Skip to content

Commit 3955b01

Browse files
cmb69cmb69
committed
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] <https://learn.microsoft.com/en-us/cpp/error-messages/tool-errors/nmake-warning-u4004> Closes GH-17545.
1 parent 8deca28 commit 3955b01

File tree

4 files changed

+47
-34
lines changed

4 files changed

+47
-34
lines changed

‎UPGRADING.INTERNALS

+7
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ PHP 8.5 INTERNALS UPGRADE NOTES
2424
2. Build system changes
2525
========================
2626

27+
- Windows build system changes
28+
. SAPI() and ADD_SOURCES() now suport the optional `duplicate_sources`
29+
parameter. If truthy, no rules to build the object files are generated.
30+
This allows to build additional variants of SAPIs (e.g. a DLL and EXE)
31+
without duplicate build rules. It is up to the SAPI maintainers to ensure
32+
that appropriate build rules are created.
33+
2734
========================
2835
3. Module changes
2936
========================

‎sapi/cli/config.w32

+4-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ ARG_ENABLE('cli', 'Build CLI version of PHP', 'yes');
44
ARG_ENABLE('cli-win32', 'Build console-less CLI version of PHP', 'no');
55

66
if (PHP_CLI == "yes") {
7-
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');
7+
SAPI('cli', 'php_cli.c php_http_parser.c php_cli_server.c', 'php.exe', '/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1');
8+
ADD_SOURCES(configure_module_dirname, 'php_cli_process_title.c ps_title.c', 'cli');
89
ADD_FLAG("LIBS_CLI", "ws2_32.lib");
910
ADD_FLAG("LIBS_CLI", "shell32.lib");
1011
ADD_FLAG("LDFLAGS_CLI", "/stack:67108864");
@@ -17,7 +18,8 @@ if (PHP_CLI == "yes") {
1718
}
1819

1920
if (PHP_CLI_WIN32 == "yes") {
20-
SAPI('cli_win32', 'cli_win32.c php_cli_process_title.c ps_title.c', 'php-win.exe', '/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1');
21+
SAPI('cli_win32', 'cli_win32.c', 'php-win.exe', '/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1');
22+
ADD_SOURCES(configure_module_dirname, ' php_cli_process_title.c ps_title.c', 'cli_win32', undefined, PHP_CLI == "yes");
2123
ADD_FLAG("LDFLAGS_CLI_WIN32", "/stack:67108864");
2224
ADD_FLAG("LIBS_CLI_WIN32", "shell32.lib");
2325
}

‎sapi/phpdbg/config.w32

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ if (PHP_PHPDBG == "yes") {
2626
}
2727

2828
if (PHP_PHPDBGS == "yes") {
29-
SAPI('phpdbgs', PHPDBG_SOURCES, PHPDBG_DLL, '/D PHP_PHPDBG_EXPORTS');
29+
SAPI('phpdbgs', PHPDBG_SOURCES, PHPDBG_DLL, PHPDBG_CFLAGS, undefined, PHP_PHPDBG == "yes");
3030
ADD_FLAG("LIBS_PHPDBGS", "ws2_32.lib user32.lib");
3131
ADD_FLAG("CFLAGS_PHPDBGS", "/D YY_NO_UNISTD_H");
3232

‎win32/build/confutils.js

+35-31
Original file line numberDiff line numberDiff line change
@@ -1189,7 +1189,7 @@ function is_pgo_desired(mod)
11891189
return eval("!!" + varname);
11901190
}
11911191

1192-
function SAPI(sapiname, file_list, makefiletarget, cflags, obj_dir)
1192+
function SAPI(sapiname, file_list, makefiletarget, cflags, obj_dir, duplicate_sources)
11931193
{
11941194
var SAPI = sapiname.toUpperCase();
11951195
var ldflags;
@@ -1213,7 +1213,7 @@ function SAPI(sapiname, file_list, makefiletarget, cflags, obj_dir)
12131213
ADD_FLAG('CFLAGS_' + SAPI, cflags);
12141214
}
12151215

1216-
ADD_SOURCES(configure_module_dirname, file_list, sapiname, obj_dir);
1216+
ADD_SOURCES(configure_module_dirname, file_list, sapiname, obj_dir, duplicate_sources);
12171217
MFO.WriteBlankLines(1);
12181218
MFO.WriteLine("# SAPI " + sapiname);
12191219
MFO.WriteBlankLines(1);
@@ -1550,7 +1550,7 @@ function EXTENSION(extname, file_list, shared, cflags, dllname, obj_dir)
15501550
extensions_enabled[extensions_enabled.length] = [extname, shared ? 'shared' : 'static', false];
15511551
}
15521552

1553-
function ADD_SOURCES(dir, file_list, target, obj_dir)
1553+
function ADD_SOURCES(dir, file_list, target, obj_dir, duplicate_sources)
15541554
{
15551555
var i;
15561556
var tv;
@@ -1654,8 +1654,10 @@ function ADD_SOURCES(dir, file_list, target, obj_dir)
16541654
srcs_by_dir[build_dir].push(i);
16551655
}
16561656

1657-
/* Create makefile build targets and dependencies. */
1658-
MFO.WriteLine(objs_line + ": " + srcs_line);
1657+
if (!duplicate_sources) {
1658+
/* Create makefile build targets and dependencies. */
1659+
MFO.WriteLine(objs_line + ": " + srcs_line);
1660+
}
16591661

