#959 Make repo_include_all setting configurable in xmd
Merged a year ago by ralph. Opened a year ago by mizdebsk.
mizdebsk/fm-orchestrator fix-957  into  master

@@ -838,6 +838,10 @@ 

              'repo_include_all': True,

          }

  

+         xmd = self.mmd.get_xmd()

+         if "mbs_options" in xmd.keys() and "repo_include_all" in xmd["mbs_options"].keys():

+             opts['extra']['repo_include_all'] = xmd["mbs_options"]["repo_include_all"]

+ 

          # edit tag with opts

          self.koji_session.editTag2(tag_name, **opts)

          return self._get_tag(tag_name)  # Return up2date taginfo

@@ -414,7 +414,8 @@ 

  

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

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

-     def test_buildroot_connect(self, custom_whitelist, blocklist):

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

+     def test_buildroot_connect(self, custom_whitelist, blocklist, repo_include_all):

          if blocklist:

              mmd = self.module.mmd()

              xmd = glib.from_variant_dict(mmd.get_xmd())

@@ -429,6 +430,15 @@ 

              mmd.set_buildopts(opts)

              self.module.modulemd = mmd.dumps()

  

+         if repo_include_all is False:

+             mmd = self.module.mmd()

+             xmd = glib.from_variant_dict(mmd.get_xmd())

+             mbs_options = xmd["mbs_options"] if "mbs_options" in xmd.keys() else {}

+             mbs_options["repo_include_all"] = False

+             xmd["mbs_options"] = mbs_options

+             mmd.set_xmd(glib.dict_values(xmd))

+             self.module.modulemd = mmd.dumps()

+ 

          builder = FakeKojiModuleBuilder(

              owner=self.module.owner, module=self.module, config=conf, tag_name='module-foo',

              components=["nginx"])

@@ -466,6 +476,14 @@ 

          expected_calls = []

          assert session.packageListBlock.mock_calls == expected_calls

  

+         expected_calls = [mock.call('module-foo', arches='i686 armv7hl x86_64',

+                                     extra={'mock.package_manager': 'dnf',

+                                            'repo_include_all': repo_include_all}),

+                           mock.call('module-foo-build', arches='i686 armv7hl x86_64',

+                                     extra={'mock.package_manager': 'dnf',

+                                            'repo_include_all': repo_include_all})]

+         assert session.editTag2.mock_calls == expected_calls

+ 

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

      def test_buildroot_connect_create_tag(self, blocklist):

          if blocklist:

I'm fine with this but I'll leave it open to leave others time to review it.

Hm, this should probably affect component re-use. i.e., if it changes from False to True or vice versa, no components are re-usable.

FWIW, seems fine here too, generally speaking. @mizdebsk - wdyt about the component re-use question?

@mizdebsk, we've got some merge conflicts here now. Do you have time to look into it?

Sorry for delayed reply

I would not be concerned about component reuse in this case:
1. First, this option only helps with some corner cases, but generally for most packages buildroot should be the same with the option set to either value. So for most packages it does not matter.
2. I think that packager toggling advanced and undocumented options like this can be expected to be aware about possible component reuse problems and submit module build with reuse disabled when necessary.
3. Component reuse is not perfect, and there are more important cases in which component reuse could be a problem, like when something in platform changes and component is reused anyway.

I will rebase this PR to fix merge conflicts.

rebased onto c549690981d0ecb887a1d278ef36ae058a4abfe6

a year ago

Makes sense, and I agree.

Here, I get

___________________________________________ TestKojiBuilder.test_buildroot_connect[False-False-False] ____________________________________________

self = <test_koji.TestKojiBuilder instance at 0x7fccf4194a28>, custom_whitelist = False, blocklist = False, repo_include_all = False

    @pytest.mark.parametrize('blocklist', [False, True])
    @pytest.mark.parametrize('custom_whitelist', [False, True])
    @pytest.mark.parametrize('repo_include_all', [False, True])
    def test_buildroot_connect(self, custom_whitelist, blocklist, repo_include_all):
    if blocklist:
        mmd = self.module.mmd()
        xmd = glib.from_variant_dict(mmd.get_xmd())
        xmd["mbs_options"] = {"blocked_packages": ["foo", "nginx"]}
        mmd.set_xmd(glib.dict_values(xmd))
        self.module.modulemd = mmd.dumps()

    if custom_whitelist:
        mmd = self.module.mmd()
        opts = mmd.get_buildopts()
        opts.set_rpm_whitelist(['custom1', 'custom2'])
        mmd.set_buildopts(opts)
        self.module.modulemd = mmd.dumps()

    if repo_include_all is False:
        mmd = self.module.mmd()
        xmd = glib.from_variant_dict(mmd.get_xmd())
>           xmd.getdefault("mbs_options", {})["repo_include_all"] = "false"
E           AttributeError: 'dict' object has no attribute 'getdefault'

tests/test_builder/test_koji.py:436: AttributeError

@mizdebsk, should we change that getdefault to just a plain old get?

rebased onto 0d29633

a year ago

Tests pass here now. Thanks @mizdebsk!

Pull-Request has been merged by ralph

a year ago