Skip to content

ext/uri: Fix static build on Windows by defining LEXBOR_STATIC#22165

Open
luthermonson wants to merge 1 commit into
php:PHP-8.5from
luthermonson:fix/uri-lexbor-static-windows
Open

ext/uri: Fix static build on Windows by defining LEXBOR_STATIC#22165
luthermonson wants to merge 1 commit into
php:PHP-8.5from
luthermonson:fix/uri-lexbor-static-windows

Conversation

@luthermonson
Copy link
Copy Markdown

@luthermonson luthermonson commented May 28, 2026 •
edited
Loading

Summary

ext/uri is always built statically and pulls in the bundled lexbor headers via /I ext/lexbor. On Windows, lexbor's LXB_API macro defaults to __declspec(dllimport) when LEXBOR_STATIC isn't defined. As a result, when PHP itself is linked statically (no DLL) on Windows, every lxb_* symbol referenced from ext/uri fails to resolve:

libphp.lib(php_uri.obj) : error LNK2019: unresolved external symbol
    __imp_lxb_url_parse referenced in function ...
... (one LNK2019 per lxb_* function reachable from ext/uri) ...

@luthermonson luthermonson mentioned this pull request May 28, 2026
Closed
@luthermonson luthermonson changed the title Fix static ext/uri build on Windows by defining LEXBOR_STATIC ext/uri: Fix static build on Windows by defining LEXBOR_STATIC May 28, 2026
@TimWolla
Copy link
Copy Markdown
Member

TimWolla commented May 28, 2026

I know nothing about Windows. Should / Must this be fixed in ext/lexbor instead?

@luthermonson luthermonson force-pushed the fix/uri-lexbor-static-windows branch from dad77e8 to da128c4 Compare May 28, 2026 13:48
@luthermonson
Copy link
Copy Markdown
Author

luthermonson commented May 28, 2026

I think your instinct is correct, i"m running a test right now with a change to lexbor and will report back

@luthermonson
Copy link
Copy Markdown
Author

luthermonson commented May 28, 2026

@TimWolla thanks for the insight , the change now in lexbor had the same results as doing the define in config.w32 and i was able to get clean static builds on all platforms.

TimWolla
TimWolla reviewed May 28, 2026
View reviewed changes
Comment thread ext/lexbor/lexbor/core/def.h
@luthermonson
Define LEXBOR_STATIC in ext/lexbor/config.w32 for static linkage
5521f86 
PHP bundles lexbor as a static-only extension (ext/lexbor/config.w32
declares EXTENSION("lexbor", ..., false /* never shared */)). On
Windows, ext/lexbor/lexbor/core/def.h defaults LXB_API to
__declspec(dllimport) when LEXBOR_STATIC is not defined, which causes
LNK2019 unresolved external symbol errors for every lxb_* function
referenced from consumers (ext/uri today, potentially others later)
when PHP itself is linked statically without a runtime DLL.

Add AC_DEFINE("LEXBOR_STATIC", 1, ...) in ext/lexbor/config.w32 so
the symbol is placed in main/config.w.h and seen by every PHP source
file that includes php.h before lexbor headers. This covers ext/uri
and any future consumer with one definition, in the right place for
configuration intent.

This change lives entirely in PHP-owned configuration (no modification
to the vendored ext/lexbor/lexbor/ tree), so it survives lexbor library
upgrades without re-application.
@luthermonson luthermonson force-pushed the fix/uri-lexbor-static-windows branch from da128c4 to 5521f86 Compare May 28, 2026 16:35
@TimWolla TimWolla requested a review from shivammathur May 28, 2026 17:38
shivammathur
shivammathur requested changes May 28, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@shivammathur shivammathur left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please limit LEXBOR_STATIC to CFLAGS_URI instead of defining it globally.

@TimWolla
Copy link
Copy Markdown
Member

TimWolla commented May 28, 2026

Please limit LEXBOR_STATIC to CFLAGS_URI instead of defining it globally.

ext/lexbor is always statically built into PHP and is intended to be reusable across different extensions (e.g. ext/uri, ext/dom). Should the define actually be limited to CFLAGS_URI then?

@shivammathur
Copy link
Copy Markdown
Member

shivammathur commented May 28, 2026

@TimWolla Yes, that is correct, my concern is in case dom is built as a shared extension (See GH-21911), ext/dom/config.w32 does not set LEXBOR_STATIC and this would set it.

@luthermonson
Copy link
Copy Markdown
Author

luthermonson commented May 28, 2026 •
edited
Loading

so go back to the ext/dom/config.w32 fix with adding /D LEXBOR_STATIC? arent we still worred about that being overwritten?

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

Reviewers

@shivammathur shivammathur shivammathur requested changes

@TimWolla TimWolla TimWolla approved these changes

@kocsismate kocsismate Awaiting requested review from kocsismate kocsismate is a code owner

Assignees

No one assigned

Labels

Category: Build System Extension: uri

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@luthermonson @TimWolla @shivammathur