From 3d87db94a24978df92c58ac5483c9cefd33b5b74 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Niklas=20Hamb=C3=BCchen?= <mail@nh2.me>
Date: Sun, 11 Aug 2019 22:03:56 +0200
Subject: [PATCH] zlib: Properly clean up static/shared distinction

This improves what commit

    e999def1 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.
---
 pkgs/development/libraries/zlib/default.nix | 58 ++++++++++++++-------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/pkgs/development/libraries/zlib/default.nix b/pkgs/development/libraries/zlib/default.nix
index 9f5af47c72680af..592abea4ea04e60 100644
--- a/pkgs/development/libraries/zlib/default.nix
+++ b/pkgs/development/libraries/zlib/default.nix
@@ -1,24 +1,19 @@
 { stdenv
 , fetchurl
-# Regarding static/shared libaries, the current behaviour is:
-#
-# - static=true,  shared=true:  builds both and moves .a to the .static output;
-#                               in this case `pkg-config` auto detection will
-#                               not work if the .static output is given as
-#                               buildInputs to another package (#66461)
-# - static=true,  shared=false: builds .a only and leaves it in the main output
-# - static=false, shared=true:  builds shared only
-#
-# To get both `.a` and shared libraries in one output,
-# you currently have to use
-#   static=false, shared=true
-# and use
-#   .overrideAttrs (old: { dontDisableStatic = true; })
-# This is because by default, upstream zlib ./configure builds both.
-, static ? true
+# Note: If `{ static = false; shared = false; }`, upstream's default is used
+#       (which is building both static and shared as of zlib 1.2.11).
 , shared ? true
+, static ? true
+# If true, a separate .static ouput is created and the .a is moved there.
+# In this case `pkg-config` will auto detection will currently not work if the
+# .static output is given as `buildInputs` to another package (#66461), because
+# the `.pc` file lists only the main output's lib dir.
+# If false, and if `{ static = true; }`, the .a stays in the main output.
+, splitStaticOutput ? static
 }:
 
+assert splitStaticOutput -> static;
+
 stdenv.mkDerivation (rec {
   name = "zlib-${version}";
   version = "1.2.11";
@@ -41,14 +36,35 @@ stdenv.mkDerivation (rec {
   '';
 
   outputs = [ "out" "dev" ]
-    ++ stdenv.lib.optional (shared && static) "static";
+    ++ stdenv.lib.optional splitStaticOutput "static";
   setOutputFlags = false;
   outputDoc = "dev"; # single tiny man3 page
 
-  configureFlags = stdenv.lib.optional shared "--shared"
-                   ++ stdenv.lib.optional (static && !shared) "--static";
+  # For zlib's ./configure (as of verion 1.2.11), the order
+  # of --static/--shared flags matters!
+  # `--shared --static` builds only static libs, while
+  # `--static --shared` builds both.
+  # So we use the latter order to be able to build both.
+  # Also, giving just `--shared` builds both,
+  # giving just `--static` builds only static,
+  # and giving nothing builds both.
+  # So we have 3 possible ways to build both:
+  # `--static --shared`, `--shared` and giving nothing.
+  # Of these, we choose `--shared`, only because that's
+  # what we did in the past and we can avoid mass rebuilds this way.
+  # As a result, we pass `--static` only when we want just static.
+  configureFlags = stdenv.lib.optional (static && !shared) "--static"
+                   ++ stdenv.lib.optional shared "--shared";
+
+  # Note we don't need to set `dontDisableStatic`, because static-disabling
+  # works by grepping for `enable-static` in the `./configure` script
+  # (see `pkgs/stdenv/generic/setup.sh`), and zlib's handwritten one does
+  # not have such.
+  # It wouldn't hurt setting `dontDisableStatic = static && !splitStaticOutput`
+  # here (in case zlib ever switches to autoconf in the future),
+  # but we don't do it simply to avoid mass rebuilds.
 
-  postInstall = stdenv.lib.optionalString (shared && static) ''
+  postInstall = stdenv.lib.optionalString splitStaticOutput ''
     moveToOutput lib/libz.a "$static"
   ''
     # jww (2015-01-06): Sometimes this library install as a .so, even on
@@ -69,6 +85,8 @@ stdenv.mkDerivation (rec {
   # to the bootstrap-tools libgcc (as uses to happen on arm/mips)
   NIX_CFLAGS_COMPILE = stdenv.lib.optionalString (!stdenv.hostPlatform.isDarwin) "-static-libgcc";
 
+  # We don't strip on static cross-compilation because of reports that native
+  # stripping corrupted the target library; see commit 12e960f5 for the report.
   dontStrip = stdenv.hostPlatform != stdenv.buildPlatform && static;
   configurePlatforms = [];