cmake: classify libraries in /usr/lib as system libraries #98565
Conversation
This file contains 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
ofborg
bot
added
10.rebuild-darwin: 501+
10.rebuild-darwin: 5001+
10.rebuild-linux: 0
labels
10 tasks
|
/rebase-staging |
|
@hannesweisbach please rebase on staging. |
|
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,
jonringer,
kalbasit,
Mic92 and
zowoq
as code owners
I hope I did that right. "cimg: 2.9.1 -> 2.9.2", commit |
|
@hannesweisbach you forgot to change the base branch. I did that for you. |
| @@ -70,6 +70,12 @@ stdenv.mkDerivation rec { | |||
| # CC_FOR_BUILD and CXX_FOR_BUILD are used to bootstrap cmake | |||
| + '' | |||
| 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 '' | |||
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
6.topic: python
8.has: documentation
10.rebuild-linux-stdenv
labels
ofborg
bot
added
10.rebuild-linux: 1-10
and removed
10.rebuild-linux: 501+
10.rebuild-linux: 5001+
labels
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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)