16601662
/* Create target subdirs if any and produce the compiler calls, /mp is respected if enabled. */
16611663
for (var k in srcs_by_dir) {
@@ -1736,38 +1738,40 @@ function ADD_SOURCES(dir, file_list, target, obj_dir)
17361738
}
17371739
}
17381740

1739-
if (PHP_MP_DISABLED) {
1740-
for (var j in srcs_by_dir[k]) {
1741-
src = file_list[srcs_by_dir[k][j]];
1741+
if (!duplicate_sources) {
1742+
if (PHP_MP_DISABLED) {
1743+
for (var j in srcs_by_dir[k]) {
1744+
src = file_list[srcs_by_dir[k][j]];
17421745

1743-
var _tmp = src.split("\\");
1744-
var filename = _tmp.pop();
1745-
obj = filename.replace(re, ".obj");
1746+
var _tmp = src.split("\\");
1747+
var filename = _tmp.pop();
1748+
obj = filename.replace(re, ".obj");
17461749

1747-
MFO.WriteLine("\t" + CMD_MOD1 + "$(CC) $(" + flags + ") $(CFLAGS) $(" + bd_flags_name + ") /c " + dir + "\\" + src + " /Fo" + sub_build + d + obj);
1750+
MFO.WriteLine("\t" + CMD_MOD1 + "$(CC) $(" + flags + ") $(CFLAGS) $(" + bd_flags_name + ") /c " + dir + "\\" + src + " /Fo" + sub_build + d + obj);
17481751

1749-
if ("clang" == PHP_ANALYZER) {
1750-
MFO.WriteLine("\t" + CMD_MOD1 + "\"$(CLANG_CL)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + dir + "\\" + src);
1751-
} else if ("cppcheck" == PHP_ANALYZER) {
1752-
MFO.WriteLine("\t\"" + CMD_MOD1 + "$(CPPCHECK)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + analyzer_base_flags + " " + dir + "\\" + src);
1753-
}else if (PHP_ANALYZER == "pvs") {
1754-
MFO.WriteLine("\t" + CMD_MOD1 + "\"$(PVS_STUDIO)\" --cl-params $(" + flags + ") $(CFLAGS) $(" + bd_flags_name + ") /c " + dir + "\\" + src + " --source-file " + dir + "\\" + src
1755-
+ " --cfg PVS-Studio.conf --errors-off \"V122 V117 V111\" ");
1752+
if ("clang" == PHP_ANALYZER) {
1753+
MFO.WriteLine("\t" + CMD_MOD1 + "\"$(CLANG_CL)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + dir + "\\" + src);
1754+
} else if ("cppcheck" == PHP_ANALYZER) {
1755+
MFO.WriteLine("\t\"" + CMD_MOD1 + "$(CPPCHECK)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + analyzer_base_flags + " " + dir + "\\" + src);
1756+
}else if (PHP_ANALYZER == "pvs") {
1757+
MFO.WriteLine("\t" + CMD_MOD1 + "\"$(PVS_STUDIO)\" --cl-params $(" + flags + ") $(CFLAGS) $(" + bd_flags_name + ") /c " + dir + "\\" + src + " --source-file " + dir + "\\" + src
1758+
+ " --cfg PVS-Studio.conf --errors-off \"V122 V117 V111\" ");
1759+
}
1760+
}
1761+
} else {
1762+
/* TODO create a response file at least for the source files to work around the cmd line length limit. */
1763+
var src_line = "";
1764+
for (var j in srcs_by_dir[k]) {
1765+
src_line += dir + "\\" + file_list[srcs_by_dir[k][j]] + " ";
17561766
}
1757-
}
1758-
} else {
1759-
/* TODO create a response file at least for the source files to work around the cmd line length limit. */
1760-
var src_line = "";
1761-
for (var j in srcs_by_dir[k]) {
1762-
src_line += dir + "\\" + file_list[srcs_by_dir[k][j]] + " ";
1763-
}
17641767

1765-
MFO.WriteLine("\t" + CMD_MOD1 + "$(CC) $(" + flags + ") $(CFLAGS) /Fo" + sub_build + d + " $(" + bd_flags_name + ") /c " + src_line);
1768+
MFO.WriteLine("\t" + CMD_MOD1 + "$(CC) $(" + flags + ") $(CFLAGS) /Fo" + sub_build + d + " $(" + bd_flags_name + ") /c " + src_line);
17661769

1767-
if ("clang" == PHP_ANALYZER) {
1768-
MFO.WriteLine("\t\"$(CLANG_CL)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + src_line);
1769-
} else if ("cppcheck" == PHP_ANALYZER) {
1770-
MFO.WriteLine("\t\"$(CPPCHECK)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + analyzer_base_flags + " " + src_line);
1770+
if ("clang" == PHP_ANALYZER) {
1771+
MFO.WriteLine("\t\"$(CLANG_CL)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + src_line);
1772+
} else if ("cppcheck" == PHP_ANALYZER) {
1773+
MFO.WriteLine("\t\"$(CPPCHECK)\" " + analyzer_base_args + " $(" + flags + "_ANALYZER) $(CFLAGS_ANALYZER) $(" + bd_flags_name + "_ANALYZER) " + analyzer_base_flags + " " + src_line);
1774+
}
17711775
}
17721776
}
17731777
}

0 commit comments

Comments
 (0)