Script to push download_file_types.pb to all platforms, via Component Updater

Script to push download_file_types.pb to all platforms, via Component Updater.

BUG=596555
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/0627b3f53c2fb7c0c7a0ae73f94f35c06d53b394
Cr-Commit-Position: refs/heads/master@{#396114}

[Nathan Parker, 2016-05-24 22:35:26]

Description was changed from

==========
Script to push download_file_types.pb to all platforms, via Component Updater.

BUG=596555
==========

to

==========
Script to push download_file_types.pb to all platforms, via Component Updater.

BUG=596555
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Nathan Parker, 2016-05-24 22:37:13]

nparker@chromium.org changed reviewers:
+ vakh@chromium.org

[Nathan Parker, 2016-05-24 22:37:14]

The design doc describes the overall flow of the file pushing:
https://docs.google.com/document/d/10OYEl7xXQ9D9d81bvwe5joVaxSVA4qDIhVWdZAPQH...

[vakh (use Gerrit instead), 2016-05-24 23:31:57]

https://codereview.chromium.org/2003323003/diff/1/chrome/browser/resources/sa...
File chrome/browser/resources/safe_browsing/gen_file_type_proto.py (right):

https://codereview.chromium.org/2003323003/diff/1/chrome/browser/resources/sa...
chrome/browser/resources/safe_browsing/gen_file_type_proto.py:182: all_dir =
os.path.abspath(opts.outfile[:opts.outfile.find("%d") - 1])
This assumes a particular format for opts.outfile, which may not be the case
since it is a user input path. That may end up confusing the user.
A more user-friendly way to deal with this may be to split opts.outfile into
opts.outpath and opts.outbasename and always enforce the path for -all operation
to be: opts.outpath + "/%d/%s/" + opts.outbasename

https://codereview.chromium.org/2003323003/diff/1/chrome/browser/resources/sa...
File chrome/browser/resources/safe_browsing/push_file_type_proto.py (right):

https://codereview.chromium.org/2003323003/diff/1/chrome/browser/resources/sa...
chrome/browser/resources/safe_browsing/push_file_type_proto.py:53: for f in
expected_files:
replace with:
print '   '.join(expected_files)

[Nathan Parker, 2016-05-24 23:55:50]

Thanks! PTAL

https://codereview.chromium.org/2003323003/diff/1/chrome/browser/resources/sa...
File chrome/browser/resources/safe_browsing/gen_file_type_proto.py (right):

https://codereview.chromium.org/2003323003/diff/1/chrome/browser/resources/sa...
chrome/browser/resources/safe_browsing/gen_file_type_proto.py:182: all_dir =
os.path.abspath(opts.outfile[:opts.outfile.find("%d") - 1])
On 2016/05/24 23:31:57, vakh wrote:
> This assumes a particular format for opts.outfile, which may not be the case
> since it is a user input path. That may end up confusing the user.
> A more user-friendly way to deal with this may be to split opts.outfile into
> opts.outpath and opts.outbasename and always enforce the path for -all
operation
> to be: opts.outpath + "/%d/%s/" + opts.outbasename

Yup, that's better.  Done.

https://codereview.chromium.org/2003323003/diff/1/chrome/browser/resources/sa...
File chrome/browser/resources/safe_browsing/push_file_type_proto.py (right):

https://codereview.chromium.org/2003323003/diff/1/chrome/browser/resources/sa...
chrome/browser/resources/safe_browsing/push_file_type_proto.py:53: for f in
expected_files:
On 2016/05/24 23:31:57, vakh wrote:
> replace with:
> print '   '.join(expected_files)

But that doesn't add the newlines. So I'd need something like

  print '\n'.join(['   '+x for x in expected_files])

which I'd argue is harder to read.  Is there a better way?

[vakh (use Gerrit instead), 2016-05-25 21:10:07]

lgtm

https://codereview.chromium.org/2003323003/diff/1/chrome/browser/resources/sa...
File chrome/browser/resources/safe_browsing/push_file_type_proto.py (right):

https://codereview.chromium.org/2003323003/diff/1/chrome/browser/resources/sa...
chrome/browser/resources/safe_browsing/push_file_type_proto.py:53: for f in
expected_files:
On 2016/05/24 23:55:50, Nathan Parker wrote:
> On 2016/05/24 23:31:57, vakh wrote:
> > replace with:
> > print '   '.join(expected_files)
> 
> But that doesn't add the newlines. So I'd need something like
> 
>   print '\n'.join(['   '+x for x in expected_files])
> 
> which I'd argue is harder to read.  Is there a better way?

