#1053 Pass buildrequired modules built against all compatible base module streams to MMDResolver.
Merged 6 months ago by jkaluza. Opened 6 months ago by jkaluza.
jkaluza/fm-orchestrator virtual-provides  into  master

@@ -281,11 +281,11 @@ 

          return query.all()

  

      @staticmethod

-     def _get_last_builds_in_stream_query(session, name, stream):

+     def _get_last_builds_in_stream_query(session, name, stream, **kwargs):

          # Prepare the subquery to find out all unique name:stream records.

          subq = session.query(

              func.max(sqlalchemy.cast(ModuleBuild.version, db.BigInteger)).label("maxversion")

-         ).filter_by(name=name, state=BUILD_STATES["ready"], stream=stream).subquery('t2')

+         ).filter_by(name=name, state=BUILD_STATES["ready"], stream=stream, **kwargs).subquery('t2')

  

          # Use the subquery to actually return all the columns for its results.

          query = session.query(ModuleBuild).join(

@@ -296,20 +296,33 @@ 

          return query

  

      @staticmethod

-     def get_last_builds_in_stream(session, name, stream):

+     def get_last_builds_in_stream(session, name, stream, **kwargs):

          """

          Returns the latest builds in "ready" state for given name:stream.

+ 

+         :param session: SQLAlchemy session.

+         :param str name: Name of the module to search builds for.

+         :param str stream: Stream of the module to search builds for.

+         :param dict kwargs: Key/value pairs passed to SQLAlchmey filter_by method

+             allowing to set additional filter for results.

+ 

          """

          # Prepare the subquery to find out all unique name:stream records.

  

-         return ModuleBuild._get_last_builds_in_stream_query(session, name, stream).all()

+         return ModuleBuild._get_last_builds_in_stream_query(session, name, stream, **kwargs).all()

  

      @staticmethod

-     def get_last_build_in_stream(session, name, stream):

+     def get_last_build_in_stream(session, name, stream, **kwargs):

          """

          Returns the latest build in "ready" state for given name:stream.

+ 

+         :param session: SQLAlchemy session.

+         :param str name: Name of the module to search builds for.

+         :param str stream: Stream of the module to search builds for.

+         :param dict kwargs: Key/value pairs passed to SQLAlchmey filter_by method

+             allowing to set additional filter for results.

          """

-         return ModuleBuild._get_last_builds_in_stream_query(session, name, stream).first()

+         return ModuleBuild._get_last_builds_in_stream_query(session, name, stream, **kwargs).first()

  

      @staticmethod

      def get_build_from_nsvc(session, name, stream, version, context, **kwargs):

@@ -320,6 +333,39 @@ 

          return session.query(ModuleBuild).filter_by(

              name=name, stream=stream, version=str(version), context=context, **kwargs).first()

  

+     @staticmethod

+     def get_last_builds_in_stream_version_lte(session, name, stream_version):

+         """

+         Returns the latest builds in "ready" state for given name:stream limited by

+         `stream_version`. The `stream_version` is int generated by `get_stream_version(...)`

+         method from "x.y.z" version string.

+         The builds returned by this method are limited by stream_version XX.YY.ZZ like this:

+             "XX0000 <= build.stream_version <= XXYYZZ".

+ 

+         :param session: SQLAlchemy session.

+         :param str name: Name of the module to search builds for.

+         :param int stream_version: Maximum stream_version to search builds for.

+         """

+         # Compute the minimal stream_version - for example for `stream_version` 281234,

+         # the minimal `stream_version` is 280000.

+         min_stream_version = (stream_version // 10000) * 10000

+ 

+         # Prepare the subquery to find out all unique name:stream records.

+         subq = session.query(

+             func.max(sqlalchemy.cast(ModuleBuild.version, db.BigInteger)).label("maxversion")

+         ).filter_by(name=name, state=BUILD_STATES["ready"]).filter(

+             stream_version <= stream_version).filter(

+                 stream_version >= min_stream_version).subquery('t2')

+ 

+         # Use the subquery to actually return all the columns for its results.

+         query = session.query(ModuleBuild).join(

+             subq, and_(

+                 ModuleBuild.name == name,

+                 ModuleBuild.stream_version <= stream_version,

+                 ModuleBuild.stream_version >= min_stream_version,

+                 sqlalchemy.cast(ModuleBuild.version, db.BigInteger) == subq.c.maxversion))

+         return query.all()

+ 

      def mmd(self):

          try:

              mmd = Modulemd.Module().new_from_string(self.modulemd)

@@ -22,10 +22,13 @@ 

  # Written by Matt Prahl <mprahl@redhat.com>

  #            Jan Kaluza <jkaluza@redhat.com>

  

- from module_build_service import log

+ from sqlalchemy.orm import aliased

+ 

+ from module_build_service import log, db

  from module_build_service.resolver.base import GenericResolver

  from module_build_service import models

  from module_build_service.errors import UnprocessableEntity

+ import sqlalchemy

  

  

  class DBResolver(GenericResolver):

@@ -37,7 +40,8 @@ 

      def __init__(self, config):

          self.config = config

  

-     def get_module_modulemds(self, name, stream, version=None, context=None, strict=False):

+     def get_module_modulemds(self, name, stream, version=None, context=None, strict=False,

+                              stream_version_lte=False):

          """

          Gets the module modulemds from the resolver.

          :param name: a string of the module's name

@@ -48,6 +52,9 @@ 

              be returned.

          :kwarg strict: Normally this function returns [] if no module can be

              found.  If strict=True, then a UnprocessableEntity is raised.

+         :kwarg stream_version_lte: If True and if the `stream` can be transformed to

+             "stream version", the returned list will include all the modules with stream version

+             less than or equal the stream version computed from `stream`.

          :return: List of Modulemd metadata instances matching the query

          """

          with models.make_session(self.config) as session:

@@ -55,8 +62,14 @@ 

                  builds = [models.ModuleBuild.get_build_from_nsvc(

                      session, name, stream, version, context)]

              elif not version and not context:

-                 builds = models.ModuleBuild.get_last_builds_in_stream(

-                     session, name, stream)

+                 if (stream_version_lte and len(str(models.ModuleBuild.get_stream_version(

+                         stream, right_pad=False))) >= 5):

+                     stream_version = models.ModuleBuild.get_stream_version(stream)

+                     builds = models.ModuleBuild.get_last_builds_in_stream_version_lte(

+                         session, name, stream_version)

+                 else:

+                     builds = models.ModuleBuild.get_last_builds_in_stream(

+                         session, name, stream)

              else:

                  raise NotImplementedError(

                      "This combination of name/stream/version/context is not implemented")

@@ -66,6 +79,60 @@ 

                      "Cannot find any module builds for %s:%s" % (name, stream))

              return [build.mmd() for build in builds]

  

+     def get_buildrequired_modulemds(self, name, stream, base_module_nsvc):

+         """

+         Returns modulemd metadata of all module builds with `name` and `stream` buildrequiring

+         base module defined by `base_module_nsvc` NSVC.

+ 

+         :param str name: Name of module to return.

+         :param str stream: Stream of module to return.

+         :param str base_module_nsvc: NSVC of base module which must be buildrequired by returned

+             modules.

+         :rtype: list

+         :return: List of modulemd metadata.

+         """

+         log.debug("Looking for %s:%s buildrequiring %s", name, stream, base_module_nsvc)

+         with models.make_session(self.config) as session:

+             query = session.query(models.ModuleBuild)

+             query = query.filter_by(name=name, stream=stream, state=models.BUILD_STATES["ready"])

+ 

+             module_br_alias = aliased(models.ModuleBuild, name='module_br')

+             # Shorten this table name for clarity in the query below

+             mb_to_br = models.module_builds_to_module_buildrequires

+             # The following joins get added:

+             # JOIN module_builds_to_module_buildrequires

+             #     ON module_builds_to_module_buildrequires.module_id = module_builds.id

+             # JOIN module_builds AS module_br

+             #     ON module_builds_to_module_buildrequires.module_buildrequire_id = module_br.id

+             query = query.join(mb_to_br, mb_to_br.c.module_id == models.ModuleBuild.id)\

+                 .join(module_br_alias, mb_to_br.c.module_buildrequire_id == module_br_alias.id)

+ 

+             # Get only modules buildrequiring particular base_module_nsvc

+             n, s, v, c = base_module_nsvc.split(":")

+             query = query.filter(

+                 module_br_alias.name == n, module_br_alias.stream == s,

+                 module_br_alias.version == v, module_br_alias.context == c)

+             query = query.order_by(

+                 sqlalchemy.cast(models.ModuleBuild.version, db.BigInteger).desc())

+             all_builds = query.all()

+ 

+             # The `all_builds` list contains builds sorted by "build.version". We need only

+             # the builds with latest version, but in all contexts.

+             builds = []

+             latest_version = None

+             for build in all_builds:

+                 if latest_version is None:

+                     latest_version = build.version

+                 if latest_version != build.version:

+                     break

+                 builds.append(build)

+ 

+             mmds = [build.mmd() for build in builds]

+             nsvcs = [":".join([mmd.get_name(), mmd.get_stream(),

+                                str(mmd.get_version()), mmd.get_context()]) for mmd in mmds]

+             log.debug("Found: %r", nsvcs)

+             return mmds

+ 

      def resolve_profiles(self, mmd, keys):

          """

          Returns a dictionary with keys set according the `keys` parameters and values

@@ -70,7 +70,8 @@ 

              query["context"] = context

          return query

  

-     def _get_modules(self, name, stream, version=None, context=None, state="ready", strict=False):

+     def _get_modules(self, name, stream, version=None, context=None, state="ready", strict=False,

+                      **kwargs):

          """Query and return modules from MBS with specific info

  

          :param str name: module's name.

@@ -88,6 +89,7 @@ 

          query = self._query_from_nsvc(name, stream, version, context, state)

          query["page"] = 1

          query["per_page"] = 10

+         query.update(kwargs)

          modules = []

  

          while True:

@@ -113,7 +115,7 @@ 

              else:

                  return None

  

-         if version is None:

+         if version is None and "stream_version_lte" not in kwargs:

              # Only return the latest version

              return [m for m in modules if m["version"] == modules[0]["version"]]

          else:

@@ -122,7 +124,8 @@ 

      def _get_module(self, name, stream, version, context, state="ready", strict=False):

          return self._get_modules(name, stream, version, context, state, strict)[0]

  

-     def get_module_modulemds(self, name, stream, version=None, context=None, strict=False):

+     def get_module_modulemds(self, name, stream, version=None, context=None, strict=False,

+                              stream_version_lte=False):

          """

          Gets the module modulemds from the resolver.

          :param name: a string of the module's name

@@ -141,7 +144,13 @@ 

          if local_modules:

              return [m.mmd() for m in local_modules]

  

-         modules = self._get_modules(name, stream, version, context, strict=strict)

+         extra_args = {}

+         if (stream_version_lte and len(str(models.ModuleBuild.get_stream_version(

+                 stream, right_pad=False))) >= 5):

+             stream_version = models.ModuleBuild.get_stream_version(stream)

+             extra_args["stream_version_lte"] = stream_version

+ 

+         modules = self._get_modules(name, stream, version, context, strict=strict, **extra_args)

          if not modules:

              return []

  

@@ -160,6 +169,30 @@ 

              mmds.append(self.extract_modulemd(yaml, strict=strict))

          return mmds

  

+     def get_buildrequired_modulemds(self, name, stream, base_module_nsvc):

+         """

+         Returns modulemd metadata of all module builds with `name` and `stream` buildrequiring

+         base module defined by `base_module_nsvc` NSVC.

+ 

+         :param str name: Name of module to return.

+         :param str stream: Stream of module to return.

+         :param str base_module_nsvc: NSVC of base module which must be buildrequired by returned

+             modules.

+         :rtype: list

+         :return: List of modulemd metadata.

+         """

+         yaml = None

+         modules = self._get_modules(name, stream, strict=False,

+                                     base_module_br=base_module_nsvc)

+         if not modules:

+             return []

+ 

+         mmds = []

+         for module in modules:

+             yaml = module['modulemd']

+             mmds.append(self.extract_modulemd(yaml))

+         return mmds

+ 

      def resolve_profiles(self, mmd, keys):

          """

          :param mmd: Modulemd.Module instance of module

@@ -95,7 +95,12 @@ 

          return mmd

  

      @abstractmethod

-     def get_module_modulemds(self, name, stream, version=None, context=None, strict=False):

+     def get_module_modulemds(self, name, stream, version=None, context=None, strict=False,

+                              stream_version_lte=None):

+         raise NotImplementedError()

+ 

+     @abstractmethod

+     def get_buildrequired_modulemds(self, name, stream, base_module_nsvc, strict=False):

          raise NotImplementedError()

  

      @abstractmethod

@@ -22,8 +22,9 @@ 

  # Written by Ralph Bean <rbean@redhat.com>

  #            Matt Prahl <mprahl@redhat.com>

  #            Jan Kaluza <jkaluza@redhat.com>

- from module_build_service import log, models, Modulemd, db

+ from module_build_service import log, models, Modulemd, db, conf

  from module_build_service.errors import StreamAmbigous

+ from module_build_service.errors import UnprocessableEntity

  from module_build_service.mmd_resolver import MMDResolver

  from module_build_service import glib

  import module_build_service.resolver

@@ -115,7 +116,8 @@ 

  

  

  def _get_mmds_from_requires(requires, mmds, recursive=False,

-                             default_streams=None, raise_if_stream_ambigous=False):

+                             default_streams=None, raise_if_stream_ambigous=False,

+                             base_module_mmds=None):

      """

      Helper method for get_mmds_required_by_module_recursively returning

      the list of module metadata objects defined by `requires` dict.

@@ -130,6 +132,9 @@ 

      :param bool raise_if_stream_ambigous: When True, raises a StreamAmbigous exception in case

          there are multiple streams for some dependency of module and the module name is not

          defined in `default_streams`, so it is not clear which stream should be used.

+     :param list base_module_mmds: List of modulemd metadata instances. When set, the

+         returned list contains MMDs build against each base module defined in

+         `base_module_mmds` list.

      :return: Dict with name:stream as a key and list with mmds as value.

      """

      default_streams = default_streams or {}

