diff --git a/docs/msvc-enable.md b/docs/msvc-enable.md index b0a9085..c4aa530 100644 --- a/docs/msvc-enable.md +++ b/docs/msvc-enable.md @@ -248,6 +248,58 @@ shims (`gem`, `bundle`) and PATHEXT resolve the way they would if the user had typed the command directly; a bare `CreateProcess` would not find `gem` (it is `gem.cmd`). +## `LIBCLANG_PATH` for Rust extension gems + +Rust extension gems built with rb-sys pull in bindgen, which loads +`libclang.dll` through the clang-sys crate. clang-sys probes the Visual +Studio install itself and does not consult PATH, and a VS install carries +both `VC\Tools\Llvm\ARM64\bin\libclang.dll` and +`VC\Tools\Llvm\x64\bin\libclang.dll`. Which one that probe reaches first +is not deterministic, so on an x64 host the build fails intermittently +with a `LoadLibraryExW failed` error. Adding the x64 Llvm `bin` to PATH +does not fix it, which was confirmed on a real machine: three +consecutive `rb msvc cargo build --release` runs in a fresh directory +failed every time with `LIBCLANG_PATH` unset (the activated PATH already +carried the x64 Llvm bin), and succeeded every time with it set. + +Both surfaces therefore set `LIBCLANG_PATH` to +`\VC\Tools\Llvm\x64\bin` when `libclang.dll` is there. +`VsDevCmd.bat` never sets this variable, so it is not part of the +activation delta and is added as a separate step derived from the +resolved `installationPath`. A `LIBCLANG_PATH` the user has already set +is left alone. + +rbmanager does not install or manage rustup, cargo, or rustc, the same +scope decision as the vcpkg dependency headers below: `rb msvc` +activates the MSVC toolchain and clang, and does not check that a Rust +toolchain exists. Users install rustup themselves. + +## Windows SDK library check + +`vswhere -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64` +confirms the MSVC compiler component is installed, but says nothing +about whether the paired Windows SDK's library files are on disk. A real +machine was found in exactly that state, and the failure is remote from +the cause: rustc's own MSVC detection reported ``linker `link.exe` not +found``, and because a uutils coreutils `link.exe` happened to sit on +PATH, the actual message was `error: 2 values required for ' +'`. A C extension under mkmf would fail the same way on +`kernel32.lib`. + +There is no stable vswhere component id to require here, since the SDK +ids carry their version (`Microsoft.VisualStudio.Component.Windows11SDK.*`). +Activation instead checks the lib tree directly, using what VsDevCmd +just reported: +`\Lib\\um\x64\kernel32.lib`. +`WindowsSDKLibVersion` is the one that names the `Lib` subdirectory in +both the 8.1 (`winv6.3`) and 10 (`10.0.x`) layouts, with +`WindowsSDKVersion`, which only the Windows 10 SDK sets, as the +fallback. When the file is absent, both surfaces refuse with an +actionable message (add the Windows SDK component in the Visual Studio +Installer) rather than handing back a half-usable environment. When +VsDevCmd reports no SDK variables at all there is nothing to check +against, so the check is skipped rather than failing closed. + ## `NoDefaultCurrentDirectoryInExePath` On machines where `NoDefaultCurrentDirectoryInExePath` is set (it was @@ -270,8 +322,11 @@ The prototype hard-codes `-arch=amd64 -host_arch=amd64` for the current arm64 mswin package exists, the arch would be derived from the active ruby's platform (or the host) and passed as `-arch=arm64` `-host_arch=amd64` (cross) or `-host_arch=arm64` (native), and the -vswhere `-requires` would name the arm64 toolset component. That is the -only place the change lands. +vswhere `-requires` would name the arm64 toolset component. The two +later additions have the same axis and are marked with the same +`TODO(arm64)`: the `LIBCLANG_PATH` selection picks the `x64` Llvm +directory, and the Windows SDK check looks under the `x64` lib +subdirectories. ## Third-party dependency dev files (scope decision) @@ -336,7 +391,8 @@ should not depend on it.) argument parser; `Program.cs`'s dispatch switch hands it everything after the `msvc` token. It is marked as a prototype and covers VS discovery, VsDevCmd activation with env-diffing, the -`NoDefaultCurrentDirectoryInExePath` clearing, and the per-shell output. +`NoDefaultCurrentDirectoryInExePath` clearing, the `LIBCLANG_PATH` +forwarding, the Windows SDK library check, and the per-shell output. It is compiler-only (phase 1). What was exercised: - `rb msvc enable powershell|cmd` prints correct assignments; the diff --git a/docs/test-plan.md b/docs/test-plan.md index 36231ea..24f8857 100644 --- a/docs/test-plan.md +++ b/docs/test-plan.md @@ -366,6 +366,25 @@ Added with the command-surface change: `enable cmd` prints the stub's assignments, and `cmd /c exit 7` runs as a command and propagates 7. +Added with `LIBCLANG_PATH` and the Windows SDK check. Both use a stub +VsDevCmd.bat placed at `\Common7\Tools\VsDevCmd.bat`, so the +`installationPath` Msvc derives from it has the real layout's shape: + +94. `LibclangPathToSet`: no Llvm tree → null; an ARM64 `libclang.dll` + alone → still null (the wrong one must never be selected); an x64 + one → its `bin` directory. With `LIBCLANG_PATH` already set in the + environment → null, whatever is on disk. +95. `Enable`/`Exec` with an x64 `libclang.dll` present → the assignment + appears in the printed output and the variable reaches the child. +96. `Enable`/`Exec` with the stub reporting a `WindowsSdkDir` and a + `WindowsSDKLibVersion` whose `um\x64\kernel32.lib` exists → + activates normally, exit 0. +97. The same with the lib tree absent, and reporting the + `WindowsSDKVersion` fallback key instead → stderr names the missing + Windows SDK, exit 1, stdout empty. +98. `WindowsSdkLibsPresent` with neither variable in the delta → true + (nothing was reported, so there is nothing to check). + ### 4.10 Msvc against real Visual Studio — RequiresVS (opt-in) Skipped unless vswhere resolves an install (use a runtime skip, e.g. diff --git a/src/rbmanager/Msvc.cs b/src/rbmanager/Msvc.cs index aa2d89e..8a18d88 100644 --- a/src/rbmanager/Msvc.cs +++ b/src/rbmanager/Msvc.cs @@ -194,7 +194,10 @@ public static int Enable(string? shell, string? vsver = null) string? year = EffectiveVsVer(vsver); string? vsdevcmd = ResolveVsDevCmd(year); if (vsdevcmd is null) return WarnMissingToolchain(year); - var env = ActivatedDelta(vsdevcmd); + var env = ActivatedDelta(vsdevcmd).ToList(); + if (!WindowsSdkLibsPresent(ByKey(env))) return WarnMissingWindowsSdk(); + if (LibclangPathToSet(vsdevcmd) is { } libclangBin) + env.Add(("LIBCLANG_PATH", libclangBin)); foreach ((string key, string value) in env) Console.WriteLine(Assignment(target, key, value)); // The mkmf gotcha: with NoDefaultCurrentDirectoryInExePath set, @@ -209,7 +212,10 @@ public static int Exec(string[] command, string? vsver = null) string? year = EffectiveVsVer(vsver); string? vsdevcmd = ResolveVsDevCmd(year); if (vsdevcmd is null) return WarnMissingToolchain(year); - var delta = ActivatedDelta(vsdevcmd); + var delta = ActivatedDelta(vsdevcmd).ToList(); + if (!WindowsSdkLibsPresent(ByKey(delta))) return WarnMissingWindowsSdk(); + if (LibclangPathToSet(vsdevcmd) is { } libclangBin) + delta.Add(("LIBCLANG_PATH", libclangBin)); // cmd.exe /c so that .cmd shims (gem, bundle) and PATHEXT resolve // the way they would if the user had typed the command directly. string commandLine = string.Join(' ', command.Select(QuoteArg)); @@ -232,6 +238,55 @@ public static int Exec(string[] command, string? vsver = null) return proc.ExitCode; } + private static Dictionary ByKey(List<(string, string)> pairs) => + pairs.ToDictionary(p => p.Item1, p => p.Item2, StringComparer.OrdinalIgnoreCase); + + // bindgen (via clang-sys, an rb-sys/rustc-bindgen dependency) probes the + // VS install for libclang.dll itself; it does not consult PATH. VS + // ships both an ARM64 and an x64 copy of libclang.dll side by side, and + // clang-sys's search picks between them non-deterministically. Grabbing + // the ARM64 one fails every build on an x64 host with a LoadLibraryExW + // error. VsDevCmd itself never sets LIBCLANG_PATH, so this is a + // separate step layered on top of ActivatedDelta. A LIBCLANG_PATH the + // user already has set is left untouched. + // TODO(arm64): "x64" becomes host-arch-dependent here, same as the + // -arch=amd64/-host_arch=amd64 in ActivatedDelta. + internal static string? LibclangPathToSet(string vsdevcmd) + { + if (Environment.GetEnvironmentVariable("LIBCLANG_PATH") is { Length: > 0 }) + return null; + // vsdevcmd is \Common7\Tools\VsDevCmd.bat. + string? installationPath = + Path.GetDirectoryName(Path.GetDirectoryName(Path.GetDirectoryName(vsdevcmd))); + if (installationPath is null) return null; + string bin = Path.Combine(installationPath, "VC", "Tools", "Llvm", "x64", "bin"); + return File.Exists(Path.Combine(bin, "libclang.dll")) ? bin : null; + } + + // vswhere's -requires only confirms the VC.Tools.x86.x64 component is + // installed, not that its paired Windows SDK's library files are + // actually on disk; that gap surfaces much later as a bewildering + // linker error (or, worse, an unrelated-looking one if a non-MSVC + // link.exe shadows the real one on PATH). There is no stable vswhere + // component id for "this SDK version's libs exist", so this checks the + // lib tree directly, keyed off what VsDevCmd itself just reported in + // the activation delta. WindowsSDKLibVersion is the Lib subdirectory + // name in both the 8.1 (winv6.3) and 10 (10.0.x) layouts; + // WindowsSDKVersion, which only the 10 SDK sets, is the fallback. + // Neither appearing means VsDevCmd reported no SDK change and there is + // nothing to check against, so the check is skipped rather than + // failing closed. + // TODO(arm64): the x64 subdirectory becomes host-arch-dependent here. + internal static bool WindowsSdkLibsPresent(IReadOnlyDictionary activated) + { + if (!activated.TryGetValue("WindowsSdkDir", out string? sdkDir)) return true; + if (!activated.TryGetValue("WindowsSDKLibVersion", out string? libVer) && + !activated.TryGetValue("WindowsSDKVersion", out libVer)) + return true; + return File.Exists(Path.Combine( + sdkDir, "Lib", libVer.TrimEnd('\\', '/'), "um", "x64", "kernel32.lib")); + } + // Runs VsDevCmd in a clean child and returns only the variables it added // or changed relative to our own (i.e. the calling shell's) environment. internal static IEnumerable<(string, string)> ActivatedDelta(string vsdevcmd) @@ -408,6 +463,26 @@ winget install Microsoft.VisualStudio.{year ?? "2022"}.BuildTools --override "-- return 1; } + // The compiler is present but has nothing to link against. Refusing here + // beats activating a half-usable environment: the failure would + // otherwise land in the linker, where a missing kernel32.lib reads as a + // gem bug rather than an incomplete VS install. + private static int WarnMissingWindowsSdk() + { + Console.Error.WriteLine(""" + rb: warning: the Visual Studio install has the MSVC compiler but no Windows SDK libraries; linking will fail. + + To set one up: + + 1. Open the Visual Studio Installer, Modify the install, and add the + "Windows 11 SDK" (or "Windows 10 SDK") component under + "Desktop development with C++". + + 2. Open a new terminal and re-run this command. + """); + return 1; + } + internal enum Shell { Cmd, PowerShell } internal static Shell ParseShell(string? shell) => shell switch diff --git a/tests/rbmanager.Tests/MsvcActivationTests.cs b/tests/rbmanager.Tests/MsvcActivationTests.cs index 2b3a1da..e08483b 100644 --- a/tests/rbmanager.Tests/MsvcActivationTests.cs +++ b/tests/rbmanager.Tests/MsvcActivationTests.cs @@ -26,6 +26,37 @@ private static Dictionary Delta(string bat) => Msvc.ActivatedDelta(bat) .ToDictionary(t => t.Item1, t => t.Item2, StringComparer.OrdinalIgnoreCase); + // A VsDevCmd.bat at the layout position Msvc derives installationPath + // from (\Common7\Tools\VsDevCmd.bat), so the sibling + // VC\Tools\Llvm tree resolves the way it does in a real install. + private static string VsLayoutBat(TempDir tmp, string root, params string[] lines) + { + string dir = tmp.At(root, "Common7", "Tools"); + Directory.CreateDirectory(dir); + string path = Path.Combine(dir, "VsDevCmd.bat"); + File.WriteAllText(path, "@echo off\r\n" + string.Join("\r\n", lines) + "\r\n"); + return path; + } + + private static void FakeLibclang(TempDir tmp, string root, string arch) + { + string dir = tmp.At(root, "VC", "Tools", "Llvm", arch, "bin"); + Directory.CreateDirectory(dir); + File.WriteAllText(Path.Combine(dir, "libclang.dll"), ""); + } + + // A Windows Kits lib tree as VsDevCmd reports it: both variables carry a + // trailing backslash. + private static (string Dir, string Version) FakeSdk(TempDir tmp, bool withLibs) + { + string dir = tmp.At("kits", "10"); + const string version = "10.0.26100.0"; + string libDir = Path.Combine(dir, "Lib", version, "um", "x64"); + Directory.CreateDirectory(withLibs ? libDir : dir); + if (withLibs) File.WriteAllText(Path.Combine(libDir, "kernel32.lib"), ""); + return (dir + "\\", version + "\\"); + } + [Fact] // case 61 public void ActivatedDelta_ReportsAddedAndChangedOnly() { @@ -283,6 +314,105 @@ public void Dispatch_RoutesEnableAndCommand() Assert.Equal(7, Msvc.Dispatch(Msvc.Parse(["cmd", "/c", "exit", "7"])!.Value)); } + [Fact] // case 94: LIBCLANG_PATH points at the x64 copy, never the ARM64 one + public void LibclangPathToSet_PrefersX64_AndSkipsWhenAbsent() + { + using var tmp = new TempDir(); + using var env = new EnvScope(); + env.Set("LIBCLANG_PATH", null); + string bat = VsLayoutBat(tmp, "vs"); + + Assert.Null(Msvc.LibclangPathToSet(bat)); // no Llvm tree at all + + FakeLibclang(tmp, "vs", "ARM64"); + Assert.Null(Msvc.LibclangPathToSet(bat)); // ARM64 alone is not a match + + FakeLibclang(tmp, "vs", "x64"); + Assert.Equal(tmp.At("vs", "VC", "Tools", "Llvm", "x64", "bin"), + Msvc.LibclangPathToSet(bat)); + } + + [Fact] // case 94: a LIBCLANG_PATH the user already set wins + public void LibclangPathToSet_UserValue_NotOverridden() + { + using var tmp = new TempDir(); + using var env = new EnvScope(); + env.Set("LIBCLANG_PATH", @"C:\mine"); + string bat = VsLayoutBat(tmp, "vs"); + FakeLibclang(tmp, "vs", "x64"); + + Assert.Null(Msvc.LibclangPathToSet(bat)); + } + + [Fact] // case 95: both surfaces carry LIBCLANG_PATH into the activation + public void EnableAndExec_SetLibclangPath() + { + using var tmp = new TempDir(); + using var env = new EnvScope(); + env.Set("LIBCLANG_PATH", null); + FakeLibclang(tmp, "vs", "x64"); + env.Set("RBMANAGER_VSDEVCMD", VsLayoutBat(tmp, "vs")); + string expected = tmp.At("vs", "VC", "Tools", "Llvm", "x64", "bin"); + + using (var cap = new ConsoleCapture()) + { + Assert.Equal(0, Msvc.Enable("powershell")); + Assert.Contains($"$env:LIBCLANG_PATH = '{expected}'", cap.OutLines); + } + + string dump = Bat(tmp, "set > \"%~1\""); + string outFile = tmp.At("env.txt"); + Assert.Equal(0, Msvc.Exec([dump, outFile])); + Assert.Contains($"LIBCLANG_PATH={expected}", File.ReadAllText(outFile)); + } + + [Fact] // case 96: an SDK whose libs are on disk activates normally + public void EnableAndExec_SdkLibsPresent_Activates() + { + using var tmp = new TempDir(); + using var env = new EnvScope(); + env.Set("LIBCLANG_PATH", null); + (string dir, string version) = FakeSdk(tmp, withLibs: true); + env.Set("RBMANAGER_VSDEVCMD", VsLayoutBat(tmp, "vs", + $"set WindowsSdkDir={dir}", $"set WindowsSDKLibVersion={version}")); + + using var cap = new ConsoleCapture(); + Assert.Equal(0, Msvc.Enable("powershell")); + Assert.Equal(0, Msvc.Exec(["cmd", "/c", "exit", "0"])); + } + + [Fact] // case 97: the component is installed but its libs are not + public void EnableAndExec_SdkLibsMissing_WarnOnStderr() + { + using var tmp = new TempDir(); + using var env = new EnvScope(); + env.Set("LIBCLANG_PATH", null); + (string dir, string version) = FakeSdk(tmp, withLibs: false); + env.Set("RBMANAGER_VSDEVCMD", VsLayoutBat(tmp, "vs", + $"set WindowsSdkDir={dir}", $"set WindowsSDKVersion={version}")); + + using (var cap = new ConsoleCapture()) + { + Assert.Equal(1, Msvc.Enable("powershell")); + Assert.Equal("", cap.Out); + Assert.Contains("no Windows SDK libraries", cap.Err); + } + + using (var cap = new ConsoleCapture()) + { + Assert.Equal(1, Msvc.Exec(["cmd", "/c", "exit", "0"])); + Assert.Contains("Windows 11 SDK", cap.Err); + } + } + + [Fact] // case 98: VsDevCmd reporting no SDK change leaves the check inert + public void WindowsSdkLibsPresent_NoSdkVarsInDelta_True() => + Assert.True(Msvc.WindowsSdkLibsPresent( + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["PATH"] = @"C:\x", + })); + [Fact] // case 71: an argument with spaces survives as one argument public void Exec_QuotesArgumentWithSpaces() {