New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 772741 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug

Blocked on:
issue 773372



Sign in to add a comment

Make `gclient runhooks` less noisy

Project Member Reported by thakis@chromium.org, Oct 8 2017

Issue description

ssia
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/8db10a6fb1d4b86909b8cb32e3b53e35624c8979

commit 8db10a6fb1d4b86909b8cb32e3b53e35624c8979
Author: Nico Weber <thakis@chromium.org>
Date: Mon Oct 09 00:36:06 2017

gclient: Stop printing newline in front of "________ running hook"

The newline is probably here to protect against a hook that doesn't print
a trailing newline.  I've never seen a hook that did that, and if we found one,
we could make the logic look like "print a trailing newline if it's not there"
-- or just fix the hook.

The newline has been around since depot_tools was created
(https://codereview.chromium.org/92087, gclient.py), back then this was in
a general "run stuff" function, not in hooks-specific code.  Maybe it made
more sense back then.

This is part of a few changes to make `gclient runhooks` less noisy.

Bug:  772741 
Change-Id: I285f76dc3f01c5acf5bbaa0be4db9f6edb9c0366
Reviewed-on: https://chromium-review.googlesource.com/706914
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>

[modify] https://crrev.com/8db10a6fb1d4b86909b8cb32e3b53e35624c8979/tests/gclient_scm_test.py
[modify] https://crrev.com/8db10a6fb1d4b86909b8cb32e3b53e35624c8979/gclient_utils.py
[modify] https://crrev.com/8db10a6fb1d4b86909b8cb32e3b53e35624c8979/tests/gclient_utils_test.py

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 9 2017

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

commit b1943703eb83ae53934b6e2ed22814daf1a182c3
Author: Nico Weber <thakis@chromium.org>
Date: Mon Oct 09 02:45:09 2017

Use the spiffy new DEPS conditionals feature to run fewer no-op hooks.

We have a bunch of hooks that used to run on all platforms, but would then
early-exit on all-but-one.  Now that we have conditionals support in DEPS,
we can run the hook only on the right platform in the first place, reducing
the amount of stuff `gclient runhooks` prints.

(Once DEPS files in v8, webrtc, ... do this too, we can remove the early-exit
logic from these scripts as well.)

No intended behavior change.

Bug:  570091 , 772741 
Change-Id: If4527833680f9834a987a0a6a382217a75fddcfd
Reviewed-on: https://chromium-review.googlesource.com/706825
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507321}
[modify] https://crrev.com/b1943703eb83ae53934b6e2ed22814daf1a182c3/DEPS

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 9 2017

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

commit f98681b809b1f9026b90fccfba6f1835c258146d
Author: Nico Weber <thakis@chromium.org>
Date: Mon Oct 09 18:08:53 2017

Make clang update script print nothing in the happy case where it needs to do nothing.

This is part of a few changes to make `gclient runhooks` less noisy.

Bug:  772741 
Change-Id: If3d8621fc282a49488beac131a3519d2c141c482
Reviewed-on: https://chromium-review.googlesource.com/706826
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507413}
[modify] https://crrev.com/f98681b809b1f9026b90fccfba6f1835c258146d/tools/clang/scripts/update.py

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 9 2017

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

commit bbdcc4931afb03517d3c7f6e6192eafc244a350c
Author: John Budorick <jbudorick@chromium.org>
Date: Mon Oct 09 18:57:09 2017

Restrict checkstyle JAR download to android and linux.

Bug:  727875 , 772741 
Change-Id: Iac36fc9426b2926c78334bedb8bfa0aaec57ab5a
Reviewed-on: https://chromium-review.googlesource.com/707300
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507432}
[modify] https://crrev.com/bbdcc4931afb03517d3c7f6e6192eafc244a350c/DEPS

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/87677e9830fec827c48d57d5dc764e0320282585

commit 87677e9830fec827c48d57d5dc764e0320282585
Author: Nico Weber <thakis@chromium.org>
Date: Mon Oct 09 19:48:26 2017

Make download_from_google_storage.py print nothing in the happy case where it needs to do nothing.

The script prints a bunch of stuff behind `if verbose` checks, but then
"verbose" was turned on by default in
https://bugs.chromium.org/p/chromium/issues/detail?id=504452

I think the wrong lesson was learned from that bug – it sounds like the
problem was that an error message was printed only if verbose was set.
After this change, the script is silent when it does nothing, and prints
something if something happens. (Arguably, it still prints too much in the
case where it successfully downloads something.)

This is part of a few changes to make `gclient runhooks` less noisy.

Bug:  772741 ,504452
Change-Id: I5823c296abe97611ba4411bab2743673b10dca4b
Reviewed-on: https://chromium-review.googlesource.com/706915
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Ryan Tseng <hinoka@chromium.org>

[modify] https://crrev.com/87677e9830fec827c48d57d5dc764e0320282585/tests/download_from_google_storage_unittests.py
[modify] https://crrev.com/87677e9830fec827c48d57d5dc764e0320282585/download_from_google_storage.py
[modify] https://crrev.com/87677e9830fec827c48d57d5dc764e0320282585/upload_to_google_storage.py

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/6bba5a9b4a01ba9e27b856108ff62f880a60fca3

