Skip to content

Guard config.w32.h from multiple inclusion #17021

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

Merged
merged 1 commit into from
Dec 8, 2024

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Dec 2, 2024

Besides that is generally good practice to avoid macro redefinitions (and symbol redeclarations), and we're doing this on POSIX platforms anyway, there is a particular issue regarding phpize builds, where config.w32.h actually includes config.pickle.h. The latter overrides some macro definitions (e.g. PHP_BUILD_SYSTEM) to define the proper values, but if config.w32.h is included multiple times, different macro definitions eventually raise C4005 compiler warnings[1], which break builds with /WX /W1 enabled.

[1] https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4005

Besides that is generally good practice to avoid macro redefinitions
(and symbol redeclarations), and we're doing this on POSIX platforms
anyway, there is a particular issue regarding phpize builds, where
config.w32.h actually includes config.pickle.h.  The latter overrides
some macro definitions (e.g. `PHP_BUILD_SYSTEM`) to define the proper
values, but if config.w32.h is included multiple times, different macro
definitions eventually raise C4005 compiler warnings[1], which break
builds with `/WX /W1` enabled.

[1] <https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4005>
@cmb69 cmb69 merged commit c0385e9 into php:master Dec 8, 2024
10 checks passed
@cmb69 cmb69 deleted the cmb/config.w32.h-guard branch December 8, 2024 22:42
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.

1 participant