Fixes for Ninja on Windows

nickclark2016 · nickclark2016 · commit 5270987e3d6e · 2025-11-30T11:51:13.000-05:00
diff --git a/modules/ninja/ninja_cpp.lua b/modules/ninja/ninja_cpp.lua
@@ -182,7 +182,13 @@ end
 
 function m.prebuildcommandsrule(cfg, toolset)
 	_p("rule prebuild")
-	_p("  command = $prebuildcommands")
+	-- Use cmd /c on Windows to ensure proper command execution with complex commands
+	-- This matches the behavior of the gmake generator which detects shell type
+	if cfg.system == p.WINDOWS then
+		_p("  command = cmd /c $prebuildcommands")
+	else
+		_p("  command = $prebuildcommands")
+	end
 	_p("  description = Running pre-build commands")
 	_p("")
 end
@@ -196,7 +202,12 @@ end
 
 function m.prelinkcommandsrule(cfg, toolset)
 	_p("rule prelink")
-	_p("  command = $prelinkcommands")
+	-- Use cmd /c on Windows to ensure proper command execution
+	if cfg.system == p.WINDOWS then
+		_p("  command = cmd /c $prelinkcommands")
+	else
+		_p("  command = $prelinkcommands")
+	end
 	_p("  description = Running pre-link commands")
 	_p("")
 end
@@ -210,7 +221,12 @@ end
 
 function m.postbuildcommandsrule(cfg, toolset)
 	_p("rule postbuild")
-	_p("  command = $postbuildcommands")
+	-- Use cmd /c on Windows to ensure proper command execution
+	if cfg.system == p.WINDOWS then
+		_p("  command = cmd /c $postbuildcommands")
+	else
+		_p("  command = $postbuildcommands")
+	end
 	_p("  description = Running post-build commands")
 	_p("")
 end
@@ -436,7 +452,7 @@ function m.getFileCxxFlags(cfg, filecfg, toolset)
 	local allExternalIncludedirs = table.join(cfg.externalincludedirs or {}, filecfg.externalincludedirs or {})
 	local allFrameworkdirs = table.join(cfg.frameworkdirs or {}, filecfg.frameworkdirs or {})
 	local allIncludedirsafter = table.join(cfg.includedirsafter or {}, filecfg.includedirsafter or {})
-	local includedirs = toolset.getincludedirs(cfg, allIncludedirs, allExternalIncludedirs, allFrameworkdirs, allIncludedirsafter)
+	local includedirs = toolset.getincludedirs(cfg, allIncludedirs, allExternalIncludedDirs, allFrameworkdirs, allIncludedirsafter)
 	flags = table.join(flags, includedirs)
 	
 	local forceincludes = toolset.getforceincludes(filecfg)
@@ -539,6 +555,13 @@ function m.getLdFlags(cfg, toolset)
 		flags = table.join(flags, rpaths)
 	end
 	
+	-- For MSVC shared libraries, add /IMPLIB: to specify the import library location
+	-- MSVC is Windows-only, so no need to check cfg.system
+	if cfg.kind == p.SHAREDLIB and toolset == p.tools.msc and not cfg.flags.NoImportLib then
+		local impLibPath = path.getrelative(cfg.workspace.location, cfg.linktarget.directory) .. "/" .. cfg.linktarget.name
+		table.insert(flags, "/IMPLIB:" .. impLibPath)
+	end
+	
 	return flags
 end
 
@@ -1112,7 +1135,16 @@ function m.linkTarget(cfg)
 	
 	local hasPostBuild = #cfg.postbuildcommands > 0 or cfg.postbuildmessage
 	
-	_p("build %s: %s %s%s", targetPath, rule, table.concat(cfg._objectFiles, " "), implicitDeps)
+	-- For MSVC shared libraries with import libraries, list the import library as an implicit output
+	-- Dependencies will reference the .lib file (linktarget), which Ninja will know how to build.
+	-- MSVC is Windows-only, so no need to check cfg.system
+	local implicitOutputs = ""
+	if cfg.kind == p.SHAREDLIB and toolset == p.tools.msc and not cfg.flags.NoImportLib then
+		local impLibPath = path.getrelative(cfg.workspace.location, cfg.linktarget.directory) .. "/" .. cfg.linktarget.name
+		implicitOutputs = " | " .. impLibPath
+	end
+	
+	_p("build %s%s: %s %s%s", targetPath, implicitOutputs, rule, table.concat(cfg._objectFiles, " "), implicitDeps)
 	
 	if cfg.kind ~= p.STATICLIB then
 		_p("  ldflags = $ldflags_%s", ninja.key(cfg))
