Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

darwin.apple_sdk: update to 10.13 #46699

Closed
wants to merge 1 commit into from

Conversation

uri-canva
Copy link
Contributor

@uri-canva uri-canva commented Sep 15, 2018

Motivation for this change

10.10 SDKs don't support type arguments in Foundation classes, leading to "type arguments cannot be applied to non-parameterized class" errors.

Based on 377cef8.

WARNING:

This will cause mass rebuilds on Darwin, and should be merged into staging I think, rather than master, please advise whether I should submit the PR against staging instead (it has merge conflicts between master and staging so if I submit it against staging it won't apply cleanly on master and vice versa).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg GrahamcOfBorg added the 6.topic: darwin Running or building packages on Darwin label Sep 15, 2018
@uri-canva
Copy link
Contributor Author

cc @copumpkin

@LnL7
Copy link
Member

LnL7 commented Sep 15, 2018

Only updating the sdk isn't an option because CoreFoundation is still at 10.10, but @copumpkin has made some pretty exciting progress there recently.

@uri-canva
Copy link
Contributor Author

I haven't figured out exactly how it all connects together, but there's two different CoreFoundation right? An impure one and a pure one. I assume the impure one is this one, from the SDKs, and the pure one is one built from source? Are you saying we can't update the frameworks because they'd use the 10.10 CoreFoundation that is still from source?

@copumpkin
Copy link
Member

@uri-canva thanks for the contribution! I'd take a look at #37403 since @matthewbauer also tried something similar. Since then he's added a package for pbzx so that at least can be less hackish. But I'd also hold off on this for a couple of reasons:

  1. As @LnL7 said, the pure CoreFoundation we have is going to break with this. I just opened stdenv/darwin: integrate a new CoreFoundation #46704 which should improve that but I'd rather hold off on changing too much at once so we can attribute weird failures precisely. Once that has stabilized, I definitely want to update the SDK and several other Apple packages
  2. I'm not 100% sure 10.13 is a good version for this. The reason is that we do try to maintain some backwards-compatibility for folks who aren't on the latest version. I think our current bound goes back to 10.11. It's quite possible that 10.13 will work just fine on older systems, but I'd want to make sure before merging.

In short, changes like this have far-reaching implications and touch a bajillion packages, so we have a fairly high bar for merging them. So just to set your expectations, I wouldn't expect this to get merged in the next few weeks (because of 1 above, and because I'll be on vacation for a couple of weeks starting next Saturday) unless one of the Darwin maintainers suddenly has far more free time than we normally do 😄

@copumpkin
Copy link
Member

copumpkin commented Sep 15, 2018

Also, to answer your specific question: yes, there are two CoreFoundation packages and this SDK defines one of them. Basically, these SDKs have dubious licensing (even just for headers) and we'd rather not have our entire stdenv depend on them, since Hydra refuses to build things like that. Note that we're currently somewhat lax around this but making the entire stdenv depend on it would be a big step in the wrong direction IMO. At the same time, the publicly available CoreFoundation doesn't include all the stuff we need, so we do (occasionally) need to refer to the SDK version. To further complicate matters, you get all sorts of weird fun when you link against both versions of CoreFoundation, often transitively, so we have workarounds for that. It's been a constant pain in the ass, but it's mostly under control right now apart from forcing us to stay on an old SDK. The PR I just opened should allow us to finally advance but has changed lots of stuff so I want to keep an eye on it for a while.

@LnL7
Copy link
Member

LnL7 commented Sep 15, 2018

I introduced CoreFoundation in the frameworks as a hacky workaround for builds that fail with our old CF, I'm pretty sure nothing will need it anymore after the update.

@matthewbauer
Copy link
Member

matthewbauer commented Sep 15, 2018

While it may be true that newer SDK will break things, it is usually really obvious looking at Hydra. Making breakages reproducible means we can just revert the bad commit. This is one of my favorite things about Nix!

I would shoot for using 10.13 SDK TBH. Apple is usually pretty good at not breaking old code so I think it should be okay. I know of a few things introduced in SDK that is already being used in the wild. Plus if anyone wants to use older macOS versions, they can just use the 18.09 or 18.03 channel. That can just be in the release notes.

@uri-canva
Copy link
Contributor Author

Thanks for the clarifications @copumpkin, that helped.

I was also a bit uncertain whether it would be better to update the stack from the bottom up when I added #define TARGET_OS_OSX 1 to Libsystem even though the source we build it from doesn't have it, because the 10.13 SDKs require it.

I will work around the need for an updated Apple SDKs in #46700, so it's not blocked.

I'm not 100% sure 10.13 is a good version for this. The reason is that we do try to maintain some backwards-compatibility for folks who aren't on the latest version.

This is related to the impure CoreFoundation right? Because if the whole stack was pure it wouldn't matter at all what version the host macOS was, since everything would be built by Nix? Or is this about source compatibility of the derivations?

@lionello
Copy link
Contributor

Was hoping that this patch would fix issues I'm having with coreutils by being able to build coreutils against libsystem ≥10.13, but it appears that it's still using the old 10.11.6. Any idea why?

@copumpkin
Copy link
Member

@lionello because that stuff is governed by a completely separate set of packages in apple-source-releases. This is only used as a last resort. The apple-source-releases thing was the next thing I was going to look at, although I'm going on vacation soon so might not get to it for a bit.

Out of curiosity, what's missing in 10.11.6 that affects coreutils? We've been intentionally hiding some comparatively new symbols for libsystem in particular because we want to remain compatible with the last 3 or so major macOS revisions, so even if we update libsystem you might not get what you want for a while.

@lionello
Copy link
Contributor

lionello commented Sep 18, 2018

Out of curiosity, what's missing in 10.11.6 that affects coreutils? We've been intentionally hiding some comparatively new symbols for libsystem in particular because we want to remain compatible with the last 3 or so major macOS revisions, so even if we update libsystem you might not get what you want for a while.

futimens, without it touch (and other utils) only handle µs resolution file times, even though the underlying FS likely had ns resolution. This means that created files never really have the exact expected time. Since libsystem 10.11.6 lacks futimens, coreutils falls back to futimesat.

@copumpkin
Copy link
Member

Ah, so it looks like futimens only appeared in 10.13, so we're unlikely to enable it for a while.

Basically, the issue we're guarding against is that libSystem is one of our very few impurities. Say for a moment that we didn't hide (or more precisely, whitelist) symbols like futimens: we build coreutils on a Hydra worker running 10.13, so we get a version of touch that expects futimens to exist. Then someone with Nix on 10.12 grabs coreutils from our binary cache and gets a linker error rather than a working tool.

The other option would be to somewhat purely "fork" our stdenv for each major macOS version, but it would mean a massive amount of rebuilds (and we're already pretty short on Mac builders) for comparatively low benefit, since the only real differences arise from subtle symbol changes in libsystem.

@uri-canva
Copy link
Contributor Author

Thanks again for all the context, it's been very helpful.
I will close this PR since I don't plan on working it for now but I'll keep an eye out on developments from others.

@uri-canva uri-canva closed this Sep 21, 2018
@uri-canva uri-canva deleted the darwin_sdk_10_13 branch September 21, 2018 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants