diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 249462ef2..ac3fc0d23 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -70,7 +70,6 @@ def CommonChecks(input_api, output_api, tests_to_black_list, run_on_python3): print('Warning: skipping most unit tests on Windows') tests_to_black_list = [ r'.*auth_test\.py$', - r'.*download_from_google_storage_unittest\.py$', r'.*gclient_smoketest\.py$', r'.*git_cl_test\.py$', r'.*git_common_test\.py$', @@ -82,7 +81,6 @@ def CommonChecks(input_api, output_api, tests_to_black_list, run_on_python3): r'.*roll_dep_test\.py$', r'.*scm_unittest\.py$', r'.*subprocess2_test\.py$', - r'.*upload_to_google_storage_unittest\.py$', ] # TODO(maruel): Make sure at least one file is modified first. diff --git a/download_from_google_storage.py b/download_from_google_storage.py index 067f772ae..806b78800 100755 --- a/download_from_google_storage.py +++ b/download_from_google_storage.py @@ -114,14 +114,17 @@ class Gsutil(object): stderr=subprocess2.PIPE, env=self.get_sub_env()) + out = out.decode('utf-8', 'replace') + err = err.decode('utf-8', 'replace') + # Parse output. - status_code_match = re.search(b'status=([0-9]+)', err) + status_code_match = re.search('status=([0-9]+)', err) if status_code_match: return (int(status_code_match.group(1)), out, err) - if (b'You are attempting to access protected data with ' - b'no configured credentials.' in err): + if ('You are attempting to access protected data with ' + 'no configured credentials.' in err): return (403, out, err) - if b'matched no objects' in err: + if 'matched no objects' in err: return (404, out, err) return (code, out, err) @@ -267,9 +270,9 @@ def _downloader_worker_thread(thread_num, q, force, base_url, else: # 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.decode())) + (thread_num, file_url, output_filename, err)) ret_codes.put((1, 'Failed to fetch file %s for %s. [Err: %s]' % - (file_url, output_filename, err.decode()))) + (file_url, output_filename, err))) continue # Fetch the file. out_q.put('%d> Downloading %s...' % (thread_num, output_filename)) @@ -282,8 +285,8 @@ def _downloader_worker_thread(thread_num, q, force, base_url, 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.decode())) - ret_codes.put((code, err.decode())) + out_q.put('%d> %s' % (thread_num, err)) + ret_codes.put((code, err)) continue remote_sha1 = get_sha1(output_filename) @@ -336,11 +339,11 @@ def _downloader_worker_thread(thread_num, q, force, base_url, elif sys.platform != 'win32': # On non-Windows platforms, key off of the custom header # "x-goog-meta-executable". - code, out, _ = gsutil.check_call('stat', file_url) + code, out, err = gsutil.check_call('stat', file_url) if code != 0: - out_q.put('%d> %s' % (thread_num, err.decode())) - ret_codes.put((code, err.decode())) - elif re.search(r'executable:\s*1', out.decode()): + out_q.put('%d> %s' % (thread_num, err)) + ret_codes.put((code, err)) + elif re.search(r'executable:\s*1', out): st = os.stat(output_filename) os.chmod(output_filename, st.st_mode | stat.S_IEXEC) diff --git a/gsutil.py b/gsutil.py index 64e4b5cbc..8d5de5df3 100755 --- a/gsutil.py +++ b/gsutil.py @@ -140,7 +140,8 @@ def run_gsutil(force_version, fallback, target, args, clean=False): # This script requires Windows Python, so invoke with depot_tools' # Python. def winpath(path): - return subprocess.check_output(['cygpath', '-w', path]).strip() + stdout = subprocess.check_output(['cygpath', '-w', path]) + return stdout.strip().decode('utf-8', 'replace') cmd = ['python.bat', winpath(__file__)] cmd.extend(args) sys.exit(subprocess.call(cmd)) diff --git a/tests/download_from_google_storage_unittest.py b/tests/download_from_google_storage_unittest.py index 7a6ed965b..4fe58ac61 100755 --- a/tests/download_from_google_storage_unittest.py +++ b/tests/download_from_google_storage_unittest.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env vpython3 # Copyright (c) 2012 The Chromium Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. @@ -71,7 +71,7 @@ class GsutilMock(object): fn() return code, out, err else: - return (0, b'', b'') + return (0, '', '') def check_call_with_retries(self, *args): return self.check_call(*args) @@ -115,16 +115,19 @@ class GstoolsUnitTests(unittest.TestCase): self.assertTrue( download_from_google_storage._validate_tar_file(tar, tar_dir)) - # Test no links. - tar_dir_link = 'for_tar_link' - os.makedirs(tar_dir_link) - link = os.path.join(tar_dir_link, 'link') - os.symlink(lorem_ipsum, link) - tar_with_links = 'with_links.tar.gz' - with tarfile.open(tar_with_links, 'w:gz') as tar: - tar.add(link) - self.assertFalse( - download_from_google_storage._validate_tar_file(tar, tar_dir_link)) + # os.symlink doesn't exist on Windows. + if sys.platform != 'win32': + # Test no links. + tar_dir_link = 'for_tar_link' + os.makedirs(tar_dir_link) + link = os.path.join(tar_dir_link, 'link') + os.symlink(lorem_ipsum, link) + tar_with_links = 'with_links.tar.gz' + with tarfile.open(tar_with_links, 'w:gz') as tar: + tar.add(link) + self.assertFalse( + download_from_google_storage._validate_tar_file( + tar, tar_dir_link)) # Test not outside. tar_dir_outside = 'outside_tar' @@ -158,8 +161,8 @@ class GstoolsUnitTests(unittest.TestCase): gsutil = download_from_google_storage.Gsutil(GSUTIL_DEFAULT_PATH, None) self.assertEqual(gsutil.path, GSUTIL_DEFAULT_PATH) code, _, err = gsutil.check_call() - self.assertEqual(code, 0) - self.assertEqual(err, b'') + self.assertEqual(code, 0, err) + self.assertEqual(err, '') def test_get_sha1(self): lorem_ipsum = os.path.join(self.base_path, 'lorem_ipsum.txt') @@ -245,8 +248,8 @@ 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, b'', b'') # ls - self.gsutil.add_expected(0, b'', b'', lambda: shutil.copyfile( + 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)) self.queue.put((None, None)) @@ -329,7 +332,7 @@ 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, b'', b'') # Return error when 'ls' is called. + self.gsutil.add_expected(1, '', '') # Return error when 'ls' 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) @@ -353,8 +356,8 @@ 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, b'', b'') # ls - self.gsutil.add_expected(101, b'', b'Test error message.') + self.gsutil.add_expected(0, '', '') # ls + self.gsutil.add_expected(101, '', 'Test error message.') code = download_from_google_storage.download_from_google_storage( input_filename=sha1_hash, base_url=self.base_url, diff --git a/tests/upload_to_google_storage_unittest.py b/tests/upload_to_google_storage_unittest.py index e91c83042..4d9bf69dc 100755 --- a/tests/upload_to_google_storage_unittest.py +++ b/tests/upload_to_google_storage_unittest.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env vpython3 # Copyright (c) 2012 The Chromium Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. @@ -7,6 +7,7 @@ import optparse import os +import posixpath try: import Queue as queue @@ -87,11 +88,12 @@ class UploadTests(unittest.TestCase): self.assertTrue(os.path.exists(tar_gz_file)) with tarfile.open(tar_gz_file, 'r:gz') as tar: content = map(lambda x: x.name, tar.getmembers()) - self.assertTrue(dirname in content) - self.assertTrue(os.path.join(dirname, 'subfolder_text.txt') in content) - self.assertTrue( - os.path.join(dirname, 'subfolder_text.txt.sha1') in content) + self.assertIn(dirname, content) + self.assertIn(posixpath.join(dirname, 'subfolder_text.txt'), content) + self.assertIn( + posixpath.join(dirname, 'subfolder_text.txt.sha1'), content) + @unittest.skipIf(sys.platform == 'win32', 'os.symlink does not exist on win') def test_validate_archive_dirs_fails(self): work_dir = os.path.join(self.base_path, 'download_test_data') with ChangedWorkingDirectory(work_dir): diff --git a/upload_to_google_storage.py b/upload_to_google_storage.py index b3b636738..bf1c40543 100755 --- a/upload_to_google_storage.py +++ b/upload_to_google_storage.py @@ -84,7 +84,7 @@ def _upload_worker( if gsutil.check_call('ls', file_url)[0] == 0 and not force: # File exists, check MD5 hash. _, out, _ = gsutil.check_call_with_retries('ls', '-L', file_url) - etag_match = re.search(r'ETag:\s+([a-z0-9]{32})', out.decode()) + etag_match = re.search(r'ETag:\s+([a-z0-9]{32})', out) if etag_match: remote_md5 = etag_match.group(1) # Calculate the MD5 checksum to match it to Google Storage's ETag.