Skip to content

Fix GH-15534: Build failure for libxml2 2.9.0 - 2.9.3 #15536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 23, 2024
Merged

Fix GH-15534: Build failure for libxml2 2.9.0 - 2.9.3 #15536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6647b8f
Fix GH-15534: Build failure for libxml2 2.9.0 - 2.9.3
petk Aug 22, 2024
56237da
Autotools: Bump minimum libxml2 version to 2.9.4
petk Aug 22, 2024
55ca034
Remove redundant include file
petk Aug 22, 2024
ca8bd23
[skip ci] Simplify UPGRADING note
petk Aug 22, 2024
ffd8ab5
Check libxml2 version on Windows
cmb69 Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions UPGRADING
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,9 @@ PHP 8.4 UPGRADE NOTES
$domain name is empty or too long, and if $variant is not
INTL_IDNA_VARIANT_UTS46.

- LibXML:
. The libxml extension now requires at least libxml2 2.9.4.

- MBString:
. Unicode data tables have been updated to Unicode 15.1.

Expand Down
2 changes: 1 addition & 1 deletion UPGRADING.INTERNALS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ PHP 8.4 INTERNALS UPGRADE NOTES
- M4 macro PHP_EVAL_LIBLINE got a new 3rd argument to override the ext_shared
checks.
- M4 macro PHP_SETUP_LIBXML doesn't define the redundant HAVE_LIBXML symbol
anymore.
anymore and requires at least libxml2 2.9.4.
- M4 macro PHP_SETUP_ICONV doesn't define the HAVE_ICONV symbol anymore.
- M4 macro PHP_OUTPUT is obsolete (use AC_CONFIG_FILES).
- TSRM/tsrm.m4 file and its TSRM_CHECK_PTHREADS M4 macro have been removed.
Expand Down
2 changes: 1 addition & 1 deletion build/php.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1934,7 +1934,7 @@ dnl
dnl Common setup macro for libxml.
dnl
AC_DEFUN([PHP_SETUP_LIBXML], [
PKG_CHECK_MODULES([LIBXML], [libxml-2.0 >= 2.9.0])
PKG_CHECK_MODULES([LIBXML], [libxml-2.0 >= 2.9.4])
PHP_EVAL_INCLINE([$LIBXML_CFLAGS])
PHP_EVAL_LIBLINE([$LIBXML_LIBS], [$1])
$2
Expand Down
18 changes: 12 additions & 6 deletions ext/libxml/config.w32
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,19 @@ if (PHP_LIBXML == "yes") {
CHECK_HEADER_ADD_INCLUDE("libxml/tree.h", "CFLAGS_LIBXML", PHP_PHP_BUILD + "\\include\\libxml2") &&
ADD_EXTENSION_DEP('libxml', 'iconv')) {

EXTENSION("libxml", "libxml.c mime_sniff.c", false /* never shared */, "/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1");
AC_DEFINE("HAVE_LIBXML", 1, "Define to 1 if the PHP extension 'libxml' is available.");
ADD_FLAG("CFLAGS_LIBXML", "/D LIBXML_STATIC /D LIBXML_STATIC_FOR_DLL /D HAVE_WIN32_THREADS ");
if (!PHP_LIBXML_SHARED) {
ADD_DEF_FILE("ext\\libxml\\php_libxml2.def");
if (GREP_HEADER("libxml/xmlversion.h", "#define\\s+LIBXML_VERSION\\s+(\\d+)", PHP_PHP_BUILD + "\\include\\libxml2") &&
Copy link
Contributor

@Jan-E Jan-E Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petk @cmb69 This change assumes that xmlversion.h is in include/libxml2/libxml/xmlversion.h which is most likely not the case. The most likely place for xmlversion.h is include/libxml. With LIBXML_VERSION 20914 I got the warning that I did not have the required libxml version.
Dropping the \\libxml2 in the last argument of GREP_HEADER did the trick. Or dropping the libxml/ in the first argument and putting the headers in include/libxml2.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. The GREP_HEADER function should be adjusted a bit, yes. Because the CHECK_HEADER_ADD_INCLUDE checks the include flags by appending them to the other flags, and this one works by a single file path only. It already checks the flags but only the empty PHP_EXTRA_INCLUDES variable (probably)...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHECK_HEADER_ADD_INCLUDE is smart enough to first check in the main include directory. With the headers in include/libxml it will find the headers. In my case:

Checking for libxml/parser.h ... ../win64build\include
Checking for libxml/tree.h ... ../win64build\include

for the x64 build

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHP_PHP_BUILD + "\\include\\libxml2" in CHECK_HEADER_ADD_INCLUDE has been there for ages. I can see it there in PHP 7.0. In PHP 5.6 it simply was

CHECK_HEADER_ADD_INCLUDE("libxml/parser.h", "CFLAGS_LIBXML")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm testing something like this now:

diff --git a/win32/build/confutils.js b/win32/build/confutils.js
index 95a4e5ce3c..b652a497d1 100644
--- a/win32/build/confutils.js
+++ b/win32/build/confutils.js
@@ -965,6 +965,11 @@ function GREP_HEADER(header_name, regex, path_to_check)
 
        if (!c) {
                /* look in the include path */
+               if (path_to_check == null) {
+                       path_to_check = php_usual_include_suspects;
+               } else {
+                       path_to_check += ";" + php_usual_include_suspects;
+               }
 
                var p = search_paths(header_name, path_to_check, "INCLUDE");
                if (typeof(p) == "string") {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change assumes that xmlversion.h is in include/libxml2/libxml/xmlversion.h which is most likely not the case. The most likely place for xmlversion.h is include/libxml. With LIBXML_VERSION 20914 I got the warning that I did not have the required libxml version.

From where did you get these libraries? I've checked a couple from windows.php.net, and even one from http://xmlsoft.org/sources/win32/64bit/, and there is always include/libxml2/libxml/*.h.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not have my build machine at hand, but I probably just used https://github.com/winlibs/libxml2

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://windows.php.net/downloads/php-sdk/deps/archives/vc11/x64/libxml2-2.9.4-1-vc11-x64.zip
And updates after that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for digging this out. So we need to fix this for best compatibility with other builds. I'll have a look at @petk's PR (thanks for that!) soon (probably the weekend).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for digging this out. So we need to fix this for best compatibility with other builds. I'll have a look at @petk's PR (thanks for that!) soon (probably the weekend).

@cmb69 Could you review the PR so that it can be merged?

+RegExp.$1 >= 20904) {

EXTENSION("libxml", "libxml.c mime_sniff.c", false /* never shared */, "/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1");
AC_DEFINE("HAVE_LIBXML", 1, "Define to 1 if the PHP extension 'libxml' is available.");
ADD_FLAG("CFLAGS_LIBXML", "/D LIBXML_STATIC /D LIBXML_STATIC_FOR_DLL /D HAVE_WIN32_THREADS ");
if (!PHP_LIBXML_SHARED) {
ADD_DEF_FILE("ext\\libxml\\php_libxml2.def");
}
PHP_INSTALL_HEADERS("ext/libxml", "php_libxml.h");
} else {
WARNING("libxml support can't be enabled, libxml version >= 2.9.4 required");
}
PHP_INSTALL_HEADERS("ext/libxml", "php_libxml.h");
} else {
WARNING("libxml support can't be enabled, iconv or libxml are missing")
PHP_LIBXML = "no"
Expand Down
Loading