Skip to content

Detect Visual Studio when only VisualStudioVersion is set - #3388

Open
PerryLink wants to merge 2 commits into
nodejs:mainfrom
PerryLink:fix/visualstudio-version-fallback
Open

PerryLink wants to merge 2 commits into
nodejs:mainfrom
PerryLink:fix/visualstudio-version-fallback

Conversation

@PerryLink

Copy link
Copy Markdown

What

When node-gyp is started from a Visual Studio command prompt that exports VisualStudioVersion
but not VSCMD_VER, the VS discovery path reports that Visual Studio is not installed:

unknown version "undefined" found at C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC
You need to install the latest version of Visual Studio
Could not find any Visual Studio installation to use

This is the state described in issue #3276 (VCINSTALLDIR and VisualStudioVersion set,
VSCMD_VER absent). On machines where the Find-VisualStudio.cs PowerShell fallback is unavailable
(restricted language mode, execution policy, no VSSetup module) discovery then fails outright.

The change

lib/find-visualstudio.js:148 — fall back to the documented VisualStudioVersion variable when
VSCMD_VER is missing:

version: process.env.VSCMD_VER || process.env.VisualStudioVersion

Behaviour is unchanged when VSCMD_VER is present, and unchanged when neither variable is set.

Verification

Real VS2022 Build Tools 17.14.37314.3 on this machine, --no-powershell so the fallback cannot mask
the problem:

  • before: exit 1 with the three messages above;
  • after: exit 0, resolves VS2022 (17.0), v143, SDK 10.0.26100.0.

New unit test (run on a VS Command Prompt that only exports VisualStudioVersion) fails before and
passes after; test/test-find-visualstudio.js 44/44; eslint clean.

Note: when the PowerShell fallback is available, the same environment still eventually resolves VS
— so the visible symptom there is the misleading unknown version "undefined" line rather than a
hard failure. The patch removes the fast path's dependency on PowerShell either way.

DCO: commit is signed off, per CONTRIBUTING.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant