Build old (pre-v145) PlatformToolset-pinned .vcxproj files

vintner only ever downloads one compiler generation, but real-world .vcxproj files are pinned to whichever PlatformToolset they were last saved under - v142 (VS2019) for anything not actively maintained is extremely common. MSBuild checks toolset "installed-ness" (MSB8020) by testing whether MSBuild/Microsoft/VC/v<schema>/Platforms/<arch>/ PlatformToolsets/<toolset>/ exists on disk - a plain file lookup our downloaded MSBuild package only satisfies for the exact generation it shipped. `install` now symlinks every historical numeric PlatformToolset name (v90 through v143) onto whichever real toolset directory is actually present, so any of them resolves transparently; Toolset.props/ .targets don't hardcode a version number, so aliasing is correct, not just a workaround. Three more MSBuild property/environment issues came with it, all found building a real years-old project against the one modern toolchain vintner installs: - VCInstallDir_<N>/VCToolsInstallDir_<N> needed a third numbering source (PlatformToolset short names from Microsoft.VCToolsVersion. v<N>.default.props) alongside the existing MSBuild schema-version and known-toolset lists, so the env-var-driven half of toolset resolution covers the same names the on-disk alias does. - VCToolsVersion must be a real version string: left unset, it falls back to a literal placeholder that then hits an unconditional version-string comparison elsewhere in Microsoft.CppBuild.targets (MSB4184). Setting it to the real installed version in turn requires CheckMSVCComponents=false, since CheckVCToolsetVersion (MSB8052) otherwise rejects an aliased PlatformToolset whenever its numeric generation doesn't match VCToolsVersion's - exactly the case aliasing creates on purpose. Everything else CheckMSVCComponents gates is diagnostic-only (MFC/ATL/Spectre presence warnings), so disabling it costs nothing else. - WindowsTargetPlatformVersion needed to become an explicit /p: global property on the msbuild command line, not just an env var: legacy .vcxproj files commonly hardcode this in a PropertyGroup, and an explicit project assignment always wins over an inherited environment variable of the same name. A command-line global property is the one thing a project file can't override. Only injected when the caller hasn't already pinned it themselves.
2026-08-03 15:57:24 +00:00 · 2026-07-25 15:41:09 +10:00
parent 4795c7c105
commit 25f751e874
7 changed files with 448 additions and 22 deletions
@@ -29,10 +29,31 @@ func msbuildEnv(cfg *wineenv.Config, paths *wineenv.Paths) map[string]string {
 		// tools onto the same UTC clock removes the mismatch.
 		"TZ": "UTC",

-		"DisableRegistryUse": "true",
-		"VCToolsVersion":     cfg.MSVCVer,
-		"VsInstallRoot":      paths.BaseWin + `\`,
-		"VSInstallDir":       paths.BaseWin + `\`,
+		// VCToolsVersion must be a real version string, not left unset:
+		// Microsoft.Cpp.VCTools.props itself falls back to the literal
+		// placeholder "VCToolsVersion_is_not_defined" whenever it's empty,
+		// and that placeholder then reaches unconditional (not gated behind
+		// CheckMSVCComponents) version-string comparisons elsewhere in
+		// Microsoft.CppBuild.targets, e.g. the SegmentHeap manifest check's
+		// VersionGreaterThanOrEquals(), which throws MSB4184 on a
+		// non-version string.
+		//
+		// CheckMSVCComponents=false is what actually makes an aliased
+		// PlatformToolset safe to combine with that real version: without
+		// it, CheckVCToolsetVersion (Microsoft.CppBuild.targets, MSB8052)
+		// rejects the combination whenever VCToolsVersion's numeric
+		// generation doesn't match PlatformToolset's - exactly the legacy-
+		// project case toolsetSuffixes/aliasPlatformToolsets exist for (a
+		// v142 project against the one real, newer compiler actually
+		// installed). Everything else CheckMSVCComponents gates
+		// (Microsoft.CppBuild.targets ~495-535) is diagnostic-only - MFC/ATL/
+		// Spectre component presence warnings, none of it feeding into the
+		// actual compile/link - so disabling it costs nothing here.
+		"DisableRegistryUse":  "true",
+		"CheckMSVCComponents": "false",
+		"VCToolsVersion":      cfg.MSVCVer,
+		"VsInstallRoot":       paths.BaseWin + `\`,
+		"VSInstallDir":        paths.BaseWin + `\`,

 		"SDKReferenceDirectoryRoot":                 paths.BaseWin + `\`,
 		"SDKExtensionDirectoryRoot":                 paths.BaseWin + `\`,
@@ -58,20 +79,29 @@ func msbuildEnv(cfg *wineenv.Config, paths *wineenv.Paths) map[string]string {
 		"Platform": msbuildPlatform(cfg.Arch),
 	}

-	// Microsoft.Cpp.props resolves the compiler/toolset location through
-	// VCInstallDir_<N>/VCToolsInstallDir_<N>, where <N> is whatever numeric
-	// suffix the installed MSBuild toolset property sheets use (e.g.
-	// .../MSBuild/Microsoft/VC/v180 -> "180"). Populate every one actually
-	// present, so a project pinned to any of them resolves to the one real
-	// toolchain that's installed.
-	matches, _ := filepath.Glob(filepath.Join(paths.BaseUnix, "MSBuild", "Microsoft", "VC", "v*"))
-	for _, m := range matches {
-		sub := reToolsetDir.FindStringSubmatch(filepath.Base(m))
-		if sub == nil {
-			continue
-		}
-		env["VCInstallDir_"+sub[1]] = paths.MSVCBaseWin + `\`
-		env["VCToolsInstallDir_"+sub[1]] = paths.MSVCDirWin + `\`
+	// VCInstallDir_<N>/VCToolsInstallDir_<N> are consulted under two
+	// completely different numbering schemes, both needing the single real
+	// toolchain behind every <N> they might ask for:
+	//
+	//  - Microsoft.Cpp.Default.props keys its early "is this toolset even
+	//    installed" check (the one MSB8020 comes from) off <N> = the
+	//    PlatformToolset suffix a .vcxproj actually declares (v142, v143,
+	//    v145, ...) - the same short name Microsoft stamps on
+	//    VC/Auxiliary/Build/Microsoft.VCToolsVersion.v<N>.default.props for
+	//    the downloaded compiler.
+	//  - Microsoft.CppBuild.targets (MSB8070) instead keys off <N> = the
+	//    MSBuild targets-schema version whose Microsoft.Cpp.props ended up
+	//    imported for this run (MSBuild/Microsoft/VC/v150|v160|v170|v180 -
+	//    fixed, shipped identically with every MSBuild release, unrelated to
+	//    which compiler is installed), to locate the specific toolset
+	//    version subfolder.
+	//
+	// Populate every <N> from both sources, all pointing at the one real
+	// toolchain that's installed, so a project pinned to any PlatformToolset
+	// resolves at every stage MSBuild checks it.
+	for _, n := range toolsetSuffixes(paths.BaseUnix) {
+		env["VCInstallDir_"+n] = paths.MSVCBaseWin + `\`
+		env["VCToolsInstallDir_"+n] = paths.MSVCDirWin + `\`
 	}

 	if strings.HasSuffix(paths.MSBuildBinDir, "amd64") {
@@ -94,6 +124,100 @@ func msbuildEnv(cfg *wineenv.Config, paths *wineenv.Paths) map[string]string {
 	return env
 }

+// toolsetSuffixes collects every numeric <N> that either VCInstallDir_<N>
+// lookup mechanism (see msbuildEnv) might be asked to resolve for this
+// installation: PlatformToolset short names from
+// vc/Auxiliary/Build/Microsoft.VCToolsVersion.v<N>.default.props, MSBuild
+// targets-schema versions from MSBuild/Microsoft/VC/v<N>, and every
+// historical PlatformToolset name (see wineenv.KnownPlatformToolsets) - a
+// project pinned to any of them all resolves to the one real toolchain
+// installed.
+func toolsetSuffixes(baseUnix string) []string {
+	seen := map[string]bool{}
+	var suffixes []string
+
+	record := func(n string) {
+		if seen[n] {
+			return
+		}
+		seen[n] = true
+		suffixes = append(suffixes, n)
+	}
+
+	add := func(dir, prefix, suffix string) {
+		matches, _ := filepath.Glob(filepath.Join(dir, "*"))
+		for _, m := range matches {
+			name := filepath.Base(m)
+			if prefix != "" {
+				if !strings.HasPrefix(name, prefix) {
+					continue
+				}
+				name = strings.TrimPrefix(name, prefix)
+			}
+			name = strings.TrimSuffix(name, suffix)
+			sub := reToolsetDir.FindStringSubmatch(name)
+			if sub == nil {
+				continue
+			}
+			record(sub[1])
+		}
+	}
+
+	add(filepath.Join(baseUnix, "vc", "Auxiliary", "Build"), "Microsoft.VCToolsVersion.", ".default.props")
+	add(filepath.Join(baseUnix, "MSBuild", "Microsoft", "VC"), "", "")
+	for _, n := range wineenv.KnownPlatformToolsets {
+		record(n)
+	}
+
+	return suffixes
+}
+
+// reGlobalProp matches an MSBuild global-property command-line switch
+// ("/p:Name=...", "-property:Name=...", case-insensitive on both the
+// -p/-property spelling and the property name) so msbuildGlobalArgs can tell
+// whether the caller already pinned a given property themselves.
+func reGlobalProp(name string) *regexp.Regexp {
+	return regexp.MustCompile(`(?i)^[-/](p|property):` + regexp.QuoteMeta(name) + `=`)
+}
+
+// msbuildGlobalArgs returns /p: switches to prepend to an MSBuild invocation,
+// one per forced property not already present in args.
+//
+// WindowsTargetPlatformVersion is the one property that needs this rather
+// than an env var: unlike VCInstallDir_<N> (an input to a props-file
+// *lookup*, so any value msbuildEnv sets is visible no matter what a project
+// pins its PlatformToolset to), WindowsTargetPlatformVersion is itself the
+// value most legacy .vcxproj files hardcode directly in a PropertyGroup -
+// and an explicit PropertyGroup assignment always wins over an inherited
+// environment variable of the same name. A command-line global property is
+// the one thing a project file can't override, which is exactly what's
+// needed here: vintner only ever installs one Windows SDK version, so - same
+// reasoning as the PlatformToolset fallback above - any project should
+// transparently build against that one installed version rather than fail
+// outright over an exact version string it happened to be pinned to when
+// last saved from a real Windows SDK selector dropdown.
+func msbuildGlobalArgs(cfg *wineenv.Config, args []string) []string {
+	forced := map[string]string{
+		"WindowsTargetPlatformVersion": cfg.SDKVer,
+	}
+
+	var out []string
+	for name, value := range forced {
+		re := reGlobalProp(name)
+		alreadySet := false
+		for _, a := range args {
+			if re.MatchString(a) {
+				alreadySet = true
+				break
+			}
+		}
+		if !alreadySet {
+			out = append(out, "/p:"+name+"="+value)
+		}
+	}
+	return out
+}
+
 func msbuildPlatform(arch string) string {
 	switch arch {
 	case "x86":
@@ -39,6 +39,14 @@ func TestMsbuildEnvBasics(t *testing.T) {
 	if env["DisableRegistryUse"] != "true" {
 		t.Errorf(`env["DisableRegistryUse"] = %q, want "true"`, env["DisableRegistryUse"])
 	}
+	if env["CheckMSVCComponents"] != "false" {
+		t.Errorf(`env["CheckMSVCComponents"] = %q, want "false" (else CheckVCToolsetVersion errors on an aliased PlatformToolset)`, env["CheckMSVCComponents"])
+	}
+	// VCToolsVersion must be a real version string (see msbuildEnv's doc
+	// comment on it: leaving it unset makes Microsoft.Cpp.VCTools.props
+	// substitute a placeholder that then breaks unconditional version
+	// comparisons elsewhere). CheckMSVCComponents=false is what keeps this
+	// safe to combine with an aliased PlatformToolset.
 	if env["VCToolsVersion"] != cfg.MSVCVer {
 		t.Errorf(`env["VCToolsVersion"] = %q, want %q`, env["VCToolsVersion"], cfg.MSVCVer)
 	}
@@ -61,7 +69,24 @@ func TestMsbuildEnvDiscoversEveryToolsetVersion(t *testing.T) {
 	cfg := &wineenv.Config{Arch: "x64", Host: "x64", DotnetHost: "amd64", MSVCVer: "14.51.36231", SDKVer: "10.0.26100.0"}
 	paths, base := newTestPaths(t, cfg)

-	for _, v := range []string{"v145", "v180", "not-a-version"} {
+	// Real layout has two independent sources feeding VCInstallDir_<N>/
+	// VCToolsInstallDir_<N> (see toolsetSuffixes' doc comment for why both
+	// are needed): the PlatformToolset short names a downloaded compiler
+	// ships default-props for, and MSBuild's own fixed schema-version dirs.
+	buildDir := filepath.Join(base, "vc", "Auxiliary", "Build")
+	if err := os.MkdirAll(buildDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	for _, name := range []string{
+		"Microsoft.VCToolsVersion.v145.default.props",
+		"Microsoft.VCToolsVersion.v143.default.props",
+		"Microsoft.VCToolsVersion.default.props", // no version suffix - must not match
+	} {
+		if err := os.WriteFile(filepath.Join(buildDir, name), nil, 0o644); err != nil {
+			t.Fatal(err)
+		}
+	}
+	for _, v := range []string{"v180", "not-a-version"} {
 		if err := os.MkdirAll(filepath.Join(base, "MSBuild", "Microsoft", "VC", v), 0o755); err != nil {
 			t.Fatal(err)
 		}
@@ -69,7 +94,7 @@ func TestMsbuildEnvDiscoversEveryToolsetVersion(t *testing.T) {

 	env := msbuildEnv(cfg, paths)

-	for _, n := range []string{"145", "180"} {
+	for _, n := range []string{"145", "143", "180"} {
 		if _, ok := env["VCInstallDir_"+n]; !ok {
 			t.Errorf("expected VCInstallDir_%s to be set", n)
 		}
@@ -82,6 +107,64 @@ func TestMsbuildEnvDiscoversEveryToolsetVersion(t *testing.T) {
 	}
 }

+func TestToolsetSuffixesDedupsOverlap(t *testing.T) {
+	base := t.TempDir()
+	buildDir := filepath.Join(base, "vc", "Auxiliary", "Build")
+	if err := os.MkdirAll(buildDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(buildDir, "Microsoft.VCToolsVersion.v180.default.props"), nil, 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.MkdirAll(filepath.Join(base, "MSBuild", "Microsoft", "VC", "v180"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+
+	got := toolsetSuffixes(base)
+	count := 0
+	for _, n := range got {
+		if n == "180" {
+			count++
+		}
+	}
+	if count != 1 {
+		t.Errorf("toolsetSuffixes() returned %q with %d entries for \"180\" (from both sources), want exactly 1", got, count)
+	}
+}
+
+func TestMsbuildGlobalArgsForcesWindowsTargetPlatformVersion(t *testing.T) {
+	cfg := &wineenv.Config{SDKVer: "10.0.26100.0"}
+
+	got := msbuildGlobalArgs(cfg, []string{"Foo.sln", "/p:Configuration=Release"})
+	want := "/p:WindowsTargetPlatformVersion=10.0.26100.0"
+	found := false
+	for _, a := range got {
+		if a == want {
+			found = true
+		}
+	}
+	if !found {
+		t.Errorf("msbuildGlobalArgs(...) = %v, want it to contain %q", got, want)
+	}
+}
+
+func TestMsbuildGlobalArgsRespectsExplicitOverride(t *testing.T) {
+	cfg := &wineenv.Config{SDKVer: "10.0.26100.0"}
+
+	for _, explicit := range []string{
+		"/p:WindowsTargetPlatformVersion=10.0.19041.0",
+		"-p:WindowsTargetPlatformVersion=10.0.19041.0",
+		"/property:WindowsTargetPlatformVersion=10.0.19041.0",
+	} {
+		got := msbuildGlobalArgs(cfg, []string{"Foo.sln", explicit})
+		for _, a := range got {
+			if reGlobalProp("WindowsTargetPlatformVersion").MatchString(a) {
+				t.Errorf("msbuildGlobalArgs with explicit %q also injected %q - should have left the caller's value alone", explicit, a)
+			}
+		}
+	}
+}
+
 func TestMsbuildEnvDetectsWDKContentRoot(t *testing.T) {
 	cfg := &wineenv.Config{Arch: "x64", Host: "x64", DotnetHost: "amd64", MSVCVer: "14.51.36231", SDKVer: "10.0.26100.0"}
 	paths, base := newTestPaths(t, cfg)
@@ -83,8 +83,10 @@ func Run(tool string, args []string) int {
 		// MSBuild: skip all filtering/toolrelay (its output is meant to be
 		// read as-is), and add the extra environment MSBuild's own
 		// toolset/SDK-detection props need on top of the generic
-		// INCLUDE/LIB/WINEPATH.
-		cmd := exec.Command(wineBin, append([]string{toolExePath}, rewritten...)...)
+		// INCLUDE/LIB/WINEPATH, plus any global properties a project file
+		// itself could otherwise override (see msbuildGlobalArgs).
+		msArgs := append(msbuildGlobalArgs(cfg, rewritten), rewritten...)
+		cmd := exec.Command(wineBin, append([]string{toolExePath}, msArgs...)...)
 		env := buildEnv(paths)
 		for k, v := range msbuildEnv(cfg, paths) {
 			env = append(env, k+"="+v)