@@ -139,6 +144,10 @@ 

      resolver = module_build_service.resolver.system_resolver

  

      for name, streams in requires.items():

+         # Base modules are already added to `mmds`.

+         if name in conf.base_module_names:

+             continue

+ 

          streams_to_try = streams.get()

          if name in default_streams:

              streams_to_try = [default_streams[name]]

@@ -152,22 +161,62 @@ 

          # previous call of this method.

          for stream in streams.get():

              ns = "%s:%s" % (name, stream)

-             if ns in mmds:

-                 continue

- 

-             mmds[ns] = resolver.get_module_modulemds(name, stream, strict=True)

-             added_mmds[ns] = mmds[ns]

+             if ns not in mmds:

+                 mmds[ns] = []

+             if ns not in added_mmds:

+                 added_mmds[ns] = []

+ 

+             if base_module_mmds:

+                 for base_module_mmd in base_module_mmds:

+                     base_module_nsvc = ":".join([

+                         base_module_mmd.get_name(), base_module_mmd.get_stream(),

+                         str(base_module_mmd.get_version()), base_module_mmd.get_context()])

+                     mmds[ns] += resolver.get_buildrequired_modulemds(

+                         name, stream, base_module_nsvc)

+             else:

+                 mmds[ns] = resolver.get_module_modulemds(name, stream, strict=True)

