From 314688f1706c03bae7271e10126418b958c76cb4 Mon Sep 17 00:00:00 2001 From: mprahl Date: Oct 02 2018 16:59:36 +0000 Subject: [PATCH 1/5] Correct the docstrings referring to the old modulemd type --- diff --git a/module_build_service/resolver/MBSResolver.py b/module_build_service/resolver/MBSResolver.py index 76ce2d0..ce63176 100644 --- a/module_build_service/resolver/MBSResolver.py +++ b/module_build_service/resolver/MBSResolver.py @@ -156,7 +156,7 @@ class MBSResolver(GenericResolver): def resolve_profiles(self, mmd, keys): """ - :param mmd: ModuleMetadata instance of module + :param mmd: Modulemd.Module instance of module :param keys: list of modulemd installation profiles to include in the result. :return: Dictionary with keys set according to `keys` param and values @@ -213,8 +213,8 @@ class MBSResolver(GenericResolver): :param strict: Normally this function returns None if no module can be found. If strict=True, then an UnprocessableEntity is raised. :return: a mapping containing buildrequire modules info in key/value pairs, - where key is koji_tag and value is the ModuleMetadata object. - :rtype: dict(str, :class:`ModuleMetadata`) + where key is koji_tag and value is the Modulemd.Module object. + :rtype: dict(str, :class:`Modulemd.Module`) """ if mmd: diff --git a/module_build_service/scheduler/handlers/modules.py b/module_build_service/scheduler/handlers/modules.py index c996887..f6bcd43 100644 --- a/module_build_service/scheduler/handlers/modules.py +++ b/module_build_service/scheduler/handlers/modules.py @@ -223,7 +223,7 @@ def wait(config, session, msg): cg_build_koji_tag = conf.koji_cg_default_build_tag if conf.system not in ['koji', 'test']: # In case of non-koji backend, we want to get the dependencies - # of the local module build based on ModuleMetadata, because the + # of the local module build based on Modulemd.Module, because the # local build is not stored in the external MBS and therefore we # cannot query it using the `query` as for Koji below. dependencies = resolver.get_module_build_dependencies( diff --git a/module_build_service/utils/mse.py b/module_build_service/utils/mse.py index 7f8c1cf..2403104 100644 --- a/module_build_service/utils/mse.py +++ b/module_build_service/utils/mse.py @@ -121,7 +121,7 @@ def _get_mmds_from_requires(session, requires, mmds, recursive=False, the list of module metadata objects defined by `requires` dict. :param session: SQLAlchemy DB session. - :param requires: Modulemetadata requires or buildrequires. + :param requires: Modulemd.Module requires or buildrequires. :param mmds: Dictionary with already handled name:streams as a keys and lists of resulting mmds as values. :param recursive: If True, the requires are checked recursively. diff --git a/module_build_service/utils/submit.py b/module_build_service/utils/submit.py index fb9058d..6822c78 100644 --- a/module_build_service/utils/submit.py +++ b/module_build_service/utils/submit.py @@ -116,7 +116,7 @@ def format_mmd(mmd, scmurl, session=None): Prepares the modulemd for the MBS. This does things such as replacing the branches of components with commit hashes and adding metadata in the xmd dictionary. - :param mmd: the ModuleMetadata object to format + :param mmd: the Modulemd.Module object to format :param scmurl: the url to the modulemd """ # Import it here, because SCM uses utils methods and fails to import From ac400fafcf29c63db5bbc81105230b7ddfd2cf0b Mon Sep 17 00:00:00 2001 From: mprahl Date: Oct 03 2018 14:11:34 +0000 Subject: [PATCH 2/5] Set the default base_module_names config option to be platform --- diff --git a/module_build_service/config.py b/module_build_service/config.py index a6c25d0..db9a8e5 100644 --- a/module_build_service/config.py +++ b/module_build_service/config.py @@ -412,7 +412,7 @@ class Config(object): 'the groups in LDAP')}, 'base_module_names': { 'type': set, - 'default': set(['platform', 'bootstrap']), + 'default': set(['platform']), 'desc': ("Set of module names which defines the product version " "(by their stream) of modules depending on them.")}, 'base_module_koji_arches': { From 54c1ed716629c319fd5178409bb85b356332321d Mon Sep 17 00:00:00 2001 From: mprahl Date: Oct 04 2018 11:19:07 +0000 Subject: [PATCH 3/5] Change the config base_module_names to be a list instead of a set to allow ordering by the admin --- diff --git a/module_build_service/config.py b/module_build_service/config.py index db9a8e5..dca1a0e 100644 --- a/module_build_service/config.py +++ b/module_build_service/config.py @@ -411,9 +411,9 @@ class Config(object): 'desc': ('The distinguished name of the container or organizational unit containing ' 'the groups in LDAP')}, 'base_module_names': { - 'type': set, - 'default': set(['platform']), - 'desc': ("Set of module names which defines the product version " + 'type': list, + 'default': ['platform'], + 'desc': ("List of base module names which define the product version " "(by their stream) of modules depending on them.")}, 'base_module_koji_arches': { 'type': dict, From cfb75b4d0f2960ec6faa8c058a494fe6070342d4 Mon Sep 17 00:00:00 2001 From: mprahl Date: Oct 04 2018 11:19:07 +0000 Subject: [PATCH 4/5] Prefix the module version based on the platform it buildrequires --- diff --git a/module_build_service/utils/submit.py b/module_build_service/utils/submit.py index 6822c78..93f6287 100644 --- a/module_build_service/utils/submit.py +++ b/module_build_service/utils/submit.py @@ -32,6 +32,7 @@ from datetime import datetime import kobo.rpmlib import requests +from gi.repository import GLib import module_build_service.scm import module_build_service.resolver @@ -207,6 +208,70 @@ def format_mmd(mmd, scmurl, session=None): mmd.set_xmd(glib.dict_values(xmd)) +def get_prefixed_version(mmd): + """ + Return the prefixed version of the module based on the buildrequired base module stream. + + :param mmd: the Modulemd.Module object to format + :return: the prefixed version + :rtype: int + """ + xmd = mmd.get_xmd() + version = mmd.get_version() + base_module_stream = None + for base_module in conf.base_module_names: + # xmd is a GLib Variant and doesn't support .get() syntax + try: + base_module_stream = xmd['mbs']['buildrequires'].get(base_module, {}).get('stream') + if base_module_stream: + # Break after finding the first base module that is buildrequired + break + except KeyError: + log.warning('The module\'s mmd is missing information in the xmd section') + return version + else: + log.warning('This module does not buildrequire a base module ({0})' + .format(' or '.join(conf.base_module_names))) + return version + + # The platform version (e.g. prefix1.2.0 => 010200) + version_prefix = '' + for char in base_module_stream: + try: + # See if the current character is an integer, signifying the version + # has started + int(char) + version_prefix += char + except ValueError: + # If version_prefix isn't set, then a digit hasn't been encountered + if version_prefix: + # If the character is a period and the version_prefix is set, then + # the loop is still processing the version part of the stream + if char == '.': + version_prefix += '.' + # If the version_prefix is set and the character is not a period or + # digit, then the remainder of the stream is a suffix like "-beta" + else: + break + + # Remove the periods and pad the numbers if necessary + version_prefix = ''.join([section.zfill(2) for section in version_prefix.split('.')]) + + if not version_prefix: + log.warning('The "{0}" stream "{1}" couldn\'t be used to prefix the module\'s ' + 'version'.format(base_module, base_module_stream)) + return version + + # Since the version must be stored as a number, we convert the string back to + # an integer which consequently drops the leading zero if there is one + new_version = int(version_prefix + str(version)) + if new_version > GLib.MAXUINT64: + log.warning('The "{0}" stream "{1}" caused the module\'s version prefix to be ' + 'too long'.format(base_module, base_module_stream)) + return version + return new_version + + def validate_mmd(mmd): """Validate module metadata @@ -397,12 +462,15 @@ def submit_module_build(username, url, mmd, scm, optional_params=None): modules = [] for mmd in mmds: + # Prefix the version of the modulemd based on the platform it buildrequires + version = get_prefixed_version(mmd) + mmd.set_version(version) + version_str = str(version) + log.debug('Checking whether module build already exists: %s.', - ":".join([mmd.get_name(), mmd.get_stream(), - str(mmd.get_version()), mmd.get_context()])) + ":".join([mmd.get_name(), mmd.get_stream(), version_str, mmd.get_context()])) module = models.ModuleBuild.get_build_from_nsvc( - db.session, mmd.get_name(), mmd.get_stream(), str(mmd.get_version()), - mmd.get_context()) + db.session, mmd.get_name(), mmd.get_stream(), version_str, mmd.get_context()) if module: if module.state != models.BUILD_STATES['failed']: err_msg = ('Module (state=%s) already exists. Only a new build or resubmission of ' @@ -438,7 +506,7 @@ def submit_module_build(username, url, mmd, scm, optional_params=None): conf, name=mmd.get_name(), stream=mmd.get_stream(), - version=str(mmd.get_version()), + version=version_str, modulemd=mmd.dumps(), scmurl=url, username=username, @@ -451,7 +519,7 @@ def submit_module_build(username, url, mmd, scm, optional_params=None): db.session.commit() modules.append(module) log.info("%s submitted build of %s, stream=%s, version=%s, context=%s", username, - mmd.get_name(), mmd.get_stream(), mmd.get_version(), mmd.get_context()) + mmd.get_name(), mmd.get_stream(), version_str, mmd.get_context()) return modules diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index 0603d6e..c626e4f 100644 --- a/tests/test_build/test_build.py +++ b/tests/test_build/test_build.py @@ -869,7 +869,7 @@ class TestBuild: build_one = models.ModuleBuild() build_one.name = 'testmodule' build_one.stream = 'master' - build_one.version = 20180205135154 + build_one.version = '2820180205135154' build_one.build_context = 'return_runtime_context' build_one.ref_build_context = 'return_runtime_context' build_one.runtime_context = '9c690d0e' @@ -994,7 +994,7 @@ class TestBuild: build_one = models.ModuleBuild() build_one.name = 'testmodule' build_one.stream = 'master' - build_one.version = 20180205135154 + build_one.version = '2820180205135154' build_one.build_context = 'return_runtime_context' build_one.ref_build_context = 'return_runtime_context' build_one.runtime_context = '9c690d0e' diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 21ce874..57c0c6d 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -29,7 +29,8 @@ import module_build_service.scm from module_build_service import models, conf from module_build_service.errors import ProgrammingError, ValidationError, UnprocessableEntity from tests import ( - reuse_component_init_data, db, reuse_shared_userspace_init_data, clean_database, init_data) + reuse_component_init_data, db, reuse_shared_userspace_init_data, clean_database, init_data, + scheduler_init_data) import mock import koji import pytest @@ -579,6 +580,22 @@ class TestUtils: is_eol = module_build_service.utils.submit._is_eol_in_pdc('mariadb', '10.1') assert is_eol + def test_get_prefixed_version_f28(self): + scheduler_init_data(1) + build_one = models.ModuleBuild.query.get(2) + v = module_build_service.utils.submit.get_prefixed_version(build_one.mmd()) + assert v == 2820180205135154 + + def test_get_prefixed_version_fl701(self): + scheduler_init_data(1) + build_one = models.ModuleBuild.query.get(2) + mmd = build_one.mmd() + xmd = glib.from_variant_dict(mmd.get_xmd()) + xmd['mbs']['buildrequires']['platform']['stream'] = 'fl7.0.1-beta' + mmd.set_xmd(glib.dict_values(xmd)) + v = module_build_service.utils.submit.get_prefixed_version(mmd) + assert v == 7000120180205135154 + class DummyModuleBuilder(GenericBuilder): """ diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 4788e49..4fdb9a1 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -685,7 +685,7 @@ class TestViews: assert data['name'] == 'testmodule' 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 From 117791ee165fd36dec99b5c1877de229f1da786e Mon Sep 17 00:00:00 2001 From: mprahl Date: Oct 04 2018 11:19:07 +0000 Subject: [PATCH 5/5] Prefix the component disttag with the platform stream --- diff --git a/module_build_service/utils/general.py b/module_build_service/utils/general.py index 45d8321..6402ed3 100644 --- a/module_build_service/utils/general.py +++ b/module_build_service/utils/general.py @@ -212,8 +212,26 @@ def get_rpm_release(module_build): mse_build_ids = module_build.siblings + [module_build.id or 0] mse_build_ids.sort() index = mse_build_ids[0] - return "{prefix}{index}+{dist_hash}".format( + try: + buildrequires = module_build.mmd().get_xmd()['mbs']['buildrequires'] + except (ValueError, KeyError): + log.warn('Module build {0} does not have buildrequires in its xmd'.format(module_build.id)) + buildrequires = None + + base_module_stream = '' + if buildrequires: + for base_module in conf.base_module_names: + base_module_stream = buildrequires.get(base_module, {}).get('stream', '') + if base_module_stream: + base_module_stream += '+' + break + else: + log.warn('Module build {0} does not buildrequire a base module ({1})' + .format(module_build.id, ' or '.join(conf.base_module_names))) + + return '{prefix}{base_module_stream}{index}+{dist_hash}'.format( prefix=conf.default_dist_tag_prefix, + base_module_stream=base_module_stream, index=index, dist_hash=dist_hash, ) diff --git a/module_build_service/utils/submit.py b/module_build_service/utils/submit.py index 93f6287..219991b 100644 --- a/module_build_service/utils/submit.py +++ b/module_build_service/utils/submit.py @@ -462,7 +462,7 @@ def submit_module_build(username, url, mmd, scm, optional_params=None): modules = [] for mmd in mmds: - # Prefix the version of the modulemd based on the platform it buildrequires + # Prefix the version of the modulemd based on the base module it buildrequires version = get_prefixed_version(mmd) mmd.set_version(version) version_str = str(version) diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 57c0c6d..1ae57b3 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -282,6 +282,12 @@ class TestUtils: assert release_one == "module+2+b8645bbb" assert release_two == "module+2+17e35784" + def test_get_rpm_release_platform_stream(self): + scheduler_init_data(1) + build_one = models.ModuleBuild.query.get(2) + release = module_build_service.utils.get_rpm_release(build_one) + assert release == 'module+f28+2+814cfa39' + @pytest.mark.parametrize('scmurl', [ ('git://pkgs.stg.fedoraproject.org/modules/testmodule.git' '?#620ec77321b2ea7b0d67d82992dda3e1d67055b4'),