commit 6bba5a9b4a01ba9e27b856108ff62f880a60fca3
Author: Nico Weber <thakis@chromium.org>
Date: Tue Oct 10 02:44:15 2017

Restore (unused) "verbose" paramter in download_from_google_storage.py

Apparently several scripts in other repros call this function directly.

Bug:  772741 
Change-Id: I486483e44a072af6cf8c75373a2da6ef5469fc2c
TBR=dpranke
Reviewed-on: https://chromium-review.googlesource.com/707937
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>

[modify] https://crrev.com/6bba5a9b4a01ba9e27b856108ff62f880a60fca3/tests/download_from_google_storage_unittests.py
[modify] https://crrev.com/6bba5a9b4a01ba9e27b856108ff62f880a60fca3/download_from_google_storage.py

Cc: mbjorge@chromium.org

Comment 8 by thakis@chromium.org, Oct 10 2017

Blockedon: 773372
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 10 2017

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

commit 03dff362ace00df6ad8825d1c8baabae5af9de94
Author: Nico Weber <thakis@chromium.org>
Date: Tue Oct 10 21:06:07 2017

Make mac toolchain update script print nothing in the happy case where it needs to do nothing.

The script now prints nothing both when it uses a local toolchain and when it
uses a hermetic toolchain that's already up-to-date, but I argue that almost
nobody was looking at that output anyways.

This is part of a few changes to make `gclient runhooks` less noisy.

Bug:  772741 
Change-Id: I4f32491894269522617cde7fd180344ae02c43bb
Reviewed-on: https://chromium-review.googlesource.com/709637
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507770}
[modify] https://crrev.com/03dff362ace00df6ad8825d1c8baabae5af9de94/build/mac_toolchain.py

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 10 2017

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

commit b4ac34edef3a7552cb38c938f00eb45885047044
Author: Nico Weber <thakis@chromium.org>
Date: Tue Oct 10 21:50:31 2017

Make linux sysroot script print nothing in the happy case where it needs to do nothing.

Since this is the only way it can print nothing, no information is getting lost.
This is part of a few changes to make `gclient runhooks` less noisy.

Bug:  772741 
Change-Id: I9d3e22d13c909294466a609289c9e36b5f94bbe1
Reviewed-on: https://chromium-review.googlesource.com/710456
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507784}
[modify] https://crrev.com/b4ac34edef3a7552cb38c938f00eb45885047044/build/linux/sysroot_scripts/install-sysroot.py

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 12 2017

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

commit 92489af91e4acddc0ad81b1a69e1d98c4615ba9c
Author: Rohit Rao <rohitrao@google.com>
Date: Thu Oct 12 21:45:48 2017

Fetch the mac toolchain on both iOS and macOS.

The toolchain is needed for iOS builds as well.

BUG= 772741 ,773328

Change-Id: I1a724af954d9b83debc772efa7142e0804034dfa
Reviewed-on: https://chromium-review.googlesource.com/716956
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508484}
[modify] https://crrev.com/92489af91e4acddc0ad81b1a69e1d98c4615ba9c/DEPS

This is much better now, but there's still more to do. It currently prints

"______ running <lengthy command>"

for each hook. Instead, it should just print "Running hook Y% (X/N) <hook name>" and if the hook produces no output (like almost all of them now do), it should overprint that line (with "\r") when printing the next hook. If a hook does produce output, the output should be preceded by the old "______ running <lengthy command>" line so that no information gets lost. Does anyone feel like giving that a try?
Cc: petermayo@chromium.org
 Issue 761558  has been merged into this issue.
Cc: scottmg@chromium.org
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/5fd1defbcc525aa11f3eef1e5c065de3cdfc6c21

commit 5fd1defbcc525aa11f3eef1e5c065de3cdfc6c21
Author: Daniel Cheng <dcheng@chromium.org>
Date: Thu Oct 19 19:54:56 2017

gclient runhooks: now with less spammy progress notifications

Bug:  772741 
Change-Id: I62ad64219ba27ee4ea26ceb603b1133dff4a2e04
Reviewed-on: https://chromium-review.googlesource.com/727194
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>

[modify] https://crrev.com/5fd1defbcc525aa11f3eef1e5c065de3cdfc6c21/gclient_utils.py
[modify] https://crrev.com/5fd1defbcc525aa11f3eef1e5c065de3cdfc6c21/gclient.py
[modify] https://crrev.com/5fd1defbcc525aa11f3eef1e5c065de3cdfc6c21/tests/gclient_scm_test.py

It's so gloriously quiet! Thanks to you both. :)

[Just need to best ol'

Running hooks:  49% (32/65) webui_node_modules   
________ running '/usr/bin/python src/tools/perf/conditionally_execute --gyp-condition fetch_telemetry_dependencies=1 src/third_party/catapult/telemetry/bin/fetch_telemetry_binary_dependencies' in '/usr/local/google/work/cr'
Not found "fetch_telemetry_dependencies=1" condition in GYP_DEFINES="[]". Skip script execution.
]
Project Member

