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
macvim: 8.2.319 -> 8.2.539 #85321
macvim: 8.2.319 -> 8.2.539 #85321
Conversation
Xcode 11.4 has an updated sys/_types/_fd_def.h header that references a new symbol from libSystem. This is a problem because we're using `/usr/bin/clang` to compile the non-Xcode portion, and this pulls in headers from Xcode's SDK. Somehow it's still linking to the Nix libraries (I can't figure out where configure finds these to put into `LDFLAGS` as we're not using the cc-wrapper). The end result is we get a linker error where this new symbol can't be found at link time, even though it's a weak import and isn't required at runtime. Ideally we'd provide a full 10.12 SDK to `/usr/bin/clang`, but we can't do that because even the DevSDK package we use for our 10.12 SDK doesn't contain everything (in particular it's missing nearly all dylibs) so we just get linker errors if we do that. Instead we'll just do a horrible hack and provide an `-isystem` path to a folder structure that contains only the 10.12 `sys/_types/_fd_def.h` header. This avoids the new symbol without causing all the errors that happen if we pull in the entire `${darwin.Libsystem}/include`.
I took another look at the built product and the I don't know if we should just give up and figure out how to make Eventully we should look into using |
I dug into |
Since |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/146 |
MacVim compiles the Vim part using `/usr/bin/clang` and the GUI part using Xcode. The Xcode portion always uses Xcode's own SDK and we have no workable alternative. The Vim portion so far has been compiling using a hybrid compilation environment, where it uses the SDK for most stuff but picks up a bunch of library linker paths (including libSystem) by virtue of Ruby's LDFLAGS. This hybrid compilation environment meant that if the SDK headers referenced a symbol that the library itself didn't have, this could produce link errors. Previously we attempted to fix this by synthesizing an include path that contained just the one header from Nix's Libsystem that referenced the missing symbol, to get rid of the reference and allow linking to work again, but this was very hacky and runs the risk of future Xcode SDK changes producing the same errors with different headers, or of future SDK versions expecting the intercepted header to contain a definition that Nix's doesn't. This new approach is to just clean up the compilation environment such that the Vim portion is compiling against the Xcode SDK as well, by sanitizing the LDFLAGS produced by the configure script so it stops referencing Nix's versions of OS libraries. This means the resulting Vim binary no longer depends at runtime on Nix for anything except the scripting language support, but that's how it's been for the MacVim binary all along anyway, and this approach should keep us insulated against future Xcode SDK changes.
I changed the |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/152 |
@lilyball Sorry, can't really review this. Merging assuming this is working well for you. |
Motivation for this change
https://github.com/macvim-dev/macvim/releases/tag/snapshot-163
This also includes a fix for compatibility with Xcode 11.4. Xcode 11.4 has an updated
sys/_types/_fd_def.h
header that references a new symbol from libSystem. This is a problem because we're using/usr/bin/clang
to compile the non-Xcode portion, and thispulls in headers from Xcode's SDK. Somehow it's still linking to the Nix libraries. The end result is we get a linker error where this new symbol can't be found at link time, even though it's a weak import and isn't required at runtime.
Ideally we'd provide a full 10.12 SDK to
/usr/bin/clang
, but we can't do that because even the DevSDK package we use for our 10.12 SDK doesn't contain everything (in particular it's missing nearly all dylibs) so we just get linker errors if we do that.Instead we'll just strip the LDFLAGS post-configure so it no longer references the Nix versions of OS libraries. This means we're not linking against Nix libSystem anymore, and that fixes the link error. This also prevents this issue from cropping up again in the future.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)