zlib: Properly clean up static/shared distinction by nh2 · Pull Request #66490 · NixOS/nixpkgs

nh2 · 2019-08-11T20:19:53Z

Includes #66486 (a simple comments change) which is for master, while this is for staging.

Motivation for this change

The previous logic was "grown" and has become pretty crazy to understand.

This improves what commit e999def (zlib: clean up static/shared distincion) described as "kind of a mess" and "confusing". And indeed it was confusing.

Now, the concept whether or not the .a file is moved to a split output is controlled by a clean variable.

The defaults remain unchanged.

The new approach also finally cleanly allows building statically but NOT using a split output, like all other autoconf-based projects in nixpkgs do (using the dontDisableStatic setting).
That is important for overlays that want to enable static libs for all packages in one go, without having to hand-patch idiosynchrasies like zlib had until now.

Until now, if you wanted the .a in the main output, the only way was to go via static=false, shared=true -- which made no sense, because you had to say static=false even though you want a static lib. That is fixed now.

Things done

Notify maintainers

cc @matthewbauer

nh2 · 2019-08-11T22:01:42Z

@GrahamcOfBorg build zlib

pkgs/development/libraries/zlib/default.nix

matthewbauer · 2019-08-13T16:27:14Z

The comments look good, but I'm a little bit surprised this needs to be a mass rebuild. I didn't think dontDisableStatic would be needed at all, --disable-static is not being passed to zlib.

splitStaticOutput definitely makes things clearer, although for the same reasons you describe in .pc, I was hoping we could eventually phase those out altogether. There are a few things that rely on this output existing though. pkg-config assumes that all of your libraries are in the same directory.

Changing the order seems reasonable though, at least to make things more clear. Not sure why it has that behavior, but my understanding was that zlib just always assumed you wanted static libraries.

nh2 · 2019-08-13T18:03:41Z

I didn't think dontDisableStatic would be needed at all, --disable-static is not being passed to zlib.

@matthewbauer Can you elaborate a bit on that? I thought that's passed in general, is there an exception for zlib somehwere that I could refer to?

for the same reasons you describe in .pc, I was hoping we could eventually phase those out altogether. There are a few things that rely on this output existing though

I wasn't sure if there's anyone who wants the zlib.static output to exist. Do you know if such someone exists?

I didn't find many uses of it; what's the best way to find them all?

Changing the order seems reasonable though, at least to make things more clear. Not sure why it has that behavior

As far as I can tell, zlib has a hand-writen ./configure script, not autoconf, and I suspect that the logic is that by default it builds both, that they added a --static option to build only static, and that later somebody added a --shared flag so that you can "turn it back on", and that the order is simply because their command line argument parsing code "grew that way". But I haven't checked.

nh2 · 2019-08-13T18:42:46Z

I thought that's passed in general, is there an exception for zlib somehwere that I could refer to?

Ah I see:

nixpkgs/pkgs/stdenv/generic/setup.sh

Lines 993 to 998 in 6490f9c

    
           # By default, disable static builds. 
        
           if [ -z "${dontDisableStatic:-}" ]; then 
        
               if [ -f "$configureScript" ] && grep -q enable-static "$configureScript"; then 
        
                   configureFlags="--disable-static $configureFlags" 
        
               fi 
        
           fi

This greps for enable-static in the ./configure file, and zlib's handwritten one doesn't have that.

I'm a little bit surprised this needs to be a mass rebuild

I've pushed a fix that should avoid mass rebuilds, with extra commentary for rationale.

nh2 · 2019-08-13T18:43:11Z

@GrahamcOfBorg eval

@GrahamcOfBorg build zlib

This improves what commit e999def zlib: clean up static/shared distincion described as "kind of a mess" and "confusing". And indeed it was confusing. Now, the concept whether or not the .a file is moved to a split output is controlled by a clean variable. The defaults remain unchanged. The new approach also finally cleanly allows building statically but NOT using a split output, like all other autoconf-based projects in nixpkgs do (using the `dontDisableStatic` setting). That is important for overlays that want to enable static libs for all packages in one go, without having to hand-patch idiosynchrasies like zlib had until now. Until now, if you wanted the .a in the main output, the only way was to go via `static=false, shared=true` -- which made no sense, because you had to say `static=false` even though you want a static lib. That is fixed now.

Ericson2314 · 2019-08-14T22:13:57Z

madler/zlib#394 may it someday be better upstream.

nh2 · 2019-08-15T00:56:28Z

splitStaticOutput definitely makes things clearer, although for the same reasons you describe in .pc, I was hoping we could eventually phase those out altogether. There are a few things that rely on this output existing though. pkg-config assumes that all of your libraries are in the same directory.

I have filed a separate issue for this task at #66658.

Our submodule now includes the fix from NixOS/nixpkgs#66490 which makes this unnecessary.