Comment 18 by bugdroid1@chromium.org, Oct 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/a21b5b314bcdef9f29d9ea1325accf3920a5f937

commit a21b5b314bcdef9f29d9ea1325accf3920a5f937
Author: Daniel Cheng <dcheng@chromium.org>
Date: Thu Oct 19 20:20:57 2017

Revert "gclient runhooks: now with less spammy progress notifications"

This reverts commit 5fd1defbcc525aa11f3eef1e5c065de3cdfc6c21.

Reason for revert: breaks v8 because hook names are optional

Original change's description:
> gclient runhooks: now with less spammy progress notifications
> 
> Bug:  772741 
> Change-Id: I62ad64219ba27ee4ea26ceb603b1133dff4a2e04
> Reviewed-on: https://chromium-review.googlesource.com/727194
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Daniel Cheng <dcheng@chromium.org>

TBR=dcheng@chromium.org,thakis@chromium.org,dpranke@chromium.org

Change-Id: I7dee0ca188f5ca34232f16e6496249fc9f49fa96
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  772741 
Reviewed-on: https://chromium-review.googlesource.com/728959
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>

[modify] https://crrev.com/a21b5b314bcdef9f29d9ea1325accf3920a5f937/gclient_utils.py
[modify] https://crrev.com/a21b5b314bcdef9f29d9ea1325accf3920a5f937/gclient.py
[modify] https://crrev.com/a21b5b314bcdef9f29d9ea1325accf3920a5f937/tests/gclient_scm_test.py

scottmg: dcheng's on that one too, https://chromium-review.googlesource.com/c/727408/
Project Member

Comment 20 by bugdroid1@chromium.org, Oct 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/a0c5f0850a4c3c5a61ca84dabfb1c788d5c4b9b1

commit a0c5f0850a4c3c5a61ca84dabfb1c788d5c4b9b1
Author: Daniel Cheng <dcheng@chromium.org>
Date: Fri Oct 20 02:31:46 2017

Reland "gclient runhooks: now with less spammy progress notifications"

This is a reland of 5fd1defbcc525aa11f3eef1e5c065de3cdfc6c21
Original change's description:
> gclient runhooks: now with less spammy progress notifications
> 
> Bug:  772741 
> Change-Id: I62ad64219ba27ee4ea26ceb603b1133dff4a2e04
> Reviewed-on: https://chromium-review.googlesource.com/727194
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Daniel Cheng <dcheng@chromium.org>

Bug:  772741 
Change-Id: Idd6fcd46a8c97716d68ed0975940705a90540a4d
Reviewed-on: https://chromium-review.googlesource.com/728961
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>

[modify] https://crrev.com/a0c5f0850a4c3c5a61ca84dabfb1c788d5c4b9b1/gclient_utils.py
[modify] https://crrev.com/a0c5f0850a4c3c5a61ca84dabfb1c788d5c4b9b1/gclient.py
[modify] https://crrev.com/a0c5f0850a4c3c5a61ca84dabfb1c788d5c4b9b1/tests/gclient_scm_test.py

Project Member

Comment 21 by bugdroid1@chromium.org, Oct 20 2017

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

commit 73a310d8bbf89d131288f5e5410607a3e661b003
Author: Daniel Cheng <dcheng@chromium.org>
Date: Fri Oct 20 20:03:19 2017

Don't emit unnecessary output from tools/perf/conditionally_execute

The output isn't interesting to most developers and just pollutes the
runhooks output.

Bug:  772741 
Change-Id: I0958b8e498479d27cc92e0d1837bf91c6e7c7753
Reviewed-on: https://chromium-review.googlesource.com/727408
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510534}
[modify] https://crrev.com/73a310d8bbf89d131288f5e5410607a3e661b003/tools/perf/conditionally_execute

Status: Fixed (was: Started)
Thanks a ton to dcheng for finishing this up!
Cc: nedngu...@google.com dpranke@chromium.org erikc...@chromium.org
 Issue 773372  has been merged into this issue.
Cc: thomasanderson@chromium.org
Should the "______ deleting ..." spam be removed too? (when running gclient sync -D)

Syncing projects: 100% (143/143), done.                                                    
Running hooks: 100% (69/69), done.                             

________ deleting 'src/third_party/mockito/src' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/android_tools/ndk' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/errorprone/lib' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/android_tools' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

WARNING: 'src/third_party/robolectric/robolectric' is no longer part of this client.  It is recommended that you manually remove it.


________ deleting 'src/third_party/custom_tabs_client/src' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/leakcanary/src' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/gvr-android-sdk/src' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/netty-tcnative/src' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/android_protobuf/src' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/requests/src' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/netty4/src' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

WARNING: 'src/third_party/elfutils/src' is no longer part of this client.  It is recommended that you manually remove it.


________ deleting 'src/third_party/apache-portable-runtime/src' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/findbugs' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/ub-uiautomator/lib' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/remoting/android/internal' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/junit/src' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

________ deleting 'src/third_party/jsr-305/src' in '/usr/local/google/home/thomasanderson/dev/chromium_official'

I have no opinion on that; I've never used `gclient sync -D`. Maybe file a bug for that and do it there if you want :-)

Sign in to add a comment