Skip 'ls' when downloading from gs.

Bug: 959167
Change-Id: I160658264dac71a9cff280cf3ba606d583d8f2b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3646593
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Joanna Wang <jojwang@chromium.org>

download_from_google_storage.py

@@ -273,9 +273,20 @@ def _downloader_worker_thread(thread_num, q, force, base_url,
           skip = False
       if skip:
         continue
-    # Check if file exists.
+
     file_url = '%s/%s' % (base_url, input_sha1_sum)
-    (code, _, err) = gsutil.check_call('ls', file_url)
+
+    try:
+      if delete:
+        os.remove(output_filename)  # Delete the file if it exists already.
+    except OSError:
+      if os.path.exists(output_filename):
+        out_q.put('%d> Warning: deleting %s failed.' % (
+            thread_num, output_filename))
+    if verbose:
+      out_q.put('%d> Downloading %s@%s...' % (
+          thread_num, output_filename, input_sha1_sum))
+    code, _, err = gsutil.check_call('cp', file_url, output_filename)
     if code != 0:
       if code == 404:
         out_q.put('%d> File %s for %s does not exist, skipping.' % (
@@ -294,25 +305,9 @@ def _downloader_worker_thread(thread_num, q, force, base_url,
         # Other error, probably auth related (bad ~/.boto, etc).
         out_q.put('%d> Failed to fetch file %s for %s, skipping. [Err: %s]' %
                   (thread_num, file_url, output_filename, err))
-        ret_codes.put((1, 'Failed to fetch file %s for %s. [Err: %s]' %
+        ret_codes.put((code, 'Failed to fetch file %s for %s. [Err: %s]' %
                        (file_url, output_filename, err)))
       continue
-    # Fetch the file.
-    if verbose:
-      out_q.put('%d> Downloading %s@%s...' %
-                (thread_num, output_filename, input_sha1_sum))
-    try:
-      if delete:
-        os.remove(output_filename)  # Delete the file if it exists already.
-    except OSError:
-      if os.path.exists(output_filename):
-        out_q.put('%d> Warning: deleting %s failed.' % (
-            thread_num, output_filename))
-    code, _, err = gsutil.check_call('cp', file_url, output_filename)
-    if code != 0:
-      out_q.put('%d> %s' % (thread_num, err))
-      ret_codes.put((code, err))
-      continue
 
     remote_sha1 = get_sha1(output_filename)
     if remote_sha1 != input_sha1_sum:

tests/download_from_google_storage_unittest.py

@@ -248,7 +248,6 @@ class DownloadTests(unittest.TestCase):
     sha1_hash = self.lorem_ipsum_sha1
     input_filename = '%s/%s' % (self.base_url, sha1_hash)
     output_filename = os.path.join(self.base_path, 'uploaded_lorem_ipsum.txt')
-    self.gsutil.add_expected(0, '', '')  # ls
     self.gsutil.add_expected(0, '', '', lambda: shutil.copyfile(
         self.lorem_ipsum, output_filename))  # cp
     self.queue.put((sha1_hash, output_filename))
@@ -258,8 +257,6 @@ class DownloadTests(unittest.TestCase):
         0, self.queue, False, self.base_url, self.gsutil,
         stdout_queue, self.ret_codes, True, False)
     expected_calls = [
-        ('check_call',
-            ('ls', input_filename)),
         ('check_call',
             ('cp', input_filename, output_filename))]
     sha1_hash = '7871c8e24da15bad8b0be2c36edc9dc77e37727f'
@@ -304,8 +301,6 @@ class DownloadTests(unittest.TestCase):
         0, self.queue, True, self.base_url, self.gsutil,
         stdout_queue, self.ret_codes, True, True, delete=False)
     expected_calls = [
-        ('check_call',
-            ('ls', input_filename)),
         ('check_call',
             ('cp', input_filename, output_filename))]
     if sys.platform != 'win32':
@@ -362,8 +357,7 @@ class DownloadTests(unittest.TestCase):
                                                            True,
                                                            True,
                                                            delete=False)
-    expected_calls += [('check_call', ('ls', input_filename)),
-                       ('check_call', ('cp', input_filename, output_filename))]
+    expected_calls += [('check_call', ('cp', input_filename, output_filename))]
     if sys.platform != 'win32':
       expected_calls.append(
           ('check_call', ('stat', 'gs://sometesturl/%s' % sha1_hash)))
@@ -389,17 +383,18 @@ class DownloadTests(unittest.TestCase):
     self.queue.put((sha1_hash, output_filename))
     self.queue.put((None, None))
     stdout_queue = queue.Queue()
-    self.gsutil.add_expected(1, '', '')  # Return error when 'ls' is called.
+    self.gsutil.add_expected(1, '', '')  # Return error when 'cp' is called.
     download_from_google_storage._downloader_worker_thread(
         0, self.queue, False, self.base_url, self.gsutil,
         stdout_queue, self.ret_codes, True, False)
     expected_output = [
+        '0> Downloading %s@%s...' % (output_filename, sha1_hash),
         '0> Failed to fetch file %s for %s, skipping. [Err: ]' % (
             input_filename, output_filename),
     ]
     expected_calls = [
         ('check_call',
-            ('ls', input_filename))
+            ('cp', input_filename, output_filename))
     ]
     expected_ret_codes = [
         (1, 'Failed to fetch file %s for %s. [Err: ]' % (
@@ -413,8 +408,7 @@ class DownloadTests(unittest.TestCase):
     sha1_hash = '7871c8e24da15bad8b0be2c36edc9dc77e37727f'
     input_filename = '%s/%s' % (self.base_url, sha1_hash)
     output_filename = os.path.join(self.base_path, 'uploaded_lorem_ipsum.txt')
-    self.gsutil.add_expected(0, '', '')  # ls
-    self.gsutil.add_expected(101, '', 'Test error message.')
+    self.gsutil.add_expected(101, '', 'Test error message.') # cp
     code = download_from_google_storage.download_from_google_storage(
         input_filename=sha1_hash,
         base_url=self.base_url,
@@ -430,8 +424,6 @@ class DownloadTests(unittest.TestCase):
         auto_platform=False,
         extract=False)
     expected_calls = [
-        ('check_call',
-            ('ls', input_filename)),
         ('check_call',
             ('cp', input_filename, output_filename))
     ]
@@ -450,8 +442,7 @@ class DownloadTests(unittest.TestCase):
     def _write_bad_file():
       with open(output_filename, 'w') as f:
         f.write('foobar')
-    self.gsutil.add_expected(0, '', '')
-    self.gsutil.add_expected(0, '', '', _write_bad_file)
+    self.gsutil.add_expected(0, '', '', _write_bad_file) # cp
     download_from_google_storage._downloader_worker_thread(
         1, q, True, self.base_url, self.gsutil, out_q, ret_codes, True, False)
     self.assertTrue(q.empty())
@@ -470,7 +461,6 @@ class DownloadTests(unittest.TestCase):
     input_filename = '%s/%s' % (self.base_url, sha1_hash)
     output_filename = os.path.join(self.base_path, 'uploaded_lorem_ipsum.txt')
     self.gsutil.add_expected(0, '', '')  # version
-    self.gsutil.add_expected(0, '', '')  # ls
     self.gsutil.add_expected(0, '', '', lambda: shutil.copyfile(
         self.lorem_ipsum, output_filename))  # cp
     code = download_from_google_storage.download_from_google_storage(
@@ -489,8 +479,6 @@ class DownloadTests(unittest.TestCase):
         extract=False)
     expected_calls = [
         ('check_call', ('version',)),
-        ('check_call',
-            ('ls', input_filename)),
         ('check_call',
             ('cp', input_filename, output_filename))]
     if sys.platform != 'win32':