From 6191d46d129c264e820d5c3f09709cb3e902ce81 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 06 2020 12:21:05 +0000 Subject: [PATCH 1/7] clone-tag --no-delete mode Fixes: https://pagure.io/koji/issue/1384 --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 8880732..3350731 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -3377,6 +3377,11 @@ def handle_clone_tag(goptions, session, args): "the dest tag")) parser.add_option('--ts', type='int', metavar="TIMESTAMP", help=_('Clone tag at last event before specific timestamp')) + parser.add_option('--no-delete', action='store_false', dest="delete", + default=True, + help=_("Don't delete any existing content in dest tag. " + "Note, that you can end with older latest builds in dest " + "than in src, if they are already tagged.")) parser.add_option('--event', type='int', help=_('Clone tag at a specific event')) parser.add_option('--repo', type='int', @@ -3644,27 +3649,28 @@ def handle_clone_tag(goptions, session, args): if not options.test: session.multiCall(batch=options.batch) # DEL builds. To keep the order we should untag builds at first - if not options.test: - session.multicall = True - for build in bdellist: - # don't delete an inherited build. - if build['tag_name'] == dsttag['name']: - # add missing 'name' field - build['name'] = build['package_name'] - chgbldlist.append(('[del]', - build['package_name'], - build['nvr'], - koji.BUILD_STATES[build['state']], - build['owner_name'], - build['tag_name'])) - # go on del builds from new tag. - if not options.test: - session.untagBuildBypass(dsttag['name'], - build, - force=options.force, - notify=options.notify) - if not options.test: - session.multiCall(batch=options.batch) + if options.delete: + if not options.test: + session.multicall = True + for build in bdellist: + # don't delete an inherited build. + if build['tag_name'] == dsttag['name']: + # add missing 'name' field + build['name'] = build['package_name'] + chgbldlist.append(('[del]', + build['package_name'], + build['nvr'], + koji.BUILD_STATES[build['state']], + build['owner_name'], + build['tag_name'])) + # go on del builds from new tag. + if not options.test: + session.untagBuildBypass(dsttag['name'], + build, + force=options.force, + notify=options.notify) + if not options.test: + session.multiCall(batch=options.batch) # ADD builds. if not options.test: session.multicall = True @@ -3715,103 +3721,104 @@ def handle_clone_tag(goptions, session, args): force=options.force) if not options.test: session.multiCall(batch=options.batch) - # DEL packages. - ninhrtpdellist = [] - inhrtpdellist = [] - for pkg in pdellist: - if pkg['tag_name'] == dsttag['name']: - ninhrtpdellist.append(pkg) - else: - inhrtpdellist.append(pkg) - session.multicall = True - # delete only non-inherited packages. - for pkg in ninhrtpdellist: - # check if package have owned builds inside. - session.listTagged(dsttag['name'], - package=pkg['package_name'], - inherit=False) - bump_builds = session.multiCall(batch=options.batch) - if not options.test: + if options.delete: + # DEL packages + ninhrtpdellist = [] + inhrtpdellist = [] + for pkg in pdellist: + if pkg['tag_name'] == dsttag['name']: + ninhrtpdellist.append(pkg) + else: + inhrtpdellist.append(pkg) session.multicall = True - for pkg, [builds] in zip(ninhrtpdellist, bump_builds): - # remove all its builds first if there are any. - for build in builds: - # add missing 'name' field. - build['name'] = build['package_name'] - chgbldlist.append(('[del]', - build['package_name'], - build['nvr'], - koji.BUILD_STATES[build['state']], - build['owner_name'], - build['tag_name'])) - # so delete latest build(s) from new tag. - if not options.test: - session.untagBuildBypass(dsttag['name'], - build, - force=options.force, - notify=options.notify) - # now safe to remove package itself since we resolved its builds. - chgpkglist.append(('[del]', - pkg['package_name'], - pkg['blocked'], - pkg['owner_name'], - pkg['tag_name'])) + # delete only non-inherited packages. + for pkg in ninhrtpdellist: + # check if package have owned builds inside. + session.listTagged(dsttag['name'], + package=pkg['package_name'], + inherit=False) + bump_builds = session.multiCall(batch=options.batch) if not options.test: - session.packageListRemove(dsttag['name'], - pkg['package_name'], - force=False) - # mark as blocked inherited packages. - for pkg in inhrtpdellist: - chgpkglist.append(('[blk]', - pkg['package_name'], - pkg['blocked'], - pkg['owner_name'], - pkg['tag_name'])) - if not options.test: - session.packageListBlock(dsttag['name'], pkg['package_name']) - if not options.test: - session.multiCall(batch=options.batch) - # DEL groups. - if not options.test: - session.multicall = True - for group in gdellist: - # Only delete a group that isn't inherited - if group['tag_id'] == dsttag['id']: + session.multicall = True + for pkg, [builds] in zip(ninhrtpdellist, bump_builds): + # remove all its builds first if there are any. + for build in builds: + # add missing 'name' field. + build['name'] = build['package_name'] + chgbldlist.append(('[del]', + build['package_name'], + build['nvr'], + koji.BUILD_STATES[build['state']], + build['owner_name'], + build['tag_name'])) + # so delete latest build(s) from new tag. + if not options.test: + session.untagBuildBypass(dsttag['name'], + build, + force=options.force, + notify=options.notify) + # now safe to remove package itself since we resolved its builds. + chgpkglist.append(('[del]', + pkg['package_name'], + pkg['blocked'], + pkg['owner_name'], + pkg['tag_name'])) if not options.test: - session.groupListRemove(dsttag['name'], - group['name'], - force=options.force) - for pkg in group['packagelist']: - chggrplist.append(('[del]', pkg['package'], group['name'])) - # mark as blocked inherited groups. - else: + session.packageListRemove(dsttag['name'], + pkg['package_name'], + force=False) + # mark as blocked inherited packages. + for pkg in inhrtpdellist: + chgpkglist.append(('[blk]', + pkg['package_name'], + pkg['blocked'], + pkg['owner_name'], + pkg['tag_name'])) if not options.test: - session.groupListBlock(dsttag['name'], group['name']) - for pkg in group['packagelist']: - chggrplist.append(('[blk]', pkg['package'], group['name'])) - if not options.test: - session.multiCall(batch=options.batch) - # DEL group pkgs. - if not options.test: - session.multicall = True - for group in grpchanges: - for pkg in grpchanges[group]['dels']: + session.packageListBlock(dsttag['name'], pkg['package_name']) + if not options.test: + session.multiCall(batch=options.batch) + # DEL groups. + if not options.test: + session.multicall = True + for group in gdellist: # Only delete a group that isn't inherited - if not grpchanges[group]['inherited']: - chggrplist.append(('[del]', pkg, group)) + if group['tag_id'] == dsttag['id']: if not options.test: - session.groupPackageListRemove(dsttag['name'], - group, - pkg, - force=options.force) + session.groupListRemove(dsttag['name'], + group['name'], + force=options.force) + for pkg in group['packagelist']: + chggrplist.append(('[del]', pkg['package'], group['name'])) + # mark as blocked inherited groups. else: - chggrplist.append(('[blk]', pkg, group)) if not options.test: - session.groupPackageListBlock(dsttag['name'], - group, - pkg) - if not options.test: - session.multiCall(batch=options.batch) + session.groupListBlock(dsttag['name'], group['name']) + for pkg in group['packagelist']: + chggrplist.append(('[blk]', pkg['package'], group['name'])) + if not options.test: + session.multiCall(batch=options.batch) + # DEL group pkgs. + if not options.test: + session.multicall = True + for group in grpchanges: + for pkg in grpchanges[group]['dels']: + # Only delete a group that isn't inherited + if not grpchanges[group]['inherited']: + chggrplist.append(('[del]', pkg, group)) + if not options.test: + session.groupPackageListRemove(dsttag['name'], + group, + pkg, + force=options.force) + else: + chggrplist.append(('[blk]', pkg, group)) + if not options.test: + session.groupPackageListBlock(dsttag['name'], + group, + pkg) + if not options.test: + session.multiCall(batch=options.batch) # print final list of actions. if options.verbose: pfmt = ' %-7s %-28s %-10s %-10s %-10s\n' diff --git a/tests/test_cli/test_clone_tag.py b/tests/test_cli/test_clone_tag.py index 1fde1ce..5b64bef 100644 --- a/tests/test_cli/test_clone_tag.py +++ b/tests/test_cli/test_clone_tag.py @@ -622,6 +622,256 @@ List of changes: [blk] cpkg group2 """) + @mock.patch('sys.stdout', new_callable=six.StringIO) + def test_handle_clone_tag_existing_dsttag_add(self, stdout): + args = ['src-tag', 'dst-tag', '--all', '-v', '--no-delete'] + self.session.multiCall.return_value = [] + self.session.listPackages.side_effect = [[{'package_id': 1, + 'package_name': 'pkg1', + 'blocked': False, + 'owner_name': 'userA', + 'tag_name': 'src-tag', + 'extra_arches': None}, + {'package_id': 2, + 'package_name': 'pkg2', + 'blocked': True, + 'owner_name': 'userB', + 'tag_name': 'src-tag-p', + 'extra_arches': 'arch3 arch4'}, + {'package_id': 3, + 'package_name': 'apkg', + 'blocked': False, + 'owner_name': 'userA', + 'tag_name': 'src-tag-p', + 'extra_arches': 'arch4'}], + [{'package_id': 1, + 'package_name': 'pkg1', + 'blocked': False, + 'owner_name': 'userA', + 'tag_name': 'src-tag', + 'extra_arches': None}, + {'package_id': 3, + 'package_name': 'apkg', + 'blocked': False, + 'owner_name': 'userA', + 'tag_name': 'src-tag-p', + 'extra_arches': 'arch4'}, + {'package_id': 4, + 'package_name': 'bpkg', + 'blocked': False, + 'owner_name': 'userC', + 'tag_name': 'src-tag', + 'extra_arches': 'arch4'}, + {'package_id': 5, + 'package_name': 'cpkg', + 'blocked': True, + 'owner_name': 'userC', + 'tag_name': 'src-tag-p', + 'extra_arches': 'arch4'}, + {'package_id': 6, + 'package_name': 'dpkg', + 'blocked': True, + 'owner_name': 'userC', + 'tag_name': 'src-tag', + 'extra_arches': 'arch4'} + ]] + self.session.listTagged.side_effect = [[{'package_name': 'pkg1', + 'nvr': 'pkg1-1.1-2', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'src-tag'}, + {'package_name': 'pkg1', + 'nvr': 'pkg1-1.0-2', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'src-tag'}, + {'package_name': 'pkg1', + 'nvr': 'pkg1-0.1-1', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'src-tag'}, + {'package_name': 'pkg1', + 'nvr': 'pkg1-1.0-1', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'src-tag'}, + {'package_name': 'pkg2', + 'nvr': 'pkg2-1.0-1', + 'state': 2, + 'owner_name': 'b_owner', + 'tag_name': 'src-tag-p'} + ], + [{'package_name': 'pkg1', + 'nvr': 'pkg1-2.1-2', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'dst-tag'}, + {'package_name': 'pkg1', + 'nvr': 'pkg1-1.0-1', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'dst-tag'}, + {'package_name': 'pkg1', + 'nvr': 'pkg1-0.1-1', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'dst-tag'}, + {'package_name': 'pkg2', + 'nvr': 'pkg2-1.0-1', + 'state': 2, + 'owner_name': 'b_owner', + 'tag_name': 'dst-tag'}, + {'package_name': 'pkg3', + 'nvr': 'pkg3-1.0-1', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'dst-tag'} + ]] + self.session.getTagGroups.side_effect = [[{'name': 'group1', + 'tag_id': 1, + 'packagelist': [ + {'package': 'pkg1', + 'blocked': False}, + {'package': 'pkg2', + 'blocked': False}, + {'package': 'pkg3', + 'blocked': False}, + {'package': 'pkg4', + 'blocked': False} + ]}, + {'name': 'group2', + 'tag_id': 1, + 'packagelist': [ + {'package': 'apkg', + 'blocked': False}, + {'package': 'bpkg', + 'blocked': False}] + }], + [{'name': 'group1', + 'tag_id': 2, + 'packagelist': [ + {'package': 'pkg1', + 'blocked': False}, + {'package': 'pkg5', + 'blocked': False} + ]}, + {'name': 'group2', + 'tag_id': 3, + 'packagelist': [ + {'package': 'apkg', + 'blocked': False}, + {'package': 'cpkg', + 'blocked': False}]}, + {'name': 'group3', + 'tag_id': 2, + 'packagelist': [ + {'package': 'cpkg', + 'blocked': False}, + {'package': 'dpkg', + 'blocked': False}]}, + {'name': 'group4', + 'tag_id': 3, + 'packagelist': [ + {'package': 'epkg', + 'blocked': False}, + {'package': 'fpkg', + 'blocked': False}]} + ]] + self.session.getTag.side_effect = [{'id': 1, + 'name': 'src-tag', + 'arches': 'arch1 arch2', + 'perm_id': 1, + 'maven_support': False, + 'maven_include_all': True, + 'locked': False}, + {'id': 2, + 'name': 'dst-tag', + 'arches': 'arch1 arch2', + 'perm_id': 1, + 'maven_support': False, + 'maven_include_all': True, + 'locked': False}] + handle_clone_tag(self.options, self.session, args) + self.activate_session.assert_called_once() + self.session.assert_has_calls([call.hasPerm('admin'), + call.getTag('src-tag'), + call.getTag('dst-tag'), + call.listPackages(event=None, + inherited=True, + tagID=1), + call.listPackages(inherited=True, + tagID=2), + call.listTagged(1, event=None, + inherit=None, + latest=None), + call.listTagged(2, inherit=False, + latest=False), + call.getTagGroups('src-tag', + event=None), + call.getTagGroups('dst-tag'), + call.packageListAdd('dst-tag', 'pkg2', + block=True, + extra_arches='arch3 arch4', + owner='userB'), + call.multiCall(batch=1000), + call.tagBuildBypass('dst-tag', { + 'owner_name': 'b_owner', + 'nvr': 'pkg1-0.1-1', + 'package_name': 'pkg1', 'state': 1, + 'tag_name': 'src-tag', + 'name': 'pkg1'}, force=None), + call.tagBuildBypass('dst-tag', { + 'owner_name': 'b_owner', + 'nvr': 'pkg1-1.0-2', + 'package_name': 'pkg1', 'state': 1, + 'tag_name': 'src-tag', + 'name': 'pkg1'}, force=None), + call.tagBuildBypass('dst-tag', { + 'owner_name': 'b_owner', + 'nvr': 'pkg1-1.1-2', + 'package_name': 'pkg1', 'state': 1, + 'tag_name': 'src-tag', + 'name': 'pkg1'}, force=None), + call.multiCall(batch=1000), + call.multiCall(batch=1000), + call.groupPackageListAdd('dst-tag', + 'group1', + 'pkg2', + force=None), + call.groupPackageListAdd('dst-tag', + 'group1', + 'pkg3', + force=None), + call.groupPackageListAdd('dst-tag', + 'group1', + 'pkg4', + force=None), + call.groupPackageListAdd('dst-tag', + 'group2', + 'bpkg', + force=None), + call.multiCall(batch=1000)]) + self.assert_console_message(stdout, """ +List of changes: + + Action Package Blocked Owner From Tag + ------- ---------------------------- ---------- ---------- ---------- + [add] pkg2 True userB src-tag-p + + Action From/To Package Build(s) State Owner From Tag + ------- ---------------------------- ---------------------------------------- ---------- ---------- ---------- + [add] pkg1 pkg1-0.1-1 COMPLETE b_owner src-tag + [add] pkg1 pkg1-1.0-2 COMPLETE b_owner src-tag + [add] pkg1 pkg1-1.1-2 COMPLETE b_owner src-tag + + Action Package Group + ------- ---------------------------- ---------------------------- + [new] pkg2 group1 + [new] pkg3 group1 + [new] pkg4 group1 + [new] bpkg group2 +""") + def test_handle_clone_tag_help(self): self.assert_help( handle_clone_tag, @@ -640,6 +890,9 @@ Options: --inherit-builds Include all builds inherited into the source tag into the dest tag --ts=TIMESTAMP Clone tag at last event before specific timestamp + --no-delete Don't delete any existing content in dest tag. Note, that + you can end with older latest builds in dest than in src, + if they are already tagged. --event=EVENT Clone tag at a specific event --repo=REPO Clone tag at a specific repo event -v, --verbose show changes From c6f7448f786143a6f5c1b35c657e7ddd5dba6fba Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 06 2020 12:21:05 +0000 Subject: [PATCH 2/7] overtag existing builds --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 3350731..d891411 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -3378,10 +3378,8 @@ def handle_clone_tag(goptions, session, args): parser.add_option('--ts', type='int', metavar="TIMESTAMP", help=_('Clone tag at last event before specific timestamp')) parser.add_option('--no-delete', action='store_false', dest="delete", - default=True, - help=_("Don't delete any existing content in dest tag. " - "Note, that you can end with older latest builds in dest " - "than in src, if they are already tagged.")) + default=True, + help=_("Don't delete any existing content in dest tag.")) parser.add_option('--event', type='int', help=_('Clone tag at a specific event')) parser.add_option('--repo', type='int', @@ -3596,6 +3594,8 @@ def handle_clone_tag(goptions, session, args): # remove it for next pass so we stay aligned with outer # loop del dstblds[nvr] + if not options.delete: + ablds.append(srcbld) else: ablds.append(srcbld) baddlist.extend(ablds) diff --git a/tests/test_cli/test_clone_tag.py b/tests/test_cli/test_clone_tag.py index 5b64bef..4ac571e 100644 --- a/tests/test_cli/test_clone_tag.py +++ b/tests/test_cli/test_clone_tag.py @@ -816,22 +816,34 @@ List of changes: call.multiCall(batch=1000), call.tagBuildBypass('dst-tag', { 'owner_name': 'b_owner', + 'nvr': 'pkg1-1.0-1', + 'package_name': 'pkg1', 'state': 1, + 'tag_name': 'src-tag', + 'name': 'pkg1'}, force=None, notify=False), + call.tagBuildBypass('dst-tag', { + 'owner_name': 'b_owner', 'nvr': 'pkg1-0.1-1', 'package_name': 'pkg1', 'state': 1, 'tag_name': 'src-tag', - 'name': 'pkg1'}, force=None), + 'name': 'pkg1'}, force=None, notify=False), call.tagBuildBypass('dst-tag', { 'owner_name': 'b_owner', 'nvr': 'pkg1-1.0-2', 'package_name': 'pkg1', 'state': 1, 'tag_name': 'src-tag', - 'name': 'pkg1'}, force=None), + 'name': 'pkg1'}, force=None, notify=False), call.tagBuildBypass('dst-tag', { 'owner_name': 'b_owner', 'nvr': 'pkg1-1.1-2', 'package_name': 'pkg1', 'state': 1, 'tag_name': 'src-tag', - 'name': 'pkg1'}, force=None), + 'name': 'pkg1'}, force=None, notify=False), + call.tagBuildBypass('dst-tag', { + 'owner_name': 'b_owner', + 'nvr': 'pkg2-1.0-1', + 'package_name': 'pkg2', 'state': 2, + 'tag_name': 'src-tag-p', + 'name': 'pkg2'}, force=None, notify=False), call.multiCall(batch=1000), call.multiCall(batch=1000), call.groupPackageListAdd('dst-tag', @@ -860,9 +872,11 @@ List of changes: Action From/To Package Build(s) State Owner From Tag ------- ---------------------------- ---------------------------------------- ---------- ---------- ---------- + [add] pkg1 pkg1-1.0-1 COMPLETE b_owner src-tag [add] pkg1 pkg1-0.1-1 COMPLETE b_owner src-tag [add] pkg1 pkg1-1.0-2 COMPLETE b_owner src-tag [add] pkg1 pkg1-1.1-2 COMPLETE b_owner src-tag + [add] pkg2 pkg2-1.0-1 DELETED b_owner src-tag-p Action Package Group ------- ---------------------------- ---------------------------- @@ -890,9 +904,7 @@ Options: --inherit-builds Include all builds inherited into the source tag into the dest tag --ts=TIMESTAMP Clone tag at last event before specific timestamp - --no-delete Don't delete any existing content in dest tag. Note, that - you can end with older latest builds in dest than in src, - if they are already tagged. + --no-delete Don't delete any existing content in dest tag. --event=EVENT Clone tag at a specific event --repo=REPO Clone tag at a specific repo event -v, --verbose show changes diff --git a/tests/test_cli/utils.py b/tests/test_cli/utils.py index a84a3e8..d1c2c9b 100644 --- a/tests/test_cli/utils.py +++ b/tests/test_cli/utils.py @@ -1,6 +1,5 @@ from __future__ import print_function from __future__ import absolute_import -import locale import mock import os import six From 79fe28ef6309038eb6ea800b1070ad283e575dc0 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 06 2020 12:21:05 +0000 Subject: [PATCH 3/7] fix ordering for retagged builds --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index d891411..1747b9f 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -3567,15 +3567,16 @@ def handle_clone_tag(goptions, session, args): dstblds = dstbldsbypkg[pkg] ablds = [] dblds = [] - # firstly, remove builds from dst tag - removed_nvrs = set(dstblds.keys()) - set(srcblds.keys()) - dnvrs = [] - for (dstnvr, dstbld) in six.iteritems(dstblds): - if dstnvr in removed_nvrs: - dnvrs.append(dstnvr) - dblds.append(dstbld) - for dnvr in dnvrs: - del dstblds[dnvr] + if options.delete: + # firstly, remove builds from dst tag + removed_nvrs = set(dstblds.keys()) - set(srcblds.keys()) + dnvrs = [] + for (dstnvr, dstbld) in six.iteritems(dstblds): + if dstnvr in removed_nvrs: + dnvrs.append(dstnvr) + dblds.append(dstbld) + for dnvr in dnvrs: + del dstblds[dnvr] # secondly, add builds from src tag and adjust the order for (nvr, srcbld) in six.iteritems(srcblds): found = False @@ -3594,14 +3595,19 @@ def handle_clone_tag(goptions, session, args): # remove it for next pass so we stay aligned with outer # loop del dstblds[nvr] - if not options.delete: - ablds.append(srcbld) else: ablds.append(srcbld) baddlist.extend(ablds) bdellist.extend(dblds) baddlist.sort(key=lambda x: x['package_name']) bdellist.sort(key=lambda x: x['package_name']) + + if not options.delete: + # even in such case we need to delete out of order builds + # to be retagged correctly later + add_ids = [x['id'] for x in baddlist] + bdellist = [x for x in bdellist if x['id'] in add_ids] + gaddlist = [] # list containing new groups to be added from src tag for (grpname, group) in six.iteritems(srcgroups): if grpname not in dstgroups: @@ -3649,28 +3655,27 @@ def handle_clone_tag(goptions, session, args): if not options.test: session.multiCall(batch=options.batch) # DEL builds. To keep the order we should untag builds at first - if options.delete: - if not options.test: - session.multicall = True - for build in bdellist: - # don't delete an inherited build. - if build['tag_name'] == dsttag['name']: - # add missing 'name' field - build['name'] = build['package_name'] - chgbldlist.append(('[del]', - build['package_name'], - build['nvr'], - koji.BUILD_STATES[build['state']], - build['owner_name'], - build['tag_name'])) - # go on del builds from new tag. - if not options.test: - session.untagBuildBypass(dsttag['name'], - build, - force=options.force, - notify=options.notify) - if not options.test: - session.multiCall(batch=options.batch) + if not options.test: + session.multicall = True + for build in bdellist: + # don't delete an inherited build. + if build['tag_name'] == dsttag['name']: + # add missing 'name' field + build['name'] = build['package_name'] + chgbldlist.append(('[del]', + build['package_name'], + build['nvr'], + koji.BUILD_STATES[build['state']], + build['owner_name'], + build['tag_name'])) + # go on del builds from new tag. + if not options.test: + session.untagBuildBypass(dsttag['name'], + build, + force=options.force, + notify=options.notify) + if not options.test: + session.multiCall(batch=options.batch) # ADD builds. if not options.test: session.multicall = True diff --git a/tests/test_cli/test_clone_tag.py b/tests/test_cli/test_clone_tag.py index 4ac571e..9891f97 100644 --- a/tests/test_cli/test_clone_tag.py +++ b/tests/test_cli/test_clone_tag.py @@ -623,160 +623,194 @@ List of changes: """) @mock.patch('sys.stdout', new_callable=six.StringIO) - def test_handle_clone_tag_existing_dsttag_add(self, stdout): + def test_handle_clone_tag_existing_dsttag_nodelete(self, stdout): args = ['src-tag', 'dst-tag', '--all', '-v', '--no-delete'] self.session.multiCall.return_value = [] self.session.listPackages.side_effect = [[{'package_id': 1, - 'package_name': 'pkg1', + 'package_name': 'pkg', 'blocked': False, 'owner_name': 'userA', 'tag_name': 'src-tag', 'extra_arches': None}, - {'package_id': 2, - 'package_name': 'pkg2', - 'blocked': True, - 'owner_name': 'userB', - 'tag_name': 'src-tag-p', - 'extra_arches': 'arch3 arch4'}, - {'package_id': 3, - 'package_name': 'apkg', - 'blocked': False, - 'owner_name': 'userA', - 'tag_name': 'src-tag-p', - 'extra_arches': 'arch4'}], - [{'package_id': 1, - 'package_name': 'pkg1', + ], + []] + self.session.listTagged.side_effect = [[{'id': 1, + 'package_name': 'pkg', + 'nvr': 'pkg-1.0-23', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'src-tag'}, + {'id': 2, + 'package_name': 'pkg', + 'nvr': 'pkg-1.0-21', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'src-tag'}, + {'id': 3, + 'package_name': 'pkg', + 'nvr': 'pkg-0.1-1', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'src-tag'}, + ], + [], + ] + self.session.getTagGroups.return_value = [] + self.session.getTag.side_effect = [{'id': 1, + 'name': 'src-tag', + 'arches': 'arch1 arch2', + 'perm_id': 1, + 'maven_support': False, + 'maven_include_all': True, + 'locked': False}, + {'id': 2, + 'name': 'dst-tag', + 'arches': 'arch1 arch2', + 'perm_id': 1, + 'maven_support': False, + 'maven_include_all': True, + 'locked': False}] + handle_clone_tag(self.options, self.session, args) + self.activate_session.assert_called_once() + self.assert_console_message(stdout, """ +List of changes: + + Action Package Blocked Owner From Tag + ------- ---------------------------- ---------- ---------- ---------- + [add] pkg False userA src-tag + + Action From/To Package Build(s) State Owner From Tag + ------- ---------------------------- ---------------------------------------- ---------- ---------- ---------- + [add] pkg pkg-0.1-1 COMPLETE b_owner src-tag + [add] pkg pkg-1.0-21 COMPLETE b_owner src-tag + [add] pkg pkg-1.0-23 COMPLETE b_owner src-tag + + Action Package Group + ------- ---------------------------- ---------------------------- +""") + + @mock.patch('sys.stdout', new_callable=six.StringIO) + def test_handle_clone_tag_existing_dsttag_nodelete_1(self, stdout): + args = ['src-tag', 'dst-tag', '--all', '-v', '--no-delete'] + self.session.multiCall.return_value = [] + self.session.listPackages.side_effect = [[{'package_id': 1, + 'package_name': 'pkg', 'blocked': False, 'owner_name': 'userA', 'tag_name': 'src-tag', 'extra_arches': None}, - {'package_id': 3, - 'package_name': 'apkg', - 'blocked': False, - 'owner_name': 'userA', - 'tag_name': 'src-tag-p', - 'extra_arches': 'arch4'}, - {'package_id': 4, - 'package_name': 'bpkg', - 'blocked': False, - 'owner_name': 'userC', - 'tag_name': 'src-tag', - 'extra_arches': 'arch4'}, - {'package_id': 5, - 'package_name': 'cpkg', - 'blocked': True, - 'owner_name': 'userC', - 'tag_name': 'src-tag-p', - 'extra_arches': 'arch4'}, - {'package_id': 6, - 'package_name': 'dpkg', - 'blocked': True, - 'owner_name': 'userC', - 'tag_name': 'src-tag', - 'extra_arches': 'arch4'} - ]] - self.session.listTagged.side_effect = [[{'package_name': 'pkg1', - 'nvr': 'pkg1-1.1-2', + ], + []] + self.session.listTagged.side_effect = [[{'package_name': 'pkg', + 'nvr': 'pkg-1.0-23', 'state': 1, 'owner_name': 'b_owner', 'tag_name': 'src-tag'}, - {'package_name': 'pkg1', - 'nvr': 'pkg1-1.0-2', + {'package_name': 'pkg', + 'nvr': 'pkg-1.0-21', 'state': 1, 'owner_name': 'b_owner', 'tag_name': 'src-tag'}, - {'package_name': 'pkg1', - 'nvr': 'pkg1-0.1-1', + {'package_name': 'pkg', + 'nvr': 'pkg-0.1-1', 'state': 1, 'owner_name': 'b_owner', 'tag_name': 'src-tag'}, - {'package_name': 'pkg1', - 'nvr': 'pkg1-1.0-1', + ], + [{'package_name': 'pkg', + 'nvr': 'pkg-1.0-23', 'state': 1, 'owner_name': 'b_owner', 'tag_name': 'src-tag'}, - {'package_name': 'pkg2', - 'nvr': 'pkg2-1.0-1', - 'state': 2, - 'owner_name': 'b_owner', - 'tag_name': 'src-tag-p'} - ], - [{'package_name': 'pkg1', - 'nvr': 'pkg1-2.1-2', + {'package_name': 'pkg', + 'nvr': 'pkg-1.0-21', 'state': 1, 'owner_name': 'b_owner', - 'tag_name': 'dst-tag'}, - {'package_name': 'pkg1', - 'nvr': 'pkg1-1.0-1', + 'tag_name': 'src-tag'}, + {'package_name': 'pkg', + 'nvr': 'pkg-0.1-1', 'state': 1, 'owner_name': 'b_owner', - 'tag_name': 'dst-tag'}, - {'package_name': 'pkg1', - 'nvr': 'pkg1-0.1-1', + 'tag_name': 'src-tag'}, + ] + ] + self.session.getTagGroups.return_value = [] + self.session.getTag.side_effect = [{'id': 1, + 'name': 'src-tag', + 'arches': 'arch1 arch2', + 'perm_id': 1, + 'maven_support': False, + 'maven_include_all': True, + 'locked': False}, + {'id': 2, + 'name': 'dst-tag', + 'arches': 'arch1 arch2', + 'perm_id': 1, + 'maven_support': False, + 'maven_include_all': True, + 'locked': False}] + handle_clone_tag(self.options, self.session, args) + self.activate_session.assert_called_once() + self.assert_console_message(stdout, """ +List of changes: + + Action Package Blocked Owner From Tag + ------- ---------------------------- ---------- ---------- ---------- + [add] pkg False userA src-tag + + Action From/To Package Build(s) State Owner From Tag + ------- ---------------------------- ---------------------------------------- ---------- ---------- ---------- + + Action Package Group + ------- ---------------------------- ---------------------------- +""") + + @mock.patch('sys.stdout', new_callable=six.StringIO) + def test_handle_clone_tag_existing_dsttag_nodelete_2(self, stdout): + args = ['src-tag', 'dst-tag', '--all', '-v', '--no-delete'] + self.session.multiCall.return_value = [] + self.session.listPackages.side_effect = [[{'package_id': 1, + 'package_name': 'pkg', + 'blocked': False, + 'owner_name': 'userA', + 'tag_name': 'src-tag', + 'extra_arches': None}, + ], + []] + self.session.listTagged.side_effect = [[{'id': 1, + 'package_name': 'pkg', + 'nvr': 'pkg-1.0-23', 'state': 1, 'owner_name': 'b_owner', - 'tag_name': 'dst-tag'}, - {'package_name': 'pkg2', - 'nvr': 'pkg2-1.0-1', - 'state': 2, + 'tag_name': 'src-tag'}, + {'id': 2, + 'package_name': 'pkg', + 'nvr': 'pkg-1.0-21', + 'state': 1, 'owner_name': 'b_owner', - 'tag_name': 'dst-tag'}, - {'package_name': 'pkg3', - 'nvr': 'pkg3-1.0-1', + 'tag_name': 'src-tag'}, + {'id': 3, + 'package_name': 'pkg', + 'nvr': 'pkg-0.1-1', 'state': 1, 'owner_name': 'b_owner', - 'tag_name': 'dst-tag'} - ]] - self.session.getTagGroups.side_effect = [[{'name': 'group1', - 'tag_id': 1, - 'packagelist': [ - {'package': 'pkg1', - 'blocked': False}, - {'package': 'pkg2', - 'blocked': False}, - {'package': 'pkg3', - 'blocked': False}, - {'package': 'pkg4', - 'blocked': False} - ]}, - {'name': 'group2', - 'tag_id': 1, - 'packagelist': [ - {'package': 'apkg', - 'blocked': False}, - {'package': 'bpkg', - 'blocked': False}] - }], - [{'name': 'group1', - 'tag_id': 2, - 'packagelist': [ - {'package': 'pkg1', - 'blocked': False}, - {'package': 'pkg5', - 'blocked': False} - ]}, - {'name': 'group2', - 'tag_id': 3, - 'packagelist': [ - {'package': 'apkg', - 'blocked': False}, - {'package': 'cpkg', - 'blocked': False}]}, - {'name': 'group3', - 'tag_id': 2, - 'packagelist': [ - {'package': 'cpkg', - 'blocked': False}, - {'package': 'dpkg', - 'blocked': False}]}, - {'name': 'group4', - 'tag_id': 3, - 'packagelist': [ - {'package': 'epkg', - 'blocked': False}, - {'package': 'fpkg', - 'blocked': False}]} - ]] + 'tag_name': 'src-tag'}, + ], + [{'id': 2, + 'package_name': 'pkg', + 'nvr': 'pkg-1.0-21', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'src-tag'}, + {'id': 3, + 'package_name': 'pkg', + 'nvr': 'pkg-0.1-1', + 'state': 1, + 'owner_name': 'b_owner', + 'tag_name': 'src-tag'}, + ] + ] + self.session.getTagGroups.return_value = [] self.session.getTag.side_effect = [{'id': 1, 'name': 'src-tag', 'arches': 'arch1 arch2', @@ -793,99 +827,20 @@ List of changes: 'locked': False}] handle_clone_tag(self.options, self.session, args) self.activate_session.assert_called_once() - self.session.assert_has_calls([call.hasPerm('admin'), - call.getTag('src-tag'), - call.getTag('dst-tag'), - call.listPackages(event=None, - inherited=True, - tagID=1), - call.listPackages(inherited=True, - tagID=2), - call.listTagged(1, event=None, - inherit=None, - latest=None), - call.listTagged(2, inherit=False, - latest=False), - call.getTagGroups('src-tag', - event=None), - call.getTagGroups('dst-tag'), - call.packageListAdd('dst-tag', 'pkg2', - block=True, - extra_arches='arch3 arch4', - owner='userB'), - call.multiCall(batch=1000), - call.tagBuildBypass('dst-tag', { - 'owner_name': 'b_owner', - 'nvr': 'pkg1-1.0-1', - 'package_name': 'pkg1', 'state': 1, - 'tag_name': 'src-tag', - 'name': 'pkg1'}, force=None, notify=False), - call.tagBuildBypass('dst-tag', { - 'owner_name': 'b_owner', - 'nvr': 'pkg1-0.1-1', - 'package_name': 'pkg1', 'state': 1, - 'tag_name': 'src-tag', - 'name': 'pkg1'}, force=None, notify=False), - call.tagBuildBypass('dst-tag', { - 'owner_name': 'b_owner', - 'nvr': 'pkg1-1.0-2', - 'package_name': 'pkg1', 'state': 1, - 'tag_name': 'src-tag', - 'name': 'pkg1'}, force=None, notify=False), - call.tagBuildBypass('dst-tag', { - 'owner_name': 'b_owner', - 'nvr': 'pkg1-1.1-2', - 'package_name': 'pkg1', 'state': 1, - 'tag_name': 'src-tag', - 'name': 'pkg1'}, force=None, notify=False), - call.tagBuildBypass('dst-tag', { - 'owner_name': 'b_owner', - 'nvr': 'pkg2-1.0-1', - 'package_name': 'pkg2', 'state': 2, - 'tag_name': 'src-tag-p', - 'name': 'pkg2'}, force=None, notify=False), - call.multiCall(batch=1000), - call.multiCall(batch=1000), - call.groupPackageListAdd('dst-tag', - 'group1', - 'pkg2', - force=None), - call.groupPackageListAdd('dst-tag', - 'group1', - 'pkg3', - force=None), - call.groupPackageListAdd('dst-tag', - 'group1', - 'pkg4', - force=None), - call.groupPackageListAdd('dst-tag', - 'group2', - 'bpkg', - force=None), - call.multiCall(batch=1000)]) self.assert_console_message(stdout, """ List of changes: Action Package Blocked Owner From Tag ------- ---------------------------- ---------- ---------- ---------- - [add] pkg2 True userB src-tag-p + [add] pkg False userA src-tag Action From/To Package Build(s) State Owner From Tag ------- ---------------------------- ---------------------------------------- ---------- ---------- ---------- - [add] pkg1 pkg1-1.0-1 COMPLETE b_owner src-tag - [add] pkg1 pkg1-0.1-1 COMPLETE b_owner src-tag - [add] pkg1 pkg1-1.0-2 COMPLETE b_owner src-tag - [add] pkg1 pkg1-1.1-2 COMPLETE b_owner src-tag - [add] pkg2 pkg2-1.0-1 DELETED b_owner src-tag-p + [add] pkg pkg-1.0-23 COMPLETE b_owner src-tag Action Package Group ------- ---------------------------- ---------------------------- - [new] pkg2 group1 - [new] pkg3 group1 - [new] pkg4 group1 - [new] bpkg group2 """) - def test_handle_clone_tag_help(self): self.assert_help( handle_clone_tag, From fe65b11531eb97132b82a825869461c46b4dd90a Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 06 2020 12:34:13 +0000 Subject: [PATCH 4/7] fix tests --- diff --git a/tests/test_cli/test_clone_tag.py b/tests/test_cli/test_clone_tag.py index 9891f97..eaa3e25 100644 --- a/tests/test_cli/test_clone_tag.py +++ b/tests/test_cli/test_clone_tag.py @@ -626,14 +626,7 @@ List of changes: def test_handle_clone_tag_existing_dsttag_nodelete(self, stdout): args = ['src-tag', 'dst-tag', '--all', '-v', '--no-delete'] self.session.multiCall.return_value = [] - self.session.listPackages.side_effect = [[{'package_id': 1, - 'package_name': 'pkg', - 'blocked': False, - 'owner_name': 'userA', - 'tag_name': 'src-tag', - 'extra_arches': None}, - ], - []] + self.session.listPackages.return_value = [] self.session.listTagged.side_effect = [[{'id': 1, 'package_name': 'pkg', 'nvr': 'pkg-1.0-23', @@ -675,17 +668,16 @@ List of changes: self.assert_console_message(stdout, """ List of changes: - Action Package Blocked Owner From Tag + Action Package Blocked Owner From Tag ------- ---------------------------- ---------- ---------- ---------- - [add] pkg False userA src-tag - Action From/To Package Build(s) State Owner From Tag + Action From/To Package Build(s) State Owner From Tag ------- ---------------------------- ---------------------------------------- ---------- ---------- ---------- - [add] pkg pkg-0.1-1 COMPLETE b_owner src-tag - [add] pkg pkg-1.0-21 COMPLETE b_owner src-tag - [add] pkg pkg-1.0-23 COMPLETE b_owner src-tag + [add] pkg pkg-0.1-1 COMPLETE b_owner src-tag + [add] pkg pkg-1.0-21 COMPLETE b_owner src-tag + [add] pkg pkg-1.0-23 COMPLETE b_owner src-tag - Action Package Group + Action Package Group ------- ---------------------------- ---------------------------- """) @@ -693,42 +685,41 @@ List of changes: def test_handle_clone_tag_existing_dsttag_nodelete_1(self, stdout): args = ['src-tag', 'dst-tag', '--all', '-v', '--no-delete'] self.session.multiCall.return_value = [] - self.session.listPackages.side_effect = [[{'package_id': 1, - 'package_name': 'pkg', - 'blocked': False, - 'owner_name': 'userA', - 'tag_name': 'src-tag', - 'extra_arches': None}, - ], - []] - self.session.listTagged.side_effect = [[{'package_name': 'pkg', + self.session.listPackages.return_value = [] + self.session.listTagged.side_effect = [[{'id': 1, + 'package_name': 'pkg', 'nvr': 'pkg-1.0-23', 'state': 1, 'owner_name': 'b_owner', 'tag_name': 'src-tag'}, - {'package_name': 'pkg', + {'id': 2, + 'package_name': 'pkg', 'nvr': 'pkg-1.0-21', 'state': 1, 'owner_name': 'b_owner', 'tag_name': 'src-tag'}, - {'package_name': 'pkg', + {'id': 3, + 'package_name': 'pkg', 'nvr': 'pkg-0.1-1', 'state': 1, 'owner_name': 'b_owner', 'tag_name': 'src-tag'}, ], - [{'package_name': 'pkg', + [{'id': 1, + 'package_name': 'pkg', 'nvr': 'pkg-1.0-23', 'state': 1, 'owner_name': 'b_owner', 'tag_name': 'src-tag'}, - {'package_name': 'pkg', - 'nvr': 'pkg-1.0-21', + {'id': 3, + 'package_name': 'pkg', + 'nvr': 'pkg-0.1-1', 'state': 1, 'owner_name': 'b_owner', 'tag_name': 'src-tag'}, - {'package_name': 'pkg', - 'nvr': 'pkg-0.1-1', + {'id': 2, + 'package_name': 'pkg', + 'nvr': 'pkg-1.0-21', 'state': 1, 'owner_name': 'b_owner', 'tag_name': 'src-tag'}, @@ -754,14 +745,15 @@ List of changes: self.assert_console_message(stdout, """ List of changes: - Action Package Blocked Owner From Tag + Action Package Blocked Owner From Tag ------- ---------------------------- ---------- ---------- ---------- - [add] pkg False userA src-tag - Action From/To Package Build(s) State Owner From Tag + Action From/To Package Build(s) State Owner From Tag ------- ---------------------------- ---------------------------------------- ---------- ---------- ---------- + [add] pkg pkg-1.0-21 COMPLETE b_owner src-tag + [add] pkg pkg-1.0-23 COMPLETE b_owner src-tag - Action Package Group + Action Package Group ------- ---------------------------- ---------------------------- """) @@ -769,14 +761,7 @@ List of changes: def test_handle_clone_tag_existing_dsttag_nodelete_2(self, stdout): args = ['src-tag', 'dst-tag', '--all', '-v', '--no-delete'] self.session.multiCall.return_value = [] - self.session.listPackages.side_effect = [[{'package_id': 1, - 'package_name': 'pkg', - 'blocked': False, - 'owner_name': 'userA', - 'tag_name': 'src-tag', - 'extra_arches': None}, - ], - []] + self.session.listPackages.return_value = [] self.session.listTagged.side_effect = [[{'id': 1, 'package_name': 'pkg', 'nvr': 'pkg-1.0-23', @@ -830,15 +815,14 @@ List of changes: self.assert_console_message(stdout, """ List of changes: - Action Package Blocked Owner From Tag + Action Package Blocked Owner From Tag ------- ---------------------------- ---------- ---------- ---------- - [add] pkg False userA src-tag - Action From/To Package Build(s) State Owner From Tag + Action From/To Package Build(s) State Owner From Tag ------- ---------------------------- ---------------------------------------- ---------- ---------- ---------- - [add] pkg pkg-1.0-23 COMPLETE b_owner src-tag + [add] pkg pkg-1.0-23 COMPLETE b_owner src-tag - Action Package Group + Action Package Group ------- ---------------------------- ---------------------------- """) def test_handle_clone_tag_help(self): From 0af9bf6106ac55596359afbe451b1ba87215231e Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Apr 09 2020 07:35:20 +0000 Subject: [PATCH 5/7] comments --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 1747b9f..73dc4b6 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -3529,6 +3529,8 @@ def handle_clone_tag(goptions, session, args): inherited=True): dstpkgs[pkg['package_name']] = pkg if options.builds: + # listTagged orders builds latest-first + # so reversing that gives us oldest-first for build in reversed(session.listTagged(srctag['id'], event=event.get('id'), inherit=options.inherit_builds, @@ -3560,9 +3562,11 @@ def handle_clone_tag(goptions, session, args): pdellist.sort(key=lambda x: x['package_name']) baddlist = [] # list containing new builds to be added from src tag bdellist = [] # list containing new builds to be removed from dst tag + # remove builds for packages that are absent from src tag for (pkg, dstblds) in six.iteritems(dstbldsbypkg): if pkg not in srcbldsbypkg: bdellist.extend(dstblds.values()) + # add and/or remove builds from dst to match src contents and order for (pkg, srcblds) in six.iteritems(srcbldsbypkg): dstblds = dstbldsbypkg[pkg] ablds = [] @@ -3581,6 +3585,9 @@ def handle_clone_tag(goptions, session, args): for (nvr, srcbld) in six.iteritems(srcblds): found = False out_of_order = [] + # note that dstblds is trimmed as we go, so we are only + # considering the tail corresponding to where we are at + # in the srcblds loop for (dstnvr, dstbld) in six.iteritems(dstblds): if nvr == dstnvr: found = True @@ -3596,6 +3603,7 @@ def handle_clone_tag(goptions, session, args): # loop del dstblds[nvr] else: + # missing from dst, so we need to add it ablds.append(srcbld) baddlist.extend(ablds) bdellist.extend(dblds) From f6277601d6bfdd9aee840251e56c2aa366024079 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Apr 09 2020 07:35:21 +0000 Subject: [PATCH 6/7] clone-tag: force extra builds last if not removing them --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 73dc4b6..72aafd3 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -3571,18 +3571,31 @@ def handle_clone_tag(goptions, session, args): dstblds = dstbldsbypkg[pkg] ablds = [] dblds = [] + # firstly, deal with extra builds in dst + removed_nvrs = set(dstblds.keys()) - set(srcblds.keys()) + bld_order = srcblds.copy() if options.delete: - # firstly, remove builds from dst tag - removed_nvrs = set(dstblds.keys()) - set(srcblds.keys()) + # mark the extra builds for deletion dnvrs = [] for (dstnvr, dstbld) in six.iteritems(dstblds): if dstnvr in removed_nvrs: dnvrs.append(dstnvr) dblds.append(dstbld) + # we also remove them from dstblds now so that they do not + # interfere with the order comparison below for dnvr in dnvrs: del dstblds[dnvr] + else: + # in the no-delete case, the extra builds should be forced + # to last in the tag + bld_order = OrderedDict() + for (dstnvr, dstbld) in six.iteritems(dstblds): + if dstnvr in removed_nvrs: + bld_order[dstnvr] = dstbld + for (nvr, srcbld) in six.iteritems(srcblds): + bld_order[nvr] = srcbld # secondly, add builds from src tag and adjust the order - for (nvr, srcbld) in six.iteritems(srcblds): + for (nvr, srcbld) in six.iteritems(bld_order): found = False out_of_order = [] # note that dstblds is trimmed as we go, so we are only From 87024eb91ca311b6c1f55010ea109276e17db2a7 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Apr 09 2020 07:35:21 +0000 Subject: [PATCH 7/7] clone-tag: avoid marking and then unmarking for deletion Between this and the last change, the filtering of bdellist is no longer needed --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 72aafd3..6007a87 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -3562,10 +3562,11 @@ def handle_clone_tag(goptions, session, args): pdellist.sort(key=lambda x: x['package_name']) baddlist = [] # list containing new builds to be added from src tag bdellist = [] # list containing new builds to be removed from dst tag - # remove builds for packages that are absent from src tag - for (pkg, dstblds) in six.iteritems(dstbldsbypkg): - if pkg not in srcbldsbypkg: - bdellist.extend(dstblds.values()) + if options.delete: + # remove builds for packages that are absent from src tag + for (pkg, dstblds) in six.iteritems(dstbldsbypkg): + if pkg not in srcbldsbypkg: + bdellist.extend(dstblds.values()) # add and/or remove builds from dst to match src contents and order for (pkg, srcblds) in six.iteritems(srcbldsbypkg): dstblds = dstbldsbypkg[pkg] @@ -3623,12 +3624,6 @@ def handle_clone_tag(goptions, session, args): baddlist.sort(key=lambda x: x['package_name']) bdellist.sort(key=lambda x: x['package_name']) - if not options.delete: - # even in such case we need to delete out of order builds - # to be retagged correctly later - add_ids = [x['id'] for x in baddlist] - bdellist = [x for x in bdellist if x['id'] in add_ids] - gaddlist = [] # list containing new groups to be added from src tag for (grpname, group) in six.iteritems(srcgroups): if grpname not in dstgroups: