#2912 dist-repo takes inherited arch when arch is not set
Merged 11 months ago by tkopecek. Opened 11 months ago by jcupova.
jcupova/koji issue-2805  into  master

file modified
+9 -4
@@ -7425,16 +7425,21 @@ 

      taginfo = session.getTag(tag)

      if not taginfo:

          parser.error(_('No such tag: %s') % tag)

+     allowed_arches = taginfo['arches'] or ''

+     if not allowed_arches:

+         for tag_inh in session.getFullInheritance(tag):

+             allowed_arches = session.getTag(tag_inh['parent_id'])['arches'] or ''

+             if allowed_arches:

+                 break

      if len(task_opts.arch) == 0:

-         arches = taginfo['arches'] or ''

-         task_opts.arch = arches.split()

+         task_opts.arch = allowed_arches.split()

          if not task_opts.arch:

              parser.error(_('No arches given and no arches associated with tag'))

      else:

          for a in task_opts.arch:

-             if not taginfo['arches']:

+             if not allowed_arches:

                  warn('Tag %s has an empty arch list' % taginfo['name'])

-             elif a not in taginfo['arches']:

+             elif a not in allowed_arches:

                  warn('%s is not in the list of tag arches' % a)

      if task_opts.multilib:

          if not os.path.exists(task_opts.multilib):

@@ -28,6 +28,28 @@ 

          'maven_include_all': False,

          'perm_id': None

      }

+     TAG_2 = {

+         'maven_support': False,

+         'locked': False,

+         'name': 'fedora28-build',

+         'extra': {},

+         'perm': None,

+         'id': 3,

+         'arches': '',

+         'maven_include_all': False,

+         'perm_id': None

+     }

+     INHERITANCE = [{'child_id': 3,

+                     'currdepth': 1,

+                     'filter': [],

+                     'intransitive': False,

+                     'maxdepth': None,

+                     'name': 'fedora-26-build',

+                     'nextdepth': None,

+                     'noconfig': False,

+                     'parent_id': 2,

+                     'pkg_filter': '',

+                     'priority': 0}]

  

      def setUp(self):

          self.task_id = 1001
@@ -150,7 +172,7 @@ 

              [self.tag_name, self.fake_key],

              stderr=expected)

  

-         # Case 3. Arch field is empty in Tag

+         # Case 3. Arch field is empty in Tag (without inheritance)

          tag = copy.copy(self.TAG)

          tag.update({'arches': None})

          self.session.getTag.return_value = tag
@@ -162,6 +184,24 @@ 

              [self.tag_name, self.fake_key],

              stderr=expected)

  

+         # Case 4. Arch field is empty in Tag (with inheritance)

+         tag_1 = copy.copy(self.TAG)

+         tag_1.update({'arches': None})

+         tag_2 = copy.copy(self.TAG)

+         tag_2.update({'arches': None})

+         self.session.getTag.side_effect = [tag_2, tag_1]

+         self.session.getFullInheritance.return_value = self.INHERITANCE

+         expected = self.format_error_message('No arches given and no arches associated with tag')

+         self.assert_system_exit(

+             handle_dist_repo,

+             self.options,

+             self.session,

+             [tag_2['name'], self.fake_key],

+             stderr=expected)

+         expected_calls = [mock.call(tag_2['name']), mock.call(tag_1['id'])]

+         self.session.getTag.assert_has_calls(expected_calls)

+         self.session.getFullInheritance.assert_called_with(tag_2['name'])

+ 

      def test_handle_dist_repo_with_comp(self):

          comp_file = 'comp.xml'

          arguments = [self.tag_name, self.fake_key, '--comp', comp_file]
@@ -188,6 +228,8 @@ 

          for a in arches:

              arguments += ['--arch', a]

  

+         self.session.getFullInheritance.return_value = []

+         self.session.getTag.return_value = {'arches': 'x86_64', 'id': 1, 'name': self.tag_name}

          expected_warn = '%s is not in the list of tag arches' % arches[0] + "\n"

          expected_warn += '%s is not in the list of tag arches' % arches[2] + "\n"

          expected_warn += '%s is not in the list of tag arches' % arches[3] + "\n"

It could be simplified to if taginfo['arches']

1) That cycle should stop on first tag which has any arch.
2) There is a whole second branch (when user specifies some arch) which compares that arch again against taginfo. This should use same test. I would place whole inheritance check before the "if" and have something like "allowed_archs" which can be used in both parts.

rebased onto 7dc4444c14d5f0d58199118dd470c69edfa45e60

11 months ago

rebased onto 9e0fbfc150e24a033fcaa5b4eb98c5a65e5a8f25

11 months ago

rebased onto 1c67966

11 months ago

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

11 months ago

Metadata Update from @mfilip:
- Pull-request tagged with: testing-done

11 months ago

Commit 562e970 fixes this pull-request

Pull-Request has been merged by tkopecek

11 months ago