cmake: respect custom RC flags with mingw #677

vszakats · 2022-07-11T16:18:57Z

Before this patch, zlib.rc was compiled using a manual command [1] when
using the MinGW toolchain. This method ignores CMAKE_RC_FLAGS and
offers no other way to pass a custom flag, breaking the build in cases
where a custom windres option is required. E.g. --target= or -I on
some platforms and configuration, in particular with llvm-windres.

This patch deletes the special case for MinGW and lets CMake compile the
.rc file via the standard way used for all WIN32.

Also:

Adjust the condition for dealing with the .rc file from NOT MINGW
to WIN32, thus omitting this unnecessary input file for non-Windows
platforms.
Delete the MinGW-specific RC flag GCC_WINDRES.
GCC_WINDRES was protecting logic that compiles fine with recent
windres versions. Also, the protected line contained obsolete,
16-bit era keywords that had no effect for a long time. [2]
Delete the MinGW-specific defaulting logic for CMAKE_RC_COMPILER.
This is done by CMake automatically, to the more portable default
value windres (there is no .exe extension in cross-toolchains).

[1] dc5a43e
[2] https://docs.microsoft.com/windows/win32/menurc/common-resource-attributes

This fixes build errors like this one:

$ cmake . -B bld -Wno-dev -DCMAKE_SYSTEM_NAME=Windows \
  -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_MESSAGE=NEVER \
  -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_SYSROOT=/usr/local/opt/mingw-w64/toolchain-x86_64 \
  -DCMAKE_C_COMPILER_TARGET=x86_64-w64-mingw32 -DCMAKE_C_COMPILER=clang \
  -DCMAKE_AR=/usr/local/opt/llvm/bin/llvm-ar \
  '-DCMAKE_RC_FLAGS=--target=pe-x86-64 -I/usr/local/opt/mingw-w64/toolchain-x86_64/x86_64-w64-mingw32/include' \
  '-DCMAKE_C_FLAGS=-Wno-unused-command-line-argument -Wno-deprecated-non-prototype -fuse-ld=lld -Wl,-s -static-libgcc'
-- The C compiler identification is Clang 15.0.3
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/local/opt/llvm/bin/clang - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Looking for sys/types.h
-- Looking for sys/types.h - found
-- Looking for stdint.h
-- Looking for stdint.h - found
-- Looking for stddef.h
-- Looking for stddef.h - found
-- Check size of off64_t
-- Check size of off64_t - done
-- Looking for fseeko
-- Looking for fseeko - found
-- Looking for unistd.h
-- Looking for unistd.h - found
-- Renaming
--     /sandbox/zlib/zconf.h
-- to 'zconf.h.included' because this file is included with zlib
-- but CMake generates it automatically in the build directory.
-- Configuring done
-- Generating done
-- Build files have been written to: /sandbox/zlib/bld
++ pwd
+ make --directory=bld --jobs=1 install DESTDIR=/sandbox/zlib/x64-msvcrt VERBOSE=1
/usr/local/Cellar/cmake/3.24.2/bin/cmake -S/sandbox/zlib -B/sandbox/zlib/bld --check-build-system CMakeFiles/Makefile.cmake 0
/usr/local/Cellar/cmake/3.24.2/bin/cmake -E cmake_progress_start /sandbox/zlib/bld/CMakeFiles /sandbox/zlib/bld//CMakeFiles/progress.marks
/Library/Developer/CommandLineTools/usr/bin/make  -f CMakeFiles/Makefile2 all
/Library/Developer/CommandLineTools/usr/bin/make  -f CMakeFiles/zlib.dir/build.make CMakeFiles/zlib.dir/depend
[  2%] Generating zlib1rc.obj
/usr/local/opt/llvm/bin/llvm-windres -D GCC_WINDRES -I /sandbox/zlib -I /sandbox/zlib/bld -o /sandbox/zlib/bld/zlib1rc.obj -i /sandbox/zlib/win32/zlib1.rc
/sandbox/zlib/win32/zlib1.rc:1:10: fatal error: 'winver.h' file not found
#include <winver.h>
         ^~~~~~~~~~
