From fe7d6a38f5ae1c9c169475a08006d7252046f4e6 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Wed, 10 Aug 2022 10:19:13 -0700 Subject: [PATCH] Replace PR #3458 with PR #3459 (preferred by upstream) --- ...lter-in-download-task-to-handle-noar.patch | 55 -- 3459.patch | 479 ++++++++++++++++++ koji.spec | 9 +- 3 files changed, 485 insertions(+), 58 deletions(-) delete mode 100644 0001-Fix-task-arch-filter-in-download-task-to-handle-noar.patch create mode 100644 3459.patch diff --git a/0001-Fix-task-arch-filter-in-download-task-to-handle-noar.patch b/0001-Fix-task-arch-filter-in-download-task-to-handle-noar.patch deleted file mode 100644 index ceac6d3..0000000 --- a/0001-Fix-task-arch-filter-in-download-task-to-handle-noar.patch +++ /dev/null @@ -1,55 +0,0 @@ -From 5a44f904747cd7defb79ee44750ad45707445729 Mon Sep 17 00:00:00 2001 -From: Adam Williamson -Date: Tue, 9 Aug 2022 11:00:10 -0700 -Subject: [PATCH] Fix task arch filter in download-task to handle noarch etc. - -In 6713d1651d73b1b9d5dec37d0c5e0d85fa41a3c5 , filtering by task -arch was added to `download-task` when `--arch` is passed - -previously, it didn't bother filtering the tasks themselves by -arch, only any files they produce. - -This introduces some problems. The most obvious one is that the -'task arch' for noarch package builds is not 'noarch' but -'whatever the arch of the builder is' - e.g. for task -90635802 , which is a noarch build, the arch is 's390x'. - -Another problem would be with i386 vs. i686: for builds that -produce i686 RPMs, the task arch is i386. So in order to get -i686 packages, you'd have to pass `--arch=i386 --arch=i686". - -Fortunately, the task "label" seems tailor-made to solve these -problems: when it's a noarch build, the label is "noarch", and -when it's an i386/i686 build, the label is "i686". So by just -including the task if its arch *or* its label is in the arches -passed on the command line, we resolve both those problems, and -hopefully any other similar ones. - -Another option here would be just to ditch the arch filter and -go back to considering all tasks and only filtering files, but -this seems more efficient if it's safe in all cases. - -Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2116674 -Signed-off-by: Adam Williamson ---- - cli/koji_cli/commands.py | 4 ++-- - 1 file changed, 2 insertions(+), 2 deletions(-) - -diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py -index 9e2cc1b3..0249497b 100644 ---- a/cli/koji_cli/commands.py -+++ b/cli/koji_cli/commands.py -@@ -6950,9 +6950,9 @@ def anon_handle_download_task(options, session, args): - build_methods_list = ['buildArch', 'build'] - rpm_file_types = ['rpm', 'src.rpm'] - for task in list_tasks: -- taskarch = task['arch'] -+ taskarches = (task['arch'], task['label']) - task_id = str(task['id']) -- if len(suboptions.arches) == 0 or taskarch in suboptions.arches: -+ if len(suboptions.arches) == 0 or any(tarch in suboptions.arches for tarch in taskarches): - files = list_task_output_all_volumes(session, task["id"]) - for filename in files: - if filename.endswith('src.rpm'): --- -2.37.1 - diff --git a/3459.patch b/3459.patch new file mode 100644 index 0000000..b522581 --- /dev/null +++ b/3459.patch @@ -0,0 +1,479 @@ +From 9f498f1cf3339f3f41d1bb80df2ada4e391624b3 Mon Sep 17 00:00:00 2001 +From: Jana Cupova +Date: Aug 10 2022 05:38:00 +0000 +Subject: Fix arch in download-task + + +Fixes: https://pagure.io/koji/issue/3456 + +--- + +diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py +index 9e2cc1b..c2b2d97 100644 +--- a/cli/koji_cli/commands.py ++++ b/cli/koji_cli/commands.py +@@ -6952,50 +6952,49 @@ def anon_handle_download_task(options, session, args): + for task in list_tasks: + taskarch = task['arch'] + task_id = str(task['id']) +- if len(suboptions.arches) == 0 or taskarch in suboptions.arches: +- files = list_task_output_all_volumes(session, task["id"]) +- for filename in files: +- if filename.endswith('src.rpm'): +- filetype = 'src.rpm' +- else: +- filetype = filename.rsplit('.', 1)[1] +- if suboptions.all and not (task['method'] in build_methods_list and +- filetype in rpm_file_types): +- if filetype != 'log': +- for volume in files[filename]: +- if suboptions.dirpertask: +- new_filename = '%s/%s' % (task_id, filename) +- else: +- if taskarch not in filename and filetype != 'src.rpm': +- part_filename = filename[:-len('.%s' % filetype)] +- new_filename = "%s.%s.%s" % (part_filename, +- taskarch, filetype) +- else: +- new_filename = filename +- downloads.append((task, filename, volume, new_filename, task_id)) +- elif task['method'] in build_methods_list: +- if filetype in rpm_file_types: +- filearch = filename.split(".")[-2] +- for volume in files[filename]: +- if len(suboptions.arches) == 0 or filearch in suboptions.arches: +- if suboptions.dirpertask: +- new_filename = '%s/%s' % (task_id, filename) +- else: +- new_filename = filename +- downloads.append((task, filename, volume, new_filename, +- task_id)) +- +- if filetype == 'log' and suboptions.logs: ++ files = list_task_output_all_volumes(session, task["id"]) ++ for filename in files: ++ if filename.endswith('src.rpm'): ++ filetype = 'src.rpm' ++ else: ++ filetype = filename.rsplit('.', 1)[1] ++ if suboptions.all and not (task['method'] in build_methods_list and ++ filetype in rpm_file_types): ++ if filetype != 'log': + for volume in files[filename]: + if suboptions.dirpertask: + new_filename = '%s/%s' % (task_id, filename) + else: +- if taskarch not in filename: +- part_filename = filename[:-len('.log')] +- new_filename = "%s.%s.log" % (part_filename, taskarch) ++ if taskarch not in filename and filetype != 'src.rpm': ++ part_filename = filename[:-len('.%s' % filetype)] ++ new_filename = "%s.%s.%s" % (part_filename, ++ taskarch, filetype) + else: + new_filename = filename + downloads.append((task, filename, volume, new_filename, task_id)) ++ elif task['method'] in build_methods_list: ++ if filetype in rpm_file_types: ++ filearch = filename.split(".")[-2] ++ for volume in files[filename]: ++ if len(suboptions.arches) == 0 or filearch in suboptions.arches: ++ if suboptions.dirpertask: ++ new_filename = '%s/%s' % (task_id, filename) ++ else: ++ new_filename = filename ++ downloads.append((task, filename, volume, new_filename, ++ task_id)) ++ ++ if filetype == 'log' and suboptions.logs: ++ for volume in files[filename]: ++ if suboptions.dirpertask: ++ new_filename = '%s/%s' % (task_id, filename) ++ else: ++ if taskarch not in filename: ++ part_filename = filename[:-len('.log')] ++ new_filename = "%s.%s.log" % (part_filename, taskarch) ++ else: ++ new_filename = filename ++ downloads.append((task, filename, volume, new_filename, task_id)) + + if len(downloads) == 0: + print("No files for download found.") +diff --git a/tests/test_cli/test_download_task.py b/tests/test_cli/test_download_task.py +index c35d4a6..3c455e3 100644 +--- a/tests/test_cli/test_download_task.py ++++ b/tests/test_cli/test_download_task.py +@@ -120,26 +120,29 @@ Default behavior without --all option downloads .rpm files only for build and bu + def test_handle_download_task_parent(self): + args = [str(self.parent_task_id), '--arch=noarch,x86_64'] + self.session.getTaskInfo.return_value = self.parent_task_info +- self.session.getTaskChildren.return_value = [{'id': 22222, +- 'method': 'buildArch', +- 'arch': 'noarch', +- 'state': 2}, +- {'id': 33333, +- 'method': 'buildArch', +- 'arch': 'x86_64', +- 'state': 2}, +- {'id': 44444, +- 'method': 'buildArch', +- 'arch': 's390', +- 'state': 2}, +- {'id': 55555, +- 'method': 'tagBuild', +- 'arch': 'noarch', +- 'state': 2} +- ] ++ self.session.getTaskChildren.return_value = [ ++ {'id': 22222, ++ 'method': 'buildArch', ++ 'arch': 'noarch', ++ 'state': 2}, ++ {'id': 33333, ++ 'method': 'buildArch', ++ 'arch': 'x86_64', ++ 'state': 2}, ++ {'id': 44444, ++ 'method': 'buildArch', ++ 'arch': 's390', ++ 'state': 2}, ++ {'id': 55555, ++ 'method': 'tagBuild', ++ 'arch': 'noarch', ++ 'state': 2} ++ ] + self.list_task_output_all_volumes.side_effect = [ ++ {}, + {'somerpm.src.rpm': ['DEFAULT', 'vol1']}, + {'somerpm.x86_64.rpm': ['DEFAULT', 'vol2']}, ++ {'somerpm.s390.rpm': ['DEFAULT']}, + {'somerpm.noarch.rpm': ['vol3'], + 'somelog.log': ['DEFAULT', 'vol1']}, + ] +@@ -156,8 +159,10 @@ Default behavior without --all option downloads .rpm files only for build and bu + self.session.getTaskInfo.assert_called_once_with(self.parent_task_id) + self.session.getTaskChildren.assert_called_once_with(self.parent_task_id) + self.assertEqual(self.list_task_output_all_volumes.mock_calls, [ ++ call(self.session, 123333), + call(self.session, 22222), + call(self.session, 33333), ++ call(self.session, 44444), + call(self.session, 55555)]) + self.assertListEqual(self.download_file.mock_calls, [ + call('https://topurl/work/tasks/3333/33333/somerpm.x86_64.rpm', +@@ -169,10 +174,11 @@ Default behavior without --all option downloads .rpm files only for build and bu + def test_handle_download_task_log(self): + args = [str(self.parent_task_id), '--log'] + self.session.getTaskInfo.return_value = self.parent_task_info +- self.session.getTaskChildren.return_value = [{'id': 22222, +- 'method': 'buildArch', +- 'arch': 'noarch', +- 'state': 2}] ++ self.session.getTaskChildren.return_value = [ ++ {'id': 22222, ++ 'method': 'buildArch', ++ 'arch': 'noarch', ++ 'state': 2}] + self.list_task_output_all_volumes.side_effect = [{}, { + 'somerpm.src.rpm': ['DEFAULT', 'vol1'], + 'somerpm.x86_64.rpm': ['DEFAULT', 'vol2'], +@@ -229,15 +235,17 @@ Default behavior without --all option downloads .rpm files only for build and bu + self.ensure_connection.assert_called_once_with(self.session, self.options) + self.session.getTaskInfo.assert_called_once_with(self.parent_task_id) + self.session.getTaskChildren.assert_called_once_with(self.parent_task_id) +- self.list_task_output_all_volumes.assert_not_called() ++ self.list_task_output_all_volumes.assert_called_once_with( ++ self.session, self.parent_task_id) + self.download_file.assert_not_called() + + def test_handle_download_parent_not_finished(self): + args = [str(self.parent_task_id)] +- self.session.getTaskInfo.return_value = {'id': self.parent_task_id, +- 'method': 'buildArch', +- 'arch': 'taskarch', +- 'state': 3} ++ self.session.getTaskInfo.return_value = { ++ 'id': self.parent_task_id, ++ 'method': 'buildArch', ++ 'arch': 'taskarch', ++ 'state': 3} + self.list_task_output_all_volumes.return_value = { + 'somerpm.src.rpm': ['DEFAULT', 'vol1'], + 'somerpm.x86_64.rpm': ['DEFAULT', 'vol2'], +@@ -265,10 +273,11 @@ Default behavior without --all option downloads .rpm files only for build and bu + def test_handle_download_child_not_finished(self): + args = [str(self.parent_task_id)] + self.session.getTaskInfo.return_value = self.parent_task_info +- self.session.getTaskChildren.return_value = [{'id': 22222, +- 'method': 'buildArch', +- 'arch': 'noarch', +- 'state': 3}] ++ self.session.getTaskChildren.return_value = [{ ++ 'id': 22222, ++ 'method': 'buildArch', ++ 'arch': 'noarch', ++ 'state': 3}] + self.list_task_output_all_volumes.side_effect = [ + {'somerpm.src.rpm': ['DEFAULT', 'vol1']}, + {'somenextrpm.src.rpm': ['DEFAULT', 'vol1']}] +@@ -374,26 +383,29 @@ Options: + def test_handle_download_task_parent_dirpertask(self): + args = [str(self.parent_task_id), '--arch=noarch,x86_64', '--dirpertask', '--log'] + self.session.getTaskInfo.return_value = self.parent_task_info +- self.session.getTaskChildren.return_value = [{'id': 22222, +- 'method': 'buildArch', +- 'arch': 'noarch', +- 'state': 2}, +- {'id': 33333, +- 'method': 'buildArch', +- 'arch': 'x86_64', +- 'state': 2}, +- {'id': 44444, +- 'method': 'buildArch', +- 'arch': 's390', +- 'state': 2}, +- {'id': 55555, +- 'method': 'tagBuild', +- 'arch': 'noarch', +- 'state': 2} +- ] ++ self.session.getTaskChildren.return_value = [ ++ {'id': 22222, ++ 'method': 'buildArch', ++ 'arch': 'noarch', ++ 'state': 2}, ++ {'id': 33333, ++ 'method': 'buildArch', ++ 'arch': 'x86_64', ++ 'state': 2}, ++ {'id': 44444, ++ 'method': 'buildArch', ++ 'arch': 's390', ++ 'state': 2}, ++ {'id': 55555, ++ 'method': 'tagBuild', ++ 'arch': 'noarch', ++ 'state': 2} ++ ] + self.list_task_output_all_volumes.side_effect = [ ++ {}, + {'somerpm.src.rpm': ['DEFAULT', 'vol1'], 'somerpm.noarch.rpm': ['DEFAULT']}, + {'somerpm.x86_64.rpm': ['DEFAULT', 'vol2']}, ++ {'somerpm.s390.rpm': ['DEFAULT']}, + {'somerpm.noarch.rpm': ['vol3'], + 'somelog.log': ['DEFAULT', 'vol1']}, + ] +@@ -410,8 +422,10 @@ Options: + self.session.getTaskInfo.assert_called_once_with(self.parent_task_id) + self.session.getTaskChildren.assert_called_once_with(self.parent_task_id) + self.assertEqual(self.list_task_output_all_volumes.mock_calls, [ ++ call(self.session, 123333), + call(self.session, 22222), + call(self.session, 33333), ++ call(self.session, 44444), + call(self.session, 55555)]) + self.assertListEqual(self.download_file.mock_calls, [ + call('https://topurl/work/tasks/2222/22222/somerpm.noarch.rpm', +@@ -430,26 +444,29 @@ Options: + def test_handle_download_task_parent_all(self): + args = [str(self.parent_task_id), '--arch=noarch,x86_64', '--all'] + self.session.getTaskInfo.return_value = self.parent_task_info +- self.session.getTaskChildren.return_value = [{'id': 22222, +- 'method': 'buildArch', +- 'arch': 'noarch', +- 'state': 2}, +- {'id': 33333, +- 'method': 'buildArch', +- 'arch': 'x86_64', +- 'state': 2}, +- {'id': 44444, +- 'method': 'buildArch', +- 'arch': 's390', +- 'state': 2}, +- {'id': 55555, +- 'method': 'tagBuild', +- 'arch': 'noarch', +- 'state': 2} +- ] ++ self.session.getTaskChildren.return_value = [ ++ {'id': 22222, ++ 'method': 'buildArch', ++ 'arch': 'noarch', ++ 'state': 2}, ++ {'id': 33333, ++ 'method': 'buildArch', ++ 'arch': 'x86_64', ++ 'state': 2}, ++ {'id': 44444, ++ 'method': 'buildArch', ++ 'arch': 's390', ++ 'state': 2}, ++ {'id': 55555, ++ 'method': 'tagBuild', ++ 'arch': 'noarch', ++ 'state': 2} ++ ] + self.list_task_output_all_volumes.side_effect = [ ++ {}, + {'somerpm.src.rpm': ['DEFAULT', 'vol1']}, + {'somerpm.json': ['DEFAULT', 'vol2']}, ++ {'somerpm.s390.rpm': ['DEFAULT']}, + {'somerpm.noarch.rpm': ['vol3'], + 'somelog.log': ['DEFAULT', 'vol1']}, + ] +@@ -466,8 +483,10 @@ Options: + self.session.getTaskInfo.assert_called_once_with(self.parent_task_id) + self.session.getTaskChildren.assert_called_once_with(self.parent_task_id) + self.assertEqual(self.list_task_output_all_volumes.mock_calls, [ ++ call(self.session, 123333), + call(self.session, 22222), + call(self.session, 33333), ++ call(self.session, 44444), + call(self.session, 55555)]) + self.assertListEqual(self.download_file.mock_calls, [ + call('https://topurl/work/tasks/3333/33333/somerpm.json', +@@ -504,23 +523,24 @@ Options: + def test_handle_download_task_parent_dirpertask_all(self): + args = [str(self.parent_task_id), '--dirpertask', '--all'] + self.session.getTaskInfo.return_value = self.parent_task_info +- self.session.getTaskChildren.return_value = [{'id': 22222, +- 'method': 'buildArch', +- 'arch': 'noarch', +- 'state': 2}, +- {'id': 33333, +- 'method': 'buildArch', +- 'arch': 'x86_64', +- 'state': 2}, +- {'id': 44444, +- 'method': 'buildArch', +- 'arch': 's390', +- 'state': 2}, +- {'id': 55555, +- 'method': 'tagBuild', +- 'arch': 'noarch', +- 'state': 2} +- ] ++ self.session.getTaskChildren.return_value = [ ++ {'id': 22222, ++ 'method': 'buildArch', ++ 'arch': 'noarch', ++ 'state': 2}, ++ {'id': 33333, ++ 'method': 'buildArch', ++ 'arch': 'x86_64', ++ 'state': 2}, ++ {'id': 44444, ++ 'method': 'buildArch', ++ 'arch': 's390', ++ 'state': 2}, ++ {'id': 55555, ++ 'method': 'tagBuild', ++ 'arch': 'noarch', ++ 'state': 2} ++ ] + self.list_task_output_all_volumes.side_effect = [ + {}, + {'somerpm.src.rpm': ['DEFAULT', 'vol1'], 'somerpm.noarch.rpm': ['DEFAULT']}, +@@ -568,11 +588,12 @@ Options: + def test_handle_download_task_log_with_arch(self): + args = [str(self.parent_task_id), '--arch=noarch', '--log'] + self.session.getTaskInfo.return_value = self.parent_task_info +- self.session.getTaskChildren.return_value = [{'id': 22222, +- 'method': 'buildArch', +- 'arch': 'noarch', +- 'state': 2}] +- self.list_task_output_all_volumes.side_effect = [{ ++ self.session.getTaskChildren.return_value = [ ++ {'id': 22222, ++ 'method': 'buildArch', ++ 'arch': 'noarch', ++ 'state': 2}] ++ self.list_task_output_all_volumes.side_effect = [{}, { + 'somerpm.src.rpm': ['DEFAULT', 'vol1'], + 'somerpm.x86_64.rpm': ['DEFAULT', 'vol2'], + 'somerpm.noarch.rpm': ['vol3'], +@@ -604,23 +625,24 @@ Options: + def test_handle_download_task_all_log(self): + args = [str(self.parent_task_id), '--all', '--log'] + self.session.getTaskInfo.return_value = self.parent_task_info +- self.session.getTaskChildren.return_value = [{'id': 22222, +- 'method': 'buildArch', +- 'arch': 'noarch', +- 'state': 2}, +- {'id': 33333, +- 'method': 'buildArch', +- 'arch': 'x86_64', +- 'state': 2}, +- {'id': 44444, +- 'method': 'buildArch', +- 'arch': 's390', +- 'state': 2}, +- {'id': 55555, +- 'method': 'tagBuild', +- 'arch': 'noarch', +- 'state': 2} +- ] ++ self.session.getTaskChildren.return_value = [ ++ {'id': 22222, ++ 'method': 'buildArch', ++ 'arch': 'noarch', ++ 'state': 2}, ++ {'id': 33333, ++ 'method': 'buildArch', ++ 'arch': 'x86_64', ++ 'state': 2}, ++ {'id': 44444, ++ 'method': 'buildArch', ++ 'arch': 's390', ++ 'state': 2}, ++ {'id': 55555, ++ 'method': 'tagBuild', ++ 'arch': 'noarch', ++ 'state': 2} ++ ] + self.list_task_output_all_volumes.side_effect = [ + {}, + {'somerpm.src.rpm': ['DEFAULT', 'vol1'], 'somerpm.noarch.rpm': ['DEFAULT']}, +@@ -671,23 +693,24 @@ Options: + def test_handle_download_task_parent_dirpertask_all_conflict_names(self): + args = [str(self.parent_task_id), '--all'] + self.session.getTaskInfo.return_value = self.parent_task_info +- self.session.getTaskChildren.return_value = [{'id': 22222, +- 'method': 'buildArch', +- 'arch': 'noarch', +- 'state': 2}, +- {'id': 33333, +- 'method': 'buildArch', +- 'arch': 'x86_64', +- 'state': 2}, +- {'id': 44444, +- 'method': 'buildArch', +- 'arch': 's390', +- 'state': 2}, +- {'id': 55555, +- 'method': 'tagBuild', +- 'arch': 'noarch', +- 'state': 2} +- ] ++ self.session.getTaskChildren.return_value = [ ++ {'id': 22222, ++ 'method': 'buildArch', ++ 'arch': 'noarch', ++ 'state': 2}, ++ {'id': 33333, ++ 'method': 'buildArch', ++ 'arch': 'x86_64', ++ 'state': 2}, ++ {'id': 44444, ++ 'method': 'buildArch', ++ 'arch': 's390', ++ 'state': 2}, ++ {'id': 55555, ++ 'method': 'tagBuild', ++ 'arch': 'noarch', ++ 'state': 2} ++ ] + self.list_task_output_all_volumes.side_effect = [ + {}, + {'somerpm.src.rpm': ['DEFAULT', 'vol1'], 'somerpm.noarch.rpm': ['DEFAULT']}, + diff --git a/koji.spec b/koji.spec index 87f7c6d..6555819 100644 --- a/koji.spec +++ b/koji.spec @@ -9,7 +9,7 @@ Name: koji Version: 1.29.1 -Release: 3%{?dist} +Release: 4%{?dist} # the included arch lib from yum's rpmUtils is GPLv2+ License: LGPLv2 and GPLv2+ Summary: Build system tools @@ -21,9 +21,9 @@ Source0: https://releases.pagure.org/koji/koji-%{version}.tar.bz2 # isn't there Patch0: 0001-Don-t-crash-in-_checkImageState-if-there-s-no-image..patch # https://bugzilla.redhat.com/show_bug.cgi?id=2116674 -# https://pagure.io/koji/pull-request/3458 +# https://pagure.io/koji/pull-request/3459 # Fix arch filtering in `koji download-task` -Patch1: 0001-Fix-task-arch-filter-in-download-task-to-handle-noar.patch +Patch1: 3459.patch # Not upstreamable Patch100: fedora-config.patch @@ -356,6 +356,9 @@ done %systemd_postun kojira.service %changelog +* Wed Aug 10 2022 Adam Williamson - 1.29.1-4 +- Replace PR #3458 with PR #3459 (preferred by upstream) + * Tue Aug 09 2022 Adam Williamson - 1.29.1-3 - Backport PR #3458 to fix download-task arch filtering. fixes rhbz#2116674