print '   ' + '\n   '.join(expected_files)

[Nathan Parker, 2016-05-25 21:15:09]

nparker@chromium.org changed reviewers:
+ thestig@chromium.org

[Nathan Parker, 2016-05-25 21:15:09]

thestig@chromium.org: Please review changes in 
  chrome/chrome_resources.gyp
and maybe
  chrome/browser/resources/safe_browsing/BUILD.gn
if you feel like it.

Thanks!

[Lei Zhang, 2016-05-25 21:44:09]

https://codereview.chromium.org/2003323003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/safe_browsing/BUILD.gn (right):

https://codereview.chromium.org/2003323003/diff/40001/chrome/browser/resource...
chrome/browser/resources/safe_browsing/BUILD.gn:69:
action("make_all_file_types_protobuf") {
Just curious why there's no GYP equivalent. Is it because one would only do this
by hand, and those who do this only use GN?

[Nathan Parker, 2016-05-26 00:38:27]

https://codereview.chromium.org/2003323003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/safe_browsing/BUILD.gn (right):

https://codereview.chromium.org/2003323003/diff/40001/chrome/browser/resource...
chrome/browser/resources/safe_browsing/BUILD.gn:69:
action("make_all_file_types_protobuf") {
On 2016/05/25 21:44:09, Lei Zhang wrote:
> Just curious why there's no GYP equivalent. Is it because one would only do
this
> by hand, and those who do this only use GN?

Yes!  You've got it.

[Lei Zhang, 2016-05-26 00:39:29]

lgtm

[Nathan Parker, 2016-05-26 00:43:44]

The CQ bit was checked by nparker@chromium.org

[commit-bot: I haz the power, 2016-05-26 00:44:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2003323003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2003323003/40001

[commit-bot: I haz the power, 2016-05-26 02:49:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-26 02:49:13]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Lei Zhang, 2016-05-26 02:50:11]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2016-05-26 02:50:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2003323003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2003323003/40001

[commit-bot: I haz the power, 2016-05-26 04:06:59]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-26 04:08:08]

Description was changed from

==========
Script to push download_file_types.pb to all platforms, via Component Updater.

BUG=596555
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Script to push download_file_types.pb to all platforms, via Component Updater.

BUG=596555
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/0627b3f53c2fb7c0c7a0ae73f94f35c06d53b394
Cr-Commit-Position: refs/heads/master@{#396114}
==========

[commit-bot: I haz the power, 2016-05-26 04:08:09]

Patchset 3 (id:??) landed as
https://crrev.com/0627b3f53c2fb7c0c7a0ae73f94f35c06d53b394
Cr-Commit-Position: refs/heads/master@{#396114}

[brucedawson, 2016-06-02 00:45:06]

brucedawson@chromium.org changed reviewers:
+ brucedawson@chromium.org

[brucedawson, 2016-06-02 00:45:07]

https://codereview.chromium.org/2003323003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/safe_browsing/gen_file_type_proto.py (right):

https://codereview.chromium.org/2003323003/diff/40001/chrome/browser/resource...
chrome/browser/resources/safe_browsing/gen_file_type_proto.py:181: print "\n\nTo
push these files, run the following:"
This message appears to print as part of a normal build. It's quite weird seeing
this message out of context. Was that intentional? Either way it probably
shouldn't be shown unless explicit requested by the user.

[vakh (use Gerrit instead), 2016-06-02 01:54:07]

On 2016/06/02 00:45:07, brucedawson wrote:
>
https://codereview.chromium.org/2003323003/diff/40001/chrome/browser/resource...
> File chrome/browser/resources/safe_browsing/gen_file_type_proto.py (right):
> 
>
https://codereview.chromium.org/2003323003/diff/40001/chrome/browser/resource...
> chrome/browser/resources/safe_browsing/gen_file_type_proto.py:181: print
"\n\nTo
> push these files, run the following:"
> This message appears to print as part of a normal build. It's quite weird
seeing
> this message out of context. Was that intentional? Either way it probably
> shouldn't be shown unless explicit requested by the user.

This has been removed via: https://codereview.chromium.org/2032793002/