+             added_mmds[ns] += mmds[ns]

  

      # Get the requires recursively.

      if recursive:

          for mmd_list in added_mmds.values():

              for mmd in mmd_list:

                  for deps in mmd.get_dependencies():

-                     mmds = _get_mmds_from_requires(deps.get_requires(), mmds, True)

+                     mmds = _get_mmds_from_requires(

+                         deps.get_requires(), mmds, True, base_module_mmds=base_module_mmds)

  

      return mmds

  

  

+ def _get_base_module_mmds(mmd):

+     """

+     Returns list of MMDs of base modules buildrequired by `mmd` including the compatible

+     old versions of the base module based on the stream version.

+ 

+     :param Modulemd mmd: Input modulemd metadata.

+     :rtype: list of Modulemd

+     :return: List of MMDs of base modules buildrequired by `mmd`.

+     """

+     seen = set()

+     ret = []

+ 

+     resolver = module_build_service.resolver.system_resolver

+     for deps in mmd.get_dependencies():

+         buildrequires = deps.get_buildrequires()

+         for name in conf.base_module_names:

+             if name not in buildrequires.keys():

+                 continue

+ 

+             for stream in buildrequires[name].get():

+                 ns = ":".join([name, stream])

+                 if ns in seen:

+                     continue

+                 seen.add(ns)

+                 ret += resolver.get_module_modulemds(name, stream, stream_version_lte=True)

+             break

+     return ret

+ 

+ 

  def get_mmds_required_by_module_recursively(

          mmd, default_streams=None, raise_if_stream_ambigous=False):

      """

@@ -199,11 +248,23 @@ 

      # handled from DB.

      mmds = {}

  

-     # At first get all the buildrequires of the module of interest.

+     # Get the MMDs of all compatible base modules based on the buildrequires.

+     base_module_mmds = _get_base_module_mmds(mmd)

+     if not base_module_mmds:

+         raise ValueError("No base module found in buildrequires section of %s" % ":".join(

+             [mmd.get_name(), mmd.get_stream(), str(mmd.get_version())]))

+ 

+     # Add base modules to `mmds`.

+     for base_module in base_module_mmds:

+         ns = ":".join([base_module.get_name(), base_module.get_stream()])

+         mmds.setdefault(ns, [])

+         mmds[ns].append(base_module)

+ 

+     # Get all the buildrequires of the module of interest.

      for deps in mmd.get_dependencies():

          mmds = _get_mmds_from_requires(

              deps.get_buildrequires(), mmds, False, default_streams,

-             raise_if_stream_ambigous)

+             raise_if_stream_ambigous, base_module_mmds)

  

      # Now get the requires of buildrequires recursively.

      for mmd_key in list(mmds.keys()):

@@ -211,11 +272,14 @@ 

              for deps in mmd.get_dependencies():

                  mmds = _get_mmds_from_requires(

                      deps.get_requires(), mmds, True, default_streams,

-                     raise_if_stream_ambigous)

+                     raise_if_stream_ambigous, base_module_mmds)

  

      # Make single list from dict of lists.

      res = []

-     for mmds_list in mmds.values():

+     for ns, mmds_list in mmds.items():

+         if len(mmds_list) == 0:

+             raise UnprocessableEntity(

+                 "Cannot find any module builds for %s" % (ns))

          res += mmds_list

      return res

  

file modified
+17 -2

@@ -93,15 +93,27 @@ 

          import_mmd(db.session, mmd)

  

  

- def init_data(data_size=10, contexts=False):

+ def init_data(data_size=10, contexts=False, multiple_stream_versions=False):

      """

      Creates data_size * 3 modules in database in different states and

      with different component builds. See _populate_data for more info.

  

      :param bool contexts: If True, multiple streams and contexts in each stream

          are generated for 'nginx' module.

+     :param bool multiple_stream_versions: If true, multiple base modules with

+         difference stream versions are generated.

      """

      clean_database()

+     if multiple_stream_versions:

+         mmd = load_mmd(os.path.join(base_dir, 'staged_data', 'platform.yaml'), True)

+         for stream in ["f28.0.0", "f29.0.0", "f29.1.0", "f29.2.0"]:

+             mmd.set_name("platform")

+             mmd.set_stream(stream)

+             import_mmd(db.session, mmd)

+             # Just to possibly confuse tests by adding another base module.

+             mmd.set_name("bootstrap")

+             mmd.set_stream(stream)

+             import_mmd(db.session, mmd)

      with make_session(conf) as session:

          _populate_data(session, data_size, contexts=contexts)

  

@@ -681,7 +693,7 @@ 

              session.add(build)

  

  

- def make_module(nsvc, requires_list, build_requires_list):

+ def make_module(nsvc, requires_list, build_requires_list, base_module=None):

      """

      Creates new models.ModuleBuild defined by `nsvc` string with requires

      and buildrequires set according to `requires_list` and `build_requires_list`.

@@ -734,6 +746,7 @@ 

      module_build = ModuleBuild()

      module_build.name = name

      module_build.stream = stream

+     module_build.stream_version = module_build.get_stream_version(stream)

      module_build.version = version

      module_build.context = context

      module_build.state = BUILD_STATES['ready']

@@ -747,6 +760,8 @@ 

      module_build.stream_build_context = context

      module_build.runtime_context = context

      module_build.modulemd = mmd.dumps()

+     if base_module:

+         module_build.buildrequires.append(base_module)

      db.session.add(module_build)

      db.session.commit()

  

@@ -8,3 +8,8 @@ 

          module:

              - MIT

          content: []

+     dependencies:

+         buildrequires:

+             platform: f28

+         requires:

+             platform: f28

@@ -9,6 +9,7 @@ 

          module: [ MIT ]

      dependencies:

          buildrequires:

+             platform: f28

              chineese_food: good

          requires:

              noodles: lomein

@@ -23,7 +23,7 @@ 

  import os

  

  from tests.test_models import init_data, module_build_from_modulemd

- from tests import init_data as init_data_contexts, clean_database

+ from tests import (init_data as init_data_contexts, clean_database)

  from module_build_service import conf, Modulemd

  from module_build_service.models import ComponentBuild, ModuleBuild, make_session

  

@@ -132,3 +132,12 @@ 

              builds = ["%s:%s:%s:%s" % (build.name, build.stream, str(build.version),

                                         build.context) for build in builds]

              assert builds == ['nginx:1:3:d5a6c0fa', 'nginx:1:3:795e97c1']

+ 

+     def test_get_last_builds_in_stream_version_lte(self):

+         init_data_contexts(1, multiple_stream_versions=True)

+         with make_session(conf) as session:

+             builds = ModuleBuild.get_last_builds_in_stream_version_lte(

+                 session, "platform", 290100)

