bzip2, zstd: Add enableStatic option
#237563
Closed
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: 0
10.rebuild-linux: 1-10
10.rebuild-linux: 1
labels
|
@ofborg build nixStatic nixStatic.passthru.tests |
l0b0
reviewed
Jun 13, 2023
| lib.optional enableStatic "--enable-static" ++ | ||
| lib.optional disableShared "--disable-shared"; | ||
|
|
||
| dontDisableStatic = if enableStatic then true else null; # null to not cause rebuild vs historic versions that didn't have the attribute at all |
There was a problem hiding this comment.
Is the purpose of this only to avoid a rebuild? If so, isn't it more important to keep the code simple?
There was a problem hiding this comment.
Is the purpose of this only to avoid a rebuild?
Yes, the usage of else null is.
With a rebuild, the entirety of nixpkgs would be rebuilt, so ofborg could not build it, this commit could not be cherry-picked easily, and it would have to target staging.
I am thinking that a followup PR to staging that removes else null could do the code simplification. But happy to hear others' opinions!
There was a problem hiding this comment.
we have no urgency here so please target staging straight away and avoid the rebuild reducing tricks
Comment on lines
51
to
53
| configureFlags = | ||
| lib.optionals linkStatic [ "--enable-static" "--disable-shared" ]; | ||
| lib.optional enableStatic "--enable-static" ++ | ||
| lib.optional disableShared "--disable-shared"; |
There was a problem hiding this comment.
Suggested change
| configureFlags = | |
| lib.optionals linkStatic [ "--enable-static" "--disable-shared" ]; | |
| lib.optional enableStatic "--enable-static" ++ | |
| lib.optional disableShared "--disable-shared"; | |
| configureFlags = lib.optional enableStatic "--enable-static" | |
| ++ lib.optional disableShared "--disable-shared"; |
| lib.optional enableStatic "--enable-static" ++ | ||
| lib.optional disableShared "--disable-shared"; | ||
|
|
||
| dontDisableStatic = if enableStatic then true else null; # null to not cause rebuild vs historic versions that didn't have the attribute at all |
There was a problem hiding this comment.
we have no urgency here so please target staging straight away and avoid the rebuild reducing tricks
| lib.optional enableStatic "--enable-static" ++ | ||
| lib.optional disableShared "--disable-shared"; | ||
|
|
||
| dontDisableStatic = if enableStatic then true else null; # null to not cause rebuild vs historic versions that didn't have the attribute at all |
There was a problem hiding this comment.
Suggested change
| dontDisableStatic = if enableStatic then true else null; # null to not cause rebuild vs historic versions that didn't have the attribute at all | |
| dontDisableStatic = enableStatic; |
12 tasks
libwebp: 1.3.0 -> 1.3.1
pypy3Packages.execnet: disable tests as they randomly crash, cleanup
The configure scripts frequently use `main` returning an implicit `int`, which causes spurious failures when CC is clang 16+. This is fixed by patching the provided macros and regenerating the scripts with autoreconfHook, though it requires some manual processing (see below). The upstream `configure.ac` is written in such a way that it requires fixups and post-processing. * Fixups are required because the original build process just cats the macros together into one file. When `aclocal` is run, it does not pick up all of them. This is worked around by catting the missing macros to a file that is picked up by autoreconfHook. * Post-processing is required to populate the version information. This is done in a subshell to avoid polluting the environment with the contents of `RELEASE`. Otherwise, the build will fail because the version C macros it expects will not be defined.
Both `_spin_lock_try` and `_spin_unlock` are private and deprecated APIs, which are not exported by any headers in the SDK. The build fails because the configure script does not define the functions before calling them, which is treated as error by clang 16. This patch replaces use of those APIs with `os_unfair_lock`, which is the recommended replacement per the deprecation messages.
In preparation for bumping the LLVM used by Darwin, this change refactors and reworks the stdenv build process. When it made sense, existing behaviors were kept to avoid causing any unwanted breakage. However, there are some differences. The reasoning and differences are discussed below. - Improved cycle times - Working on the Darwin stdenv was a tedious process because `allowedRequisites` determined what was allowed between stages. If you made a mistake, you might have to wait a considerable amount of time for the build to fail. Using assertions makes many errors fail at evaluation time and makes moving things around safer and easier to do. - Decoupling from bootstrap tools - The stdenv build process builds as much as it can in the early stages to remove the requirement that the bootstrap tools need bumped in order to bump the stdenv itself. This should lower the barrier to updates and make it easier to bump in the future. It also allows changes to be made without requiring additional tools be added to the bootstrap tools. - Patterned after the Linux stdenv - I tried to follow the patterns established in the Linux stdenv with adaptations made to Darwin’s needs. My hope is this makes the Darwin stdenv more approable for non-Darwin developers who made need to interact with it. It also allowed some of the hacks to be removed. - Documentation - Comments were added explaining what was happening and why things were being done. This is particular important for some stages that might not be obvious (such as the sysctl stage). - Cleanup - Converting the intermediate `allowedRequisites` to assertions revealed that many packages were being referenced that no longer exist or have been renamed. Removing them reduces clutter and should help make the stdenv bootstrap process be more understandable.
swift-corelibs fails to build due to a missing header and an invalid pointer conversion. Patches are provided to fix both of these issues.
Switching the build system to cmake makes it easier to make changes to the build (particularly which dependencies to link). It also removes a lot of manual build steps and fixes the issue identified by @emilazy in NixOS#238791. Fixes NixOS#238791.
Upstream swift-corelibs links against icu on Linux, so it is not necessarily tied to the version of ICU provided by Apple for Darwin. swift-corelibs and qtwebkit are the only two packages that link against darwin.ICU. Switching to the nixpkgs icu will allow the Darwin-specific package to be deprecated and removed eventually.
swift-corelibs uses libcurl to implement `NSURLSession` in Foundation via the symbols exported by CF. Foundation is not build on Darwin, and these symbols are not exported by the system CoreFoundation. By not linking against libcurl, this breaks a cycle between CF and libcurl. That should allow libcurl to drop the patch disabling linking against the SystemConfiguration and restore NAT64 support. Unfortunately, the Darwin stdenv bootstrap still needs to build dependencies that use `fetchFromGitHub`. While it can drop curl from the final stdenv, it still needs to use it during the stdenv bootstrap.
@emilazy found a bug in NixOS#234861. Specifically, the hook is not actually applied. e2fsprogs links against darwin.CF, but since it cannot find the framework by rpath, it crashes. Since the hook should always be used, it is copied directly to `nix-support/setup-hook` instead of providing it as an attribute. This preserves dropping the hook in the cross-compilation case while providing it for everything else that needs it. To avoid further churn and due to the complexity of building the stdenv with the hook active, this change required the stdenv rework.
nh2
requested review from
ajs124,
lukegb,
mweinelt,
matthewbauer,
zowoq,
winterqt,
figsoda,
Mic92,
marsam,
a team and
Ericson2314
as code owners
|
Closing to make a new PR for Edit: Bah, the close operation reverted itself after a minute with Github 's |
12 tasks
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.
Edit: Converted to
stagingPR at #243161Re-open of #237449, this time without accidentally subscribing many reviewers because of the wrong source branch in the PR.
Description of changes
Supports #61575.
The fact that these libs currently force-disable shared libraries when static ones are enabled was found during working on nh2/static-haskell-nix#116.
FYI @jonathanlking @aherrmann
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)