Skip to content

zlib: use default configure script on windows #428871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
greg-hellings wants to merge 1 commit into
base: staging
Choose a base branch
from
+18 −8
Open

zlib: use default configure script on windows #428871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions pkgs/development/libraries/zlib/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ stdenv.mkDerivation (finalAttrs: {
hash = "sha256-mpOyt9/ax3zrpaVYpYDnRmfdb+3kWFuR7vtg8Dty3yM=";
};

patches = lib.optional stdenv.hostPlatform.isMinGW ./mingw-shared.patch;

postPatch = lib.optionalString stdenv.hostPlatform.isDarwin ''
substituteInPlace configure \
--replace '/usr/bin/libtool' '${stdenv.cc.targetPrefix}ar' \
Expand All @@ -59,8 +61,6 @@ stdenv.mkDerivation (finalAttrs: {
setOutputFlags = false;
outputDoc = "dev"; # single tiny man3 page

dontConfigure = stdenv.hostPlatform.isMinGW;

preConfigure = lib.optionalString (stdenv.hostPlatform != stdenv.buildPlatform) ''
export CHOST=${stdenv.hostPlatform.config}
'';
Expand Down Expand Up @@ -105,7 +105,8 @@ stdenv.mkDerivation (finalAttrs: {
# Non-typical naming confuses libtool which then refuses to use zlib's DLL
# in some cases, e.g. when compiling libpng.
+ lib.optionalString (stdenv.hostPlatform.isMinGW && shared) ''
ln -s zlib1.dll $out/bin/libz.dll
mkdir -p $out/bin
mv $out/lib/*.dll* $out/bin
Copy link
Copy Markdown
Member

@Ericson2314 Ericson2314 Jul 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be a symlink instead like before?

Copy link
Copy Markdown
Contributor Author

@greg-hellings greg-hellings Jul 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have written about the details of why extensively in other PRs related to Windows cross builds, but the short answer is no. DLL files properly belong under the bin/ folder on Windows, not symlinked there.

Copy link
Copy Markdown
Contributor

@RossSmyth RossSmyth Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought there was a setup hook that did this for you, but I guess not.

Copy link
Copy Markdown
Contributor Author

@greg-hellings greg-hellings Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought there was a setup hook that did this for you, but I guess not.

Most packages seem to have it automatically done, but I believe that is because of updates to autotools over the years and setting the destination folders properly nowadays. In the early days of MinGW, the post fixup had to be done to every single package as autotools didn't have any real knowledge of DLL files. However, zlib uses its own homegrown system so as not to rely on autotools, so I think that's why it is not handled by the standard scripts and options. If there is an automatic hook that would handle this properly, then I would be happy to use it, instead.

Copy link
Copy Markdown
Contributor

@RossSmyth RossSmyth Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It happens with my Rust projects, which do not use autotools. Looks like it's called win-dll-link

Copy link
Copy Markdown
Member

@Ericson2314 Ericson2314 Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's fine to symlink transitive DLLs I don't get why it's not fine to symlink DLLs from lib.

I would but the static libs / stub libs in $dev/lib, and the DLLs in $out/lib.

Copy link
Copy Markdown
Contributor

@RossSmyth RossSmyth Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can also change the hook (in a future PR)

Copy link
Copy Markdown
Contributor Author

@greg-hellings greg-hellings Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DLLs properly belong in the bin/ folder on Windows targets. This is because Windows has an extremely limited search path for loading dynamic libraries, but it will always look in the same directory alongside the executing binary. Therefore, the DLLs do not belong in $out/lib but instead belong in $out/bin. This has been established practice for the MinGW packages due to the limitations in Windows search paths and it is how every other package in pkgsCross.mingwW64 handles it, it's how autotools handles it, it's how CMake handles it. The final target's $out/bin when building a .exe gets the dependency and transitive dependency DLL files symlinked into place because there is no equivalent to setting LD paths for Windows. There simply is no precedent anywhere else, including in other nixpkgs Windows targets, for putting the DLL file in lib/. It doesn't belong there. It is not a linker-related file, it is a binary executable on Windows.

Stub libraries and fully static libraries end up in lib/ because they are used at link time but are not necessary to distribute alongside binary files unless you are intentionally distributing build products to enable a downstream dev environment. They could live in $dev/lib or $out/lib depending on the purpose of the parent package, but that's a moot discussion here.

The only reason this difference is exposed here is because zlib has a custom, hand-rolled build system instead of using something like autotools. This change is just maintaining how the library already works, but enabling it to be built for Windows with LLVM compilers. This move is not introducing anything new or altering existing behavior. Here is the current output of building pkgsCross.mingwW64.zlib without this patch:

image

We really should put this information up in the packaging guidelines somewhere so it doesn't need to be reiterated on every pull request.

Copy link
Copy Markdown
Contributor

@RossSmyth RossSmyth Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am very aware how windows search paths work. I am just wondering out loud if we can automate it, because us devs who care about Windows is a much much smaller population than the other major platforms.

Copy link
Copy Markdown
Member

@Ericson2314 Ericson2314 Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#431820

I am opening a separate issue for this point, which is a general disagreement on policy separate from zlib in particular.

'';

env =
Expand All @@ -114,7 +115,7 @@ stdenv.mkDerivation (finalAttrs: {
# to the bootstrap-tools libgcc (as uses to happen on arm/mips)
NIX_CFLAGS_COMPILE = "-static-libgcc";
}
// lib.optionalAttrs (stdenv.hostPlatform.linker == "lld") {
// lib.optionalAttrs (stdenv.hostPlatform.linker == "lld" && !stdenv.hostPlatform.isWindows) {
# lld 16 enables --no-undefined-version by default
# This makes configure think it can't build dynamic libraries
# this may be removed when a version is packaged with https://github.com/madler/zlib/issues/960 fixed
Expand All @@ -138,10 +139,6 @@ stdenv.mkDerivation (finalAttrs: {
makeFlags = [
"PREFIX=${stdenv.cc.targetPrefix}"
]
++ lib.optionals stdenv.hostPlatform.isMinGW [
"-f"
"win32/Makefile.gcc"
]
++ lib.optionals shared [
# Note that as of writing (zlib 1.2.11), this flag only has an effect
# for Windows as it is specific to `win32/Makefile.gcc`.
Expand Down
13 changes: 13 additions & 0 deletions pkgs/development/libraries/zlib/mingw-shared.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/configure b/configure
index c55098a..07f7c08 100755
--- a/configure
+++ b/configure
@@ -243,6 +243,8 @@ if test "$gcc" -eq 1 && ($cc -c $test.c) >> configure.log 2>&1; then
echo "If this doesn't work for you, try win32/Makefile.gcc." | tee -a configure.log
LDSHARED=${LDSHARED-"$cc -shared"}
LDSHAREDLIBC=""
+ shared_ext='.dll'
+ SHAREDLIB='libz.dll'
EXE='.exe' ;;
QNX*) # This is for QNX6. I suppose that the QNX rule below is for QNX2,QNX4
# (alain.bonnefoy@icbt.com)
Loading