Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2925863002:
check_gn_headers: use pinned depot_tools (Closed)
Descriptioncheck_gn_headers: use pinned depot_tools
build/check_gn_headers.py currently assumes depot_tools is in $PATH.
This is false on some systems.
Use the DEPS-pinned depot_tools instead.
Bug: 729811
Review-Url: https://codereview.chromium.org/2925863002
Cr-Commit-Position: refs/heads/master@{#477559}
Committed: https://chromium.googlesource.com/chromium/src/+/6a40e94018f930fd154f12a309b0a211e9e5f915
Patch Set 1 #
Total comments: 4
Patch Set 2 : fix depot_tools path #Messages
Total messages: 18 (9 generated)
PTAL
nodir@chromium.org changed reviewers: + wychen@chromium.org
+wychen
lgtm
The CQ bit was checked by nodir@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks for fixing this! Sorry I didn't notice LUCI bots are affected.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
https://codereview.chromium.org/2925863002/diff/1/build/check_gn_headers.py File build/check_gn_headers.py (right): https://codereview.chromium.org/2925863002/diff/1/build/check_gn_headers.py#n... build/check_gn_headers.py:31: cmd = [os.path.join(DEPOT_TOOLS_DIR, 'ninja'), '-C', out_dir, '-t', 'deps'] I don't have src/depot_tools in my checkout. Is there a pending CL adding that? Would it be safer to prepend the pinned depot_tools to PATH? If we are sure src/depot_tools would be available on all bots and dev environments, then this is fine.
https://codereview.chromium.org/2925863002/diff/1/build/check_gn_headers.py File build/check_gn_headers.py (right): https://codereview.chromium.org/2925863002/diff/1/build/check_gn_headers.py#n... build/check_gn_headers.py:24: DEPOT_TOOLS_DIR = os.path.join(SRC_DIR, 'depot_tools') This needs to be os.path.join(SRC_DIR, 'third_party', 'depot_tools'). https://codereview.chromium.org/2925863002/diff/1/build/check_gn_headers.py#n... build/check_gn_headers.py:31: cmd = [os.path.join(DEPOT_TOOLS_DIR, 'ninja'), '-C', out_dir, '-t', 'deps'] On 2017/06/07 01:27:03, wychen wrote: > I don't have src/depot_tools in my checkout. Is there a pending CL adding that? > Would it be safer to prepend the pinned depot_tools to PATH? If we are sure > src/depot_tools would be available on all bots and dev environments, then this > is fine. Good catch, see comment above.
lgtm with the third_party fix. https://codereview.chromium.org/2925863002/diff/1/build/check_gn_headers.py File build/check_gn_headers.py (right): https://codereview.chromium.org/2925863002/diff/1/build/check_gn_headers.py#n... build/check_gn_headers.py:31: cmd = [os.path.join(DEPOT_TOOLS_DIR, 'ninja'), '-C', out_dir, '-t', 'deps'] On 2017/06/07 01:29:53, Dirk Pranke (slow) wrote: > On 2017/06/07 01:27:03, wychen wrote: > > I don't have src/depot_tools in my checkout. Is there a pending CL adding > that? > > Would it be safer to prepend the pinned depot_tools to PATH? If we are sure > > src/depot_tools would be available on all bots and dev environments, then this > > is fine. > > Good catch, see comment above. Ah. I haven't updated to https://chromium-review.googlesource.com/c/522802/.
The CQ bit was checked by nodir@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org, wychen@chromium.org Link to the patchset: https://codereview.chromium.org/2925863002/#ps20001 (title: "fix depot_tools path")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496809462475070,
"parent_rev": "25808b7ff3e61f9951ab104d7148f31e0820f0fb", "commit_rev":
"6a40e94018f930fd154f12a309b0a211e9e5f915"}
Message was sent while issue was closed.
Description was changed from ========== check_gn_headers: use pinned depot_tools build/check_gn_headers.py currently assumes depot_tools is in $PATH. This is false on some systems. Use the DEPS-pinned depot_tools instead. R=dpranke@chromium.org Bug: 729811 ========== to ========== check_gn_headers: use pinned depot_tools build/check_gn_headers.py currently assumes depot_tools is in $PATH. This is false on some systems. Use the DEPS-pinned depot_tools instead. Bug: 729811 Review-Url: https://codereview.chromium.org/2925863002 Cr-Commit-Position: refs/heads/master@{#477559} Committed: https://chromium.googlesource.com/chromium/src/+/6a40e94018f930fd154f12a309b0... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/6a40e94018f930fd154f12a309b0... |