1 error generated.
llvm-rc: Preprocessing failed.
make[2]: *** [zlib1rc.obj] Error 1
make[1]: *** [CMakeFiles/zlib.dir/all] Error 2
make: *** [all] Error 2

No luck with our bugfix patch sadly: madler/zlib#677

vszakats · 2022-10-14T11:58:20Z

@madler Any love for this PR?

vszakats · 2022-10-25T19:59:44Z

@nmoinvaz Can you enable CI workflows for this PR please?

nmoinvaz · 2022-10-25T20:30:17Z

Hey @vszakats, sorry I don't have access. I'm just a pleb like everybody else here. 😂

vszakats · 2022-10-25T20:53:19Z

Oh, okay, no worries, I do understand the feeling! :)

[ as a shot in the abyss why this PR is not going anywhere, I was about to reinstate the Windows 16-bit bits, if maybe this is a reason for the silence. For the sake of all those Windows 3.11 users out there. One can only guess. ]

vszakats · 2022-10-25T22:49:51Z

Updated the patch to keep support for these legacy bits:

Windows 16-bit Resource flags.
MinGW releases that do not support those Windows 16-bit Resource flags.

Please let me know if there is any other concern.

vszakats · 2022-10-26T00:51:56Z

Added a build error log to the top post.

vszakats · 2023-01-10T15:09:33Z

Any interest in merging this? Or concerns to address that would allow merging?

This one omits building a shared zlib, and thus doesn't need windres. Ref: madler/zlib#677

vszakats · 2023-07-31T08:59:16Z

If using 1K more of RAM on Windows 16-bit is fine with zlib.dll, the second and third commits can be deleted. Let me know.

vszakats · 2023-08-18T12:38:20Z

Deleted the Windows 16-bit legacy resource flags once again.

Before this patch, `zlib.rc` was compiled using a manual command [1] when using the MinGW toolchain. This method ignores `CMAKE_RC_FLAGS` and offers no other way to pass a custom flag, breaking the build in cases where a custom windres option is required. E.g. `--target=` or `-I` on some platforms and configuration, in particular with llvm-windres. This patch deletes the special case for MinGW and lets CMake compile the `.rc` file via the standard way used for all `WIN32`. Also: - Adjust the condition for dealing with the `.rc` file from `NOT MINGW` to `WIN32`, thus omitting this unnecessary input file for non-Windows platforms. - Delete the MinGW-specific RC flag `GCC_WINDRES`. `GCC_WINDRES` was protecting logic that compiles fine with recent `windres` versions. Also, the protected line contained obsolete, 16-bit era keywords that had no effect for a long time. [2] - Delete the MinGW-specific defaulting logic for `CMAKE_RC_COMPILER`. This is done by CMake automatically, to the more portable default value `windres` (there is no `.exe` extension in cross-toolchains). [1] dc5a43e [2] https://docs.microsoft.com/windows/win32/menurc/common-resource-attributes

In case these are considered important.

Just in case any old MinGW release may not support those Windows 16-bit .rc options.

This reverts commit 174ce5b.

This reverts commit a67dbb3.

Use a less convoluted, shorter workaround instead. It appears that zlib is either uninterested in accepting bugfixes, or thinks that the correct fix might be wrong, or may break compatibility with 20+ year old CMake releases, or possibly breaking some old, undefined system, or may hit a niche bug somebody heard about, and/or just afraid to make any changes, or even let its CI run on them. PR (abandoned): madler/zlib#677

Neustradamus · 2023-08-28T02:53:36Z

@vszakats: Why have you closed this PR?

vszakats · 2023-08-28T07:57:11Z

