Fix phar make paths

[csandanov]

I guess no one noticed this error because the source phar_path_check.c normally never remade but if you get a different timestamp (older) than the target phar_path_check.re (as a result of source archive unpacking), you'll get this error:

re2c: error: cannot open source file: ext/phar/phar_path_check.re
make: *** [Makefile:193: /usr/src/php/ext/phar/phar_path_check.c] Error 1

Tries to look for ext/phar/phar_path_check.re from /usr/src/php/ext/phar/.

https://bugs.php.net/bug.php?id=75587

[weltling]

Were it not more reliable to use the full path here?

Thanks.

[csandanov]

@weltling updated to full paths

[nikic]

The same pattern is used for a bunch of other re2c invocations we use (https://github.com/php/php-src/search?p=1&q=top_srcdir&type=&utf8=%E2%9C%93). The cd $top_srcdir seems like the part that should make this work correctly. Are you building phar out-of-tree?

[weltling]

I was just checking more, too. It seems a relative path is used explicitly to avoid diffs in the generated file. @csandanov perhps you could post a configure command, so one could check what happens behind the scenes? In the makefile, there are several vars declared after the configure, that were to evaluate.

Thanks.

[csandanov]

I use docker-php-ext-install script that runs docker-php-ext-configure from the official php docker image.

[weltling]

@csandanov thanks for the link. It is clear now what happens - here it switches into the extension directory and uses phpize in the configure script. This seems, well, unusual :) The extensions in the core are actually not foreseen for phpize. It might occasionally work this way, though, but only by luck. The issue is, that the extensions in the core are supposed to have top_srcdir as the source root directory of PHP, say [top_srcdir]/ext/phar, and that's how it is expected to be. But phpize will set top_srcdir to the actual extension dir, say my/ext/phar[top_srcdir], and such cases won't work then.

It can also have unexpected effects. As phpize is installed on the system, so the php dev packages are, obviously. Now, extensions from the source checkout can look for some includes, which are possibly going to be from the actual source checkout and not from the installed php dev package. If such config.m4 needs to work under both phpize and regular source compilation, it needs to somehow tell whether phpize is used. I'm not sure yet, how to handle this, as it still won't fix the include issues, at least. Clearly, the Docker script tries to do the quick compilation, but the way it is done is questionable. Need to check more or perhaps someone yet could have better idea.

Thanks.

[tianon]

Apologies for the unorthodox method of building extensions! (I can take at least partial blame for all that)

See docker-library/php#39 (comment) for some of the discussion that led us down that road, and docker-library/php#41 for the initial PR which added said scripts.

We've actually found that the majority of modules compile just fine this way, and there are only a few exceptions (such as odbc, although we found a reasonable workaround for folks who need that: docker-library/php#103 (comment)). Some of the other issues we run into often are that Debian likes to keep header files in odd places, and PHP's configure scripts won't always pick them up properly (the Debian PHP package has patches for exactly these sorts of things).

Regarding whether phpize from -dev packages will be a problem -- that shouldn't ever be a problem unless the user of the Docker image is doing something very weird. The way the images are built, we download PHP's source directly from the official PHP releases, build and install the absolute minimal set of it, and leave the same source around inside the image for folks to compile additional modules after-the-fact from these scripts.

We try to keep the base itself as minimal as possible since unlike a normal package manager, folks can't really pick and choose bits of their base image very easily in order to install extensions afterwards, so when we found that most extensions would compile fine after-the-fact with phpize, we jumped all over it because it seemed like a perfect solution.

Hopefully that's some useful history on why we do this, and makes it a little easier to understand, even if it's no easier to find it to be anything less than absolutely insane. 😇 ❤️

[bukka]

@weltling I think that $srcdir should be fine and always worked for me in json and jsond:

https://github.com/php/php-src/blob/69affa985c653d4884afe7272134bd8cc6c6a13a/ext/json/Makefile.frag

https://github.com/bukka/php-jsond/blob/b411e73a381e805b2c9c488edbee39b6bd5b3bc1/Makefile.frag

So think this PR should be fine to merge unless I missed some phar specific bit...

[weltling]

@bukka yeah, you use -i which means no line information is inserted. If it's inserted, the diffs on #line items will be to see. It depends on how much the debug information is needed. I had another idea, that one could use a check like whether $top_srcdir/php_phar.h exists, to tell whether it's a phpize call or not. Correspondingly it'd use a full path, or a relative path to generate. I had no time yet to check this, not generating debug info were of course an approach, too.

Thanks.

[bukka]

Ah yeah good catch. I don't use debugging symbols for re2c because I find better to debug the actual generated code which is much more useful IMHO!

[csandanov]

Shouldn't we also apply this to 7.0?