Download all files, skip downloaded files

Fixes: https://pagure.io/koji/issue/3499

cli/koji_cli/commands.py

@@ -7001,27 +7001,11 @@ def anon_handle_download_task(options, session, args):
         print("No files for download found.")
         return
 
-    downloads_new_names = [(new_filename, vol) for (_, _, vol, new_filename, _) in downloads]
-    if not suboptions.dirpertask:
-        not_uniques = list({x for x in downloads_new_names if downloads_new_names.count(x) > 1})
-        if not_uniques:
-            files_dict = {}
-            for nu in not_uniques:
-                if not nu[0].endswith('src.rpm'):
-                    for (_, _, vol, new_filename, task_id) in downloads:
-                        if new_filename == nu[0] and vol == nu[1]:
-                            files_dict.setdefault(new_filename, {'vol': vol, 'tasks': []})
-                            files_dict[new_filename]['tasks'].append(task_id)
-            for key, value in files_dict.items():
-                warn('Duplicate file %s for volume %s (tasks [%s])' % (key, value['vol'],
-                                                                       ", ".join(value['tasks'])))
-            if files_dict != {}:
-                error("Download files names conflict, use --dirpertask")
-
     # perform the download
     number = 0
     pathinfo = koji.PathInfo(topdir=suboptions.topurl)
-    srpm_downloaded = []
+    files_downloaded = []
+    dirpertask_msg = False
     for (task, filename, volume, new_filename, task_id) in downloads:
         if suboptions.dirpertask:
             koji.ensuredir(task_id)
@@ -7036,18 +7020,17 @@ def anon_handle_download_task(options, session, args):
         if '..' in filename:
             error('Invalid file name: %s' % filename)
         url = '%s/%s/%s' % (pathinfo.work(volume), pathinfo.taskrelpath(task["id"]), filename)
-        if not new_filename.endswith('src.rpm'):
+        if (new_filename, volume) not in files_downloaded:
             download_file(url, new_filename, quiet=suboptions.quiet,
                           noprogress=suboptions.noprogress, size=len(downloads), num=number)
+            files_downloaded.append((new_filename, volume))
         else:
-            if (new_filename, volume) not in srpm_downloaded:
+            if not suboptions.quiet:
-                download_file(url, new_filename, quiet=suboptions.quiet,
+                print("Downloading [%d/%d]: %s" % (number, len(downloads), new_filename))
-                              noprogress=suboptions.noprogress, size=len(downloads), num=number)
+                print("File %s already downloaded, skipping" % new_filename)
-                srpm_downloaded.append((new_filename, volume))
+            dirpertask_msg = True
-            else:
+    if dirpertask_msg:
-                if not suboptions.quiet:
+        warn("Duplicate files, for download all duplicate files use --dirpertask.")
-                    print("Downloading [%d/%d] %s" % (number, len(downloads), new_filename))
-                    print("File %s already downloaded, skipping" % new_filename)
 
 
 def anon_handle_wait_repo(options, session, args):

tests/test_cli/test_download_task.py

@@ -722,16 +722,9 @@ Options:
         # Run it and check immediate output
         # args: task_id --dirpertask --log
         # expected: failure
-        self.assert_system_exit(
+        anon_handle_download_task(self.options, self.session, args)
-            anon_handle_download_task,
+        actual = self.stderr.getvalue()
-            self.options, self.session, args,
+        expected = 'Duplicate files, for download all duplicate files use --dirpertask.\n'
-            stderr="Duplicate file somerpm.noarch.rpm for volume DEFAULT (tasks [22222, 55555])\n"
-                   "Download files names conflict, use --dirpertask\n",
-            stdout='',
-            activate_session=None,
-            exit_code=1)
-        actual = self.stdout.getvalue()
-        expected = ''
         self.assertMultiLineEqual(actual, expected)
         # Finally, assert that things were called as we expected.
         self.ensure_connection.assert_called_once_with(self.session, self.options)
@@ -743,7 +736,20 @@ Options:
             call(self.session, 33333),
             call(self.session, 44444),
             call(self.session, 55555)])
-        self.assertListEqual(self.download_file.mock_calls, [])
+        self.assertListEqual(self.download_file.mock_calls, [
+            call('https://topurl/work/tasks/2222/22222/somerpm.src.rpm',
+                 'somerpm.src.rpm', quiet=None, noprogress=None, size=7, num=1),
+            call('https://topurl/vol/vol1/work/tasks/2222/22222/somerpm.src.rpm',
+                 'vol1/somerpm.src.rpm', quiet=None, noprogress=None, size=7, num=2),
+            call('https://topurl/work/tasks/2222/22222/somerpm.noarch.rpm',
+                 'somerpm.noarch.rpm', quiet=None, noprogress=None, size=7, num=3),
+            call('https://topurl/work/tasks/3333/33333/somerpm.x86_64.rpm',
+                 'somerpm.x86_64.rpm', quiet=None, noprogress=None, size=7, num=4),
+            call('https://topurl/vol/vol2/work/tasks/3333/33333/somerpm.x86_64.rpm',
+                 'vol2/somerpm.x86_64.rpm', quiet=None, noprogress=None, size=7, num=5),
+            call('https://topurl/vol/vol2/work/tasks/4444/44444/somerpm.s390.rpm',
+                 'vol2/somerpm.s390.rpm', quiet=None, noprogress=None, size=7, num=6),
+        ])
 
     def test_handle_download_task_without_all_json_not_downloaded(self):
         args = [str(self.parent_task_id)]
@@ -811,7 +817,7 @@ Options:
         rv = anon_handle_download_task(self.options, self.session, args)
 
         actual = self.stdout.getvalue()
-        expected = 'Downloading [3/9] somerpm.src.rpm\n' \
+        expected = 'Downloading [3/9]: somerpm.src.rpm\n' \
                    'File somerpm.src.rpm already downloaded, skipping\n'
         self.assertMultiLineEqual(actual, expected)
         # Finally, assert that things were called as we expected.