zlib: Properly clean up static/shared distinction

[nh2]

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

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.
• Tested all boolean combinations of shared, static and splitStaticOutput, checking that the files in the outputs are as expected.

Notify maintainers

cc @matthewbauer

[nh2]

@GrahamcOfBorg build zlib

[matthewbauer]

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]

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]

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]

@GrahamcOfBorg eval

@GrahamcOfBorg build zlib

[Ericson2314]

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

[nh2]

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.