Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(326)

Issue 2246573002: Pass path to gn binary to cronet_licenses.py when building for iOS. (Closed)

Created:
4 years, 4 months ago by sdefresne
Modified:
4 years, 4 months ago
Reviewers:
justincohen
CC:
chromium-reviews, cbentzel+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Pass path to gn binary to cronet_licenses.py when building for iOS. The bot used on iOS have a different repository structure where Chromium repository is not the primary gclient repository, and the buildtools is not in the search path of gclient. This cause failure when invoking gn through searching the PATH as the wrapper from depot_tools is found and it does not find the real binary. Instead update cronet_licenses.py to accept the path to the gn binary and pass the path to the binary in //buildtools/mac when building for iOS. Fixes the following compilation error: python ../../components/cronet/tools/cronet_licenses.py license cronet/LICENSE --gn gn.py: Could not find gn executable at: /b/build/slave/chrome-ios-official/build/buildtools/mac/gn Traceback (most recent call last): File "../../components/cronet/tools/cronet_licenses.py", line 130, in <module> sys.exit(main()) File "../../components/cronet/tools/cronet_licenses.py", line 108, in main third_party_dirs = FindThirdPartyDeps(os.getcwd()) File "../../components/cronet/tools/cronet_licenses.py", line 74, in FindThirdPartyDeps subprocess.check_output(["gn", "gen", tmp_dir]) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 573, in check_output raise CalledProcessError(retcode, cmd, output=output) subprocess.CalledProcessError: Command '['gn', 'gen', '/b/build/slave/chrome-ios-official/build/src/out/Release-iphoneos/tmppFeoga']' returned non-zero exit status 2 BUG=None Committed: https://crrev.com/2228c82b13e3663d176b7e76b34eb6d360f0a8f7 Cr-Commit-Position: refs/heads/master@{#411766}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -6 lines) Patch
M components/cronet/ios/BUILD.gn View 2 chunks +6 lines, -0 lines 0 comments Download
M components/cronet/tools/cronet_licenses.py View 2 chunks +9 lines, -6 lines 0 comments Download

Messages

Total messages: 16 (10 generated)
sdefresne
I've added //build/util/LASTCHANGE to inputs because after adding //buildtools/$host_os/gn, the script would only have been ...
4 years, 4 months ago (2016-08-12 20:02:56 UTC) #4
sdefresne
Please take a look and send to CQ if lgty.
4 years, 4 months ago (2016-08-12 20:03:15 UTC) #5
justincohen
lgtm
4 years, 4 months ago (2016-08-12 20:13:46 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2246573002/1
4 years, 4 months ago (2016-08-12 20:51:09 UTC) #12
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 4 months ago (2016-08-12 21:11:30 UTC) #14
commit-bot: I haz the power
4 years, 4 months ago (2016-08-12 21:15:57 UTC) #16
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/2228c82b13e3663d176b7e76b34eb6d360f0a8f7
Cr-Commit-Position: refs/heads/master@{#411766}

Powered by Google App Engine
This is Rietveld 408576698