#988 Don't filter RPMs of reused components from a module that depends on itself
Merged 11 months ago by mprahl. Opened a year ago by mprahl.

@@ -43,7 +43,7 @@ 

  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.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 @@ 

          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 @@ 

              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"],

@@ -76,7 +76,7 @@ 

      ]

  

  

- 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 @@ 

      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

  

@@ -167,7 +167,7 @@ 

      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 @@ 

      :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 @@ 

      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)

  

@@ -215,7 +219,7 @@ 

      :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 +244,7 @@ 

          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

  

@@ -37,7 +37,7 @@ 

  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 @@ 

          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)

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.

rebased onto f422b82

a year ago

@jkaluza and @ralph could you please review?

2 new commits added

  • Don't filter RPMs of reused components from a module that depends on itself
  • Make _get_reusable_module a public method so it can be used elsewhere
a year ago

Thanks for the review @ralph. @jkaluza could you also review?

+1, there is just old variable name in comment, but you can merge even without fixing that.

2 new commits added

  • Don't filter RPMs of reused components from a module that depends on itself
  • Make _get_reusable_module a public method so it can be used elsewhere
11 months ago

@jkaluza thanks for the review. I fixed the comment and I'll wait for the CI to pass before merging.

Pull-Request has been merged by mprahl

11 months ago