Allow use of non-default Xcode in GN for Mac. |
|||||||||||
Issue descriptionAdd a new variable to GN to allow the user to select hermetic xcode. Then update every callsite to tools that come bundled with xcode (ld, ibtool, etc.) to explicitly reference hermetic xcode if necessary.
,
Sep 29 2016
tldr: I'm going to add a gn variable that toggles hermetic. When hermetic is on, all xcode binaries will be referenced by absolute path. First let me describe the current behavior. Chrome uses one binary that is only available through xcode: ibtool And many binaries available through either xcode or command line tools: dsymutil, libtool, strip, ld, strings...[probably more] By default, shims for these binaries [with the same name] live in /usr/bin. 1. If the environment variable DEVELOPER_DIR is set, the implementation will forward to a binary with the same name that lives in the Xcode install at that directory. 2. If `xcode-select -p` returns a directory, same behavior as (1) 3. Otherwise, puts up a user-prompt to install Xcode or command line tools and logs a message to console. If I take a machine with no command line tools or xcode install [xcode-select -p returns an error message], but with a DEVELOPER_DIR set, GN seems happy, but ninja is unhappy. All of the binaries I listed work fine from the command line, so I bet we're not forwarding the environment variable in linker_driver.py. This is perhaps WAI. We could theoretically fix this problem by forcing users to set DEVELOPER_DIR, and changing the behavior of linker_driver.py. Note that there's no GN-only fix, since many binaries used at build time require this as well. This is the simplest solution, but not very clean and may cause confusion. The alternative is add a GN arg that replaces every reference to these binaries with an absolute path [if hermetic]. This has the benefit of being explicit with no room for confusion. [e.g. right now bots are compiling with hermetic GN, but that actually means that they're pulling the SKD from the hermetic xcode, but still using tools from system install. This is pretty terrifying]. This also means that users can very easily have a local hermetic-toolchain without worrying about leaking environment variables to other contexts. Downsides: A) Presumably a fair amount of plumbing. B) Removes a layer of indirection afforded by the shims in /usr/bin. If Xcode hierarchy changes, we'll need to update the absolute paths manually.
,
Sep 29 2016
,
Sep 29 2016
Both clang++ and ld expect a functional Xcode install either from DEVELOPER_DIR or xcode-select. I guess we could set up a dummy environment for ninja to use when compiling targets [I believe Windows does something similar], but the cost/benefit tradeoff is looking increasingly bad. All of this is to work around just setting a DEVELOPER_DIR environment variable. I looked into the issue I listed in c#2 with setting DEVELOPER_DIR - goma doesn't pass on the env DEVELOPER_DIR, which is a simple bug to fix.
,
Sep 29 2016
I think using the arg is the right way to go; it's certainly the most consistent with the way we do builds elsewhere, where we try hard not to rely on the environment and make everything be specified explicitly. We did get rid of the gyp-mac-tool in the migration, and moved to a number of individual scripts. It's possible that doing that made this particular problem harder to solve, and if we should move back to a single wrapper that seems fine, too.
,
Sep 29 2016
To clarify, there are two paths forward:
Let {xcode_tools} be the set of binaries that come bundled with Xcode. The list includes clang, dsymutil, libtool, strip, ld, strings...[Yes, we bundle clang separately]
1) [The better solution]: Prior to calling any binary in {xcode_tools}, make sure that DEVELOPER_DIR is set. [xcode-select also works, but affects a user profile, rather than a shell]
clang and ld require DEVELOPER_DIR. ibtool does not. I haven't checked the other binaries.
2) [The easy solution]: Set DEVELOPER_DIR prior to invoking ninja or gn.
Both dpranke and justincohen prefer (1). sdrefresne preferred (2), but that was when we were asking him to do (1).
,
Sep 29 2016
FWIW, I'm happy to help with (1) if that means it gets done instead of (2), but I do also have other things on my plate ;).
,
Sep 29 2016
+ brettw, to comment on (1a)
thakis, to comment on (1b)
(1) requires two changes:
(1a) Update GN to support setting an environment variable. I think this makes more sense then plumbing an environment variable through to each invocation of {xcode_tools}.
(1b) Update GN to export the command [use this DEVELOPER_DIR] to ninja files. Update Ninja to parse this command and set the environment variable. I suspect this will have similar logic to NinjaMain::ToolMSVC.
,
Sep 29 2016
I would only want to do (1) if it meant we were also able to clean up / consolidate the similar logic we have for visual studio and the env files on windows. +scottmg/+brucedawson to get pulled into that thread ...
,
Sep 29 2016
If GN just sets environment variables, they''ll be lost by the time you build. If you want environment variables to be set when you build, you need to set them in some way like we do on Windows via scripts that wrap the commands. And if you want that, you can do that without changes to the GN binary itself.
,
Sep 29 2016
I'm not quite clear on if everyone has to set DEVELOPER_DIR, or if it's only people that are opting out of hermetic? If the latter, then I would go with "easy solution" for sure. Or is it the other way around, where we need to force set the var to point to our hermetic toolchain?
,
Sep 29 2016
By default, DEVELOPER_DIR is not set on bots or developer machines. Instead, both currently use xcode-select to point to a system install of Xcode. If DEVELOPER_DIR is set, then xcode-select isn't used. We need to set DEVELOPER_DIR for hermetic builds, the only question is how we set it.
,
Sep 29 2016
I chatted with brettw: c#8, (1a): brettw suggests prototyping the desired behavior by modifying exec_script (or decorating or whatever) to set the appropriate environment variable.
,
Sep 30 2016
The following revision refers to this bug: https://chrome-internal.googlesource.com/goma/client/+/6994af99b0ba177173464088081ca5a335f1e6c7 commit 6994af99b0ba177173464088081ca5a335f1e6c7 Author: Erik Chen <erikchen@google.com> Date: Thu Sep 29 21:13:52 2016
,
Sep 30 2016
I've got all targets in a Chromium checkout building with hermetic xcode [no command line tools, no xcode install]. I suspect more work will have to be done to get official builds and other repositories compiling with hermetic. I'm less worried about other repos - since we don't have to move them over to hermetic [and some of them like webrtc I think are already on a different toolchain]. GN args: dcheck_always_on = false is_component_build = false is_debug = false symbol_level = 1 use_goma = true enable_nacl = true use_system_xcode = false goma_dir = "/Users/erikchen/projects/goma/client/out/gn" Proof of concept CLs: https://codereview.chromium.org/2387723002/#ps20001 https://codereview.chromium.org/2378623006/ Requires custom build of goma including patch from c#14. Potential concerns: When I build, I get a **large** number of warnings that have identical prefixes: """ld: warning: unknown dwarf DW_FORM_strp (offset=0x41534800) is too big in...""" I haven't counted, but this may be emitted for every translation unit. Other thoughts: In my prototype CL, I stopped trying to plumb DEVELOPER_DIR correctly to python scripts and just started setting it directly. All of these scripts use the GN "action" tag. I considered modifying GN (ninja_action_target_writer.cc) to add a prefix field that would be prepended to the action. I ended up deciding that there were few enough scripts that needed DEVELOPER_DIR that this wasn't necessary. Note that third_party/WebKit/Source/build/scripts/in_generator.py actually gets used by many scripts[CL doesn't bother adding plumbing for all of them].
,
Sep 30 2016
One of my concerns is that it's very easy for someone to add a new script that requires DEVELOPER_DIR plumbing, but doesn't have it. Since almost all bots and dev machines have an xcode install, this will silently succeed [and different versions of the toolchain will be used]. This is an argument for some type of global GN flag that would modify all emitted actions. [e.g. modify ninja_action_target_writer.cc and associated plumbing]
,
Sep 30 2016
To clarify the DW_FORM_strp warning, it appears to be emitted once per TU being linked into a static library/binary. e.g. """ [1873/27054] LINK ./boringssl_spake25519_test ld: warning: unknown dwarf DW_FORM_strp (offset=0x41534800) is too big in obj/third_party/boringssl/boringssl_spake25519_test/malloc.o ld: warning: unknown dwarf DW_FORM_strp (offset=0x41534800) is too big in obj/third_party/boringssl/libboringssl.a(err_data.o) ld: warning: unknown dwarf DW_FORM_strp (offset=0x41534800) is too big in obj/third_party/boringssl/libboringssl.a(generic.o) ld: warning: unknown dwarf DW_FORM_strp (offset=0x41534800) is too big in obj/third_party/boringssl/libboringssl.a(cpu-aarch64-linux.o) ... """ """ [8674/45897] SOLINK libboringssl_fuzzer.dylib libboringssl_fuzzer.dylib.TOC ld: warning: unknown dwarf DW_FORM_strp (offset=0x41534800) is too big in obj/third_party/boringssl/boringssl_fuzzer/err_data.o ld: warning: unknown dwarf DW_FORM_strp (offset=0x41534800) is too big in obj/third_party/boringssl/boringssl_fuzzer/generic.o ld: warning: unknown dwarf DW_FORM_strp (offset=0x41534800) is too big in obj/third_party/boringssl/boringssl_fuzzer/cpu-aarch64-linux.o ld: warning: unknown dwarf DW_FORM_strp (offset=0x41534800) is too big in obj/third_party/boringssl/boringssl_fuzzer/cpu-arm-linux.o ld: warning: unknown dwarf DW_FORM_strp (offset=0x41534800) is too big in obj/third_party/boringssl/boringssl_fuzzer/cpu-arm.o ld: warning: unknown dwarf DW_FORM_strp (offset=0x41534800) is too big in obj/third_party/boringssl/boringssl_fuzzer/cpu-ppc64le.o ld: warning: unknown dwarf DW_FORM_strp (offset=0x41534800) is too big in obj/third_party/boringssl/boringssl_fuzzer/x25519-x86_64.o ... """
,
Sep 30 2016
Switching to xcode5.1.1 on a non-hermetic build also gives the same "ld: warning..." emissions. So this is definitely due to using an outdated ld.
,
Oct 1 2016
Split ld warning into Issue 652008 .
,
Oct 4 2016
Regarding #6, my reason for preferring 2) is that any developer writing a script to use in an action target will have to wonder whether it needs to support setting DEVELOPER_DIR in the BUILD.gn file (i.e. it is error prone).
,
Oct 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/crashpad/crashpad.git/+/1e6dbcb3008f6eebe02ca33d8c36c8922931dad3 commit 1e6dbcb3008f6eebe02ca33d8c36c8922931dad3 Author: Erik Chen <erikchen@chromium.org> Date: Tue Oct 04 00:31:48 2016 Support passing DEVELOPER_DIR to mig.py BUG= chromium:651267 Change-Id: If02f9bac603237677d348869d05d7b4d0b31909e Reviewed-on: https://chromium-review.googlesource.com/392486 Reviewed-by: Mark Mentovai <mark@chromium.org> [modify] https://crrev.com/1e6dbcb3008f6eebe02ca33d8c36c8922931dad3/util/mach/mig.py
,
Oct 5 2016
,
Oct 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b58a723ab22333fe40575f6992c3cb8c7dde0b0a commit b58a723ab22333fe40575f6992c3cb8c7dde0b0a Author: erikchen <erikchen@chromium.org> Date: Thu Oct 06 20:58:56 2016 Add a variable use_system_xcode to GN. Start plumbing it through to actions that require binaries from the Xcode toolchain. BUG= 651267 TBR=sdefresne@chromium.org Review-Url: https://codereview.chromium.org/2388063003 Cr-Commit-Position: refs/heads/master@{#423673} [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/build/config/mac/base_rules.gni [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/build/config/mac/compile_xib.py [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/build/config/mac/xcrun.py [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/build/toolchain/mac/BUILD.gn [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/build/toolchain/toolchain.gni [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/chrome/BUILD.gn [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/chrome/app/nibs/BUILD.gn [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/chrome/app/nibs/generate_localizer.py [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/chrome/tools/build/mac/run_verify_order.py [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/components/policy/BUILD.gn [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/third_party/WebKit/Source/build/scripts/rule_bison.py [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/third_party/WebKit/Source/core/BUILD.gn
,
Oct 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/717ca1227414e7ea84b4676ec4b1572491654ae1 commit 717ca1227414e7ea84b4676ec4b1572491654ae1 Author: erikchen <erikchen@chromium.org> Date: Tue Oct 11 00:39:01 2016 More changes to support hermetic Xcode toolchain in GN. BUG= 651267 Review-Url: https://codereview.chromium.org/2403583002 Cr-Commit-Position: refs/heads/master@{#424307} [modify] https://crrev.com/717ca1227414e7ea84b4676ec4b1572491654ae1/build/secondary/third_party/crashpad/crashpad/util/BUILD.gn [modify] https://crrev.com/717ca1227414e7ea84b4676ec4b1572491654ae1/chrome/BUILD.gn [modify] https://crrev.com/717ca1227414e7ea84b4676ec4b1572491654ae1/chrome/tools/build/mac/copy_keystone_framework.py [modify] https://crrev.com/717ca1227414e7ea84b4676ec4b1572491654ae1/third_party/WebKit/Source/build/scripts/gperf.py [modify] https://crrev.com/717ca1227414e7ea84b4676ec4b1572491654ae1/third_party/WebKit/Source/build/scripts/in_generator.py [modify] https://crrev.com/717ca1227414e7ea84b4676ec4b1572491654ae1/third_party/WebKit/Source/build/scripts/scripts.gni [modify] https://crrev.com/717ca1227414e7ea84b4676ec4b1572491654ae1/third_party/WebKit/Source/core/BUILD.gn [modify] https://crrev.com/717ca1227414e7ea84b4676ec4b1572491654ae1/third_party/WebKit/Source/platform/BUILD.gn
,
Oct 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6646d70c3d5ccbcb9ffcf8426762bf68babcc136 commit 6646d70c3d5ccbcb9ffcf8426762bf68babcc136 Author: erikchen <erikchen@chromium.org> Date: Wed Oct 12 23:38:38 2016 Turn on hermetic toolchain for the FORCE_MAC_TOOLCHAIN fyi bot. BUG= 651267 Review-Url: https://codereview.chromium.org/2414793002 Cr-Commit-Position: refs/heads/master@{#424905} [modify] https://crrev.com/6646d70c3d5ccbcb9ffcf8426762bf68babcc136/tools/mb/mb_config.pyl
,
Oct 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6218c34684254f1c090bf501f6c46b0a69b63ec5 commit 6218c34684254f1c090bf501f6c46b0a69b63ec5 Author: erikchen <erikchen@chromium.org> Date: Thu Oct 13 21:34:45 2016 Remove direct references to hermetic mac toolchain. This ensures that the hermetic toolchain is only used if use_system_xcode is false. This CL also causes two changes: * svn is assumed to be installed on the system. It is not pulled from the hermetic toolchain. * mac_sdk_build was used to populate the SDK version in the Info.plist. This was being populated with a different version than the SDK being used to build Chrome, which is incorrect. BUG= 651267 Review-Url: https://codereview.chromium.org/2412353003 Cr-Commit-Position: refs/heads/master@{#425170} [modify] https://crrev.com/6218c34684254f1c090bf501f6c46b0a69b63ec5/build/config/mac/mac_sdk.gni [modify] https://crrev.com/6218c34684254f1c090bf501f6c46b0a69b63ec5/build/config/mac/rules.gni [modify] https://crrev.com/6218c34684254f1c090bf501f6c46b0a69b63ec5/build/config/mac/sdk_info.py [modify] https://crrev.com/6218c34684254f1c090bf501f6c46b0a69b63ec5/build/gyp_environment.py [modify] https://crrev.com/6218c34684254f1c090bf501f6c46b0a69b63ec5/build/mac/find_sdk.py [modify] https://crrev.com/6218c34684254f1c090bf501f6c46b0a69b63ec5/build/mac_toolchain.py [modify] https://crrev.com/6218c34684254f1c090bf501f6c46b0a69b63ec5/tools/clang/scripts/update.py
,
Oct 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/native_client/src/native_client.git/+/cb2b66010be5078b3c533be58727c7f474cfd20c commit cb2b66010be5078b3c533be58727c7f474cfd20c Author: erikchen <erikchen@chromium.org> Date: Thu Oct 13 23:57:54 2016 Plumb use_system_xcode through native client. NOTRY=true BUG= chromium:651267 Review-Url: https://codereview.chromium.org/2393473002 [modify] https://crrev.com/cb2b66010be5078b3c533be58727c7f474cfd20c/src/trusted/service_runtime/BUILD.gn [modify] https://crrev.com/cb2b66010be5078b3c533be58727c7f474cfd20c/src/trusted/service_runtime/osx/run_mig.py
,
Oct 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e4292073dac379dba1f418b78497397a484f43bc commit e4292073dac379dba1f418b78497397a484f43bc Author: justincohen <justincohen@chromium.org> Date: Fri Oct 14 19:07:17 2016 Fix hermetic argument to run_verify_order. s/developer-dir/developer_dir. BUG= 651267 Review-Url: https://codereview.chromium.org/2421893002 Cr-Commit-Position: refs/heads/master@{#425414} [modify] https://crrev.com/e4292073dac379dba1f418b78497397a484f43bc/chrome/BUILD.gn
,
Oct 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1a69ed79d1c994433a1f73455038d499a62e77cb commit 1a69ed79d1c994433a1f73455038d499a62e77cb Author: nacl-deps-roller <nacl-deps-roller@chromium.org> Date: Fri Oct 14 19:31:02 2016 Roll src/native_client/ be714d07c..c9a4a41cf (3 commits). https://chromium.googlesource.com/native_client/src/native_client.git/+log/be714d07cf63..c9a4a41cfda3 $ git log be714d07c..c9a4a41cf --date=short --no-merges --format='%ad %ae %s' 2016-10-14 bradnelson Fix breakpad 32-bit build. 2016-10-13 bradnelson Revert use of hexfloat for wasm in spec2k tests. 2016-10-13 erikchen Plumb use_system_xcode through native client. BUG=None,651267 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_dbg_32_ng,linux_nacl_sdk_build TBR=mseaborn@chromium.org Review-Url: https://codereview.chromium.org/2416303003 Cr-Commit-Position: refs/heads/master@{#425430} [modify] https://crrev.com/1a69ed79d1c994433a1f73455038d499a62e77cb/DEPS
,
Oct 19 2016
,
Oct 25 2016
,
Oct 26 2016
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b58a723ab22333fe40575f6992c3cb8c7dde0b0a commit b58a723ab22333fe40575f6992c3cb8c7dde0b0a Author: erikchen <erikchen@chromium.org> Date: Thu Oct 06 20:58:56 2016 Add a variable use_system_xcode to GN. Start plumbing it through to actions that require binaries from the Xcode toolchain. BUG= 651267 TBR=sdefresne@chromium.org Review-Url: https://codereview.chromium.org/2388063003 Cr-Commit-Position: refs/heads/master@{#423673} [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/build/config/mac/base_rules.gni [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/build/config/mac/compile_xib.py [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/build/config/mac/xcrun.py [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/build/toolchain/mac/BUILD.gn [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/build/toolchain/toolchain.gni [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/chrome/BUILD.gn [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/chrome/app/nibs/BUILD.gn [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/chrome/app/nibs/generate_localizer.py [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/chrome/tools/build/mac/run_verify_order.py [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/components/policy/BUILD.gn [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/third_party/WebKit/Source/build/scripts/rule_bison.py [modify] https://crrev.com/b58a723ab22333fe40575f6992c3cb8c7dde0b0a/third_party/WebKit/Source/core/BUILD.gn
,
Nov 4 2016
[Automated comment] removing mislabelled merge-merged-2840 |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by erikc...@chromium.org
, Sep 29 2016