From f422b82ce7a4cf28549507e5a7742aa150627cde Mon Sep 17 00:00:00 2001 From: mprahl Date: Aug 09 2018 23:25:35 +0000 Subject: [PATCH 1/2] Make _get_reusable_module a public method so it can be used elsewhere --- diff --git a/module_build_service/utils/reuse.py b/module_build_service/utils/reuse.py index 395aa6c..8f39bc9 100644 --- a/module_build_service/utils/reuse.py +++ b/module_build_service/utils/reuse.py @@ -76,7 +76,7 @@ def reuse_component(component, previous_component_build, ] -def _get_reusable_module(session, module): +def get_reusable_module(session, module): """ Returns previous module build of the module `module` in case it can be used as a source module to get the components to reuse from. @@ -123,7 +123,7 @@ def attempt_to_reuse_all_components(builder, session, module): False is returned, no component has been reused. """ - previous_module_build = _get_reusable_module(session, module) + previous_module_build = get_reusable_module(session, module) if not previous_module_build: return False @@ -188,7 +188,7 @@ def get_reusable_components(session, module, component_names): if conf.system not in ['koji', 'test']: return [None] * len(component_names) - previous_module_build = _get_reusable_module(session, module) + previous_module_build = get_reusable_module(session, module) if not previous_module_build: return [None] * len(component_names) @@ -215,7 +215,7 @@ def get_reusable_component(session, module, component_name, :param component_name: the name of the component (RPM) that you'd like to reuse a previous build of :param previous_module_build: the ModuleBuild instances of a module build - which contains the components to reuse. If not passed, _get_reusable_module + which contains the components to reuse. If not passed, get_reusable_module is called to get the ModuleBuild instance. Consider passing the ModuleBuild instance in case you plan to call get_reusable_component repeatedly for the same module to make this method faster. @@ -240,7 +240,7 @@ def get_reusable_component(session, module, component_name, return None if not previous_module_build: - previous_module_build = _get_reusable_module(session, module) + previous_module_build = get_reusable_module(session, module) if not previous_module_build: return None From d54422adeb99f70455de08cb59406436fbf1d74a Mon Sep 17 00:00:00 2001 From: mprahl Date: Aug 13 2018 15:13:40 +0000 Subject: [PATCH 2/2] Don't filter RPMs of reused components from a module that depends on itself If a module has filters set and it buildrequires itself, issues occur during component reuse. In that event, the filtered RPMs from the previous module build will not be available in the buildroot because the NVRs in the filters will match the RPMs from the reused component. Co-authored-by: Jan Kaluza --- diff --git a/module_build_service/builder/KojiModuleBuilder.py b/module_build_service/builder/KojiModuleBuilder.py index ac26097..9900e92 100644 --- a/module_build_service/builder/KojiModuleBuilder.py +++ b/module_build_service/builder/KojiModuleBuilder.py @@ -43,7 +43,7 @@ except ImportError: import munch from OpenSSL.SSL import SysCallError -from module_build_service import log, conf +from module_build_service import log, conf, models import module_build_service.scm import module_build_service.utils from module_build_service.builder.utils import execute_cmd @@ -51,6 +51,7 @@ from module_build_service.errors import ProgrammingError from module_build_service.builder.base import GenericBuilder from module_build_service.builder.KojiContentGenerator import KojiContentGenerator +from module_build_service.utils import get_reusable_components, get_reusable_module logging.basicConfig(level=logging.DEBUG) @@ -222,6 +223,68 @@ class KojiModuleBuilder(GenericBuilder): return ready @staticmethod + def _get_filtered_rpms_on_self_dep(module_build, filtered_rpms_of_dep): + # filtered_rpms will contain the NVRs of non-reusable component's RPMs + filtered_rpms = list(set(filtered_rpms_of_dep)) + with models.make_session(conf) as db_session: + # Get a module build that can be reused, which will likely be the + # build dep that is used since it relies on itself + reusable_module = get_reusable_module(db_session, module_build) + if not reusable_module: + return filtered_rpms + koji_session = KojiModuleBuilder.get_session(conf, None) + # Get all the RPMs and builds of the reusable module in Koji + rpms, builds = koji_session.listTaggedRPMS(reusable_module.koji_tag, latest=True) + # Convert the list to a dict where each key is the build_id + builds = {build['build_id']: build for build in builds} + # Create a mapping of package (SRPM) to the RPMs in NVR format + package_to_rpms = {} + for rpm in rpms: + package = builds[rpm['build_id']]['name'] + if package not in package_to_rpms: + package_to_rpms[package] = [] + package_to_rpms[package].append(kobo.rpmlib.make_nvr(rpm)) + + components_in_module = [c.package for c in module_build.component_builds] + reusable_components = get_reusable_components( + db_session, module_build, components_in_module, + previous_module_build=reusable_module) + # Loop through all the reusable components to find if any of their RPMs are + # being filtered + for reusable_component in reusable_components: + # reusable_component will be None if the component can't be reused + if not reusable_component: + continue + # We must get the component name from the NVR and not from + # reusable_component.package because macros such as those used + # by SCLs can change the name of the underlying build + component_name = kobo.rpmlib.parse_nvr(reusable_component.nvr)['name'] + + if component_name not in package_to_rpms: + continue + + # Loop through the RPMs associted with the reusable component + for nvr in package_to_rpms[component_name]: + parsed_nvr = kobo.rpmlib.parse_nvr(nvr) + # Don't compare with the epoch + parsed_nvr['epoch'] = None + # Loop through all the filtered RPMs to find a match with the reusable + # component's RPMs. + for nvr2 in list(filtered_rpms): + parsed_nvr2 = kobo.rpmlib.parse_nvr(nvr2) + # Don't compare with the epoch + parsed_nvr2['epoch'] = None + # Only remove the filter if we are going to reuse a component with + # the same exact NVR + if parsed_nvr == parsed_nvr2: + filtered_rpms.remove(nvr2) + # Since filtered_rpms was cast to a set and then back + # to a list above, we know there won't be duplicate RPMS, + # so we can just break here. + break + return filtered_rpms + + @staticmethod def get_disttag_srpm(disttag, module_build): # Taken from Karsten's create-distmacro-pkg.sh @@ -243,7 +306,13 @@ class KojiModuleBuilder(GenericBuilder): if req_data["filtered_rpms"]: filter_conflicts += "# Filtered rpms from %s module:\n" % ( req_name) - for nvr in req_data["filtered_rpms"]: + # Check if the module depends on itself + if req_name == module_build.name: + filtered_rpms = KojiModuleBuilder._get_filtered_rpms_on_self_dep( + module_build, req_data["filtered_rpms"]) + else: + filtered_rpms = req_data["filtered_rpms"] + for nvr in filtered_rpms: parsed_nvr = kobo.rpmlib.parse_nvr(nvr) filter_conflicts += "Conflicts: %s = %s:%s-%s\n" % ( parsed_nvr["name"], parsed_nvr["epoch"], diff --git a/module_build_service/utils/reuse.py b/module_build_service/utils/reuse.py index 8f39bc9..5f6ffc9 100644 --- a/module_build_service/utils/reuse.py +++ b/module_build_service/utils/reuse.py @@ -167,7 +167,7 @@ def attempt_to_reuse_all_components(builder, session, module): return True -def get_reusable_components(session, module, component_names): +def get_reusable_components(session, module, component_names, previous_module_build=None): """ Returns the list of ComponentBuild instances belonging to previous module build which can be reused in the build of module `module`. @@ -181,6 +181,9 @@ def get_reusable_components(session, module, component_names): :param session: SQLAlchemy database session :param module: the ModuleBuild object of module being built. :param component_names: List of component names to be reused. + :kwarg previous_module_build: the ModuleBuild instance of a module build + which contains the components to reuse. If not passed, get_reusable_module + is called to get the ModuleBuild instance. :return: List of ComponentBuild instances to reuse in the same order as `component_names` """ @@ -188,7 +191,8 @@ def get_reusable_components(session, module, component_names): if conf.system not in ['koji', 'test']: return [None] * len(component_names) - previous_module_build = get_reusable_module(session, module) + if not previous_module_build: + previous_module_build = get_reusable_module(session, module) if not previous_module_build: return [None] * len(component_names) diff --git a/tests/test_builder/test_koji.py b/tests/test_builder/test_koji.py index 0ce9e4f..4495891 100644 --- a/tests/test_builder/test_koji.py +++ b/tests/test_builder/test_koji.py @@ -37,7 +37,7 @@ from module_build_service import glib import pytest from mock import patch, MagicMock -from tests import conf, init_data +from tests import conf, init_data, reuse_component_init_data from module_build_service.builder.KojiModuleBuilder import KojiModuleBuilder @@ -530,3 +530,79 @@ class TestKojiBuilder: ret = KojiModuleBuilder.get_built_rpms_in_module_build(self.module) assert set(ret) == set( ['bar-2:1.30-4.el8+1308+551bfa71', 'tar-2:1.30-4.el8+1308+551bfa71']) + + @pytest.mark.parametrize('br_filtered_rpms,expected', ( + ( + ['perl-Tangerine-0.23-1.module+0+d027b723', 'not-in-tag-5.0-1.module+0+d027b723'], + ['not-in-tag-5.0-1.module+0+d027b723'] + ), + ( + ['perl-Tangerine-0.23-1.module+0+d027b723', + 'perl-List-Compare-0.53-5.module+0+d027b723'], + [] + ), + ( + ['perl-Tangerine-0.23-1.module+0+d027b723', + 'perl-List-Compare-0.53-5.module+0+d027b723', + 'perl-Tangerine-0.23-1.module+0+d027b723'], + [] + ), + ( + ['perl-Tangerine-0.23-1.module+0+diff_module', 'not-in-tag-5.0-1.module+0+d027b723'], + ['perl-Tangerine-0.23-1.module+0+diff_module', 'not-in-tag-5.0-1.module+0+d027b723'] + ), + ( + [], + [] + ), + )) + @patch('module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session') + def test_get_filtered_rpms_on_self_dep(self, get_session, br_filtered_rpms, expected): + session = MagicMock() + session.listTaggedRPMS.return_value = ( + [ + { + 'build_id': 12345, + 'epoch': None, + 'name': 'perl-Tangerine', + 'release': '1.module+0+d027b723', + 'version': '0.23' + }, + { + 'build_id': 23456, + 'epoch': None, + 'name': 'perl-List-Compare', + 'release': '5.module+0+d027b723', + 'version': '0.53' + }, + { + 'build_id': 34567, + 'epoch': None, + 'name': 'tangerine', + 'release': '3.module+0+d027b723', + 'version': '0.22' + } + ], + [ + { + 'build_id': 12345, + 'name': 'perl-Tangerine', + 'nvr': 'perl-Tangerine-0.23-1.module+0+d027b723' + }, + { + 'build_id': 23456, + 'name': 'perl-List-Compare', + 'nvr': 'perl-List-Compare-0.53-5.module+0+d027b723' + }, + { + 'build_id': 34567, + 'name': 'tangerine', + 'nvr': 'tangerine-0.22-3.module+0+d027b723' + } + ] + ) + get_session.return_value = session + reuse_component_init_data() + current_module = module_build_service.models.ModuleBuild.query.get(3) + rv = KojiModuleBuilder._get_filtered_rpms_on_self_dep(current_module, br_filtered_rpms) + assert set(rv) == set(expected)