-
Notifications
You must be signed in to change notification settings - Fork 8k
Fix GREP_HEADER on Windows #15619
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.
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
Fix GREP_HEADER on Windows #15619
Conversation
This fixes the libxml version check when the libxml/xmlversion.h is located elsewhere than libxml2 include directory.
I originally applied only this: #15536 (comment)
Without the changes in both file_get_contents() statements I got the following output for the x86 build:
Checking for libxml/parser.h ... ../win32build\include
Checking for libxml/tree.h ... ../win32build\include
Checking for libxml/xmlversion.h ... ../win32build\include
Problem reading ../win32build\include
But with the complete patch it works as expected in my build environment:
Checking for library libxml2_a_dll.lib;libxml2_a.lib ... ..\win32build\lib\libxml2_a_dll.lib
Checking for library libiconv_a.lib;iconv_a.lib;libiconv.lib;iconv.lib ... ..\win32build\lib\libiconv_a.lib
Checking for libxml/parser.h ... ../win32build\include
Checking for libxml/tree.h ... ../win32build\include
Checking for libxml/xmlversion.h ... ../win32build\include
Enabling extension ext\libxml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, no time to thoroughly check this. Feel free to merge, or not.
Sorry, no time to thoroughly check this. Feel free to merge, or not.
@petk What is the status? Can this be merged?
Due to #16199, I had a closer look, and I think the patch is correct. I'm going to apply to PHP-8.4.
Note, though, that the php_usual_*_suspects are broken anyway for builds without --with-php-build=..., because
php-src/win32/build/confutils.js
Line 3557 in 2d8a93c
is always true (at least for regular builds; haven't checked phpize builds), due to php_build_option_handle() wich is called earlier. This is all a terrible mess.
Anyhow, thanks for the PR!
I still need to retest this a bit if it covers all cases correctly. This check never actually worked properly as the filename needs to be appended to the file_get_contents path.
This fixes the libxml version check when the libxml/xmlversion.h is located elsewhere than libxml2 include directory.
Fixes #15536 (comment)