Repository navigation
Windows build system defines HAVE_<header>_H as 0 or 1 #15501
Copy link
Copy link
Closed
Description
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 0However most of these headers are used in the code like this:
#ifdef HAVE_FOOBAR_H
...
#endifAffected 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");
}
Activity
cmb69 commentedon Aug 20, 2024
Going from
#define FOO 0to#undef FOOshould 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 commentedon Aug 20, 2024
Perhaps it can be done in PHP 8.4 then. Ok, appended the PR for this.
Fix phpGH-15501: Windows HAVE_<header>_H macros defined to 1 or undef…