I'm slightly confused why this is necessary, as the LIBZIP_CFLAGS are already passed to PHP_NEW_EXTENSION.

Unfortunately I kind of lack knowledge about the internals of the build system. Fact is, that if a custom libzip path is defined all "prechecks" during "configure" run fine, but during the build phase the "-Ipath/to/libzip/include" CFLAG is missing, which results in zip.h not being found. I added the PHP_EVAL_INCLINE which i have "stolen" from the gd extension. Can i provide any details?

I'm struggling to reproduce this, even using a custom path. How are you calling configure?

Here is a minimal Dockerfile that will reproduce the issue. You need to make sure that the system does not have any zip.h already installed.

It will fail with:

In file included from main/internal_functions_cli.c:33:0:
/home/php-7.3.5/ext/zip/php_zip.h:31:10: fatal error: zip.h: No such file or directory
#include <zip.h>
^~~~~~~
compilation terminated.

Which is obvious when having a look at the CC line. The include path is missing.

/bin/bash /home/php-7.3.5/libtool --silent --preserve-dup-deps --mode=compile cc -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 -Imain/ -I/home/php-7.3.5/main/ -DPHP_ATOM_INC -I/home/php-7.3.5/include -I/home/php-7.3.5/main -I/home/php-7.3.5 -I/home/php-7.3.5/ext/date/lib -I/home/php-7.3.5/TSRM -I/home/php-7.3.5/Zend -g -O2 -fvisibility=hidden -DZEND_SIGNALS -c main/internal_functions_cli.c -o main/internal_functions_cli.lo

FROM ubuntu:18.04

WORKDIR /home

RUN \
    apt-get update &&\
    apt-get -y install \
        build-essential \
        zlib1g-dev \
        cmake \
        curl

RUN curl -O -# https://libzip.org/download/libzip-1.5.2.tar.gz && \
    tar xf libzip-1.5.2.tar.gz && \
    mkdir libzip && \
    cd libzip-1.5.2 && \
    cmake . -DCMAKE_INSTALL_PREFIX:PATH=../libzip -DCMAKE_VERBOSE_MAKEFILE=ON -DBUILD_SHARED_LIBS=OFF -DENABLE_BZIP2=OFF && \
    cmake --build . -- -j$(sysctl -n hw.ncpu) && \
    make install

RUN curl -O -# https://www.php.net/distributions/php-7.3.5.tar.gz && \
    tar xf php-7.3.5.tar.gz && \
    cd php-7.3.5 && \
    ./configure --disable-all --enable-zip --with-libzip=/home/libzip && \
    make

Thanks, @paresy. This makes it clear, since I didn't realise you were talking about 7.3.5.

Your patch is indeed correct.

@hughmcmaster What's the relevant difference between 7.3 and 7.4 here? While 7.4 exclusively uses pkg-config, the way they work with LIBZIP_CFLAGS looks the same to me.

Here is a Dockerfile with the same options against master. Maybe i am mistaken, but it seems like zip support is totally broken. It is not build at all.

FROM ubuntu:18.04

WORKDIR /home

RUN \
    apt-get update &&\
    apt-get -y install \
        build-essential \
        autoconf \
        automake \
        libtool \
        bison \
        re2c \
        zlib1g-dev \
        cmake \
        curl \
        git

RUN curl -O -# https://libzip.org/download/libzip-1.5.2.tar.gz && \
    tar xf libzip-1.5.2.tar.gz && \
    mkdir libzip && \
    cd libzip-1.5.2 && \
    cmake . -DCMAKE_INSTALL_PREFIX:PATH=../libzip -DCMAKE_VERBOSE_MAKEFILE=ON -DBUILD_SHARED_LIBS=OFF -DENABLE_BZIP2=OFF && \
    cmake --build . -- -j$(sysctl -n hw.ncpu) && \
    make install

RUN git clone --depth 1 git://github.com/php/php-src && \
    cd php-src && \
    ./buildconf && \
    ./configure --disable-all --enable-zip --with-libzip=/home/libzip && \
    make

@nikic, PHP_NEW_EXTENSION correctly adds LIBZIP_CFLAGS when compiling php_zip.c and zip_stream.c, so compiling these sources works correctly.

Unfortunately, the LIBZIP_CFLAGS are not present when compiling main/internal_functions_cli.c with the zip extension enabled. This causes the C preprocessor to fail when attempting to #include "zip.h". Basically, $INCLUDES needs to have $LIBZIP_CFLAGS, since that information isn't known outside of ext/zip.

Note that this situation only occurs when the headers are not installed in a standard system search path.

The 7.3.x, 7.4 and master branches all have this problem. All other extensions converted to use PKG_CHECK_MODULES call PHP_EVAL_INCLINE and PHP_EVAL_LIBLINE sequentially. For some reason, it is missing in the zip extension.

PR #4189 includes a patch to fix this problem in 7.4 and master.

@hughmcmaster Ah, thank you, that makes a lot of sense! Ideally we would not even include zip.h inside php_zip.h and only include it in implementation files. But to avoid larger changes, I guess that this fix is fine (though we should drop the CFLAGS from the NEW_EXTENSION call, as those are redundant now).

Unfortunately, the LIBZIP_CFLAGS are not present when compiling main/internal_functions_cli.c with the zip extension enabled

Still don't understand... don't see any include of php_zip.h in main/internal_functions_cli.c or anywhere else

@remicollet internal_functions.c includes the headers for all statically linked extensions. If you don't see php_zip.h in there, you probably have a shared build.

@nikic ty (my latest build was with --disable-all.... so...)

Both 4189 / 4190 target phpmaster, which is obviously wrong

Both need to be rebased

Merged as a0c9d08 into 7.2+. Thanks!