pnggroup / libpng

LIBPNG: Portable Network Graphics support, official libpng repository
http://libpng.sf.net
Other
1.25k stars 611 forks source link

CMakeLists: Fix gawk error with MSVC #547

Closed danyeaw closed 6 months ago

danyeaw commented 6 months ago

For Windows users building with CMake and MSVC, if they have Git Bash on their path, CMake picks up gawk and then fails with the following error:

Building project libpng (1.6.39)
CMake Deprecation Warning at CMakeLists.txt:13 (cmake_minimum_required):
  Compatibility with CMake < 2.8.12 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.

CMake Deprecation Warning at CMakeLists.txt:14 (cmake_policy):
  Compatibility with CMake < 2.8.12 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.

-- Configuring done (0.2s)
-- Generating done (0.1s)
-- Build files have been written to: R:/src/test/gvsbuild/build/build/x64/debug/libpng/_gvsbuild-cmake
[1/59] Generating scripts/symbols.chk
FAILED: scripts/symbols.chk R:/src/test/gvsbuild/build/build/x64/debug/libpng/_gvsbuild-cmake/scripts/symbols.chk
cmd.exe /C "cd /D R:\src\test\gvsbuild\build\build\x64\debug\libpng\_gvsbuild-cmake && R:\src\test\gvsbuild\build\tools\cmake-3.26.4-windows-x86_64\bin\cmake.exe -DINPUT=R:/src/test/gvsbuild/build/build/x64/debug/libpng/_gvsbuild-cmake/scripts/symbols.out -DOUTPUT=R:/src/test/gvsbuild/build/build/x64/debug/libpng/_gvsbuild-cmake/scripts/symbols.chk -P R:/src/test/gvsbuild/build/build/x64/debug/libpng/_gvsbuild-cmake/scripts/genchk.cmake"
gawk: fatal: can't open source file `R:/src/test/gvsbuild/build/build/x64/debug/libpng/_gvsbuild-cmake/scripts/checksym.awk' for reading (No such file or directory)

This PR updates the CMakeLists to not use awk when building with MSVC. This would be a unique setup that someone would want to use awk during a non-MSYS2/Cygwin build.

ctruta commented 6 months ago

Thank you, @danyeaw, but I'm not sure I understand the need for your proposed change.

If awk is present, then the AWK variable is defined. Otherwise, it isn't. That makes the extra test (OR MSVC) superfluous. If there is a reason for the checks against ANDROID and IOS, that's because awk is present on the development platforms (Linux/Mac), and yet, it is not applicable for cross-compiling for the target platforms (Android/iOS).

Moreover, you seem to pick up a wrong version of libpng (1.6.39) which gives a rather baffling deprecation warning about cmake_minimum_required. The minimum required CMake version in was raised from 3.0.2 to 3.1 in libpng-1.6.36. I cannot imagine what is the actual libpng version that you're picking up, but it cannot be anything newer than what we released, like, 5 years ago.

danyeaw commented 6 months ago

Hi @ctruta thanks so much for the feedback.

Most MSVC users won't have awk, so this conditional will be true for them. However, on GitHub Actions, they do have awk on the path, and this will ensure that the overall conditional is also true as well for them.

I also missed the 2nd conditional looking for awk, so I added that to the PR.

The traceback is pretty old, so please ignore the version info and warnings, we are now using the latest. A lot of users have run in to this problem when building libpng with Gvsbuild: https://github.com/wingtk/gvsbuild/issues/984

I patched it downstream here: https://github.com/wingtk/gvsbuild/pull/1245

Here is the latest run showing that we can now compile successfully with this change:

Building project libpng (1.6.42)
(Stripping trailing CRs from patch; use --binary to disable.)
patching file CMakeLists.txt
Hunk #1 succeeded at 314 (offset -34 lines).
Hunk #2 succeeded at 542 (offset -34 lines).
Hunk #3 succeeded at 556 (offset -34 lines).
(Stripping trailing CRs from patch; use --binary to disable.)
patching file CMakeLists.txt
-- The C compiler identification is MSVC 19.37.32826.1
-- The ASM compiler identification is MSVC
-- Found assembler: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.37.32822/bin/HostX64/x64/cl.exe
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.37.32822/bin/HostX64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Found ZLIB: C:/gtk-build/gtk/x64/release/lib/zlib1.lib (found version "1.3.1")  
-- Configuring done (9.5s)
-- Generating done (0.0s)
CMake Warning:
  Manually-specified variables were not used by the project:
    GTK_DIR
