Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
807463005:
Add support for tar.gz archive files to download from download_from_google_storage (Closed)
|
Created:
5 years, 11 months ago by ricow1 Modified:
5 years, 5 months ago CC:
chromium-reviews, cmp-cc_chromium.org, dpranke+depot_tools_chromium.org, iannucci+depot_tools_chromium.org Target Ref:
refs/heads/master Project:
depot_tools Visibility:
Public. |
DescriptionAdd support for tar.gz archive files to download from download_from_google_storage
Also, add support for directly setting the public read flag on uploaded files.
The support for tar.gz allows the uploaded bundle to be a single tar.gz file instead of a lot of individual files using the directory support already in. The benefit here is that it is much easier to update the dependency. Simply clean out the existing files, copy in the new ones, create a tar.gz file, with the same name as the directory + 'tar.gz'. If the directory name and file name does not match up we will not clean up existing artifacts on download (i.e., there can be left over files after extracting).
I am doing this because I am moving a bunch of the dart dependencies to gcs, and a lot of our dependencies is much easier to manage with this in. If you don't like this, I can simply wrap the download script in another python script and do the logic there, but this may be handy for other people as well.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=295872
Patch Set 1 #
Total comments: 10
Patch Set 2 : #
Total comments: 4
Patch Set 3 : #
Total comments: 18
Patch Set 4 : #Patch Set 5 : #
Total comments: 7
Patch Set 6 : add some tests #
Total comments: 8
Patch Set 7 : address comments #
Total comments: 20
Patch Set 8 : fixes #
Total comments: 1
Messages
Total messages: 34 (7 generated)
ricow@google.com changed reviewers: + hinoka@google.com
Please let me know if this is something you want in and I will update the tests https://codereview.chromium.org/807463005/diff/1/upload_to_google_storage.py File upload_to_google_storage.py (left): https://codereview.chromium.org/807463005/diff/1/upload_to_google_storage.py#... upload_to_google_storage.py:214: parser.add_option('-g', '--gsutil_path', default=GSUTIL_DEFAULT_PATH, this does not seem to be supported anyway, so removing
hinoka@chromium.org changed reviewers: + hinoka@chromium.org
I like having tar.gz support for this script. Thanks for the patch. A couple of things: 1. Because dealing with relative directories is tricky, I think its better for upload_to_google_storage.py to have also take care of tarring (in addition to uploading). Litmus test: If the script does something different depending on which directory its in, thats no good. Otherwise I can envision lots of people uploading malformed or otherwise incorrectly rooted tarballs 2. Do a verification to make sure: 2a. ".." isn't in any path string. Thats a red flag there. 2b. Path is a relative path https://codereview.chromium.org/807463005/diff/1/download_from_google_storage.py File download_from_google_storage.py (right): https://codereview.chromium.org/807463005/diff/1/download_from_google_storage... download_from_google_storage.py:244: or not output_filename.endswith('tar.gz')): The second one doesn't seem to be necessary. Eg. it could be .tgz https://codereview.chromium.org/807463005/diff/1/download_from_google_storage... download_from_google_storage.py:245: out_q.put('%d> Warning: %s is not a tar.gz archive.' % ( s/Warning/Error/ https://codereview.chromium.org/807463005/diff/1/download_from_google_storage... download_from_google_storage.py:251: extract_dir = output_filename[0:len(output_filename)-7] os.path.splitext()[0] https://codereview.chromium.org/807463005/diff/1/upload_to_google_storage.py File upload_to_google_storage.py (left): https://codereview.chromium.org/807463005/diff/1/upload_to_google_storage.py#... upload_to_google_storage.py:214: parser.add_option('-g', '--gsutil_path', default=GSUTIL_DEFAULT_PATH, On 2015/01/19 16:02:28, ricow1 wrote: > this does not seem to be supported anyway, so removing Acknowledged. https://codereview.chromium.org/807463005/diff/1/upload_to_google_storage.py File upload_to_google_storage.py (right): https://codereview.chromium.org/807463005/diff/1/upload_to_google_storage.py#... upload_to_google_storage.py:226: parser.add_option('-p', '--public', action='store_true', Why? In general, I prefer to set this on a bucket basis (set the default ACL) instead of an object basis. This is to force us to keep public buckets fully public and private buckets fully private. IMO setting object ACLs individually leads to pain and suffering.
please take another look. Regarding testing: I have updated the existing tests to take the extra arguments, but how do you propose to test this? Should I add more files (tar.gz) to tests/gstools https://codereview.chromium.org/807463005/diff/1/download_from_google_storage.py File download_from_google_storage.py (right): https://codereview.chromium.org/807463005/diff/1/download_from_google_storage... download_from_google_storage.py:244: or not output_filename.endswith('tar.gz')): On 2015/01/20 19:28:52, hinoka wrote: > The second one doesn't seem to be necessary. Eg. it could be .tgz removed second check (although we could easily require this now that I am adding support to the upload script https://codereview.chromium.org/807463005/diff/1/download_from_google_storage... download_from_google_storage.py:245: out_q.put('%d> Warning: %s is not a tar.gz archive.' % ( On 2015/01/20 19:28:52, hinoka wrote: > s/Warning/Error/ Done. https://codereview.chromium.org/807463005/diff/1/download_from_google_storage... download_from_google_storage.py:251: extract_dir = output_filename[0:len(output_filename)-7] On 2015/01/20 19:28:52, hinoka wrote: > os.path.splitext()[0] Not really, assume foobar.tar.gz, that will give me foobar.tar https://codereview.chromium.org/807463005/diff/1/upload_to_google_storage.py File upload_to_google_storage.py (right): https://codereview.chromium.org/807463005/diff/1/upload_to_google_storage.py#... upload_to_google_storage.py:226: parser.add_option('-p', '--public', action='store_true', On 2015/01/20 19:28:52, hinoka wrote: > Why? > > In general, I prefer to set this on a bucket basis (set the default ACL) instead > of an object basis. This is to force us to keep public buckets fully public and > private buckets fully private. IMO setting object ACLs individually leads to > pain and suffering. Valid point, removed support for this
ping
another friendly ping, this is blocking us on git migration. I can still just do a small wrapper if you don't want this in.
https://codereview.chromium.org/807463005/diff/20001/download_from_google_sto... File download_from_google_storage.py (right): https://codereview.chromium.org/807463005/diff/20001/download_from_google_sto... download_from_google_storage.py:244: or not output_filename.endswith('tar.gz')): '.tar.gz' to match with expectations on 251 https://codereview.chromium.org/807463005/diff/20001/download_from_google_sto... download_from_google_storage.py:249: tar = tarfile.open(output_filename, 'r:gz') Also validate all paths in the tarfile here (Only having checks in the upload scripts isn't going to prevent malicious out-of-band uploads). May have to iterate through each file.
https://codereview.chromium.org/807463005/diff/20001/download_from_google_sto... File download_from_google_storage.py (right): https://codereview.chromium.org/807463005/diff/20001/download_from_google_sto... download_from_google_storage.py:244: or not output_filename.endswith('tar.gz')): On 2015/02/05 18:44:16, hinoka wrote: > '.tar.gz' to match with expectations on 251 Done. https://codereview.chromium.org/807463005/diff/20001/download_from_google_sto... download_from_google_storage.py:249: tar = tarfile.open(output_filename, 'r:gz') On 2015/02/05 18:44:16, hinoka wrote: > Also validate all paths in the tarfile here (Only having checks in the upload > scripts isn't going to prevent malicious out-of-band uploads). May have to > iterate through each file. Added check
Looks good in general, time for tests. Particularly, I'm not entirely sure about the path verification. Adding some comprehensive test cases for that would help. https://codereview.chromium.org/807463005/diff/40001/download_from_google_sto... File download_from_google_storage.py (right): https://codereview.chromium.org/807463005/diff/40001/download_from_google_sto... download_from_google_storage.py:209: return reduce(lambda x, y: x + 1 if not y.startswith(prefix) else x, files, 0) 1. Still prefer any(map(...)) 2. This doesn't look like it'd verify something like foo/bar/baz/../../../../../../../bin/bash. You need to do some verification on the os.realpath() version of the path. https://codereview.chromium.org/807463005/diff/40001/download_from_google_sto... download_from_google_storage.py:260: out_q.put('%d> extracting %s...' % (thread_num, extract_dir)) nit: s/extracting/Extracting/ https://codereview.chromium.org/807463005/diff/40001/download_from_google_sto... download_from_google_storage.py:405: help='Extract a downloaded tar.gz file after download. ' s/after download// (its redundant) https://codereview.chromium.org/807463005/diff/40001/download_from_google_sto... download_from_google_storage.py:406: 'Leaves the tar.gz file around for sha verification1' s/sha verification1/sha1 verification/ https://codereview.chromium.org/807463005/diff/40001/download_from_google_sto... download_from_google_storage.py:408: 'file already exists, this is deleted (to get a ' s/this/it/ https://codereview.chromium.org/807463005/diff/40001/upload_to_google_storage.py File upload_to_google_storage.py (right): https://codereview.chromium.org/807463005/diff/40001/upload_to_google_storage... upload_to_google_storage.py:216: def validate_archive_dirs(dirs): This seems a little confusing to read... Really what we want is to know if: 1. The path contains ".." 2. The path contains a symlink In fact a better way might be to just utilize tarfile.add() filter= capabilities. https://codereview.chromium.org/807463005/diff/40001/upload_to_google_storage... upload_to_google_storage.py:218: return name in next(os.walk('.'))[1] Why are you walking from cwd? https://codereview.chromium.org/807463005/diff/40001/upload_to_google_storage... upload_to_google_storage.py:219: return reduce(lambda x, y: x + 1 if not just_below_cwd(y) else x, dirs, 0) I prefer any(map(fn, dirs)) over reduce() https://codereview.chromium.org/807463005/diff/40001/upload_to_google_storage... upload_to_google_storage.py:247: if options.archive: Also add an isdir() check.
ptal, re testing, see my previous comment: "Regarding testing: I have updated the existing tests to take the extra arguments, but how do you propose to test this? Should I add more files (tar.gz) to tests/gstools" https://codereview.chromium.org/807463005/diff/40001/download_from_google_sto... File download_from_google_storage.py (right): https://codereview.chromium.org/807463005/diff/40001/download_from_google_sto... download_from_google_storage.py:209: return reduce(lambda x, y: x + 1 if not y.startswith(prefix) else x, files, 0) On 2015/02/06 18:54:35, hinoka wrote: > 1. Still prefer any(map(...)) > 2. This doesn't look like it'd verify something like > foo/bar/baz/../../../../../../../bin/bash. You need to do some verification on > the os.realpath() version of the path. Added more checks https://codereview.chromium.org/807463005/diff/40001/download_from_google_sto... download_from_google_storage.py:260: out_q.put('%d> extracting %s...' % (thread_num, extract_dir)) On 2015/02/06 18:54:35, hinoka wrote: > nit: s/extracting/Extracting/ Done. https://codereview.chromium.org/807463005/diff/40001/download_from_google_sto... download_from_google_storage.py:405: help='Extract a downloaded tar.gz file after download. ' On 2015/02/06 18:54:35, hinoka wrote: > s/after download// (its redundant) Done. https://codereview.chromium.org/807463005/diff/40001/download_from_google_sto... download_from_google_storage.py:406: 'Leaves the tar.gz file around for sha verification1' On 2015/02/06 18:54:35, hinoka wrote: > s/sha verification1/sha1 verification/ Done. https://codereview.chromium.org/807463005/diff/40001/download_from_google_sto... download_from_google_storage.py:408: 'file already exists, this is deleted (to get a ' On 2015/02/06 18:54:35, hinoka wrote: > s/this/it/ Done. https://codereview.chromium.org/807463005/diff/40001/upload_to_google_storage.py File upload_to_google_storage.py (right): https://codereview.chromium.org/807463005/diff/40001/upload_to_google_storage... upload_to_google_storage.py:216: def validate_archive_dirs(dirs): On 2015/02/06 18:54:35, hinoka wrote: > This seems a little confusing to read... Really what we want is to know if: > 1. The path contains ".." > 2. The path contains a symlink > > In fact a better way might be to just utilize tarfile.add() filter= > capabilities. Added isdir, not islink, not .. in path. Requiring the dir to be just below cwd actually makes sure it does not contain .. anyway https://codereview.chromium.org/807463005/diff/40001/upload_to_google_storage... upload_to_google_storage.py:218: return name in next(os.walk('.'))[1] On 2015/02/06 18:54:35, hinoka wrote: > Why are you walking from cwd? Look at the parser.error at the callsite (which I now changed to also include info on .. and symlinks. I don't think it makes sense to have the tar.gz.sha1 other places than where the directory is extracted. And this makes all the code more simple (i.e., you don't have to do path manipulation both here and in the extraction) https://codereview.chromium.org/807463005/diff/40001/upload_to_google_storage... upload_to_google_storage.py:219: return reduce(lambda x, y: x + 1 if not just_below_cwd(y) else x, dirs, 0) On 2015/02/06 18:54:35, hinoka wrote: > I prefer any(map(fn, dirs)) over reduce() done https://codereview.chromium.org/807463005/diff/40001/upload_to_google_storage... upload_to_google_storage.py:247: if options.archive: On 2015/02/06 18:54:35, hinoka wrote: > Also add an isdir() check. Added in validate_archive_dirs
Re: testing For the _downloader_worker_thread() portion I think its sufficient to just add code coverage. Good question about add tarfiles to gstools... if we can keep binary files out of the repo and just craft tarfiles in the testing code, that would be the best. I think its more important to validate the correctness of _validate_tar_file() (By crafting up working + malicious tar files, and making sure it properly rejects the malicious ones) just to ensure we're not introducing a new security hole. https://codereview.chromium.org/807463005/diff/80001/download_from_google_sto... File download_from_google_storage.py (right): https://codereview.chromium.org/807463005/diff/80001/download_from_google_sto... download_from_google_storage.py:207: def _validate_tar_file(tar, prefix): Also check for symbolic/hard links when decompressing. Its more important to do so while extracting than archiving. https://docs.python.org/2/library/tarfile.html#tarfile.TarInfo.issym https://codereview.chromium.org/807463005/diff/80001/download_from_google_sto... download_from_google_storage.py:209: if any(map(lambda x: '..' in x, files)): how about def _validate(tarinfo): """Returns false if the tarinfo is something we explicitly forbid.""" if tarinfo.issym() or tarinfo.islnk(): return False if '..' in tarinfo.name or not x.startswith(prefix): return False return True and then just return not any(map(_validate, tar.getmembers())) Would also be a good idea to bubble up the exact failure up a level. Maybe in that case a try/except/else would be better. https://codereview.chromium.org/807463005/diff/80001/download_from_google_sto... download_from_google_storage.py:257: if _validate_tar_file(tar, os.path.basename(extract_dir)): "_validate_tar_file" implies it would return True if valid, but this does the opposite. https://codereview.chromium.org/807463005/diff/80001/upload_to_google_storage.py File upload_to_google_storage.py (right): https://codereview.chromium.org/807463005/diff/80001/upload_to_google_storage... upload_to_google_storage.py:217: # We don't allow .. in paths in our archives. Each of these deserve their own error message.
better late than never, here is an updated version with some tests. https://codereview.chromium.org/807463005/diff/80001/download_from_google_sto... File download_from_google_storage.py (right): https://codereview.chromium.org/807463005/diff/80001/download_from_google_sto... download_from_google_storage.py:207: def _validate_tar_file(tar, prefix): On 2015/02/11 00:16:57, hinoka wrote: > Also check for symbolic/hard links when decompressing. Its more important to do > so while extracting than archiving. > > https://docs.python.org/2/library/tarfile.html#tarfile.TarInfo.issym > Changed to your suggestion below https://codereview.chromium.org/807463005/diff/80001/download_from_google_sto... download_from_google_storage.py:209: if any(map(lambda x: '..' in x, files)): On 2015/02/11 00:16:57, hinoka wrote: > how about > > def _validate(tarinfo): > """Returns false if the tarinfo is something we explicitly forbid.""" > if tarinfo.issym() or tarinfo.islnk(): > return False > if '..' in tarinfo.name or not x.startswith(prefix): > return False > return True > > and then just > > return not any(map(_validate, tar.getmembers())) > > Would also be a good idea to bubble up the exact failure up a level. > > Maybe in that case a try/except/else would be better. Done. https://codereview.chromium.org/807463005/diff/80001/download_from_google_sto... download_from_google_storage.py:257: if _validate_tar_file(tar, os.path.basename(extract_dir)): On 2015/02/11 00:16:57, hinoka wrote: > "_validate_tar_file" implies it would return True if valid, but this does the > opposite. Done.
PTAL
ping
mostly good, last comments https://codereview.chromium.org/807463005/diff/100001/download_from_google_st... File download_from_google_storage.py (right): https://codereview.chromium.org/807463005/diff/100001/download_from_google_st... download_from_google_storage.py:251: tar = tarfile.open(output_filename, 'r:gz') with tarfile.open(...) as tar: https://codereview.chromium.org/807463005/diff/100001/download_from_google_st... download_from_google_storage.py:269: out_q.put('%d> Extracting %s to %s' % (thread_num, output_filename, Is this supposed to be in this if block? Also seems redundant with line 259. Lets also add in # of files being extracted to the message. https://codereview.chromium.org/807463005/diff/100001/tests/download_from_goo... File tests/download_from_google_storage_unittests.py (right): https://codereview.chromium.org/807463005/diff/100001/tests/download_from_goo... tests/download_from_google_storage_unittests.py:90: os.makedirs(tar_dir) use absolute paths os.path.join(self.base_path, tar_dir) https://codereview.chromium.org/807463005/diff/100001/tests/download_from_goo... tests/download_from_google_storage_unittests.py:262: output_filename = os.path.join(self.base_path, 'tarfolder.tar.gz') can you just construct the tarfile here instead of checking in a binary file?
address comments
https://codereview.chromium.org/807463005/diff/100001/download_from_google_st... File download_from_google_storage.py (right): https://codereview.chromium.org/807463005/diff/100001/download_from_google_st... download_from_google_storage.py:251: tar = tarfile.open(output_filename, 'r:gz') On 2015/06/24 18:54:38, hinoka wrote: > with tarfile.open(...) as tar: Done. https://codereview.chromium.org/807463005/diff/100001/download_from_google_st... download_from_google_storage.py:269: out_q.put('%d> Extracting %s to %s' % (thread_num, output_filename, On 2015/06/24 18:54:38, hinoka wrote: > Is this supposed to be in this if block? Also seems redundant with line 259. > > Lets also add in # of files being extracted to the message. Removed printing above, added count https://codereview.chromium.org/807463005/diff/100001/tests/download_from_goo... File tests/download_from_google_storage_unittests.py (right): https://codereview.chromium.org/807463005/diff/100001/tests/download_from_goo... tests/download_from_google_storage_unittests.py:90: os.makedirs(tar_dir) On 2015/06/24 18:54:38, hinoka wrote: > use absolute paths > > os.path.join(self.base_path, tar_dir) We already changed into self.base_path, but done https://codereview.chromium.org/807463005/diff/100001/tests/download_from_goo... tests/download_from_google_storage_unittests.py:262: output_filename = os.path.join(self.base_path, 'tarfolder.tar.gz') On 2015/06/24 18:54:38, hinoka wrote: > can you just construct the tarfile here instead of checking in a binary file? Done.
lgtm % comments https://codereview.chromium.org/807463005/diff/120001/tests/download_from_goo... File tests/download_from_google_storage_unittests.py (right): https://codereview.chromium.org/807463005/diff/120001/tests/download_from_goo... tests/download_from_google_storage_unittests.py:61: 2 newlines (top level block) https://codereview.chromium.org/807463005/diff/120001/tests/download_from_goo... tests/download_from_google_storage_unittests.py:75: 2 newlines https://codereview.chromium.org/807463005/diff/120001/tests/download_from_goo... tests/download_from_google_storage_unittests.py:297: remove newline (since not a top level block) https://codereview.chromium.org/807463005/diff/120001/tests/upload_to_google_... File tests/upload_to_google_storage_unittests.py (right): https://codereview.chromium.org/807463005/diff/120001/tests/upload_to_google_... tests/upload_to_google_storage_unittests.py:71: dirname = 'subfolder' remove extra space https://codereview.chromium.org/807463005/diff/120001/tests/upload_to_google_... tests/upload_to_google_storage_unittests.py:78: content= map(lambda x: x.name, tar.getmembers()) space before = https://codereview.chromium.org/807463005/diff/120001/upload_to_google_storag... File upload_to_google_storage.py (right): https://codereview.chromium.org/807463005/diff/120001/upload_to_google_storag... upload_to_google_storage.py:209: top level function, 2 newlines https://codereview.chromium.org/807463005/diff/120001/upload_to_google_storag... upload_to_google_storage.py:218: 2 newlines https://codereview.chromium.org/807463005/diff/120001/upload_to_google_storag... upload_to_google_storage.py:232: remove 1 newline so there are 2 total https://codereview.chromium.org/807463005/diff/120001/upload_to_google_storag... upload_to_google_storage.py:265: remove newline (inside function, just 1 is enough) https://codereview.chromium.org/807463005/diff/120001/upload_to_google_storag... upload_to_google_storage.py:267: if validate_archive_dirs(input_filenames): if not validate_archive_dirs()?
fixes
https://codereview.chromium.org/807463005/diff/120001/tests/download_from_goo... File tests/download_from_google_storage_unittests.py (right): https://codereview.chromium.org/807463005/diff/120001/tests/download_from_goo... tests/download_from_google_storage_unittests.py:61: On 2015/06/25 17:19:14, Ryan Tseng wrote: > 2 newlines (top level block) Done. https://codereview.chromium.org/807463005/diff/120001/tests/download_from_goo... tests/download_from_google_storage_unittests.py:75: On 2015/06/25 17:19:14, Ryan Tseng wrote: > 2 newlines Done. https://codereview.chromium.org/807463005/diff/120001/tests/download_from_goo... tests/download_from_google_storage_unittests.py:297: On 2015/06/25 17:19:14, Ryan Tseng wrote: > remove newline (since not a top level block) Done. https://codereview.chromium.org/807463005/diff/120001/tests/upload_to_google_... File tests/upload_to_google_storage_unittests.py (right): https://codereview.chromium.org/807463005/diff/120001/tests/upload_to_google_... tests/upload_to_google_storage_unittests.py:71: dirname = 'subfolder' On 2015/06/25 17:19:14, Ryan Tseng wrote: > remove extra space Done. https://codereview.chromium.org/807463005/diff/120001/tests/upload_to_google_... tests/upload_to_google_storage_unittests.py:78: content= map(lambda x: x.name, tar.getmembers()) On 2015/06/25 17:19:14, Ryan Tseng wrote: > space before = Done. https://codereview.chromium.org/807463005/diff/120001/upload_to_google_storag... File upload_to_google_storage.py (right): https://codereview.chromium.org/807463005/diff/120001/upload_to_google_storag... upload_to_google_storage.py:209: On 2015/06/25 17:19:14, Ryan Tseng wrote: > top level function, 2 newlines Done. https://codereview.chromium.org/807463005/diff/120001/upload_to_google_storag... upload_to_google_storage.py:218: On 2015/06/25 17:19:14, Ryan Tseng wrote: > 2 newlines Done. https://codereview.chromium.org/807463005/diff/120001/upload_to_google_storag... upload_to_google_storage.py:232: On 2015/06/25 17:19:14, Ryan Tseng wrote: > remove 1 newline so there are 2 total Done. https://codereview.chromium.org/807463005/diff/120001/upload_to_google_storag... upload_to_google_storage.py:265: On 2015/06/25 17:19:14, Ryan Tseng wrote: > remove newline (inside function, just 1 is enough) Done. https://codereview.chromium.org/807463005/diff/120001/upload_to_google_storag... upload_to_google_storage.py:267: if validate_archive_dirs(input_filenames): On 2015/06/25 17:19:15, Ryan Tseng wrote: > if not validate_archive_dirs()? Done.
The CQ bit was checked by ricow@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from hinoka@google.com Link to the patchset: https://codereview.chromium.org/807463005/#ps140001 (title: "fixes")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/807463005/140001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: depot_tools_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/depot_tools_pre...)
Ryan: looking at the owners file, you probably need to lgtm this with your @chromium.org account
lgtm
The CQ bit was checked by ricow@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/807463005/140001
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as http://src.chromium.org/viewvc/chrome?view=rev&revision=295872
Message was sent while issue was closed.
azarchs@chromium.org changed reviewers: + azarchs@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/807463005/diff/140001/upload_to_google_storag... File upload_to_google_storage.py (right): https://codereview.chromium.org/807463005/diff/140001/upload_to_google_storag... upload_to_google_storage.py:242: help='Archive directory as a tar.gz file') This conflicts with (and duplicates?) the -z option below and has broken one of the chrome android downstream bots.
Message was sent while issue was closed.
A revert of this CL (patchset #8 id:140001) has been created in https://codereview.chromium.org/1209033006/ by boliu@chromium.org. The reason for reverting is: This conflicts with (and duplicates?) the -z option below and has broken one of the chrome android downstream bots. Safer to revert than trying to fix. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
