Remove uses of any env vars when running gclient hooks |
|
Issue description
Now that we have proper support for arguments and conditionals in gclient, we should remove any dependencies on env vars from the hooks, and make sure they are using flags instead.
This command, when run from the directory above src/ (on a Mac), gives the following list of files that reference environ:
$ grep -l environ $(cd src && grep "\.py" DEPS buildtools/DEPS | sort | uniq | sed -e "s/^DEPS://" -e "s/'action': \['python', //" -e "s/,.*$//" -e "/#/d" -e "s/\]$//" -e "s/'//g" | awk '{print $1}')
src/third_party/depot_tools/download_from_google_storage.py
src/build/android/play_services/update.py
src/third_party/gvr-android-sdk/test-apks/update.py
src/third_party/WebKit/Source/devtools/scripts/local_node/node.py
src/third_party/depot_tools/download_from_google_storage.py
src/build/get_syzygy_binaries.py
src/build/download_nacl_toolchains.py
src/build/landmines.py
src/third_party/binutils/download.py
src/build/linux/sysroot_scripts/install-sysroot.py
src/build/mac_toolchain.py
src/build/vs_toolchain.py
src/third_party/instrumented_libraries/scripts/download_binaries.py
src/tools/clang/scripts/update.py
it's possible some of these references may still be needed and appropriate, but many can be removed.
See also bug 718157 for removing the GYP_DEFINES uses explicitly.
,
Feb 16 2018
Looking at landmines.py: There is only one use of the env vars throughout all our landmines config scripts: https://cs.chromium.org/chromium/src/build/get_landmines.py?q=get_landmines.py&sq=package:chromium&dr&l=78 One option would be to just naively remove it. This would clobber all chromium android arm32 builds once. If that's acceptable, it'd be the easiest path forward. In the new version we'd just drop support for differentiating target_arch. If that's not acceptable, we'd need to find a way to pass target_arch to the landmines script.
,
Feb 16 2018
That's acceptable. landmine changes are quite rare.
,
Feb 16 2018
Looked closer. It's a little bit more. Win, mac, and linux platform are defined through the host. Only android through the OS setting in gyp_defines: https://cs.chromium.org/chromium/src/build/landmine_utils.py?sq=package:chromium&dr&q=package:%5E(chromium)$+file:(/%7C%5E)landmine_utils(%5C.(swig%7Cpy%7Cspt)$%7C/(__init__%5C.(swig%7Cpy%7Cspt))?$)&l=79 I could remove the whole android condition (and the entries in the get_landmines script). This would once clobber all android bots and from then on, we'd not be able to do android-specific clobber - only host-os-linux specific. Not required so often... once every 2 years it seems :)
,
Feb 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d76f98f52683e21606b35d15b156e28a72cb85f4 commit d76f98f52683e21606b35d15b156e28a72cb85f4 Author: Michael Achenbach <machenbach@chromium.org> Date: Fri Feb 16 22:40:01 2018 [build] Simplify landmines and strip off gyp-dependent options This prepares removing the option to make landmines target-platform dependent, as this depends on gyp environment variables. Instead we'll be able to only depend on host_os. This will cause clobber on all Android builds, as Android-specific landmines will no longer be supported. In follow-ups, we'll remove use of landmine_utils.platform() in other client projects and then remove the definition here. Bug: 756691 Change-Id: I08c1f4ecd05f5f4331bf1ab9eedda22419af2942 Reviewed-on: https://chromium-review.googlesource.com/924114 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> Cr-Commit-Position: refs/heads/master@{#537436} [modify] https://crrev.com/d76f98f52683e21606b35d15b156e28a72cb85f4/build/get_landmines.py [modify] https://crrev.com/d76f98f52683e21606b35d15b156e28a72cb85f4/build/landmine_utils.py
,
Feb 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/77290e0d7f3750f52bc11e121922d79de3cf9c0d commit 77290e0d7f3750f52bc11e121922d79de3cf9c0d Author: Michael Achenbach <machenbach@chromium.org> Date: Fri Feb 16 23:28:14 2018 [build] Prepare removal of gyp-defines from landmine Depends on: https://crrev.com/c/924114 TBR=yangguo@chromium.org Bug: chromium:756691 Change-Id: Ibbc3bd35e8727296c1539edc961e3184830575ff Reviewed-on: https://chromium-review.googlesource.com/924609 Reviewed-by: Michael Achenbach <machenbach@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> Cr-Commit-Position: refs/heads/master@{#51341} [modify] https://crrev.com/77290e0d7f3750f52bc11e121922d79de3cf9c0d/tools/get_landmines.py
,
Feb 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/libyuv/libyuv.git/+/29ba52ed28dfe25c018c070781e20f0780b18b06 commit 29ba52ed28dfe25c018c070781e20f0780b18b06 Author: Michael Achenbach <machenbach@chromium.org> Date: Fri Feb 16 23:38:18 2018 [build] Prepare removal of gyp-defines from landmine Depends on: https://crrev.com/c/924114 Bug: chromium:756691 Change-Id: I4fb6dcbdf0d5e134ef3756b231d8f98d9d41b403 Reviewed-on: https://chromium-review.googlesource.com/924617 Reviewed-by: Frank Barchard <fbarchard@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> [modify] https://crrev.com/29ba52ed28dfe25c018c070781e20f0780b18b06/tools_libyuv/get_landmines.py
,
Feb 19 2018
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/500874effdada6f847092e98fe6f4a0b87d5c8b4 commit 500874effdada6f847092e98fe6f4a0b87d5c8b4 Author: Michael Achenbach <machenbach@chromium.org> Date: Mon Feb 19 22:09:45 2018 [build] Prepare removal of gyp-defines from landmines Depends on Chromium to roll: https://crrev.com/c/924114 This will clobber all Android builds once, since after this, we can't make Android-specific landmines anymore. Bug: chromium:756691 Change-Id: Ic7588329e567e3f6e596b04de8f990dc720eb153 Reviewed-on: https://webrtc-review.googlesource.com/54721 Reviewed-by: Patrik Höglund <phoglund@webrtc.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> Cr-Commit-Position: refs/heads/master@{#22091} [modify] https://crrev.com/500874effdada6f847092e98fe6f4a0b87d5c8b4/tools_webrtc/get_landmines.py
,
Feb 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d2fc4b94138c365bbb5d7fee3b885b2cf81cd5f8 commit d2fc4b94138c365bbb5d7fee3b885b2cf81cd5f8 Author: Michael Achenbach <machenbach@chromium.org> Date: Tue Feb 20 23:27:59 2018 [build] Remove gyp usage from landmines script References from client projects to removed methods have been removed in previous CLs. Bug: 756691 Change-Id: I405ec09301455884f00348aadf82e865e85df721 Reviewed-on: https://chromium-review.googlesource.com/925683 Reviewed-by: Bruce Dawson <brucedawson@chromium.org> Reviewed-by: Robbie Iannucci <iannucci@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> Cr-Commit-Position: refs/heads/master@{#537929} [modify] https://crrev.com/d2fc4b94138c365bbb5d7fee3b885b2cf81cd5f8/build/landmine_utils.py [modify] https://crrev.com/d2fc4b94138c365bbb5d7fee3b885b2cf81cd5f8/build/landmines.py
,
Feb 20 2018
Landmine scripts are now gyp-free. Note, they are not env-variable free. They still reference CHROMIUM_OUT_DIR.
,
Feb 21 2018
The following revision refers to this bug: https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/d13d68dccec6beffb55aae98a8e8021370483428 commit d13d68dccec6beffb55aae98a8e8021370483428 Author: Justin Cohen <justincohen@google.com> Date: Wed Feb 21 05:33:08 2018
,
Feb 21 2018
The following revision refers to this bug: https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/a6343d2fcc298697f0f3ec49a2b7ec4bbe03374b commit a6343d2fcc298697f0f3ec49a2b7ec4bbe03374b Author: Justin Cohen <justincohen@google.com> Date: Wed Feb 21 15:53:31 2018 |
|
►
Sign in to add a comment |
|
Comment 1 by dpranke@chromium.org
, Aug 18 2017