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

[petk]

Description

When using CHECK_HEADER_ADD_INCLUDE in *.w32 Windows build system files there is an inconsistency with Autotools build system where most header check macros are of style undefined/defined to 1.

For example, this:

CHECK_HEADER_ADD_INCLUDE("foobar.h", "CFLAGS", "..\\path;" + php_usual_include_suspects);

Will define the HAVE_FOOBAR_H to a value 0 in the main/config.w32.h:

#define HAVE_FOOBAR_H 0

However most of these headers are used in the code like this:

#ifdef HAVE_FOOBAR_H
...
#endif

Affected headers in php-src:
zlib.h
sql.h
sqlext.h
timelib_config.h
tidy.h
tidybuffio.h

Since php-src Windows environment is very locked to exact libraries and the build can be mostly reproduced this probably isn't much of an issue but it might affect PHP extensions out there.

PHP Version

PHP 8.2+

Operating System

Windows

Patch that fixes this in a "simple" way but it is a BC break for community extensions:

diff --git a/ext/com_dotnet/com_dotnet.c b/ext/com_dotnet/com_dotnet.c
index f52554fb3d..3ce2daddaa 100644
--- a/ext/com_dotnet/com_dotnet.c
+++ b/ext/com_dotnet/com_dotnet.c
@@ -20,7 +20,7 @@
 
 #include "php.h"
 
-#if HAVE_MSCOREE_H
+#ifdef HAVE_MSCOREE_H
 # include "php_ini.h"
 # include "ext/standard/info.h"
 # include "php_com_dotnet.h"
diff --git a/ext/com_dotnet/com_extension.c b/ext/com_dotnet/com_extension.c
index e6e14a45a9..1f18d2a857 100644
--- a/ext/com_dotnet/com_extension.c
+++ b/ext/com_dotnet/com_extension.c
@@ -195,7 +195,7 @@ PHP_MINIT_FUNCTION(com_dotnet)
 	tmp->create_object = php_com_object_new;
 	tmp->get_iterator = php_com_iter_get;
 
-#if HAVE_MSCOREE_H
+#ifdef HAVE_MSCOREE_H
 	tmp = register_class_dotnet(php_com_variant_class_entry);
 	tmp->default_object_handlers = &php_com_object_handlers;
 	tmp->create_object = php_com_object_new;
@@ -216,7 +216,7 @@ PHP_MINIT_FUNCTION(com_dotnet)
 PHP_MSHUTDOWN_FUNCTION(com_dotnet)
 {
 	UNREGISTER_INI_ENTRIES();
-#if HAVE_MSCOREE_H
+#ifdef HAVE_MSCOREE_H
 	if (COMG(dotnet_runtime_stuff)) {
 		php_com_dotnet_mshutdown();
 	}
@@ -239,7 +239,7 @@ PHP_RINIT_FUNCTION(com_dotnet)
 /* {{{ PHP_RSHUTDOWN_FUNCTION */
 PHP_RSHUTDOWN_FUNCTION(com_dotnet)
 {
-#if HAVE_MSCOREE_H
+#ifdef HAVE_MSCOREE_H
 	if (COMG(dotnet_runtime_stuff)) {
 		php_com_dotnet_rshutdown();
 	}
@@ -257,7 +257,7 @@ PHP_MINFO_FUNCTION(com_dotnet)
 	php_info_print_table_row(2, "COM support", "enabled");
 	php_info_print_table_row(2, "DCOM support", COMG(allow_dcom) ? "enabled" : "disabled");
 
-#if HAVE_MSCOREE_H
+#ifdef HAVE_MSCOREE_H
 	php_info_print_table_row(2, ".Net support", "enabled");
 #else
 	php_info_print_table_row(2, ".Net support", "not present in this build");
diff --git a/ext/com_dotnet/com_extension.stub.php b/ext/com_dotnet/com_extension.stub.php
index 3207871f85..0bbe976279 100644
--- a/ext/com_dotnet/com_extension.stub.php
+++ b/ext/com_dotnet/com_extension.stub.php
@@ -363,7 +363,7 @@ class com extends variant
     public function __construct(string $module_name, array|string|null $server_name = null, int $codepage = CP_ACP, string $typelib = "") {}
 }
 
-#if HAVE_MSCOREE_H
+#ifdef HAVE_MSCOREE_H
 class dotnet extends variant
 {
     public function __construct(string $assembly_name, string $datatype_name, int $codepage = CP_ACP) {}
diff --git a/ext/com_dotnet/com_extension_arginfo.h b/ext/com_dotnet/com_extension_arginfo.h
index c75fa6df9c..91a3cb650f 100644
Binary files a/ext/com_dotnet/com_extension_arginfo.h and b/ext/com_dotnet/com_extension_arginfo.h differ
diff --git a/win32/build/confutils.js b/win32/build/confutils.js
index 78d4291f2f..522e26fb81 100644
--- a/win32/build/confutils.js
+++ b/win32/build/confutils.js
@@ -1040,9 +1040,9 @@ function CHECK_HEADER_ADD_INCLUDE(header_name, flag_name, path_to_check, use_env
 		add_to_flag_only = true;
 	}
 
-	if (typeof(add_to_flag_only) != "undefined") {
+	if (typeof(add_to_flag_only) != "undefined" && have) {
 		ADD_FLAG(flag_name, "/DHAVE_" + sym + "=" + have);
-	} else if (!configure_hdr.Exists('HAVE_' + sym)) {
+	} else if (!configure_hdr.Exists('HAVE_' + sym) && have) {
 		AC_DEFINE("HAVE_" + sym, have, "have the " + header_name + " header file");
 	}

[cmb69]

[…], but it might affect PHP extensions out there.

Going from #define FOO 0 to #undef FOO should not affect existing code using #if FOO, since MSVC and clang-cl will treat it the same (and that seems to be standard behavior). So the only problem would be if an extension already uses #ifdef FOO (or such), because that behavior would change. If autotools already use the undefined/defined to 1 style, the behavior on Windows would now match other platforms. But of course, there are some Windows only extensions, and there may be subtle issues, where the current behavior on Windows is intended to be different to other platforms (although that would likely hint at a bug).

Anyhow, I think matching autotools behavior more closely is overall a good thing, but we should not do this for stable branches, but only for master. If we document that change in UPGRADING.INTERNALS, it shouldn't be too hard for external extensions to catch up (if there is even the need to).

Do you want to submit your patch as PR?

[petk]

Perhaps it can be done in PHP 8.4 then. Ok, appended the PR for this.