Skip to content

Fix GH-15501: Windows HAVE_<header>_H macros defined to 1 or undefined #15508

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
Aug 20, 2024

Conversation

petk
Copy link
Member

@petk petk commented Aug 20, 2024

I should probably fine tune this explanation to make it easier to understand but anyway...

Previously the CHECK_HEADER_ADD_INCLUDE function defined the HAVE_<header>_H preprocessor macros to value 0 or 1 whether the <header.h> file was found. This syncs it with Autotools build system where most of these macros are either undefined or defined to 1.

In possible edge cases where such macros might be intentionally used like this without being aware that HAVE_HEADER_H can be 0 or 1 on Windows:

| #ifdef HAVE_HEADER_H
| ...
| #endif

there is backwards incompatibility for PECL extensions in case the header wouldn't exist on Windows such code wouldn't execute. However, this is considered a bug if such case is present. From the Autotools point of view, the check is correct though and should be used with ifdef/defined() checks.

Help text is also synced to Autotools style:
Define to 1 if you have the <header.h> header file.

…ined

Previously the CHECK_HEADER_ADD_INCLUDE function defined the
`HAVE_<header>_H` preprocessor macros to value 0 or 1 whether the
`<header.h>` file was found. This syncs it with Autotools build system
where most of these macros are either undefined or defined to 1.

In possible edge cases where such macros might be intentionally used
like this without being aware that HAVE_HEADER_H can be 0 or 1 on
Windows:

| #ifdef HAVE_HEADER_H
| ...
| #endif

there is backwards incompatibility for PECL extensions in case the
header wouldn't exist on Windows such code wouldn't execute. However,
this is considered a bug if such case is present. From the Autotools
point of view, the check is correct though and should be used with
ifdef/defined() checks.

Help text is also synced to Autotools style:
`Define to 1 if you have the <header.h> header file.`
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you! In my opinon a good idea to match the autotools behavior as close as possible.

@petk petk merged commit 660a860 into php:master Aug 20, 2024
10 checks passed
@petk petk deleted the patch-windows-CHECK_HEADER_ADD_INCLUDE branch August 20, 2024 19:10
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.

Windows build system defines HAVE_<header>_H as 0 or 1
2 participants