New issue
Advanced search Search tips

Issue 616141 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Regression



Sign in to add a comment

regression: safe_browsing causes build noise

Project Member Reported by thakis@chromium.org, May 31 2016

Issue description

1. ninja -C out/gn

Expected: Silent build.
Actual:

[3188/19054] ACTION //chrome/browser/resources/safe_browsing:make_all_file_types_protobuf(//build/toolchain/linux:clang_x64)


To push these files, run the following:
python chrome/browser/resources/safe_browsing/push_file_type_proto.py -d /usr/local/google/home/thakis/src/chrome/src/out/gn/gen/chrome/browser/resources/safe_browsing/


The build should be silent.


Probably a regression from  https://codereview.chromium.org/2003323003
 
Why is that getting built?  Nothing depends on it.  My intention was that it would only be run manually, so the output would only be seen by the human running it.
Ah, I see you didn't specify a target, so that must mean "all."  Is there a way to exclude this from "all"?
'all' is all (in gn). It shouldn't include stuff that shouldn't run. What does this target do?
It builds the binary proto of the download_file_types.asciipb, and it does it for every platform. Then it gives instructions on how to push that (via a script) to GCS so it can be released as a component update.  This will be run by only a handful of people who are allowed to push this component update.  It needs to build the proto with the same script that builds as part of the normal build process, where it gets included as a ResourcePack.

I could remove the instructions it prints on stdout, but then the user will need to find the correct build directory to push the files it just built.  If they get it wrong and pick the wrong build directory, they may push some old stale files and break stuff.
Just make this a script that people run manually. `tools/foo/bar.py out/gn` isn't harder to type than `ninja -C out/gn foo_bar`, right? (The script could shell out to ninja if it needs to make sure something's built.
Sure, I can have the script call ninja, rather than have ninja generate the command to run.  Then I can remove the output to stdout.
(FYI I have a CL pending, and have been trying to land it since yesterday.
https://codereview.chromium.org/2032793002/)
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a04097da7297bfe1a78b99d5393c123d992644db

commit a04097da7297bfe1a78b99d5393c123d992644db
Author: nparker <nparker@chromium.org>
Date: Thu Jun 02 21:34:38 2016

Remove the stdout clutter from gen_file_type_proto.py

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

Review-Url: https://codereview.chromium.org/2032793002
Cr-Commit-Position: refs/heads/master@{#397510}

[modify] https://crrev.com/a04097da7297bfe1a78b99d5393c123d992644db/chrome/browser/resources/safe_browsing/gen_file_type_proto.py

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a2842ec02fb283d1fa3da22a311f8b05d78c7c9

commit 6a2842ec02fb283d1fa3da22a311f8b05d78c7c9
Author: nparker <nparker@chromium.org>
Date: Wed Jun 08 16:19:10 2016

Switch the file-type pusher to invoke Ninja

R=vakh
BUG= 616141 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2040053002
Cr-Commit-Position: refs/heads/master@{#398588}

[modify] https://crrev.com/6a2842ec02fb283d1fa3da22a311f8b05d78c7c9/chrome/browser/resources/safe_browsing/README.md
[modify] https://crrev.com/6a2842ec02fb283d1fa3da22a311f8b05d78c7c9/chrome/browser/resources/safe_browsing/push_file_type_proto.py

Components: Services>Safebrowsing
Status: Fixed (was: Assigned)

Sign in to add a comment