GetPlatforms in build/mac_toolchain.py isn't quite right |
||||
Issue description
build/mac_files/Xcode.app was out-of-date on my machine.
It turned out to be due to me having `target_os = ['ios']` in my .glient, which causes mac_toolchain.py to believe that I'm _only_ interested in iOS builds, while the sematics of that field are that I'm _also_ interested in iOS builds. (For example, target_os = ['android', 'fuchsia'] on Linux means you'll get a checkout that can do linux, android, and fuchsia builds.)
So I tried to hack up mac_toolchain.py to use what's in there in addition to 'mac' instead of replacement, and since there's a loop over the results of GetPlatforms() further down I thought it might work:
$ git diff
diff --git a/build/mac_toolchain.py b/build/mac_toolchain.py
index 534f9d1982be..33933b6932fa 100755
--- a/build/mac_toolchain.py
+++ b/build/mac_toolchain.py
@@ -57,14 +57,14 @@ def PlatformMeetsHermeticXcodeRequirements(target_os):
def GetPlatforms():
- default_target_os = ["mac"]
+ target_os = set(["mac"])
try:
env = {}
execfile(GCLIENT_CONFIG, env, env)
- return env.get('target_os', default_target_os)
+ target_os |= set(env.get('target_os', default_target_os))
except:
pass
- return default_target_os
+ return target_os
def ReadStampFile(target_os):
However, unlike on Windows, build/mac_files doesn't put the toolchain output in a subdirectory with the hash, so the two toolchains clobber each other as far as I can tell. That's not good.
Things we might want to do:
1. Add toolchain hash to dir, so we end up with build/mac_files/8e2002/Xcode.app and build/mac_files/8c1002/Xcode.app
2. Use same toolchain for mac and ios; the ios toolchain needs a mac toolchain for host stuff anyhow (not sure about this one)
,
Jul 13 2017
Until Apple's ibtool stops causing problems, we can't use hermetic for developers. Running Xcode locally plus building with a hermetic will cause the world to implode. For the time being iOS hermetic is bots only.
,
Jul 14 2017
In that case, the bots should get the iOS hermetic toolchain using FORCE_IOS_TOOLCHAIN, and that way developers will *always* pull the macOS toolchain.
,
Jul 21 2017
[mac triage] taking out of triage
,
Aug 2 2017
Just got bit by this today, because I picked up https://crrev.com/c/570459 after a gclient sync, on a (Google) corp MacOS machine. I had target_os = [ 'ios' ] in my .gclient, per the iOS build instructions, so I wasn't getting the new toolchain. My workaround was to set target_os to [ 'mac', 'ios' ]. After another gclient sync, I'm happily building Chromium again. Up until now, I didn't realize we're using a hermetic toolchain for corp machines. The error messages were a bit confusing, because I do have my own Xcode with the 10.12 SDK installed. Fortunately, the error messages did contain build/mac/find_sdk.py, which was enough to start figuring things out. This might be worth a PSA?
,
Aug 2 2017
Or we could just fix this bug and make things Just Work, i.e. erikchen, can you implement comment 3?
,
Aug 2 2017
Chatted with Justin, this is on his to-do list, but low priority.
,
Aug 2 2017
In that case, a brief message to the right mailing list could save some eng time. Note that this doesn't only impact folks who are actively working on iOS -- I ended up with target_os = [ 'ios' ] because I needed to figure out a build failure at some point.
,
Aug 2 2017
I'm lost here. Can someone re-state which combinations do or don't work, and what the proposed fixes are?
,
Aug 2 2017
The fix here is a one-line change (or deleting more than one line). Not fixing this is disruptive for everyone who works on both chrome/mac and chrome/ios. Can we please just fix? dpranke: If you have `target_os = ['ios']` in your .gclient, then we don't download the hermetic mac toolchain (which kind of breaks the mac build). (We do download the hermetic ios toolchain instead, which currently isn't used for anything.)
,
Aug 2 2017
This isn't a 1-line fix, since we need to fix all the bots that currently rely on this logic to get the IOS toolchain.
,
Aug 2 2017
we could hack in a fix by detecting if this looks like a corp device, and if so, forcing the macOS toolchain. This *might* have problems if there are any stray corp-images floating around in our fleet. This would also prevent users from using the IOS hermetic toolchain, but maybe there aren't any?
,
Aug 2 2017
Doesn't that just make it 3 lines then though? In the recipe, check for ios, if set, set env var, in script check for env var?
,
Aug 2 2017
when you say that using the hermetic toolchain doesn't work for developers, are you really saying *ios developers* who need to be running xcode interactively? If I just want to build and run tests locally without running xcode, shouldn't the hermetic toolchain work just as well for me locally as it would on a bot?
,
Aug 2 2017
The trick is identifying the necessary and sufficient recipes to set the env var. And then dealing with fallout when you inevitably over or under-specify the appropriate recipes. I've put together a 3-line hack to deal with this problem until Justin has time to plumb through the ENV variable for iOS: https://chromium-review.googlesource.com/c/598776
,
Aug 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a211039cca8b7afe0edc753d051772a5ec9d63d7 commit a211039cca8b7afe0edc753d051772a5ec9d63d7 Author: erikchen <erikchen@chromium.org> Date: Wed Aug 02 21:11:18 2017 Temporary workaround for macOS hermetic toolchain not being used. Users who specify an 'ios' solution in .gclient will pull the iOS toolchain. This is used by 'ios' bots to pull the toolchain. But macOS developers who happen to specify an 'ios' solution will also pull the iOS toolchain. This CL is a temporary hack to work around this: if the machine appears to be a corp machine, always pull the macOS toolchain. Bug: 742527 Change-Id: Ic49785078f0e15c06f44f61a0daab54dd552ec9e Reviewed-on: https://chromium-review.googlesource.com/598776 Reviewed-by: Nico Weber <thakis@chromium.org> Reviewed-by: Justin Cohen <justincohen@chromium.org> Commit-Queue: Erik Chen <erikchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#491498} [modify] https://crrev.com/a211039cca8b7afe0edc753d051772a5ec9d63d7/build/mac_toolchain.py
,
Aug 3 2017
FWIW: I'm running into this problem (or one very much like it :-}) @ p491708 (i.e. after the above commit). See issue 752092 (which I think I'm going to close as a dup of this). Removing target = ['ios'] from my .gclient fixed the problem, but I'd love a longer-term fix if possible.
,
Aug 3 2017
Issue 752092 has been merged into this issue.
,
Aug 3 2017
rdsmith, are you synced up? Past 491498 this is supposed to just work.
,
Aug 3 2017
@thakis: See c#17. I'm seeing it at p491708, which I think is > 491498 :-}.
,
Aug 3 2017
rdsmith: Are you on a corp image? Do you have a directory at '/Library/GoogleCorpSupport/'?
,
Aug 3 2017
@erikchen: Yes, and yes.
,
Aug 3 2017
Minor mistake that I don't think is relevant: I'm at 491609, not 491708.
,
Aug 3 2017
At Erik's offline request, I re-added the target_os line and re-ran gclient sync, and I don't have the problem anymore. The results of cat ./build/mac_files/toolchain_build_revision were 8E2002-3 both before and after the change. So I'm not an issue anymore.
,
Aug 14 2017
,
Aug 30 2017
I just ran into this. I had target_os = ['ios'] in my .gclient file, which apparently prevented gclient sync from downloading the new sdk. Removing that line made it work.
,
Aug 31 2017
A few questions and comments: thakis@ Mac puts it's toolchain in build/mac_files, ios put it's toolchain build/ios_files -- so they shouldn't clobber one another. Is that not what you are seeing? hans@ With erikchen@'s change I'm curious why the mac toolchain wasn't pulled. Is _IsCorpMachine is False for you? If `target_os = ['ios']` means mac + ios, does that mean all the ios bots and ios developers should also be pulling build/mac_files/Xcode? iOS builds don't need this, but it doesn't roll that often. It means about 3 minutes of time per roll and an extra 2.1GB on the bots and dev machines. But I'm not really sure how users are still seeing this unless _IsCorpMachine is False.
,
Aug 31 2017
Oh, I may have found an issue: https://chromium-review.googlesource.com/c/chromium/src/+/646449
,
Aug 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d09a66e5742982e0d2a8dbdee3726ddd8abbea1 commit 9d09a66e5742982e0d2a8dbdee3726ddd8abbea1 Author: Justin Cohen <justincohen@google.com> Date: Thu Aug 31 22:05:36 2017 Tweak logic in mac_toolchain.py - Download mac_toolchain always. - Download ios_toolchain if set in target_os and FORCE_MAC_TOOLCHAIN This means all bots and developers will always have mac_toolchain, which will fix the problem where sometimes dev set target_os=['ios'] and stop getting the mac toolchain. iOS-only dev and bots will also get mac_toolchain rolls, but this roll doesn't happen often, and only adds a few minutes and few hundred MB occasionally. Bug: 742527 Change-Id: I6387c7b886cda96d9555591a750e21da94986228 Reviewed-on: https://chromium-review.googlesource.com/646449 Reviewed-by: Nico Weber <thakis@chromium.org> Reviewed-by: Erik Chen <erikchen@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Justin Cohen <justincohen@chromium.org> Cr-Commit-Position: refs/heads/master@{#499033} [modify] https://crrev.com/9d09a66e5742982e0d2a8dbdee3726ddd8abbea1/build/mac_toolchain.py
,
Aug 31 2017
Hopefully now fixed! |
||||
►
Sign in to add a comment |
||||
Comment 1 by erikc...@chromium.org
, Jul 13 2017