diff --git a/modules/ninja/tests/_tests.lua b/modules/ninja/tests/_tests.lua
@@ -15,6 +15,7 @@ return {
     "test_ninja_custom_build.lua",
     "test_ninja_custom_rules.lua",
     "test_ninja_dependson.lua",
+    "test_ninja_implib.lua",
     "test_ninja_pch.lua",
     "test_ninja_perfile_config.lua",
     "test_ninja_project.lua",
diff --git a/modules/ninja/tests/test_ninja_build_commands.lua b/modules/ninja/tests/test_ninja_build_commands.lua
@@ -0,0 +1,196 @@
+--
+-- test_ninja_build_commands.lua
+-- Test pre-build, pre-link, and post-build commands for Ninja.
+-- Author: Nick Clark
+-- Copyright (c) 2025 Jess Perkins and the Premake project
+--
+
+local suite = test.declare("ninja_build_commands")
+
+local p = premake
+local ninja = p.modules.ninja
+local cpp = ninja.cpp
+
+
+--
+-- Setup and teardown
+--
+
+local wks, prj
+
+function suite.setup()
+	p.action.set("ninja")
+	wks, prj = test.createWorkspace()
+end
+
+local function prepare()
+	local cfg = test.getconfig(prj, "Debug")
+	return cfg
+end
+
+
+---
+-- Pre-build command tests
+---
+
+--
+-- Check that prebuild commands use cmd /c on Windows
+--
+
+function suite.prebuildCommands_usesCmdOnWindows()
+	system "Windows"
+	files { "test.cpp" }
+	prebuildcommands { "echo test" }
+	
+	local cfg = prepare()
+	cpp.buildPreBuildEvents(cfg)
+	
+	test.capture [[
+build bin/Debug/MyProject.prebuild: prebuild
+  prebuildcommands = echo test
+	]]
+end
+
+
+--
+-- Check that prebuild commands don't use cmd /c on Linux
+--
+
+function suite.prebuildCommands_noCmdOnLinux()
+	system "Linux"
+	files { "test.cpp" }
+	prebuildcommands { "echo test" }
+	
+	local cfg = prepare()
+	cpp.buildPreBuildEvents(cfg)
+	
+	test.capture [[
+build bin/Debug/MyProject.prebuild: prebuild
+  prebuildcommands = echo test
+	]]
+end
+
+
+--
+-- Check that prebuild with message and commands combines them properly
+--
+
+function suite.prebuildMessageAndCommands_onWindows()
+	system "Windows"
+	files { "test.cpp" }
+	prebuildmessage "Running prebuild"
+	prebuildcommands { "echo test" }
+	
+	local cfg = prepare()
+	cpp.buildPreBuildEvents(cfg)
+	
+	test.capture [[
+build bin/Debug/MyProject.prebuild: prebuild
+  prebuildcommands = echo "Running prebuild" && echo test
+	]]
+end
+
+
+---
+-- Pre-link command tests
+---
+
+--
+-- Check that prelink commands use cmd /c on Windows
+--
+
+function suite.prelinkCommands_usesCmdOnWindows()
+	system "Windows"
+	files { "test.cpp" }
+	prelinkcommands { "echo test" }
+	
+	local cfg = prepare()
+	cfg._objectFiles = { "obj/Debug/test.obj" }
+	cpp.buildPreLinkEvents(cfg, cfg._objectFiles)
+	
+	test.capture [[
+build bin/Debug/MyProject.prelinkevents: prelink obj/Debug/test.obj
+  prelinkcommands = echo test
+	]]
+end
+
+
+--
+-- Check that prelink commands don't use cmd /c on Linux
+--
+
+function suite.prelinkCommands_noCmdOnLinux()
+	system "Linux"
+	files { "test.cpp" }
+	prelinkcommands { "echo test" }
+	
+	local cfg = prepare()
+	cfg._objectFiles = { "obj/Debug/test.obj" }
+	cpp.buildPreLinkEvents(cfg, cfg._objectFiles)
+	
+	test.capture [[
+build bin/Debug/MyProject.prelinkevents: prelink obj/Debug/test.obj
+  prelinkcommands = echo test
+	]]
+end
+
+
+---
+-- Post-build command tests
+---
+
+--
+-- Check that postbuild commands use cmd /c on Windows
+--
+
+function suite.postbuildCommands_usesCmdOnWindows()
+	system "Windows"
+	files { "test.cpp" }
+	postbuildcommands { "echo test" }
+	
+	local cfg = prepare()
+	local targetPath = "bin/Debug/MyProject.exe"
+	cpp.buildPostBuildEvents(cfg, targetPath)
+	
+	test.capture [[
+build bin/Debug/MyProject.postbuild: postbuild | bin/Debug/MyProject.exe
+  postbuildcommands = echo test
+	]]
+end
+
+
+--
+-- Check that postbuild commands don't use cmd /c on Linux
+--
+
+function suite.postbuildCommands_noCmdOnLinux()
+	system "Linux"
+	files { "test.cpp" }
+	postbuildcommands { "echo test" }
+	
+	local cfg = prepare()
+	local targetPath = "bin/Debug/MyProject"
+	cpp.buildPostBuildEvents(cfg, targetPath)
+	
+	test.capture [[
+build bin/Debug/MyProject.postbuild: postbuild | bin/Debug/MyProject
+  postbuildcommands = echo test
+	]]
+end
+
+
+--
+-- Check that complex commands with quotes work on Windows
+--
+
+function suite.prebuildCommands_withQuotedPaths_onWindows()
+	system "Windows"
+	files { "test.cpp" }
+	prebuildcommands { "{COPYFILE} %[src/file.txt] %[dest/file.txt]" }
+	
+	local cfg = prepare()
+	cpp.buildPreBuildEvents(cfg)
+	
+	-- The command should have quotes around paths
+	test.string_contains(premake.captured(), 'copy /B /Y')
+end
diff --git a/modules/ninja/tests/test_ninja_build_rules.lua b/modules/ninja/tests/test_ninja_build_rules.lua
@@ -397,11 +397,12 @@ rule copy
 --
 
 	function suite.prebuildCommandsRule()
+		system "Windows"
 		local cfg = prepare()
 		cpp.prebuildcommandsrule(cfg)
 		test.capture [[
 rule prebuild
-  command = $prebuildcommands
+  command = cmd /c $prebuildcommands
   description = Running pre-build commands
 
 		]]
@@ -429,11 +430,12 @@ rule prebuildmessage
 --
 
 	function suite.prelinkCommandsRule()
+		system "Windows"
 		local cfg = prepare()
 		cpp.prelinkcommandsrule(cfg)
 		test.capture [[
 rule prelink
-  command = $prelinkcommands
+  command = cmd /c $prelinkcommands
   description = Running pre-link commands
 
 		]]
@@ -461,11 +463,12 @@ rule prelinkmessage
 --
 
 	function suite.postbuildCommandsRule()
+		system "Windows"
 		local cfg = prepare()
 		cpp.postbuildcommandsrule(cfg)
 		test.capture [[
 rule postbuild
-  command = $postbuildcommands
+  command = cmd /c $postbuildcommands
   description = Running post-build commands
 
 		]]
@@ -488,6 +491,57 @@ rule postbuildmessage
 	end
 
 
+--
+-- Check the pre-build commands rule on Linux doesn't use cmd /c.
+--
+
+	function suite.prebuildCommandsRule_onLinux()
+		system "Linux"
+		local cfg = prepare()
+		cpp.prebuildcommandsrule(cfg)
+		test.capture [[
+rule prebuild
+  command = $prebuildcommands
+  description = Running pre-build commands
+
+		]]
+	end
+
+
+--
+-- Check the pre-link commands rule on Linux doesn't use cmd /c.
+--
+
+	function suite.prelinkCommandsRule_onLinux()
+		system "Linux"
+		local cfg = prepare()
+		cpp.prelinkcommandsrule(cfg)
+		test.capture [[
+rule prelink
+  command = $prelinkcommands
+  description = Running pre-link commands
+
+		]]
+	end
+
+
+--
+-- Check the post-build commands rule on Linux doesn't use cmd /c.
+--
+
+	function suite.postbuildCommandsRule_onLinux()
+		system "Linux"
+		local cfg = prepare()
+		cpp.postbuildcommandsrule(cfg)
+		test.capture [[
+rule postbuild
+  command = $postbuildcommands
+  description = Running post-build commands
+
+		]]
+	end
+
+
 --
 -- Check the custom command rule.
 --
diff --git a/modules/ninja/tests/test_ninja_implib.lua b/modules/ninja/tests/test_ninja_implib.lua
