#2564 external repos can have specified arch list
Merged 4 years ago by tkopecek. Opened 4 years ago by tkopecek.
tkopecek/koji issue2538  into  master

file modified
+3
@@ -5550,6 +5550,9 @@ 

          # generate repo url list, starting with our local premerge repo

          repos = ['file://' + localdir + '/']

          for repo in external_repos:

+             if repo['arches'] and arch not in repo['arches'].split():

+                 # ignore external repo with non-relevant archlist

+                 continue

              ext_url = repo['url']

              # substitute $arch in the url with the arch of the repo we're generating

              ext_url = ext_url.replace('$arch', arch)

file modified
+13 -2
@@ -4973,8 +4973,11 @@ 

          if external_repos:

              print("External repos:")

              for rinfo in external_repos:

+                 if not rinfo['arches']:

+                     rinfo['arches'] = 'inherited from tag'

+                     # TODO else intersection of arches?

                  print("  %(priority)3i %(external_repo_name)s "

-                       "(%(url)s, merge mode: %(merge_mode)s)" % rinfo)

+                       "(%(url)s, merge mode: %(merge_mode)s), arches: %(arches)s" % rinfo)

          print("Inheritance:")

          for parent in session.getInheritanceData(info['id'], **event_opts):

              parent['flags'] = format_inheritance_flags(parent)
@@ -5520,6 +5523,8 @@ 

      parser.add_option("-p", "--priority", type='int',

                        help=_("Set priority (when adding to tag)"))

      parser.add_option("-m", "--mode", help=_("Set merge mode"))

+     parser.add_option("-a", "--arches", metavar="ARCH1,ARCH2, ...",

+                       help=_("Use only subset of arches from given repo"))

      (options, args) = parser.parse_args(args)

      activate_session(session, goptions)

      if options.mode:
@@ -5549,6 +5554,8 @@ 

              callopts = {}

              if options.mode:

                  callopts['merge_mode'] = options.mode

+             if options.arches:

+                 callopts['arches'] = options.arches

              session.addExternalRepoToTag(tag, rinfo['name'], priority, **callopts)

              print("Added external repo %s to tag %s (priority %i)"

                    % (rinfo['name'], tag, priority))
@@ -5567,6 +5574,8 @@ 

      parser.add_option("-m", "--mode", metavar="MODE",

                        help=_("Edit the merge mode of the repo for the tag specified by --tag. "

                               "Options: %s.") % ", ".join(koji.REPO_MERGE_MODES))

+     parser.add_option("-a", "--arches", metavar="ARCH1,ARCH2, ...",

+                       help=_("Use only subset of arches from given repo"))

      (options, args) = parser.parse_args(args)

      if len(args) != 1:

          parser.error(_("Incorrect number of arguments"))
@@ -5581,12 +5590,14 @@ 

              tag_repo_opts['priority'] = options.priority

          if options.mode:

              tag_repo_opts['merge_mode'] = options.mode

+         if options.arches is not None:

+             tag_repo_opts['arches'] = options.arches

          if not tag_repo_opts:

              parser.error(_("At least, one of priority and merge mode should be specified"))

          tag_repo_opts['tag_info'] = options.tag

          tag_repo_opts['repo_info'] = args[0]

      else:

-         for k in ('priority', 'mode'):

+         for k in ('priority', 'mode', 'arches'):

              if getattr(options, k) is not None:

                  parser.error(_("If %s is specified, --tag must be specified as well") % k)

  

@@ -0,0 +1,9 @@ 

+ -- upgrade script to migrate the Koji database schema

+ -- from version 1.23 to 1.24

+ 

+ 

+ BEGIN;

+ 

+ ALTER TABLE tag_external_repos ADD COLUMN arches TEXT;

+ 

+ COMMIT;

file modified
+3 -2
@@ -465,7 +465,7 @@ 

  -- fake repo id for internal stuff (needed for unique index)

  INSERT INTO external_repo (id, name) VALUES (0, 'INTERNAL');

  

- create table external_repo_config (

+ CREATE TABLE external_repo_config (

  	external_repo_id INTEGER NOT NULL REFERENCES external_repo(id),

  	url TEXT NOT NULL,

  -- versioned - see earlier description of versioning
@@ -481,11 +481,12 @@ 

  	UNIQUE (external_repo_id, active)

  ) WITHOUT OIDS;

  

- create table tag_external_repos (

+ CREATE TABLE tag_external_repos (

  	tag_id INTEGER NOT NULL REFERENCES tag(id),

  	external_repo_id INTEGER NOT NULL REFERENCES external_repo(id),

  	priority INTEGER NOT NULL,

  	merge_mode TEXT NOT NULL DEFAULT 'koji',

+ 	arches TEXT,

  -- versioned - see earlier description of versioning

  	create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(),

  	revoke_event INTEGER REFERENCES events(id),

file modified
+16 -7
@@ -3675,7 +3675,7 @@ 

      update.execute()

  

  

- def add_external_repo_to_tag(tag_info, repo_info, priority, merge_mode='koji'):

+ def add_external_repo_to_tag(tag_info, repo_info, priority, merge_mode='koji', arches=None):

      """Add an external repo to a tag"""

  

      context.session.assertPerm('tag')
@@ -3691,6 +3691,9 @@ 

      repo = get_external_repo(repo_info, strict=True)

      repo_id = repo['id']

  

+     if arches is not None:

+         arches = koji.parse_arches(arches, strict=True)

+ 

      tag_repos = get_tag_external_repos(tag_info=tag_id)

      if [tr for tr in tag_repos if tr['external_repo_id'] == repo_id]:

          raise koji.GenericError('tag %s already associated with external repo %s' %
@@ -3701,7 +3704,7 @@ 

  

      insert = InsertProcessor('tag_external_repos')

      insert.set(tag_id=tag_id, external_repo_id=repo_id, priority=priority,

-                merge_mode=merge_mode)

+                merge_mode=merge_mode, arches=arches)

      insert.make_create()

      insert.execute()

  
@@ -3726,7 +3729,7 @@ 

      update.execute()

  

  

- def edit_tag_external_repo(tag_info, repo_info, priority=None, merge_mode=None):

+ def edit_tag_external_repo(tag_info, repo_info, priority=None, merge_mode=None, arches=None):

      """Edit a tag<->external repo association

      This allows you to update the priority and merge_mode without removing/adding the repo.

  
@@ -3746,8 +3749,11 @@ 

                                  (repo['name'], tag['name']))

      tag_repo = tag_repos[0]

  

+     if arches is not None:

+         arches = koji.parse_arches(arches, strict=True)

+ 

      data = {}

-     for k in ('priority', 'merge_mode'):

+     for k in ('priority', 'merge_mode', 'arches'):

          val = locals().get(k)

          # None value means no change

          if val is not None and val != tag_repo[k]:
@@ -3755,7 +3761,7 @@ 

      if not data:

          return False

      else:

-         for k in ('priority', 'merge_mode'):

+         for k in ('priority', 'merge_mode', 'arches'):

              data.setdefault(k, tag_repo[k])

          remove_external_repo_from_tag(tag_id, repo_id)

          add_external_repo_to_tag(tag_id, repo_id, **data)
@@ -3800,6 +3806,7 @@ 

          'tag.name': 'tag_name',

          'url': 'url',

          'merge_mode': 'merge_mode',

+         'arches': 'arches',

      }

      columns, aliases = zip(*fields.items())

  
@@ -10549,7 +10556,7 @@ 

      deleteExternalRepo = staticmethod(delete_external_repo)

  

      def addExternalRepoToTag(self, tag_info, repo_info, priority,

-                              merge_mode='koji'):

+                              merge_mode='koji', arches=None):

          """Add an external repo to a tag.

  

          :param tag_info: Tag name or ID number
@@ -10558,9 +10565,11 @@ 

          :param str merge_mode: This must be one of the values of the

                                 koji.REPO_MERGE_MODES set. If unspecified,

                                 the default is "koji".

+         :param str arches: space-separated list of arches handled by the repo.

          """

          # wrap the local method so we don't expose the event parameter

-         add_external_repo_to_tag(tag_info, repo_info, priority, merge_mode)

+         add_external_repo_to_tag(tag_info, repo_info, priority,

+                                  merge_mode=merge_mode, arches=arches)

  

      def removeExternalRepoFromTag(self, tag_info, repo_info):

          """

@@ -91,6 +91,8 @@ 

                          --tag.

    -m MODE, --mode=MODE  Edit the merge mode of the repo for the tag specified

                          by --tag. Options: %s.

+   -a ARCH1,ARCH2, ..., --arches=ARCH1,ARCH2, ...

+                         Use only subset of arches from given repo

  """ % (self.progname, ', '.join(koji.REPO_MERGE_MODES)))

  

  

@@ -18,7 +18,8 @@ 

          self.get_tag_external_repos.return_value = [{'external_repo_id': 11,

                                                       'tag_id': 1,

                                                       'priority': 5,

-                                                      'merge_mode': 'simple'}]

+                                                      'merge_mode': 'simple',

+                                                      'arches': 'x86_64 i686'}]

  

          self.remove_external_repo_from_tag = mock.patch(

              'kojihub.remove_external_repo_from_tag').start()
@@ -33,7 +34,8 @@ 

          self.get_external_repo.assert_called_once_with('ext_repo', strict=True)

          self.get_tag_external_repos.assert_called_once_with(tag_info=1, repo_info=11)

          self.remove_external_repo_from_tag.assert_called_once_with(1, 11)

-         self.add_external_repo_to_tag.assert_called_once_with(1, 11, priority=6, merge_mode='bare')

+         self.add_external_repo_to_tag.assert_called_once_with(1, 11, priority=6, merge_mode='bare',

+                                                               arches='x86_64 i686')

          self.assertTrue(rv)

  

      def test_edit_no_tag_repo(self):
@@ -74,9 +76,10 @@ 

          self.get_tag_external_repos.return_value = [{'external_repo_id': 11,

                                                       'tag_id': 1,

                                                       'priority': 5,

-                                                      'merge_mode': None}]

+                                                      'merge_mode': None,

+                                                      'arches': None}]

          rv = kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=None, merge_mode='simple')

          self.remove_external_repo_from_tag.assert_called_once_with(1, 11)

-         self.add_external_repo_to_tag.assert_called_once_with(1, 11,

+         self.add_external_repo_to_tag.assert_called_once_with(1, 11, arches=None,

                                                                priority=5, merge_mode='simple')

          self.assertTrue(rv)

This needs clearer handling of the separators. In particular, the builder code expects space (repo['arches'].split()), but the cli suggests commas (metavar="ARCH1,ARCH2, ...").

Elsewhere in Koji, we tend to use spaces to separate arches in a list since this makes splitting easier. However, passing a space separated list to an --arch option is awkward because of the quoting needed. I suggest accepting either one in the cli and passing a space separated list to the hub call. I thought we had code to do this for archlists somewhere else, but I can't seem to find it right now.

ah, yes, we've that koji.parse_arches - added

rebased onto 4e6b8d55f412ae8af7a6f1d8782be0b626648304

4 years ago

1 new commit added

  • hub: use CTE for build_references
4 years ago

2 new commits added

  • parse arches
  • external repos can have specified arch list
4 years ago

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

4 years ago

rebased onto a2523ee7c9da97944b83315ad04f4171edf964dc

4 years ago

rebased onto c79114b4b381bb1c594823175504f03bd32bc128

4 years ago

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

4 years ago

ah, yes, we've that koji.parse_arches - added

I don't see this in the current change set. Was it lost somewhere in the rebase?

Without it, my concerns above still apply.

I think the best place to handle this is in the hub call. I'd like to be sure that the arch list is sanitized to space separated before it goes into the db.

1 new commit added

  • parse arches
4 years ago

Hmm, bad rebase. Updated.

Metadata Update from @tkopecek:
- Pull-request untagged with: testing-done

4 years ago

rebased onto 99f440f

4 years ago

2 new commits added

  • parse arches
  • external repos can have specified arch list
4 years ago

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

4 years ago

Commit 5e3e7c4 fixes this pull-request

Pull-Request has been merged by tkopecek

4 years ago