Seeing this pending for a year+ (through 2 releases, and without zero feedback, not even allowing tests to run) and seeing other fixes (for even worse bugs) with 5+ years of wait, it appears that zlib is uninterested to touch CMake or even give any feedback to know what it wants.

I think this patch is solid and the workarounds for this bug are painful, so if someone is willing to deal with the "process" at zlib, or zlib wants to cherry pick it manually, it would improve things for everyone. Feel free to do it.

vszakats changed the title ~~cmake: fix to respect custom RC flags with mingw~~ cmake: respect custom RC flags with mingw + cleanups Jul 17, 2022

vszakats changed the title ~~cmake: respect custom RC flags with mingw + cleanups~~ cmake: respect custom RC flags with mingw + cleanups Jul 28, 2022

vszakats changed the title ~~cmake: respect custom RC flags with mingw + cleanups~~ cmake: respect custom RC flags with mingw + delete GCC_WINDRES Aug 17, 2022

Neustradamus mentioned this pull request Aug 18, 2022

Please make new release #686

Closed

vszakats added a commit to curl/curl-for-win that referenced this pull request Oct 14, 2022

zlib 1.2.13 [ci skip]

8e6ba39

No luck with our bugfix patch sadly: madler/zlib#677

vszakats changed the title ~~cmake: respect custom RC flags with mingw + delete GCC_WINDRES~~ cmake: respect custom RC flags with mingw Oct 25, 2022

vszakats added a commit to curl/curl-for-win that referenced this pull request Mar 23, 2023

zlib.sh: add alternative workaround for upstream windres issue [ci skip]

99c19d4

This one omits building a shared zlib, and thus doesn't need windres. Ref: madler/zlib#677

Neustradamus mentioned this pull request Jul 30, 2023

CMake and Zlib #831

Open

vszakats added 5 commits August 18, 2023 14:07

restore Windows 16-bit resource flags

da79d56

In case these are considered important.

restore GCC_WINDRES

1ce99e0

Just in case any old MinGW release may not support those Windows 16-bit .rc options.

Revert "restore GCC_WINDRES"

0a45f55

This reverts commit 174ce5b.

Revert "restore Windows 16-bit resource flags"

454869f

This reverts commit a67dbb3.

vszakats closed this Aug 19, 2023

vszakats deleted the mingw-rc branch August 19, 2023 16:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmake: respect custom RC flags with mingw #677

cmake: respect custom RC flags with mingw #677

vszakats commented Jul 11, 2022 •

edited

Loading

vszakats commented Oct 14, 2022

vszakats commented Oct 25, 2022

nmoinvaz commented Oct 25, 2022

vszakats commented Oct 25, 2022

vszakats commented Oct 25, 2022 •

edited

Loading

vszakats commented Oct 26, 2022

vszakats commented Jan 10, 2023

vszakats commented Jul 31, 2023 •

edited

Loading

vszakats commented Aug 18, 2023

Neustradamus commented Aug 28, 2023

vszakats commented Aug 28, 2023 •

edited

Loading

cmake: respect custom RC flags with mingw #677

cmake: respect custom RC flags with mingw #677

Conversation

vszakats commented Jul 11, 2022 • edited Loading

vszakats commented Oct 14, 2022

vszakats commented Oct 25, 2022

nmoinvaz commented Oct 25, 2022

vszakats commented Oct 25, 2022

vszakats commented Oct 25, 2022 • edited Loading

vszakats commented Oct 26, 2022

vszakats commented Jan 10, 2023

vszakats commented Jul 31, 2023 • edited Loading

vszakats commented Aug 18, 2023

Neustradamus commented Aug 28, 2023

vszakats commented Aug 28, 2023 • edited Loading

vszakats commented Jul 11, 2022 •

edited

Loading

vszakats commented Oct 25, 2022 •

edited

Loading

vszakats commented Jul 31, 2023 •

edited

Loading

vszakats commented Aug 28, 2023 •

edited

Loading