Regression related to zlib1.rc in the mingw build. #1038
Comments
|
and MinGW and Cygwin testing has been disabled here. |
|
And reenabled here: 7667ca5 The mentioned commit adds the rc-file on win32 platforms which (I guess) is correct for mingw also on arm, so I think this is correct. I don't know enough about win to judge if it's really needed. My first guess would be, that on arm maybe windres produces others files than .obj, so an additional check would be needed, or cmake itself detects the output wrong. For me this looks like the second, but it doesn't seem like there was an issue created that got fixed. I'm back home on wednesday where I have access to a machine with mingw (no arm). Will this work with -DGCC_WINDRES or is something else needed? If so, I could try to replicate the test from the link. |
|
Before regression: Current changes: |
|
The regression is detected starting with this change It looks like CMAKE cannot handle it, at least for aarch64 and custom command is needed. |
|
You mean like the link I posted that said cmake doesn't detect this correct is right? I have to check if the difference is really needed or if we can just delete the #ifdef in the rc-file (and the whole -DGCC_WINRES stuff). If not I guess just reading the rc_compiler name and reacting on that instead of the misdetected .obj extension should be sufficient. Or maybe turn it around and check for the extension MSVC produces as this seems to work and check for that (or better NOT that) to set the define. This or the other way, everything is easy doable without any custom_command overhead, but not before wednesday. |
|
It looks like the link provided earlier is a good lead. It was a confirmation that before 1af1bb6 aarch64-w64-mingw32 build was fine. It will be great if zlib CI can be extended with aarc64-w64-mingw build. Please let me know if you need help with that. |
|
Earlier fix proposal: if(MINGW)
set_source_files_properties(win32/zlib1.rc PROPERTIES COMPILE_FLAGS "-DGCC_WINDRES")
endif()(edit: Custom .rc files aren't expected on Cygwin, though the rest of the script enables it for this platform, so maybe this line should also be enabled for it.) Though to repeat, the Refs: |
|
Hope a perfect solution for it! |
|
set_source_files_properties(win32/zlib1.rc PROPERTIES Does the same with less commands. #677 suggests to just not make any difference which would make the whole line obsolete. |
|
The line is necessary unless the obsolete |
The zlib Windows build started to fail, probably because of this: madler/zlib#1038 Cloning into 'zlib'... make: *** No rule to make target 'zconf.h', needed by 'adler32.o'. We'll pin the zlib version for now to unblock the Windows build.
The zlib Windows build started to fail, probably because of this: madler/zlib#1038 Cloning into 'zlib'... make: *** No rule to make target 'zconf.h', needed by 'adler32.o'. We'll pin the zlib version for now to unblock the Windows build. Signed-off-by: Lucian Petrut <[email protected]>
The zlib Windows build started to fail, probably because of this: madler/zlib#1038 Cloning into 'zlib'... make: *** No rule to make target 'zconf.h', needed by 'adler32.o'. We'll pin the zlib version for now to unblock the Windows build. Signed-off-by: Lucian Petrut <[email protected]> (cherry picked from commit ba9270d)
The zlib Windows build started to fail, probably because of this: madler/zlib#1038 Cloning into 'zlib'... make: *** No rule to make target 'zconf.h', needed by 'adler32.o'. We'll pin the zlib version for now to unblock the Windows build. Signed-off-by: Lucian Petrut <[email protected]> (cherry picked from commit ba9270d)
|
Then I would suggest you just remove line and remove the cruft that causes it. At least if @madler is fine with it. But I have to admit, it's always fascinating to see how much movement comes after such a change compared to how many people cared in the four weeks this change was in the making. |
Tried that. Got no feedback from @madler for 2.5 years. This is besides the point. This issue (broken mingw-builds) is fixed by the change suggested above, either your one-liner, or the other.
I submitted this patch as a PR early to your fork. You rejected abruptly saying it was a work-in-progress. Now it's too late? |
|
@Vollstrecker: @vszakats PR here: And in this repo here: |
The feedback was in other threads, which was that there was a huge number of cmake pull requests that I had no ability to evaluate or almagamate. I was looking for someone to take a unified approach to a massive update of cmake for zlib. @Vollstrecker stepped up and did that. |
I'm fine with whatever there is agreement on here. I don't know enough to evaluate it myself. |
|
@madler Thank for jumping in, and no worries. If optimizing these resource bytes for Windows 3.x isn't of any explicit importance for you, these legacy flags can be safely deleted, along with all These two patch need to be applied for this: |
aarch64-w64-mingw32-windres -O coff -DNO_FSEEKO -DZLIB_DLL -D_LARGEFILE64_SOURCE=1 -I /home/runner/work/mingw-woarm64-build/mingw-woarm64-build/build/zlib -I /home/runner/work/mingw-woarm64-build/mingw-woarm64-build/code/zlib /home/runner/work/mingw-woarm64-build/mingw-woarm64-build/code/zlib/win32/zlib1.rc CMakeFiles/zlib.dir/win32/zlib1.rc.res
aarch64-w64-mingw32-windres: /home/runner/work/mingw-woarm64-build/mingw-woarm64-build/code/zlib/win32/zlib1.rc:7: syntax error
more information here
https://github.com/Windows-on-ARM-Experiments/mingw-woarm64-build/actions/runs/13085470592/job/36517132074
This change might be the reason of the regression.
1af1bb6
The text was updated successfully, but these errors were encountered: