cmake: classify libraries in /usr/lib as system libraries#98565
Closed
hannesweisbach wants to merge 2 commits intoNixOS:stagingfrom
Closed
cmake: classify libraries in /usr/lib as system libraries#98565hannesweisbach wants to merge 2 commits intoNixOS:stagingfrom
hannesweisbach wants to merge 2 commits intoNixOS:stagingfrom
Conversation
ofborg
bot
added
the
10.rebuild-darwin-stdenv
ofborg
bot
added
10.rebuild-darwin: 501+
10 tasks
Member
|
/rebase-staging |
Member
|
@hannesweisbach please rebase on staging. |
Contributor
Member
|
Would not this make builds less hermetic on Darwin (especially when not using sandbox)? |
This change is only relevant for App Bundle builds on macOS. For an App Bundle every non-system runtime dependency gets copied recursively into the App Bundle to make a standalone distributable package. The replacement of every occurence of '/usr' by '/var/empty' by Nix thwarts App Bundle builds, because libraries in /usr/lib do not get correctly classified as system libraries. This patch reverts this single path name back to '/usr/lib', so that packaging App Bundles on macOS works. FYI: within CMake the occurence is also guarded by an 'if(APPLE)' statement.
hannesweisbach
requested review from
FRidh,
Mic92,
jonringer,
kalbasit and
zowoq
as code owners
Contributor
Author
I hope I did that right. "cimg: 2.9.1 -> 2.9.2", commit |
ofborg
bot
added
6.topic: golang
Member
|
@hannesweisbach you forgot to change the base branch. I did that for you. |
| configureFlags="--parallel=''${NIX_BUILD_CORES:-1} CC=$CC_FOR_BUILD CXX=$CXX_FOR_BUILD $configureFlags" | ||
| '' | ||
| # Detect dylibs in /usr/lib as system libraries on Darwin to correctly build App bundles | ||
| + stdenv.lib.optionalString stdenv.isDarwin '' |
Member
There was a problem hiding this comment.
Suggested change
| + stdenv.lib.optionalString stdenv.isDarwin '' | |
| + lib.optionalString stdenv.isDarwin '' |
As per recent contributing change.
ofborg
bot
removed
6.topic: golang
ofborg
bot
added
10.rebuild-linux: 1-10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation for this change
This change is only relevant for App Bundle builds on macOS. For an App Bundle
every non-system runtime dependency gets copied recursively into the App Bundle
to make a standalone distributable package.
The replacement of every occurence of '/usr' by '/var/empty' by Nix thwarts App
Bundle builds, because libraries in /usr/lib do not get correctly classified as
system libraries.
This patch reverts this single path name back to '/usr/lib', so that packaging
App Bundles on macOS works. FYI: within CMake the occurence is also guarded by
an 'if(APPLE)' statement.
The substituteInPlace matches only a single line: https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/GetPrerequisites.cmake#L522
Let me know, if a proper patch is preferred instead.
Draft, because I'm still building ;)Edit: Building the world succeeded
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)