+             builds = set(["%s:%s:%s:%s" % (build.name, build.stream, str(build.version),

+                                            build.context) for build in builds])

+             assert builds == set(['platform:f29.0.0:3:00000000', 'platform:f29.1.0:3:00000000'])

@@ -22,11 +22,14 @@ 

  

  import os

  

+ from datetime import datetime

  from mock import patch, PropertyMock

  import pytest

  

  import module_build_service.resolver as mbs_resolver

  from module_build_service import app, db, models, glib, utils, Modulemd

+ from module_build_service.utils import import_mmd, load_mmd

+ from module_build_service.models import ModuleBuild

  import tests

  

  

@@ -38,6 +41,54 @@ 

      def setup_method(self):

          tests.reuse_component_init_data()

  

+     def test_get_buildrequired_modulemds(self):

+         mmd = load_mmd(os.path.join(base_dir, 'staged_data', 'platform.yaml'), True)

+         mmd.set_stream('f30.1.3')

+         import_mmd(db.session, mmd)

+         platform_f300103 = ModuleBuild.query.filter_by(stream='f30.1.3').one()

+         mmd.set_name("testmodule")

+         mmd.set_stream("master")

+         mmd.set_version(20170109091357)

+         mmd.set_context("123")

+         build = ModuleBuild(

+             name='testmodule',

+             stream='master',

+             version=20170109091357,

+             state=5,

+             build_context='dd4de1c346dcf09ce77d38cd4e75094ec1c08ec3',

+             runtime_context='ec4de1c346dcf09ce77d38cd4e75094ec1c08ef7',

+             context='7c29193d',

+             koji_tag='module-testmodule-master-20170109091357-7c29193d',

+             scmurl='git://pkgs.stg.fedoraproject.org/modules/testmodule.git?#ff1ea79',

+             batch=3,

+             owner='Dr. Pepper',

+             time_submitted=datetime(2018, 11, 15, 16, 8, 18),

+             time_modified=datetime(2018, 11, 15, 16, 19, 35),

+             rebuild_strategy='changed-and-after',

+             modulemd=mmd.dumps()

+         )

+         build.buildrequires.append(platform_f300103)

+         db.session.add(build)

+         db.session.commit()

+ 

+         resolver = mbs_resolver.GenericResolver.create(tests.conf, backend='db')

+         result = resolver.get_buildrequired_modulemds(

+             "testmodule", "master", platform_f300103.mmd().dup_nsvc())

+         nsvcs = set([m.dup_nsvc() for m in result])

+         assert nsvcs == set(['testmodule:master:20170109091357:123'])

+ 

+     @pytest.mark.parametrize('stream_versions', [False, True])

+     def test_get_module_modulemds_stream_versions(self, stream_versions):

+         tests.init_data(1, multiple_stream_versions=True)

+         resolver = mbs_resolver.GenericResolver.create(tests.conf, backend='db')

+         result = resolver.get_module_modulemds(

+             "platform", "f29.1.0", stream_version_lte=stream_versions)

+         nsvcs = set([mmd.dup_nsvc() for mmd in result])

+         if stream_versions:

+             assert nsvcs == set(['platform:f29.1.0:3:00000000', 'platform:f29.0.0:3:00000000'])

+         else:

+             assert nsvcs == set(['platform:f29.1.0:3:00000000'])

+ 

      @pytest.mark.parametrize('empty_buildrequires', [False, True])

      def test_get_module_build_dependencies(self, empty_buildrequires):

          """

@@ -52,22 +52,22 @@ 

          Generates gtk:1, gtk:2, foo:1 and foo:2 modules requiring the

          platform:f28 and platform:f29 modules.

          """

-         make_module("gtk:1:0:c2", {"platform": ["f28"]}, {})

-         make_module("gtk:1:0:c3", {"platform": ["f29"]}, {})

-         make_module("gtk:2:0:c4", {"platform": ["f28"]}, {})

-         make_module("gtk:2:0:c5", {"platform": ["f29"]}, {})

-         make_module("foo:1:0:c2", {"platform": ["f28"]}, {})

-         make_module("foo:1:0:c3", {"platform": ["f29"]}, {})

-         make_module("foo:2:0:c4", {"platform": ["f28"]}, {})

-         make_module("foo:2:0:c5", {"platform": ["f29"]}, {})

-         make_module("platform:f28:0:c10", {}, {})

-         make_module("platform:f29:0:c11", {}, {})

-         make_module("app:1:0:c6", {"platform": ["f29"]}, {})

+         platform_f28 = make_module("platform:f28:0:c10", {}, {})

+         platform_f29 = make_module("platform:f29:0:c11", {}, {})

+         make_module("gtk:1:0:c2", {"platform": ["f28"]}, {}, platform_f28)

+         make_module("gtk:1:0:c3", {"platform": ["f29"]}, {}, platform_f29)

+         make_module("gtk:2:0:c4", {"platform": ["f28"]}, {}, platform_f28)

+         make_module("gtk:2:0:c5", {"platform": ["f29"]}, {}, platform_f29)

+         make_module("foo:1:0:c2", {"platform": ["f28"]}, {}, platform_f28)

+         make_module("foo:1:0:c3", {"platform": ["f29"]}, {}, platform_f29)

+         make_module("foo:2:0:c4", {"platform": ["f28"]}, {}, platform_f28)

+         make_module("foo:2:0:c5", {"platform": ["f29"]}, {}, platform_f29)

+         make_module("app:1:0:c6", {"platform": ["f29"]}, {}, platform_f29)

  

      def test_generate_expanded_mmds_context(self):

          self._generate_default_modules()

          module_build = make_module(

-             "app:1:0:c1", {"gtk": ["1", "2"]}, {"gtk": ["1", "2"]})

