Replace PR #3458 with PR #3459 (preferred by upstream)

epel9
Adam Williamson 3 years ago
parent 91ab5c7b45
commit fe7d6a38f5

@ -1,55 +0,0 @@
From 5a44f904747cd7defb79ee44750ad45707445729 Mon Sep 17 00:00:00 2001
From: Adam Williamson <awilliam@redhat.com>
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 <awilliam@redhat.com>
---
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

@ -0,0 +1,479 @@
From 9f498f1cf3339f3f41d1bb80df2ada4e391624b3 Mon Sep 17 00:00:00 2001
From: Jana Cupova <root@localhost>
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']},

@ -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 <awilliam@redhat.com> - 1.29.1-4
- Replace PR #3458 with PR #3459 (preferred by upstream)
* Tue Aug 09 2022 Adam Williamson <awilliam@redhat.com> - 1.29.1-3
- Backport PR #3458 to fix download-task arch filtering. fixes rhbz#2116674

Loading…
Cancel
Save