-- Build files have been written to: C:/gtk-build/build/x64/release/libpng/_gvsbuild-cmake
[1/19] Building C object CMakeFiles\png_shared.dir\pngerror.c.obj
[2/19] Building C object CMakeFiles\png_shared.dir\png.c.obj
[3/19] Building C object CMakeFiles\png_shared.dir\pngmem.c.obj
[4/19] Building C object CMakeFiles\png_shared.dir\pngget.c.obj
[5/19] Building C object CMakeFiles\png_shared.dir\pngpread.c.obj
[6/19] Building C object CMakeFiles\png_shared.dir\pngread.c.obj
[7/19] Building C object CMakeFiles\png_shared.dir\pngrio.c.obj
[8/19] Building C object CMakeFiles\png_shared.dir\pngrtran.c.obj
[9/19] Building C object CMakeFiles\png_shared.dir\pngrutil.c.obj
[10/19] Building C object CMakeFiles\png_shared.dir\pngset.c.obj
[11/19] Building C object CMakeFiles\png_shared.dir\pngtrans.c.obj
[12/19] Building C object CMakeFiles\png_shared.dir\pngwio.c.obj
[13/19] Building C object CMakeFiles\png_shared.dir\pngwrite.c.obj
[14/19] Building C object CMakeFiles\png_shared.dir\pngwtran.c.obj
[15/19] Building C object CMakeFiles\png_shared.dir\pngwutil.c.obj
[16/19] Building C object CMakeFiles\png_sha-- Install configuration: "RelWithDebInfo"
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/lib/libpng16.lib
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/bin/libpng16.dll
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/include/png.h
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/include/pngconf.h
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/include/pnglibconf.h
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/include/libpng16/png.h
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/include/libpng16/pngconf.h
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/include/libpng16/pnglibconf.h
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/bin/libpng-config
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/bin/libpng16-config
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/share/man/man3/libpng.3
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/share/man/man3/libpngpf.3
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/share/man/man5/png.5
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/lib/pkgconfig/libpng.pc
-- Up-to-date: C:/gtk-build/build/x64/release/libpng-rel/bin/libpng-config
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/lib/pkgconfig/libpng16.pc
-- Up-to-date: C:/gtk-build/build/x64/release/libpng-rel/bin/libpng16-config
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/lib/libpng/libpng16.cmake
-- Installing: C:/gtk-build/build/x64/release/libpng-rel/lib/libpng/libpng16-relwithdebinfo.cmake
red.dir\intel\intel_init.c.obj
[17/19] Building C object CMakeFiles\png_shared.dir\intel\filter_sse2_intrinsics.c.obj
[18/19] Linking C shared library libpng16.dll
[18/19] Install the project...

We were patching around this before by renaming the whole C:\Program Files\Git\bin directory so that libpng couldn't pick up gawk from Git Bash.

jbowler commented 6 months ago

@ctruta; this looks like an error in cmake, possibly caused by the recent move of the cmake support files to the 'scripts' subdirectory. The "missing" file is:

R:/src/test/gvsbuild/build/build/x64/debug/libpng/_gvsbuild-cmake/scripts/checksym.awk

That's in the build directory, but checksym.awk is a source file, from the original log:

-- Build files have been written to: R:/src/test/gvsbuild/build/build/x64/debug/libpng/_gvsbuild-cmake

I don't know why that would happen (not being a cmake expert) but so far as I know it has always worked to use gawk or awk (or the one-true-awk) in Windows build environments.

The OP's second build uses completely different directories:

-- Build files have been written to: C:/gtk-build/build/x64/release/libpng/_gvsbuild-cmake So it may also be an OP error and a spurious fix (i.e. make two changes and don't test them independently to see which actually fixed the problem.)

ctruta commented 6 months ago

@danyeaw thanks for the report. @jbowler thanks for the clarification. @danoli3 thanks for the fix.

The problem is not about MSVC not having awk specifically, but rather, about all kinds of builds where awk isn't being used for some reason (e.g. Android, iOS, MSVC, or Emscripten).

A correct fix is in issue #546. See https://github.com/pnggroup/libpng/pull/546/commits/9353f26c41aec8d3cab04c63722c51c1a03c2d7c

I am closing this as a duplicate of #546. I will apply the fix (albeit with modifications to make sure this kind of error can be traced in the future), and I intend to publish a new libpng release in a day or two.

danyeaw commented 6 months ago

@ctruta many thanks!

I tried to apply https://github.com/pnggroup/libpng/commit/9353f26c41aec8d3cab04c63722c51c1a03c2d7c as a patch, but I am still getting a gawk: fatal: can't open source file C:/gtk-build/build/x64/release/libpng/_gvsbuild-cmake/scripts/cmake/checksym.awk' for reading (No such file or directory) error. Maybe I just need to be patient and wait for the full release, but please let me know if there is something I can help test.

ctruta commented 6 months ago

@danoli3 has a typo in that commit, which is later fixed in f6be6709954dd86ba011f57aed07695cebdf7fc0. (I wish all of his commits were squashed into a single one.)

So, @danyeaw, if you just hand-edit your patched CMake file as follows:

-  execute_process(COMMAND "${AWK}" -f "${BINDIR}/scripts/cmake/checksym.awk"
+  execute_process(COMMAND "${AWK}" -f "${BINDIR}/scripts/checksym.awk"

that should solve your problem (or so I think).

danyeaw commented 6 months ago

Thanks, that worked!

ctruta commented 6 months ago

The fix is in e7ba9c0dfc1446af9f1e118325ba1dd91b8d5851.