Skip to content

Commit

Permalink
Be smarter about where we attempt magic PATH. (#3449)
Browse files Browse the repository at this point in the history 
* Be smarter about where we attempt magic PATH.

We attempt to, if we can't find their compiler for cl, clang, etc.
to try and provide paths for them from VS. However, we did this in a
location that got invoked for each level of expanding presets (preset
inheritance). This was causing issues.
Instead, we can just do this at the very end before we return the
preset.

* update changelog, remove stray TODO
  • Loading branch information
@gcampbell-msft
gcampbell-msft authored Nov 16, 2023
1 parent 9e5cd11 commit 8d4eb87
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 126 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# What's New?

## 1.16.31
Bug Fixes:
- Refactor our attempt to add VS paths to PATH for cl, clang, etc. so that we fix issues with using the wrong compiler. [PR #3449](https://github.com/microsoft/vscode-cmake-tools/pull/3449)

## 1.16.30
Bug Fixes:
- Fixed an issue where finding cl.exe and ninja from Visual Studio was broken. [PR #3445](https://github.com/microsoft/vscode-cmake-tools/pull/3445)
Expand Down
241 changes: 115 additions & 126 deletions src/preset.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,117 @@ export async function expandConfigurePreset(folder: string, name: string, worksp
// Other fields can be copied by reference for simplicity
merge(expandedPreset, preset);

let compilerEnv = EnvironmentUtils.createPreserveNull();
// [Windows Only] If CMAKE_CXX_COMPILER or CMAKE_C_COMPILER is set as cl, clang, clang-cl, clang-cpp and clang++,
// but they are not on PATH, then set the env automatically.
if (process.platform === 'win32') {
if (preset.cacheVariables) {
const cxxCompiler = getStringValueFromCacheVar(preset.cacheVariables['CMAKE_CXX_COMPILER'])?.toLowerCase();
const cCompiler = getStringValueFromCacheVar(preset.cacheVariables['CMAKE_C_COMPILER'])?.toLowerCase();
// The env variables for the supported compilers are the same.
const compilerName: string | undefined = util.isSupportedCompiler(cxxCompiler) || util.isSupportedCompiler(cCompiler);
if (compilerName) {
const compilerLocation = await execute('where.exe', [compilerName], null, {
environment: EnvironmentUtils.create(expandedPreset.environment),
silent: true,
encoding: 'utf8',
shell: true
}).result;
if (!compilerLocation.stdout) {
// Not on PATH, need to set env
const arch = getArchitecture(preset);
const toolset = getToolset(preset);

// Get version info for all VS instances.
const vsInstalls = await vsInstallations();

// The VS installation to grab developer environment from.
let vsInstall: VSInstallation | undefined;

// VS generators starting with Visual Studio 15 2017 support CMAKE_GENERATOR_INSTANCE.
// If supported, we should respect this value when defined. If not defined, we should
// set it to ensure CMake chooses the same VS instance as we use here.
// Note that if the user sets this in a toolchain file we won't know about it,
// which could cause configuration to fail. However the user can workaround this by launching
// vscode from the dev prompt of their desired instance.
// https://cmake.org/cmake/help/latest/variable/CMAKE_GENERATOR_INSTANCE.html
let vsGeneratorVersion: number | undefined;
const matches = preset.generator?.match(/Visual Studio (?<version>\d+)/);
if (matches && matches.groups?.version) {
vsGeneratorVersion = parseInt(matches.groups.version);
const useCMakeGeneratorInstance = !isNaN(vsGeneratorVersion) && vsGeneratorVersion >= 15;
const cmakeGeneratorInstance = getStringValueFromCacheVar(preset.cacheVariables['CMAKE_GENERATOR_INSTANCE']);
if (useCMakeGeneratorInstance && cmakeGeneratorInstance) {
const cmakeGeneratorInstanceNormalized = path.normalize(cmakeGeneratorInstance);
vsInstall = vsInstalls.find((vs) => vs.installationPath
&& path.normalize(vs.installationPath) === cmakeGeneratorInstanceNormalized);

if (!vsInstall) {
log.warning(localize('specified.vs.not.found',
"Configure preset {0}: Visual Studio instance specified by {1} was not found, falling back on default instance lookup behavior.",
preset.name, `CMAKE_GENERATOR_INSTANCE="${cmakeGeneratorInstance}"`));
}
}
}

// If VS instance wasn't chosen using CMAKE_GENERATOR_INSTANCE, look up a matching instance
// that supports the specified toolset.
if (!vsInstall) {
// sort VS installs in order of descending version. This ensures we choose the latest supported install first.
vsInstalls.sort((a, b) => -compareVersions(a.installationVersion, b.installationVersion));

for (const vs of vsInstalls) {
// Check for existence of vcvars script to determine whether desired host/target architecture is supported.
// toolset.host will be set by getToolset.
if (await getVcVarsBatScript(vs, toolset.host!, arch)) {
// If a toolset version is specified then check to make sure this vs instance has it installed.
if (toolset.version) {
const availableToolsets = await enumerateMsvcToolsets(vs.installationPath, vs.installationVersion);
// forcing non-null due to false positive (toolset.version is checked in conditional)
if (availableToolsets?.find(t => t.startsWith(toolset.version!))) {
vsInstall = vs;
break;
}
} else if (!vsGeneratorVersion || vs.installationVersion.startsWith(vsGeneratorVersion.toString())) {
// If no toolset version specified then choose the latest VS instance for the given generator
vsInstall = vs;
break;
}
}
}
}

if (!vsInstall) {
log.error(localize('specified.cl.not.found',
"Configure preset {0}: Compiler {1} with toolset {2} and architecture {3} was not found, you may need to run the 'CMake: Scan for Compilers' command if this toolset exists on your computer.",
preset.name, `"${compilerName}.exe"`, toolset.version ? `"${toolset.version},${toolset.host}"` : `"${toolset.host}"`, `"${arch}"`));
} else {
log.info(localize('using.vs.instance', "Using developer environment from Visual Studio (instance {0}, version {1}, installed at {2})", vsInstall.instanceId, vsInstall.installationVersion, `"${vsInstall.installationPath}"`));
const vsEnv = await varsForVSInstallation(vsInstall, toolset.host!, arch, toolset.version);
compilerEnv = vsEnv ?? EnvironmentUtils.create();

// if ninja isn't on path, try to look for it in a VS install
const ninjaLoc = await execute('where.exe', ['ninja'], null, {
environment: EnvironmentUtils.create(preset.environment),
silent: true,
encoding: 'utf8',
shell: true
}).result;
if (!ninjaLoc.stdout) {
const vsCMakePaths = await paths.vsCMakePaths(vsInstall.instanceId);
if (vsCMakePaths.ninja) {
log.warning(localize('ninja.not.set', 'Ninja is not set on PATH, trying to use {0}', vsCMakePaths.ninja));
compilerEnv['PATH'] = `${path.dirname(vsCMakePaths.ninja)};${compilerEnv['PATH']}`;
}
}

expandedPreset.environment = EnvironmentUtils.mergePreserveNull([expandedPreset.environment, compilerEnv]);
}
}
}
}
}

return expandedPreset;
}

Expand Down Expand Up @@ -840,21 +951,21 @@ function parseToolset(toolset: string): Toolset {
async function expandConfigurePresetImpl(folder: string, name: string, workspaceFolder: string, sourceDir: string, allowUserPreset: boolean = false): Promise<ConfigurePreset | null> {
let preset = getPresetByName(configurePresets(folder), name);
if (preset) {
return expandConfigurePresetHelper(folder, name, preset, workspaceFolder, sourceDir);
return expandConfigurePresetHelper(folder, preset, workspaceFolder, sourceDir);
}

if (allowUserPreset) {
preset = getPresetByName(userConfigurePresets(folder), name);
if (preset) {
return expandConfigurePresetHelper(folder, name, preset, workspaceFolder, sourceDir, true);
return expandConfigurePresetHelper(folder, preset, workspaceFolder, sourceDir, true);
}
}

log.error(localize('config.preset.not.found', 'Could not find configure preset with name {0}', name));
return null;
}

async function expandConfigurePresetHelper(folder: string, name: string, preset: ConfigurePreset, workspaceFolder: string, sourceDir: string, allowUserPreset: boolean = false) {
async function expandConfigurePresetHelper(folder: string, preset: ConfigurePreset, workspaceFolder: string, sourceDir: string, allowUserPreset: boolean = false) {
if (preset.__expanded) {
return preset;
}
Expand Down Expand Up @@ -919,129 +1030,7 @@ async function expandConfigurePresetHelper(folder: string, name: string, preset:
}

inheritedEnv = EnvironmentUtils.mergePreserveNull([process.env, inheritedEnv]);

let compilerEnv = EnvironmentUtils.createPreserveNull();
const expansionOpts: ExpansionOptions = await getExpansionOptions(workspaceFolder, sourceDir, preset);
expansionOpts.envOverride = inheritedEnv;

// [Windows Only] If CMAKE_CXX_COMPILER or CMAKE_C_COMPILER is set as cl, clang, clang-cl, clang-cpp and clang++,
// but they are not on PATH, then set the env automatically.
if (process.platform === 'win32') {
if (preset.cacheVariables) {
const cxxCompiler = getStringValueFromCacheVar(preset.cacheVariables['CMAKE_CXX_COMPILER'])?.toLowerCase();
const cCompiler = getStringValueFromCacheVar(preset.cacheVariables['CMAKE_C_COMPILER'])?.toLowerCase();
// The env variables for the supported compilers are the same.
const compilerName: string | undefined = util.isSupportedCompiler(cxxCompiler) || util.isSupportedCompiler(cCompiler);
const expandedPreset: ConfigurePreset = { name };
const expansionOpts: ExpansionOptions = await getExpansionOptions(workspaceFolder, sourceDir, preset);
if (compilerName) {
expandedPreset.environment = EnvironmentUtils.mergePreserveNull([inheritedEnv, preset.environment]);
for (const key in expandedPreset.environment) {
if (expandedPreset.environment[key]) {
expandedPreset.environment[key] = await expandString(expandedPreset.environment[key]!, expansionOpts);
}
}
const compilerLocation = await execute('where.exe', [compilerName], null, {
environment: EnvironmentUtils.create(expandedPreset.environment),
silent: true,
encoding: 'utf8',
shell: true
}).result;
if (!compilerLocation.stdout) {
// Not on PATH, need to set env
const arch = getArchitecture(preset);
const toolset = getToolset(preset);

// Get version info for all VS instances.
const vsInstalls = await vsInstallations();

// The VS installation to grab developer environment from.
let vsInstall: VSInstallation | undefined;

// VS generators starting with Visual Studio 15 2017 support CMAKE_GENERATOR_INSTANCE.
// If supported, we should respect this value when defined. If not defined, we should
// set it to ensure CMake chooses the same VS instance as we use here.
// Note that if the user sets this in a toolchain file we won't know about it,
// which could cause configuration to fail. However the user can workaround this by launching
// vscode from the dev prompt of their desired instance.
// https://cmake.org/cmake/help/latest/variable/CMAKE_GENERATOR_INSTANCE.html
let vsGeneratorVersion: number | undefined;
const matches = preset.generator?.match(/Visual Studio (?<version>\d+)/);
if (matches && matches.groups?.version) {
vsGeneratorVersion = parseInt(matches.groups.version);
const useCMakeGeneratorInstance = !isNaN(vsGeneratorVersion) && vsGeneratorVersion >= 15;
const cmakeGeneratorInstance = getStringValueFromCacheVar(preset.cacheVariables['CMAKE_GENERATOR_INSTANCE']);
if (useCMakeGeneratorInstance && cmakeGeneratorInstance) {
const cmakeGeneratorInstanceNormalized = path.normalize(cmakeGeneratorInstance);
vsInstall = vsInstalls.find((vs) => vs.installationPath
&& path.normalize(vs.installationPath) === cmakeGeneratorInstanceNormalized);

if (!vsInstall) {
log.warning(localize('specified.vs.not.found',
"Configure preset {0}: Visual Studio instance specified by {1} was not found, falling back on default instance lookup behavior.",
preset.name, `CMAKE_GENERATOR_INSTANCE="${cmakeGeneratorInstance}"`));
}
}
}

// If VS instance wasn't chosen using CMAKE_GENERATOR_INSTANCE, look up a matching instance
// that supports the specified toolset.
if (!vsInstall) {
// sort VS installs in order of descending version. This ensures we choose the latest supported install first.
vsInstalls.sort((a, b) => -compareVersions(a.installationVersion, b.installationVersion));

for (const vs of vsInstalls) {
// Check for existence of vcvars script to determine whether desired host/target architecture is supported.
// toolset.host will be set by getToolset.
if (await getVcVarsBatScript(vs, toolset.host!, arch)) {
// If a toolset version is specified then check to make sure this vs instance has it installed.
if (toolset.version) {
const availableToolsets = await enumerateMsvcToolsets(vs.installationPath, vs.installationVersion);
// forcing non-null due to false positive (toolset.version is checked in conditional)
if (availableToolsets?.find(t => t.startsWith(toolset.version!))) {
vsInstall = vs;
break;
}
} else if (!vsGeneratorVersion || vs.installationVersion.startsWith(vsGeneratorVersion.toString())) {
// If no toolset version specified then choose the latest VS instance for the given generator
vsInstall = vs;
break;
}
}
}
}

if (!vsInstall) {
log.error(localize('specified.cl.not.found',
"Configure preset {0}: Compiler {1} with toolset {2} and architecture {3} was not found, you may need to run the 'CMake: Scan for Compilers' command if this toolset exists on your computer.",
preset.name, `"${compilerName}.exe"`, toolset.version ? `"${toolset.version},${toolset.host}"` : `"${toolset.host}"`, `"${arch}"`));
} else {
log.info(localize('using.vs.instance', "Using developer environment from Visual Studio (instance {0}, version {1}, installed at {2})", vsInstall.instanceId, vsInstall.installationVersion, `"${vsInstall.installationPath}"`));
const vsEnv = await varsForVSInstallation(vsInstall, toolset.host!, arch, toolset.version);
compilerEnv = vsEnv ?? EnvironmentUtils.create();

// if ninja isn't on path, try to look for it in a VS install
const ninjaLoc = await execute('where.exe', ['ninja'], null, {
environment: EnvironmentUtils.create(preset.environment),
silent: true,
encoding: 'utf8',
shell: true
}).result;
if (!ninjaLoc.stdout) {
const vsCMakePaths = await paths.vsCMakePaths(vsInstall.instanceId);
if (vsCMakePaths.ninja) {
log.warning(localize('ninja.not.set', 'Ninja is not set on PATH, trying to use {0}', vsCMakePaths.ninja));
compilerEnv['PATH'] = `${path.dirname(vsCMakePaths.ninja)};${compilerEnv['PATH']}`;
}
}
}
}
}
}
}

compilerEnv = EnvironmentUtils.mergePreserveNull([inheritedEnv, compilerEnv]);
preset.environment = EnvironmentUtils.mergePreserveNull([compilerEnv, preset.environment]);
preset.environment = EnvironmentUtils.mergePreserveNull([inheritedEnv, preset.environment]);

preset.__expanded = true;
return preset;
Expand Down

0 comments on commit 8d4eb87

Please sign in to comment.