Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 59 additions & 3 deletions docs/msvc-enable.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
`<installationPath>\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 '<FILES>
<FILES>'`. 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:
`<WindowsSdkDir>\Lib\<WindowsSDKLibVersion>\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
Expand All @@ -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)

Expand Down Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions docs/test-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<root>\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.
Expand Down
79 changes: 77 additions & 2 deletions src/rbmanager/Msvc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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));
Expand All @@ -232,6 +238,55 @@ public static int Exec(string[] command, string? vsver = null)
return proc.ExitCode;
}

private static Dictionary<string, string> 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 <installationPath>\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<string, string> 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)
Expand Down Expand Up @@ -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
Expand Down
130 changes: 130 additions & 0 deletions tests/rbmanager.Tests/MsvcActivationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,37 @@ private static Dictionary<string, string> 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 (<installationPath>\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()
{
Expand Down Expand Up @@ -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<string, string>(StringComparer.OrdinalIgnoreCase)
{
["PATH"] = @"C:\x",
}));

[Fact] // case 71: an argument with spaces survives as one argument
public void Exec_QuotesArgumentWithSpaces()
{
Expand Down
Loading