pnggroup / libpng

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

[Build][CMake][Windows] Issue to build on Windows when cygwin (awk ) is present in the PATH #571

Open SAP-BI opened 1 month ago

SAP-BI commented 1 month ago

Hello,

The CMake build on our build system is not working on Windows.

To work around this issue, I modified the following line: https://github.com/pnggroup/libpng/blob/e4a31f024b6158aaaf55a43502f574d5f5d1c894/CMakeLists.txt#L355 to :

if(NOT WIN32)
find_program(AWK NAMES gawk awk)
endif()

Our build system has the Cygwin bin folder in the PATH for various reasons, so both awk and gawk are present.

If AWK is found in the PATH, it is used due to the line: GitHub Link to CMakeLists.txt

Best regards, Ghis

jbowler commented 1 month ago

Awk works just fine and is preferred. The scripts (scripts/*.awk) are designed to work with awk, not gawk.

SAP-BI commented 1 month ago

Thanks for looking. Yes awk works fine , the linux build works fine too .

But on windows, the fact that awk are in the PATH (a border effect because cygwin are in the PATH) don't mean we want to use it (especially on Windows ).

AWK is really mandatory to build libpng on windows ?

jbowler commented 1 month ago

The primary use of awk is to handle the configuration, however there is a "pre-built" configuration which corresponds to what is found in the system-installed libpng on most, maybe all, modern operating systems. Most of the time that is the only use of awk but symbol prefixing does require it. IRC the only other use, for building DLL symbol tables, got removed.

Both configure and cmake check that awk works, so if one is found on the %path% it won't get used if it is broken. In any case in both build systems and all the Makefiles (IRC) "awk" is used via a #define of AWK, so:

AWK=path-to-awk ../configure
AWK=path-to-awk make -f scripts/makefile.<system>
cmake -DAWK=path-to-awk ..

Those builds should all use the program "path-to-awk" regardless of spurious stuff in %path%. (I just verified this for cmake and configure). cmake also have a way of forcing the prebuilt header to be used:

cmake -DAWK=0 ..

Of course that removes the ability to configure libpng which you should almost certainly be doing if you are building your own! Using one of the minconfigs in contrib/conftest or contrib/pngminim, or some variation of it, is an extremely good idea since it reduces the size of the library massively. For example on an AMD64 the size of the DLL with "everything" using CFLAGS="-Wall -Wextra -O2 -g -fexceptions" and gcc-14 is:

   text    data     bss     dec     hex filename
 220672    1840       8  222520   36538 libpng16.so

But there is no way you use all the APIs (simplified, progressive, sequential, by-row, whole image...) So to just use the simplified API:

cp contrib/conftest/simple.dfa pngusr.dfa
cd build
CFLAGS="-Wall -Wextra -O2 -g -fexceptions" cmake ..
size libpng16.so
   text    data     bss     dec     hex filename
 115655    1352       8  117015   1c917 libpng16.so

Ignore the error about STDERR if you see it; it's a recently introduced bug in pngtest.c, it doesn't affect the build (and pngtest.c doesn't work with the simplified API!)

Doing this halves the size of the DLL while retaining all the read and write functionality that most programs require! If you use the sequential reader just to read the size reduction is even more spectacular:

cp contrib/pngminim/decoder/pngusr.dfa
rm -rf build
cd build
CFLAGS="-Wall -Wextra -O2 -g -fexceptions" cmake ..
size libpng16.so
   text    data     bss     dec     hex filename
  32752     976       8   33736    83c8 libpng16.so

Yep, that's all you need to do decode a PNG row-by-row in a way that is completely conformant to the specification. On Windows you probably want eliminate the stdio support too.

Bear in mind that searching for awk on the path is a convenience for people who don't have a standard setup, if the path is set up in a way such that the wrong programs are found that's something that needs to be fixed.

jbowler commented 3 days ago

@SAP-BI please close this.