…xpkgs#66490

Our submodule now includes the fix from NixOS/nixpkgs#66490 which makes this unnecessary.

…xpkgs#66490

Our submodule now includes the fix from NixOS/nixpkgs#66490 which makes this unnecessary.

nh2 requested a review from matthewbauer August 11, 2019 20:20

andrew-d reviewed Aug 12, 2019

View reviewed changes

pkgs/development/libraries/zlib/default.nix Outdated Show resolved Hide resolved

nh2 force-pushed the zlib-static-shared-split-cleanup branch from b6e2ad4 to bd66819 Compare August 13, 2019 00:18

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Aug 13, 2019

nixpkgs: Include zlib cleanup from NixOS/nixpkgs#66490

642a257

nh2 force-pushed the zlib-static-shared-split-cleanup branch from bd66819 to 5647474 Compare August 13, 2019 18:42

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Aug 13, 2019

nixpkgs: Include zlib cleanup from NixOS/nixpkgs#66490

f984dfa

This was referenced Aug 14, 2019

Fully static Haskell executables - overview issue #43795

Open

Upgrading to nixpkgs master nh2/static-haskell-nix#6

Closed

nh2 force-pushed the zlib-static-shared-split-cleanup branch from 5647474 to 3d87db9 Compare August 14, 2019 22:06

nh2 requested a review from Ericson2314 as a code owner August 14, 2019 22:06

ofborg bot removed 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` labels Aug 14, 2019

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Aug 14, 2019

nixpkgs: Include zlib cleanup from NixOS/nixpkgs#66490

4f57531

This was referenced Aug 14, 2019

survey: statify: Remove hardcoded link of zlib+ncurses nh2/static-haskell-nix#42

Closed

WIP Control static and shared linking completely independently madler/zlib#394

Open

Get rid of zlib.static output #66658

Open

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Aug 15, 2019

survey: Remove curl -lz, no longer necessary.

2c7967c

Our submodule now includes the fix from NixOS/nixpkgs#66490 which makes this unnecessary.

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Aug 16, 2019

nixpkgs: Include zlib cleanup from NixOS/nixpkgs#66490

c8efdf3

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Aug 16, 2019

survey: Remove curl -lz, no longer necessary.

5baf249

Our submodule now includes the fix from NixOS/nixpkgs#66490 which makes this unnecessary.

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Aug 16, 2019

nixpkgs: Include zlib cleanup from NixOS/nixpkgs#66490

19186c1

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Aug 16, 2019

survey: Remove curl -lz, no longer necessary.

cffe6bf

Our submodule now includes the fix from NixOS/nixpkgs#66490 which makes this unnecessary.

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Aug 16, 2019

nixpkgs: Update submodule, add changes for zlib cleanup from NixOS/ni…

7296d34

…xpkgs#66490

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Aug 16, 2019

survey: Remove curl -lz, no longer necessary.

f9bdfa0

Our submodule now includes the fix from NixOS/nixpkgs#66490 which makes this unnecessary.

nh2 merged commit aa99a26 into NixOS:master Aug 17, 2019

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Aug 17, 2019

nixpkgs: Update submodule, add changes for zlib cleanup from NixOS/ni…

7a2d291

…xpkgs#66490

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Aug 17, 2019

survey: Remove curl -lz, no longer necessary.

2ed2d3a

Our submodule now includes the fix from NixOS/nixpkgs#66490 which makes this unnecessary.

doronbehar mentioned this pull request Feb 16, 2026

Tracking issue for zlib #490999

Open

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

zlib: Properly clean up static/shared distinction#66490

zlib: Properly clean up static/shared distinction#66490
nh2 merged 1 commit intoNixOS:masterfrom
nh2:zlib-static-shared-split-cleanup

nh2 commented Aug 11, 2019

Uh oh!

nh2 commented Aug 11, 2019

Uh oh!

Uh oh!

matthewbauer commented Aug 13, 2019

Uh oh!

nh2 commented Aug 13, 2019

Uh oh!

nh2 commented Aug 13, 2019

Uh oh!

nh2 commented Aug 13, 2019

Uh oh!

Ericson2314 commented Aug 14, 2019

Uh oh!

nh2 commented Aug 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Comments

Conversation

nh2 commented Aug 11, 2019

Motivation for this change

Things done

Notify maintainers

Uh oh!

nh2 commented Aug 11, 2019

Uh oh!

Uh oh!

matthewbauer commented Aug 13, 2019

Uh oh!

nh2 commented Aug 13, 2019

Uh oh!

nh2 commented Aug 13, 2019

Uh oh!

nh2 commented Aug 13, 2019

Uh oh!

Ericson2314 commented Aug 14, 2019

Uh oh!

nh2 commented Aug 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants