From 3955b01653f6442b8e099013b23d74fced66422c Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 22 Jan 2025 16:14:50 +0100 Subject: [PATCH] Avoid duplicate build rules On Windows, the cli and phpdbg SAPIs have variants (cli-win32 and phpdbgs, respectively) which are build by default. However, the variants share some files, what leads to duplicate build rules in the generated Makefile. NMake throws warning U4004[1], but proceeds happily, ignoring the second build rule. That means that different flags for duplicate rules are ignored, hinting at a potential problem. We solve this by introducing an additional (optional) argument to `SAPI()` and `ADD_SOURCES()` which can be used to avoid such duplicate build rules. It's left to the SAPI maintainers to make sure that appropriate rules are created. We fix this for phpdbgs right away, which currently couldn't be build without phpdbg due to the missing define; we remove the unused `PHP_PHPDBG_EXPORTS` flag altogether. [1] Closes GH-17545. --- UPGRADING.INTERNALS | 7 +++++ sapi/cli/config.w32 | 6 ++-- sapi/phpdbg/config.w32 | 2 +- win32/build/confutils.js | 66 +++++++++++++++++++++------------------- 4 files changed, 47 insertions(+), 34 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index d7a63e93b3d..b963ead5cc8 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -24,6 +24,13 @@ PHP 8.5 INTERNALS UPGRADE NOTES 2. Build system changes ======================== +- Windows build system changes + . SAPI() and ADD_SOURCES() now suport the optional `duplicate_sources` + parameter. If truthy, no rules to build the object files are generated. + This allows to build additional variants of SAPIs (e.g. a DLL and EXE) + without duplicate build rules. It is up to the SAPI maintainers to ensure + that appropriate build rules are created. + ======================== 3. Module changes ======================== diff --git a/sapi/cli/config.w32 b/sapi/cli/config.w32 index 2a3475e5de6..093bf03a51d 100644 --- a/sapi/cli/config.w32 +++ b/sapi/cli/config.w32 @@ -4,7 +4,8 @@ ARG_ENABLE('cli', 'Build CLI version of PHP', 'yes'); ARG_ENABLE('cli-win32', 'Build console-less CLI version of PHP', 'no'); if (PHP_CLI == "yes") { - SAPI('cli', 'php_cli.c php_http_parser.c php_cli_server.c php_cli_process_title.c ps_title.c', 'php.exe', '/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1'); + SAPI('cli', 'php_cli.c php_http_parser.c php_cli_server.c', 'php.exe', '/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1'); + ADD_SOURCES(configure_module_dirname, 'php_cli_process_title.c ps_title.c', 'cli'); ADD_FLAG("LIBS_CLI", "ws2_32.lib"); ADD_FLAG("LIBS_CLI", "shell32.lib"); ADD_FLAG("LDFLAGS_CLI", "/stack:67108864"); @@ -17,7 +18,8 @@ if (PHP_CLI == "yes") { } if (PHP_CLI_WIN32 == "yes") { - SAPI('cli_win32', 'cli_win32.c php_cli_process_title.c ps_title.c', 'php-win.exe', '/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1'); + SAPI('cli_win32', 'cli_win32.c', 'php-win.exe', '/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1'); + ADD_SOURCES(configure_module_dirname, ' php_cli_process_title.c ps_title.c', 'cli_win32', undefined, PHP_CLI == "yes"); ADD_FLAG("LDFLAGS_CLI_WIN32", "/stack:67108864"); ADD_FLAG("LIBS_CLI_WIN32", "shell32.lib"); } diff --git a/sapi/phpdbg/config.w32 b/sapi/phpdbg/config.w32 index 26b6b4b75e4..6b1177b43bd 100644 --- a/sapi/phpdbg/config.w32 +++ b/sapi/phpdbg/config.w32 @@ -26,7 +26,7 @@ if (PHP_PHPDBG == "yes") { } if (PHP_PHPDBGS == "yes") { - SAPI('phpdbgs', PHPDBG_SOURCES, PHPDBG_DLL, '/D PHP_PHPDBG_EXPORTS'); + SAPI('phpdbgs', PHPDBG_SOURCES, PHPDBG_DLL, PHPDBG_CFLAGS, undefined, PHP_PHPDBG == "yes"); ADD_FLAG("LIBS_PHPDBGS", "ws2_32.lib user32.lib"); ADD_FLAG("CFLAGS_PHPDBGS", "/D YY_NO_UNISTD_H"); diff --git a/win32/build/confutils.js b/win32/build/confutils.js index cbc2f6deda8..fed05f11e5d 100644 --- a/win32/build/confutils.js +++ b/win32/build/confutils.js @@ -1189,7 +1189,7 @@ function is_pgo_desired(mod) return eval("!!" + varname); } -function SAPI(sapiname, file_list, makefiletarget, cflags, obj_dir) +function SAPI(sapiname, file_list, makefiletarget, cflags, obj_dir, duplicate_sources) { var SAPI = sapiname.toUpperCase(); var ldflags; @@ -1213,7 +1213,7 @@ function SAPI(sapiname, file_list, makefiletarget, cflags, obj_dir) ADD_FLAG('CFLAGS_' + SAPI, cflags); } - ADD_SOURCES(configure_module_dirname, file_list, sapiname, obj_dir); + ADD_SOURCES(configure_module_dirname, file_list, sapiname, obj_dir, duplicate_sources); MFO.WriteBlankLines(1); MFO.WriteLine("# SAPI " + sapiname); MFO.WriteBlankLines(1); @@ -1550,7 +1550,7 @@ function EXTENSION(extname, file_list, shared, cflags, dllname, obj_dir) extensions_enabled[extensions_enabled.length] = [extname, shared ? 'shared' : 'static', false]; } -function ADD_SOURCES(dir, file_list, target, obj_dir) +function ADD_SOURCES(dir, file_list, target, obj_dir, duplicate_sources) { var i; var tv; @@ -1654,8 +1654,10 @@ function ADD_SOURCES(dir, file_list, target, obj_dir) srcs_by_dir[build_dir].push(i); } - /* Create makefile build targets and dependencies. */ - MFO.WriteLine(objs_line + ": " + srcs_line); + if (!duplicate_sources) { + /* Create makefile build targets and dependencies. */ + MFO.WriteLine(objs_line + ": " + srcs_line); + } /* Create target subdirs if any and produce the compiler calls, /mp is respected if enabled. */ for (var k in srcs_by_dir) { @@ -1736,39 +1738,41 @@ function ADD_SOURCES(dir, file_list, target, obj_dir) } } - if (PHP_MP_DISABLED) { - for (var j in srcs_by_dir[k]) { - src = file_list[srcs_by_dir[k][j]]; + if (!duplicate_sources) { + if (PHP_MP_DISABLED) { + for (var j in srcs_by_dir[k]) { + src = file_list[srcs_by_dir[k][j]]; - var _tmp = src.split("\\"); - var filename = _tmp.pop(); - obj = filename.replace(re, ".obj"); + var _tmp = src.split("\\"); + var filename = _tmp.pop(); + obj = filename.replace(re, ".obj"); - MFO.WriteLine("\t" + CMD_MOD1 + "$(CC) $(" + flags + ") $(CFLAGS) $(" + bd_flags_name + ") /c " + dir + "\\" + src + " /Fo" + sub_build + d + obj); + MFO.WriteLine("\t" + CMD_MOD1 + "$(CC) $(" + flags + ") $(CFLAGS) $(" + bd_flags_name + ") /c " + dir + "\\" + src + " /Fo" + sub_build + d + obj); + + if ("clang" == PHP_ANALYZER) { + MFO.WriteLine("\t" + CMD_MOD1 + "\"$(CLANG_CL)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + dir + "\\" + src); + } else if ("cppcheck" == PHP_ANALYZER) { + MFO.WriteLine("\t\"" + CMD_MOD1 + "$(CPPCHECK)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + analyzer_base_flags + " " + dir + "\\" + src); + }else if (PHP_ANALYZER == "pvs") { + MFO.WriteLine("\t" + CMD_MOD1 + "\"$(PVS_STUDIO)\" --cl-params $(" + flags + ") $(CFLAGS) $(" + bd_flags_name + ") /c " + dir + "\\" + src + " --source-file " + dir + "\\" + src + + " --cfg PVS-Studio.conf --errors-off \"V122 V117 V111\" "); + } + } + } else { + /* TODO create a response file at least for the source files to work around the cmd line length limit. */ + var src_line = ""; + for (var j in srcs_by_dir[k]) { + src_line += dir + "\\" + file_list[srcs_by_dir[k][j]] + " "; + } + + MFO.WriteLine("\t" + CMD_MOD1 + "$(CC) $(" + flags + ") $(CFLAGS) /Fo" + sub_build + d + " $(" + bd_flags_name + ") /c " + src_line); if ("clang" == PHP_ANALYZER) { - MFO.WriteLine("\t" + CMD_MOD1 + "\"$(CLANG_CL)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + dir + "\\" + src); + MFO.WriteLine("\t\"$(CLANG_CL)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + src_line); } else if ("cppcheck" == PHP_ANALYZER) { - MFO.WriteLine("\t\"" + CMD_MOD1 + "$(CPPCHECK)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + analyzer_base_flags + " " + dir + "\\" + src); - }else if (PHP_ANALYZER == "pvs") { - MFO.WriteLine("\t" + CMD_MOD1 + "\"$(PVS_STUDIO)\" --cl-params $(" + flags + ") $(CFLAGS) $(" + bd_flags_name + ") /c " + dir + "\\" + src + " --source-file " + dir + "\\" + src - + " --cfg PVS-Studio.conf --errors-off \"V122 V117 V111\" "); + MFO.WriteLine("\t\"$(CPPCHECK)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + analyzer_base_flags + " " + src_line); } } - } else { - /* TODO create a response file at least for the source files to work around the cmd line length limit. */ - var src_line = ""; - for (var j in srcs_by_dir[k]) { - src_line += dir + "\\" + file_list[srcs_by_dir[k][j]] + " "; - } - - MFO.WriteLine("\t" + CMD_MOD1 + "$(CC) $(" + flags + ") $(CFLAGS) /Fo" + sub_build + d + " $(" + bd_flags_name + ") /c " + src_line); - - if ("clang" == PHP_ANALYZER) { - MFO.WriteLine("\t\"$(CLANG_CL)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + src_line); - } else if ("cppcheck" == PHP_ANALYZER) { - MFO.WriteLine("\t\"$(CPPCHECK)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + analyzer_base_flags + " " + src_line); - } } }