Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2183053006:
Add mac hermetic support to build/config/mac/sdk_info.py. (Closed)
|
Created:
4 years, 4 months ago by justincohen Modified:
4 years, 4 months ago CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd mac hermetic support to build/config/mac/sdk_info.py.
BUG=632229
Committed: https://crrev.com/254e30c78ff58abdfe16235f687cf43f6c072296
Cr-Commit-Position: refs/heads/master@{#408487}
Patch Set 1 #Patch Set 2 : Wrap ios toolchain in is_ios block #
Total comments: 3
Messages
Total messages: 23 (11 generated)
justincohen@chromium.org changed reviewers: + erikchen@chromium.org, sdefresne@chromium.org
With this CL, it looks like mac builds are trying to load iOS information, which
will break hermetic builds (which have no iphoneos or iphonesimulator SDK)
$ gn args out/gn
Waiting for editor on "/chromium/src/out/gn/args.gn"...
Generating files...
ERROR at //build/config/ios/ios_sdk.gni:76:7: Script returned non-zero exit
code.
exec_script("//build/config/mac/sdk_info.py", [ ios_sdk_name ], "scope")
^----------
Current dir: /chromium/src/out/gn/
Command: python -- /chromium/src/build/config/mac/sdk_info.py iphonesimulator
Returned 1.
stderr:
xcodebuild: error: SDK "iphonesimulator" cannot be located.
xcodebuild: error: SDK "iphonesimulator" cannot be located.
xcrun: error: unable to lookup item 'Path' in SDK 'iphonesimulator'
Traceback (most recent call last):
File "/chromium/src/build/config/mac/sdk_info.py", line 69, in <module>
FillSDKPathAndVersion(settings, sys.argv[1], settings['xcode_version'])
File "/chromium/src/build/config/mac/sdk_info.py", line 42, in
FillSDKPathAndVersion
'xcrun', '-sdk', platform, '--show-sdk-path']).strip()
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 '['xcrun', '-sdk', 'iphonesimulator',
'--show-sdk-path']' returned non-zero exit status 1
See //build/toolchain/mac/BUILD.gn:11:1: whence it was imported.
import("//build/config/ios/ios_sdk.gni")
^--------------------------------------
See //BUILD.gn:65:1: which caused the file to be included.
group("gn_all") {
^----------------
Description was changed from ========== Add mac hermetic support to build/config/mac/sdk_info.py. BUG= ========== to ========== Add mac hermetic support to build/config/mac/sdk_info.py. BUG=632229 ==========
lgtm % issues with ios configuration. [I updated the CL description to point to the relevant bug].
sdefresne@ ptal
lgtm but please ask review by dpranke or brettw for build/toolchain https://codereview.chromium.org/2183053006/diff/20001/build/toolchain/mac/BUI... File build/toolchain/mac/BUILD.gn (right): https://codereview.chromium.org/2183053006/diff/20001/build/toolchain/mac/BUI... build/toolchain/mac/BUILD.gn:405: if (is_ios) { I don't think this is necessary, instead you could do the following at the top of the file which should work: if (is_ios) { import("//build/config/ios/ios_sdk.gni") } else { additional_toolchains = [] }
https://codereview.chromium.org/2183053006/diff/20001/build/toolchain/mac/BUI... File build/toolchain/mac/BUILD.gn (right): https://codereview.chromium.org/2183053006/diff/20001/build/toolchain/mac/BUI... build/toolchain/mac/BUILD.gn:405: if (is_ios) { On 2016/07/28 17:36:31, sdefresne wrote: > I don't think this is necessary, instead you could do the following at the top > of the file which should work: > > if (is_ios) { > import("//build/config/ios/ios_sdk.gni") > } else { > additional_toolchains = [] > } Generating files... ERROR at //build/toolchain/mac/BUILD.gn:14:27: Assignment had no effect. additional_toolchains = [] ^ You set the variable "additional_toolchains" here and it was unused before it went out of scope. See //BUILD.gn:65:1: which caused the file to be included. group("gn_all") { ^----------------
On 2016/07/28 17:59:40, justincohen wrote: > https://codereview.chromium.org/2183053006/diff/20001/build/toolchain/mac/BUI... > File build/toolchain/mac/BUILD.gn (right): > > https://codereview.chromium.org/2183053006/diff/20001/build/toolchain/mac/BUI... > build/toolchain/mac/BUILD.gn:405: if (is_ios) { > On 2016/07/28 17:36:31, sdefresne wrote: > > I don't think this is necessary, instead you could do the following at the top > > of the file which should work: > > > > if (is_ios) { > > import("//build/config/ios/ios_sdk.gni") > > } else { > > additional_toolchains = [] > > } > > > Generating files... > ERROR at //build/toolchain/mac/BUILD.gn:14:27: Assignment had no effect. > additional_toolchains = [] > ^ > You set the variable "additional_toolchains" here and it was unused before it > went > out of scope. > See //BUILD.gn:65:1: which caused the file to be included. > group("gn_all") { > ^---------------- Too bad. lgtm as is then.
still lgtm
justincohen@chromium.org changed reviewers: + brettw@chromium.org, dpranke@google.com
ptal!
The CQ bit was checked by justincohen@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...
dpranke@chromium.org changed reviewers: + dpranke@chromium.org
lgtm https://codereview.chromium.org/2183053006/diff/20001/build/toolchain/mac/BUI... File build/toolchain/mac/BUILD.gn (right): https://codereview.chromium.org/2183053006/diff/20001/build/toolchain/mac/BUI... build/toolchain/mac/BUILD.gn:405: if (is_ios) { That, plus it's probably better hygiene to not declare these targets on non-ios builds.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by justincohen@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add mac hermetic support to build/config/mac/sdk_info.py. BUG=632229 ========== to ========== Add mac hermetic support to build/config/mac/sdk_info.py. BUG=632229 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add mac hermetic support to build/config/mac/sdk_info.py. BUG=632229 ========== to ========== Add mac hermetic support to build/config/mac/sdk_info.py. BUG=632229 Committed: https://crrev.com/254e30c78ff58abdfe16235f687cf43f6c072296 Cr-Commit-Position: refs/heads/master@{#408487} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/254e30c78ff58abdfe16235f687cf43f6c072296 Cr-Commit-Position: refs/heads/master@{#408487} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
