Skip to content

Create separate lexbor extension #18538

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kocsismate
Copy link
Member

In preparation of #14461 (https://wiki.php.net/rfc/url_parsing_api)

Windows support is pretty much TBD, as I have no way to test it.

@kocsismate kocsismate force-pushed the lexbor-extension branch 2 times, most recently from 9e9cb4c to c228b4a Compare May 13, 2025 04:34
Co-authored-by: Gina Peter Banyard <girgias@php.net>
@TimWolla
Copy link
Member

@cmb69 for Windows maybe?

@cmb69
Copy link
Member

cmb69 commented May 13, 2025

I haven't actually checked that (can do later if necessary), but I think we need to install the actual lexbor headers; cf.

PHP_INSTALL_HEADERS("ext/pcre", "php_pcre.h pcre2lib/");

I think that also needs to be done for POSIX systems, so phpize builds can work.

@nielsdos
Copy link
Member

Definitely.

Also, you need to keep the selectors-adapted directory in ext/dom. Due to technical reasons this is a port of the upstream sources adapted to libxml2, so it has a dependency on libxml.

@kocsismate
Copy link
Member Author

I think we need to install the actual lexbor headers; cf.

I tried to follow what ext/dom did when it used to have the lexbor sources:

https://github.com/php/php-src/blob/master/ext/dom/config.w32#L19

it didn't install the lexbor headers, other than putting them on the include path. Nevertheless, I'm trying out the suggestion.

Also, you need to keep the selectors-adapted directory in ext/dom.

Did I move it accidentally? 🤔 I tried to keep it in place, because I noticed that it was a PHP specific adaptation :)

@@ -26,5 +26,5 @@ if (PHP_LEXBOR == "yes") {

AC_DEFINE("HAVE_LEXBOR", 1, "Define to 1 if the PHP extension 'lexbor' is available.");

PHP_INSTALL_HEADERS("ext/lexbor", "php_lexbor.h");
PHP_INSTALL_HEADERS("ext/lexbor", "php_lexbor.h /");
Copy link
Member

Choose a reason for hiding this comment

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

Windows is still broken, based on the pcre one that cmb linked, perhaps it must be this?

PHP_INSTALL_HEADERS("ext/lexbor", "php_lexbor.h lexbor/");

@nielsdos
Copy link
Member

Did I move it accidentally? 🤔 I tried to keep it in place, because I noticed that it was a PHP specific adaptation :)

Ah sorry I saw it was moved but didn't properly check it is indeed still in dom, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants