From 1cd64d336ad550c67246f2e23ed88f47b13465d6 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Fri, 16 Nov 2018 17:23:24 +0800 Subject: [PATCH] Fix some minor issues * Fix failure test after rebasing on master branch. * Fix some grammar issues. * Only check stream collision modules on new created module build. * Logging message properly. Signed-off-by: Chenxiong Qi --- module_build_service/utils/submit.py | 30 +++++++-------------- module_build_service/utils/ursine.py | 40 ++++++++++++++-------------- tests/__init__.py | 7 ++--- tests/test_utils/test_ursine.py | 2 +- tests/test_views/test_views.py | 3 ++- 5 files changed, 36 insertions(+), 46 deletions(-) diff --git a/module_build_service/utils/submit.py b/module_build_service/utils/submit.py index f07a5d80..8bfa4dab 100644 --- a/module_build_service/utils/submit.py +++ b/module_build_service/utils/submit.py @@ -431,24 +431,16 @@ def record_component_builds(mmd, module, initial_batch=1, return batch -def mmd_set_default_nsv(mmd, filename, stream=None): - # Mimic the way how default values are generated for modules that are stored in SCM - # We can take filename as the module name as opposed to repo name, - # and also we can take numeric representation of current datetime - # as opposed to datetime of the last commit - dt = datetime.utcfromtimestamp(int(time.time())) - def_name = str(os.path.splitext(os.path.basename(filename))[0]) - def_version = int(dt.strftime("%Y%m%d%H%M%S")) - mmd.set_name(mmd.get_name() or def_name) - mmd.set_stream(stream or mmd.get_stream() or "master") - mmd.set_version(mmd.get_version() or def_version) - - def submit_module_build_from_yaml(username, handle, stream=None, skiptests=False, optional_params=None): yaml_file = handle.read() mmd = load_mmd(yaml_file) - mmd_set_default_nsv(mmd, handle.filename, stream=stream) + dt = datetime.utcfromtimestamp(int(time.time())) + def_name = str(os.path.splitext(os.path.basename(handle.filename))[0]) + def_version = int(dt.strftime("%Y%m%d%H%M%S")) + mmd.set_name(mmd.get_name() or def_name) + mmd.set_stream(stream or mmd.get_stream() or "master") + mmd.set_version(mmd.get_version() or def_version) if skiptests: buildopts = mmd.get_rpm_buildopts() buildopts["macros"] = buildopts.get("macros", "") + "\n\n%__spec_check_pre exit 0\n" @@ -557,13 +549,6 @@ def submit_module_build(username, url, mmd, optional_params=None): modules = [] for mmd in mmds: - # Whatever this expanded modulemd exists, check and record possible - # stream collision modules. Corresponding modules could be updated - # before an existing module is resubmitted again, so MBS has to ensure - # stream collision modules list is up-to-date. - log.info('Start to handle potential module stream collision.') - record_stream_collision_modules(mmd) - # Prefix the version of the modulemd based on the base module it buildrequires version = get_prefixed_version(mmd) mmd.set_version(version) @@ -602,6 +587,9 @@ def submit_module_build(username, url, mmd, optional_params=None): module.transition(conf, transition_to, "Resubmitted by %s" % username) log.info("Resumed existing module build in previous state %s" % module.state) else: + log.info('Start to handle potential module stream collision.') + record_stream_collision_modules(mmd) + log.debug('Creating new module build') module = models.ModuleBuild.create( db.session, diff --git a/module_build_service/utils/ursine.py b/module_build_service/utils/ursine.py index 644ba261..f7a0017c 100644 --- a/module_build_service/utils/ursine.py +++ b/module_build_service/utils/ursine.py @@ -67,11 +67,11 @@ def find_build_tags_from_external_repos(koji_session, repo_infos): conf.koji_external_repo_url_prefix.rstrip('/')) tag_names = [] for info in repo_infos: - om = re.match(re_external_repo_url, info['url']) - if om: - name = om.groups()[0] + match = re.match(re_external_repo_url, info['url']) + if match: + name = match.groups()[0] if koji_session.getTag(name) is None: - log.warning('Ignore found tag %s because no tag info is found ' + log.warning('Ignoring the found tag %s because no tag info was found ' 'with this name.', name) else: tag_names.append(name) @@ -86,9 +86,9 @@ def find_module_koji_tags(koji_session, build_tag): """ Find module koji tags from parents of build tag through the tag inheritance - MBS supports a few of prefixes which is configured in + MBS supports a few prefixes which are configured in ``conf.koij_tag_prefixes``. Tags with a configured prefix will be - considered as a koji tag. + considered as a module's koji tag. :param koji_session: instance of Koji client session. :type koji_session: ClientSession @@ -112,12 +112,12 @@ def get_modulemds_from_ursine_content(tag): Background of module build based on ursine content: Each module build buildrequires a platform module, which is a presudo-module - used to connect to an external repository whose packages could be present + used to connect to an external repository whose packages will be present in the buildroot. In practice, the external repo is generated from a build tag which could inherit from a few module koji_tags so that those module's RPMs could be build dependencies for some specific packages. - So, this method is to find out all module koji_tags from the build tag + So, this function is to find out all module koji_tags from the build tag and return corresponding module metadata. :param str tag: a base module's koji_tag. @@ -130,7 +130,7 @@ def get_modulemds_from_ursine_content(tag): repos = koji_session.getExternalRepoList(tag) build_tags = find_build_tags_from_external_repos(koji_session, repos) if not build_tags: - log.info('No external repo containing ursine content is found.') + log.debug('No external repo containing ursine content is found.') return [] modulemds = [] for tag in build_tags: @@ -146,23 +146,23 @@ def get_modulemds_from_ursine_content(tag): def find_stream_collision_modules(buildrequired_modules, koji_tag): """ - Find out modules from ursine content which are buildrequires but with a - different stream. + Find buildrequired modules that are part of the ursine content represented + by the koji_tag but with a different stream. :param dict buildrequired_modules: a mapping of buildrequires, which is just the ``xmd/mbs/buildrequires``. This mapping is used to determine if a module found from ursine content is a buildrequire with different stream. - :param str koji_tag: a base module's koji_tag. Modules will be retrieve from - ursince content associated with this koji_tag and check if there are - collision modules. + :param str koji_tag: a base module's koji_tag. Modules will be retrieved from + ursine content associated with this koji_tag and check if there are + modules that collide. :return: a list of NSVC of collision modules. If no collision module is - found, false value is returned. + found, an empty list is returned. :rtype: list[str] """ ursine_modulemds = get_modulemds_from_ursine_content(koji_tag) if not ursine_modulemds: - log.info('No module metadata is found from ursine content.') - return + log.debug('No module metadata is found from ursine content.') + return [] collision_modules = [ item.dup_nsvc() @@ -189,14 +189,14 @@ def record_stream_collision_modules(mmd): Find out modules from ursine content and record those that are buildrequire module but have different stream. - Note that, this handle depends on the result of module stream expansion. + Note that this depends on the result of module stream expansion. MBS supports multiple base modules via option conf.base_module_names. A base module name could be platform in most cases, but there could be others for particular cases in practice. So, each expanded base module stored in ``xmd/mbs/buildrequires`` will be handled and will have a new key/value pair ``stream_collision_modules: [N-S-V-C, ...]``. This key/value - will be handled in module event handler then. + will then be handled by the module event handler. As a result, a new item is added xmd/mbs/buildrequires/platform/stream_collision_modules, which is a list of NSVC strings. Each of them is the module added to ursine @@ -211,7 +211,7 @@ def record_stream_collision_modules(mmd): for module_name in conf.base_module_names: base_module_info = buildrequires.get(module_name) if base_module_info is None: - log.warning( + log.info( 'Base module %s is not a buildrequire of module %s. ' 'Skip handling module stream collision for this base module.', mmd.get_name()) diff --git a/tests/__init__.py b/tests/__init__.py index e81463aa..cd66d453 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -708,9 +708,10 @@ def make_module(nsvc, requires_list=None, build_requires_list=None, base_module= section in module metadata. :type filtered_rpms: list[str] :param dict xmd: a mapping representing XMD section in module metadata. A - custom xmd could be passed for particular test purpose and some default - key/value pairs are added if not present. - :param bool store_to_db: whether to store create module metadata to database. + custom xmd could be passed for testing a particular scenario and some + default key/value pairs are added if not present. + :param bool store_to_db: whether to store created module metadata to the + database. :return: New Module Build if set to store module metadata to database, otherwise the module metadata is returned. :rtype: ModuleBuild or Modulemd.Module diff --git a/tests/test_utils/test_ursine.py b/tests/test_utils/test_ursine.py index 0d23271a..11134fd2 100644 --- a/tests/test_utils/test_ursine.py +++ b/tests/test_utils/test_ursine.py @@ -212,7 +212,7 @@ class TestRecordStreamCollisionModules: with patch.object(ursine, 'log') as log: ursine.record_stream_collision_modules(fake_mmd) - log.warning.assert_called_once() + log.info.assert_called_once() find_stream_collision_modules.assert_not_called() assert original_xmd == glib.from_variant_dict(fake_mmd.get_xmd()) diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 85b4095e..bec1a81f 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -1216,10 +1216,11 @@ class TestViews: (['f28'], None), (['f28'], ['f28']), )) + @patch('module_build_service.utils.submit.record_stream_collision_modules') @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') def test_submit_build_dep_override( - self, mocked_scm, mocked_get_user, br_override_streams, req_override_streams): + self, mocked_scm, mocked_get_user, rscm, br_override_streams, req_override_streams): init_data(data_size=1, multiple_stream_versions=True) FakeSCM(mocked_scm, 'testmodule', 'testmodule_platform_f290000.yaml', '620ec77321b2ea7b0d67d82992dda3e1d67055b4')