php_glob.c interferes with build targets

[Lewiscowles1986]

Description

The following code:

main/php_glob.c ported from WIN32 to all build targets temporarily in #18164

causes an error when building php wasm

docker buildx bake --set default.args.PHP_VERSION=8.5.0alpha1

using https://github.com/derickr/php-wasm-builder

Resulted in this output:

32.04 /local/src/php-src/main/php_glob.c:207:1: error: static declaration of 'reallocarray' follows non-static declaration
32.04   207 | reallocarray(void *optr, size_t nmemb, size_t size)
32.04       | ^
32.04 /local/src/emsdk/upstream/emscripten/cache/sysroot/include/stdlib.h:150:7: note: previous declaration is here
32.04   150 | void *reallocarray (void *, size_t, size_t);
32.04       |       ^
32.04 1 error generated.
32.05 make: *** [Makefile:1720: main/php_glob.lo] Error 1
32.05 make: *** Waiting for unfinished jobs....
33.67 emmake: error: 'make -j4' failed (returned 2)

But I expected this output instead:

warning: undefined symbol: getdtablesize (referenced by root reference (e.g. compiled C/C++ code))
warning: undefined symbol: makecontext (referenced by root reference (e.g. compiled C/C++ code))
warning: undefined symbol: php_glob (referenced by root reference (e.g. compiled C/C++ code))
warning: undefined symbol: php_globfree (referenced by root reference (e.g. compiled C/C++ code))
warning: undefined symbol: posix_spawnp (referenced by root reference (e.g. compiled C/C++ code))
warning: undefined symbol: swapcontext (referenced by root reference (e.g. compiled C/C++ code))
emcc: warning: warnings in JS library compilation [-Wjs-compiler]

Working around this by deleting the file leads to a different output:

28.22 make: *** No rule to make target '/local/src/php-src/main/php_glob.c', needed by 'main/php_glob.lo'.  Stop.
28.22 make: *** Waiting for unfinished jobs....
29.26 emmake: error: 'make -j4' failed (returned 2)

Using --enable-system-glob causes more failures

270.7 /local/src/php-src/ext/standard/dir.c:429:7: error: use of undeclared identifier 'PHP_GLOB_BRACE'
270.7   429 |         if ((PHP_GLOB_AVAILABLE_FLAGS & flags) != flags) {
270.7       |              ^~~~~~~~~~~~~~~~~~~~~~~~
270.7 /local/src/php-src/main/php_glob.h:177:39: note: expanded from macro 'PHP_GLOB_AVAILABLE_FLAGS'
270.7   177 | #define PHP_GLOB_AVAILABLE_FLAGS (0 | PHP_GLOB_BRACE | PHP_GLOB_MARK | PHP_GLOB_NOSORT | PHP_GLOB_NOCHECK | PHP_GLOB_NOESCAPE | PHP_GLOB_ERR | PHP_GLOB_ONLYDIR)
270.7       |                                       ^~~~~~~~~~~~~~
270.8 2 errors generated.
270.8 make: *** [Makefile:1450: ext/standard/dir.lo] Error 1

But, deleting the section for realloc (because I failed to deactivate it) works, as does using touch with an empty file. Proving it is indeed not needed outside of win32...

PHP Version

N/A (8.5 alpha targetted for build)

Operating System

debian bookworm

[iluuu1994]

It seems the HAVE_REALLOCARRAY is incorrectly detected by ./configure. Hence, this sounds like a derickr/php-wasm-builder problem. You should probably create an issue there. /cc @derickr

[Lewiscowles1986]

The issues is in having unnecesarry (even the file says temporary) php_glob.c from win32, moved outside of win32; I'm unsure why the file is included at-all by default; and I've read the PR's. PHP_CFLAGS didn't touch it, and neither did explicitly setting CFLAGS to define.

If you'd like I can close, or you can. The fact it exists now means anyone else affected can work around decisions

[ndossche]

I'm unsure why the file is included at-all by default; and I've read the PR's.

As the PR says in the first paragraph, this is done such that one implementation of glob is used, which solves the problem that different platforms have inconsistencies.

[Lewiscowles1986]

suffice to say, as with most of my interactions, I dont want to take up more of your time arguing, but I disagree. Thank you for your time and attention, sorry that both sides could not come to agreement

[iluuu1994]

I'm not sure why you're getting defensive. Niels just explained why this file exists, i.e. to have uniform glob behavior that isn't platform dependant. The comment you're referring to doesn't apply to the whole file, just the reallocarray polyfill. Normally, ./configure should check whether reallocarray natively exists and if so, it will declare the HAVE_REALLOCARRAY macro in main/php_config.h. Clearly that fails with derickr/php-wasm-builder for some reason, which is why I say this is likely an issue elsewhere.

[derickr]

I had a look at this, and it sees that emscripten itself does odd things. Although it defines reallocarray in stdlib.h, when trying to compile it, it doesn't find the symbol.

I've filed a bug report at emscripten-core/emscripten#24771