+             "app:1:0:c1", {"gtk": ["1", "2"]}, {"platform": ["f28"], "gtk": ["1", "2"]})

          mmds = module_build_service.utils.generate_expanded_mmds(

              db.session, module_build.mmd())

          contexts = set([mmd.get_context() for mmd in mmds])

@@ -75,35 +75,39 @@ 

  

      @pytest.mark.parametrize(

          'requires,build_requires,stream_ambigous,expected_xmd,expected_buildrequires', [

-             ({"gtk": ["1", "2"]}, {"gtk": ["1", "2"]}, True,

+             ({"gtk": ["1", "2"]},

+              {"platform": ["f28"], "gtk": ["1", "2"]}, True,

               set([

                   frozenset(['platform:f28:0:c10', 'gtk:2:0:c4']),

                   frozenset(['platform:f28:0:c10', 'gtk:1:0:c2'])

               ]),

               set([

-                  frozenset(['gtk:1']),

-                  frozenset(['gtk:2']),

+                  frozenset(['gtk:1', 'platform:f28']),

+                  frozenset(['gtk:2', 'platform:f28']),

               ])),

  

-             ({"foo": ["1"]}, {"foo": ["1"], "gtk": ["1", "2"]}, True,

+             ({"foo": ["1"]},

+              {"platform": ["f28"], "foo": ["1"], "gtk": ["1", "2"]}, True,

               set([

                   frozenset(['foo:1:0:c2', 'gtk:1:0:c2', 'platform:f28:0:c10']),

                   frozenset(['foo:1:0:c2', 'gtk:2:0:c4', 'platform:f28:0:c10'])

               ]),

               set([

-                  frozenset(['foo:1', 'gtk:1']),

-                  frozenset(['foo:1', 'gtk:2'])

+                  frozenset(['foo:1', 'gtk:1', 'platform:f28']),

+                  frozenset(['foo:1', 'gtk:2', 'platform:f28'])

               ])),

  

-             ({"gtk": ["1"], "foo": ["1"]}, {"gtk": ["1"], "foo": ["1"]}, False,

+             ({"gtk": ["1"], "foo": ["1"]},

+              {"platform": ["f28"], "gtk": ["1"], "foo": ["1"]}, False,

               set([

                   frozenset(['foo:1:0:c2', 'gtk:1:0:c2', 'platform:f28:0:c10'])

               ]),

               set([

-                  frozenset(['foo:1', 'gtk:1'])

+                  frozenset(['foo:1', 'gtk:1', 'platform:f28'])

               ])),

  

-             ({"gtk": ["1"], "foo": ["1"]}, {"gtk": ["1"], "foo": ["1"], "platform": ["f28"]}, False,

+             ({"gtk": ["1"], "foo": ["1"]},

+              {"gtk": ["1"], "foo": ["1"], "platform": ["f28"]}, False,

               set([

                   frozenset(['foo:1:0:c2', 'gtk:1:0:c2', 'platform:f28:0:c10'])

               ]),

@@ -111,44 +115,47 @@ 

                   frozenset(['foo:1', 'gtk:1', 'platform:f28'])

               ])),

  

-             ({"gtk": ["-2"], "foo": ["-2"]}, {"gtk": ["-2"], "foo": ["-2"]}, True,

+             ({"gtk": ["-2"], "foo": ["-2"]},

+              {"platform": ["f28"], "gtk": ["-2"], "foo": ["-2"]}, True,

               set([

                   frozenset(['foo:1:0:c2', 'gtk:1:0:c2', 'platform:f28:0:c10'])

               ]),

               set([

-                  frozenset(['foo:1', 'gtk:1'])

+                  frozenset(['foo:1', 'gtk:1', 'platform:f28'])

               ])),

  

-             ({"gtk": ["1"], "foo": ["1"]}, {"gtk": ["-1", "1"], "foo": ["-2", "1"]}, False,

+             ({"gtk": ["1"], "foo": ["1"]},

+              {"platform": ["f28"], "gtk": ["-1", "1"], "foo": ["-2", "1"]}, False,

               set([

                   frozenset(['foo:1:0:c2', 'gtk:1:0:c2', 'platform:f28:0:c10'])

               ]),

               set([

-                  frozenset(['foo:1', 'gtk:1'])

+                  frozenset(['foo:1', 'gtk:1', 'platform:f28'])

               ])),

  

-             ({"gtk": ["1"], "foo": ["1"]}, {"gtk": ["1"]}, False,

+             ({"gtk": ["1"], "foo": ["1"]},

+              {"platform": ["f28"], "gtk": ["1"]}, False,

               set([

                   frozenset(['gtk:1:0:c2', 'platform:f28:0:c10'])

               ]),

               set([

-                  frozenset(['gtk:1'])

+                  frozenset(['gtk:1', 'platform:f28'])

               ])),

  

-             ({"gtk": []}, {"gtk": ["1"]}, True,

+             ({"gtk": []}, {"platform": ["f28"], "gtk": ["1"]}, True,

               set([

                   frozenset(['gtk:1:0:c2', 'platform:f28:0:c10'])

               ]),

               set([

-                  frozenset(['gtk:1'])

+                  frozenset(['gtk:1', 'platform:f28'])

               ])),

  

-             ({}, {"app": ["1"]}, False,

+             ({}, {"platform": ["f29"], "app": ["1"]}, False,

               set([

                   frozenset(['app:1:0:c6', 'platform:f29:0:c11'])

               ]),

               set([

-                  frozenset(['app:1'])

+                  frozenset(['app:1', 'platform:f29'])

               ])),

          ])

      def test_generate_expanded_mmds_buildrequires(

@@ -204,33 +211,36 @@ 

          assert buildrequires_per_mmd_buildrequires == expected_buildrequires

  

      @pytest.mark.parametrize('requires,build_requires,expected', [

-         ({"gtk": ["1", "2"]}, {"gtk": ["1", "2"]},

+         ({"gtk": ["1", "2"]}, {"platform": [], "gtk": ["1", "2"]},

           set([

               frozenset(['gtk:1']),

               frozenset(['gtk:2']),

           ])),

  

-         ({"gtk": ["1", "2"]}, {"gtk": ["1"]},

+         ({"gtk": ["1", "2"]}, {"platform": [], "gtk": ["1"]},

           set([

               frozenset(['gtk:1', 'gtk:2']),

           ])),

  

-         ({"gtk": ["1"], "foo": ["1"]}, {"gtk": ["1"], "foo": ["1"]},

+         ({"gtk": ["1"], "foo": ["1"]},

+          {"platform": [], "gtk": ["1"], "foo": ["1"]},

           set([

               frozenset(['foo:1', 'gtk:1']),

           ])),

  

-         ({"gtk": ["-2"], "foo": ["-2"]}, {"gtk": ["-2"], "foo": ["-2"]},

+         ({"gtk": ["-2"], "foo": ["-2"]},

+          {"platform": [], "gtk": ["-2"], "foo": ["-2"]},

           set([

               frozenset(['foo:1', 'gtk:1']),

           ])),

  

-         ({"gtk": ["-1", "1"], "foo": ["-2", "1"]}, {"gtk": ["-1", "1"], "foo": ["-2", "1"]},

+         ({"gtk": ["-1", "1"], "foo": ["-2", "1"]},

+          {"platform": [], "gtk": ["-1", "1"], "foo": ["-2", "1"]},

           set([

               frozenset(['foo:1', 'gtk:1']),

           ])),

  

-         ({"gtk": [], "foo": []}, {"gtk": ["1"], "foo": ["1"]},

+         ({"gtk": [], "foo": []}, {"platform": [], "gtk": ["1"], "foo": ["1"]},

           set([

               frozenset([]),

           ])),

@@ -255,28 +265,29 @@ 

          assert requires_per_mmd == expected

  

      @pytest.mark.parametrize('requires,build_requires,expected', [

-         ({}, {"gtk": ["1", "2"]},

+         ({}, {"platform": [], "gtk": ["1", "2"]},

           ['platform:f29:0:c11', 'gtk:2:0:c4', 'gtk:2:0:c5',

            'platform:f28:0:c10', 'gtk:1:0:c2', 'gtk:1:0:c3']),

  

-         ({}, {"gtk": ["1"], "foo": ["1"]},

+         ({}, {"platform": [], "gtk": ["1"], "foo": ["1"]},

           ['platform:f28:0:c10', 'gtk:1:0:c2', 'gtk:1:0:c3',

            'foo:1:0:c2', 'foo:1:0:c3', 'platform:f29:0:c11']),

  

          ({}, {"gtk": ["1"], "foo": ["1"], "platform": ["f28"]},

-          ['platform:f28:0:c10', 'gtk:1:0:c2', 'gtk:1:0:c3',

-           'foo:1:0:c2', 'foo:1:0:c3', 'platform:f29:0:c11']),

+          ['platform:f28:0:c10', 'gtk:1:0:c2',

+           'foo:1:0:c2']),

  

-         ([{}, {}], [{"gtk": ["1"], "foo": ["1"]}, {"gtk": ["2"], "foo": ["2"]}],

+         ([{}, {}], [{"platform": [], "gtk": ["1"], "foo": ["1"]},

+                     {"platform": [], "gtk": ["2"], "foo": ["2"]}],

           ['foo:1:0:c2', 'foo:1:0:c3', 'foo:2:0:c4', 'foo:2:0:c5',

            'platform:f28:0:c10', 'platform:f29:0:c11', 'gtk:1:0:c2',

            'gtk:1:0:c3', 'gtk:2:0:c4', 'gtk:2:0:c5']),

  

-         ({}, {"gtk": ["-2"], "foo": ["-2"]},

+         ({}, {"platform": [], "gtk": ["-2"], "foo": ["-2"]},

           ['foo:1:0:c2', 'foo:1:0:c3', 'platform:f29:0:c11',

            'platform:f28:0:c10', 'gtk:1:0:c2', 'gtk:1:0:c3']),

  

-         ({}, {"gtk": ["-1", "1"], "foo": ["-2", "1"]},

+         ({}, {"platform": [], "gtk": ["-1", "1"], "foo": ["-2", "1"]},

           ['foo:1:0:c2', 'foo:1:0:c3', 'platform:f29:0:c11',

            'platform:f28:0:c10', 'gtk:1:0:c2', 'gtk:1:0:c3']),

      ])

@@ -292,23 +303,23 @@ 

          and lorem:1 modules which require base:f29 module requiring

          platform:f29 module :).

          """

-         make_module("gtk:1:0:c2", {"foo": ["unknown"]}, {})

-         make_module("gtk:1:1:c2", {"foo": ["1"]}, {})

-         make_module("foo:1:0:c2", {"bar": ["unknown"]}, {})

-         make_module("foo:1:1:c2", {"bar": ["1"], "lorem": ["1"]}, {})

-         make_module("bar:1:0:c2", {"base": ["unknown"]}, {})

-         make_module("bar:1:1:c2", {"base": ["f29"]}, {})

-         make_module("lorem:1:0:c2", {"base": ["unknown"]}, {})

-         make_module("lorem:1:1:c2", {"base": ["f29"]}, {})

-         make_module("base:f29:0:c3", {"platform": ["f29"]}, {})

-         make_module("platform:f29:0:c11", {}, {})

+         base_module = make_module("platform:f29:0:c11", {}, {})

+         make_module("gtk:1:0:c2", {"foo": ["unknown"]}, {}, base_module)

+         make_module("gtk:1:1:c2", {"foo": ["1"]}, {}, base_module)

+         make_module("foo:1:0:c2", {"bar": ["unknown"]}, {}, base_module)

+         make_module("foo:1:1:c2", {"bar": ["1"], "lorem": ["1"]}, {}, base_module)

+         make_module("bar:1:0:c2", {"base": ["unknown"]}, {}, base_module)

+         make_module("bar:1:1:c2", {"base": ["f29"]}, {}, base_module)

+         make_module("lorem:1:0:c2", {"base": ["unknown"]}, {}, base_module)

+         make_module("lorem:1:1:c2", {"base": ["f29"]}, {}, base_module)

+         make_module("base:f29:0:c3", {"platform": ["f29"]}, {}, base_module)

  

      @pytest.mark.parametrize('requires,build_requires,expected', [

-         ({}, {"gtk": ["1"]},

+         ({}, {"platform": [], "gtk": ["1"]},

           ['foo:1:1:c2', 'base:f29:0:c3', 'platform:f29:0:c11',

            'bar:1:1:c2', 'gtk:1:1:c2', 'lorem:1:1:c2']),

  

-         ({}, {"foo": ["1"]},

+         ({}, {"platform": [], "foo": ["1"]},

           ['foo:1:1:c2', 'base:f29:0:c3', 'platform:f29:0:c11',

            'bar:1:1:c2', 'lorem:1:1:c2']),

      ])

@@ -317,3 +328,27 @@ 

          self._generate_default_modules_recursion()

          nsvcs = self._get_mmds_required_by_module_recursively(module_build)

          assert set(nsvcs) == set(expected)

+ 

+     def _generate_default_modules_modules_multiple_stream_versions(self):

+         """

+         Generates the gtk:1 module requiring foo:1 module requiring bar:1

+         and lorem:1 modules which require base:f29 module requiring

+         platform:f29 module :).

+         """

+         f290000 = make_module("platform:f29.0.0:0:c11", {}, {})

+         f290100 = make_module("platform:f29.1.0:0:c11", {}, {})

+         f290200 = make_module("platform:f29.2.0:0:c11", {}, {})

+         make_module("gtk:1:0:c2", {"platform": ["f29"]}, {}, f290000)

+         make_module("gtk:1:1:c2", {"platform": ["f29"]}, {}, f290100)

+         make_module("gtk:1:2:c2", {"platform": ["f29"]}, {}, f290100)

+         make_module("gtk:1:3:c2", {"platform": ["f29"]}, {}, f290200)

+ 

+     @pytest.mark.parametrize('requires,build_requires,expected', [

+         ({}, {"platform": ["f29.1.0"], "gtk": ["1"]},

+          ['platform:f29.0.0:0:c11', 'gtk:1:0:c2', 'gtk:1:2:c2', 'platform:f29.1.0:0:c11']),

+     ])

+     def test_get_required_modules_stream_versions(self, requires, build_requires, expected):

+         module_build = make_module("app:1:0:c1", requires, build_requires)

+         self._generate_default_modules_modules_multiple_stream_versions()

+         nsvcs = self._get_mmds_required_by_module_recursively(module_build)

+         assert set(nsvcs) == set(expected)

@@ -893,7 +893,7 @@ 

          assert data['name'] == 'fakemodule'

          assert data['scmurl'] == ('git://pkgs.stg.fedoraproject.org/modules/testmodule.git'

                                    '?#68931c90de214d9d13feefbd35246a81b6cb8d49')

-         assert data['version'] == '1'

+         assert data['version'] == '281'

          assert data['time_submitted'] is not None

          assert data['time_modified'] is not None

          assert data['time_completed'] is None

Imagine we have "platform:f29.0.0" and "platform:f29.1.0" base modules.
We also have "DBI" module we want to build agaisnt "platform:f29.1.0".
This "DBI" module depends on "perl" module which is only build against
"platform:f29.0.0".

Currently, DBI build would fail to resolve the dependencies, because
it wouldn't find "perl" module, because it is built against different
platform stream.

This PR changes the MSE code to include buildrequired module builds built
against all the compatible platform streams.

It does so by introducing following changes:

  • MSE code uses new get_base_module_mmds() method to find out all the
    compatible platform modules. This needed new methods in DBResolver
    and MBSResolver.
  • For each buildrequired module defined by name:stream, the MSE code then
    finds particular NSVC built against each compatible platform module.

Side effect of these code changes is that every module now must buildrequire
some base module.

@jkaluza could you fix the failing test? In the mean-time, I'll start reviewing.

This causes problems in Python 3 since the division of two integers can be a float:

>>> (281234 / 10000) * 10000
281234.0

You could do something like this instead:

>>> import math
>>> math.floor((281234 / 10000)) * 10000
280000

Or you can just learn python and use // :)

Optional: An interesting trick is you could replace this if statement with:

mmds.setdefault(ns, [])

"possible" => "possibly"
"by another" => "by adding another"

@jkaluza I just left a couple of minor comments. Once they are addressed and the tests pass, +1.

rebased onto 077ea9a4473d5bbc91663c860ce4133e7e9b538f

6 months ago

rebased onto 6451c9346bbfb0749dba924616d1c5cf02d5c1b9

6 months ago

rebased onto f2a236b

6 months ago

@mprahl, everything should be fixed now.

Pull-Request has been merged by jkaluza

6 months ago