Fix Ninja env for VS 2015 and map toolsets to versions (#2524)

CHANGELOG.md

@@ -16,6 +16,8 @@ Bug Fixes:
 - Remove problematic environment variables from the debugger environment. [#2442](https://github.com/microsoft/vscode-cmake-tools/issues/2442)
 - Fix preferredGenerator "Watcom WMake" not working. [#2500](https://github.com/microsoft/vscode-cmake-tools/issues/2500)
 - Exclude environment variables from debugging if the values have newlines. [#2515](https://github.com/microsoft/vscode-cmake-tools/issues/2515)
+- Correctly configure the build environment when using VS 2015 and Ninja in CMakePresets.json. [#2516](https://github.com/microsoft/vscode-cmake-tools/issues/2516)
+- Select the correct VS toolset for Ninja generators with CMake Presets. [#2423](https://github.com/microsoft/vscode-cmake-tools/issues/2423)
 
 ## 1.10.5
 Bug Fixes:

src/installs/visual-studio.ts

@@ -176,14 +176,19 @@ export function getHostTargetArchString(hostArch: string, targetArch?: string, a
 }
 
 // Gets the MSVC toolsets installed for a given VS install.
-export async function enumerateMsvcToolsets(vsInstallRoot: string): Promise<string[] | undefined> {
-    const toolsetDir = path.join(vsInstallRoot, 'VC\\Tools\\MSVC');
-    if (await fs.exists(toolsetDir)) {
-        const dirContents = await fs.readdir(toolsetDir, { 'withFileTypes': true });
-        // Only the toolsets should be this directory (each in their own directories), but filter out anything else just in case.
-        // Sort in descending order, so if searching with a 1- or 2-component version (e.g. 14.27) we'll choose the latest version first
-        return dirContents.filter(item => item.isDirectory()).map(dir => dir.name)
-            .sort((a, b) => util.compareVersions(a, b)).reverse();
+export async function enumerateMsvcToolsets(vsInstallRoot: string, vsVersion: string): Promise<string[] | undefined> {
+    const version = parseInt(vsVersion);
+    if (version < 15) {
+        return [`${version}.0`];
+    } else {
+        const toolsetDir = path.join(vsInstallRoot, 'VC\\Tools\\MSVC');
+        if (await fs.exists(toolsetDir)) {
+            const dirContents = await fs.readdir(toolsetDir, { 'withFileTypes': true });
+            // Only the toolsets should be this directory (each in their own directories), but filter out anything else just in case.
+            // Sort in descending order, so if searching with a 1- or 2-component version (e.g. 14.27) we'll choose the latest version first
+            return dirContents.filter(item => item.isDirectory()).map(dir => dir.name)
+                .sort((a, b) => util.compareVersions(a, b)).reverse();
+        }
     }
 
     return undefined;
@@ -192,7 +197,7 @@ export async function enumerateMsvcToolsets(vsInstallRoot: string): Promise<stri
 // Filters the given vsInstalls to those which have the given toolset.
 export function filterVSInstallationsByMsvcToolset(vsInstalls: VSInstallation[], toolsetVersion: string): VSInstallation[] {
     return vsInstalls.filter(async vs => {
-        const availableToolsets = await enumerateMsvcToolsets(vs.installationPath);
+        const availableToolsets = await enumerateMsvcToolsets(vs.installationPath, vs.installationVersion);
         return availableToolsets?.find(t => t.startsWith(toolsetVersion));
     });
 }
@@ -452,7 +457,7 @@ export async function varsForVSInstallation(inst: VSInstallation, hostArch: stri
     }
 
     const devBatArgs = [getHostTargetArchString(hostArch, targetArch, majorVersion < 15)];
-    if (toolsetVersion) {
+    if (toolsetVersion && majorVersion >= 15) {
         devBatArgs.push(`-vcvars_ver=${toolsetVersion}`);
     }
     const variables = await collectDevBatVars(hostArch, devbat, devBatArgs, majorVersion, common_dir);

src/preset.ts

@@ -765,7 +765,7 @@ export async function expandConfigurePreset(folder: string, name: string, worksp
     return expandedPreset;
 }
 
-function getArchitecture(preset: ConfigurePreset) {
+export function getArchitecture(preset: ConfigurePreset) {
     if (util.isString(preset.architecture)) {
         return preset.architecture;
     } else if (preset.architecture && preset.architecture.value) {
@@ -775,7 +775,7 @@ function getArchitecture(preset: ConfigurePreset) {
     return 'x86';
 }
 
-function getToolset(preset: ConfigurePreset): Toolset {
+export function getToolset(preset: ConfigurePreset): Toolset {
     let result: Toolset | undefined;
     if (util.isString(preset.toolset)) {
         result = parseToolset(preset.toolset);
@@ -792,7 +792,7 @@ function getToolset(preset: ConfigurePreset): Toolset {
             log.warning(noToolsetArchWarning);
             result.host = 'x86';
         }
-        if (!result.version) {
+        if (!result.version && result.name !== latestToolsetName) {
             log.warning(localize('no.cl.toolset.version', 'Configure preset {0}: No toolset version specified for cl.exe, using latest by default', preset.name));
         }
     } else {
@@ -802,6 +802,18 @@ function getToolset(preset: ConfigurePreset): Toolset {
     return result;
 }
 
+const toolsetToVersion: { [key: string]: string } = {
+    'v100': '10.0',
+    'v110': '11.0',
+    'v120': '12.0',
+    'v140': '14.0',
+    'v141': '14.16',
+    'v142': '14.29'
+    // don't include the latest version - the compiler version changes frequently and it will be picked by default anyway.
+    // NOTE: the latest toolset name (below) should be kept up to date.
+};
+const latestToolsetName = 'v143';
+
 // We don't support all of these options for Kit lookup right now, but might in the future.
 function parseToolset(toolset: string): Toolset {
     const toolsetOptions = toolset.split(',');
@@ -809,7 +821,12 @@ function parseToolset(toolset: string): Toolset {
     const result: Toolset = {};
     for (const option of toolsetOptions) {
         if (option.indexOf('=') < 0) {
-            result.name = option;
+            const version = toolsetToVersion[option];
+            if (version) {
+                result.version = version;
+            } else {
+                result.name = option;
+            }
         } else {
             const keyValue = option.split('=');
             switch (keyValue[0].toLowerCase()) {
@@ -979,7 +996,7 @@ async function expandConfigurePresetHelper(folder: string, preset: ConfigurePres
                             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);
+                                    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;

test/unit-tests/presets.test.ts

@@ -1,7 +1,59 @@
-import { Condition, evaluateCondition } from '../../src/preset';
+import { Condition, evaluateCondition, getArchitecture, getToolset } from '../../src/preset';
 import { expect } from '@test/util';
 
 suite('Preset tests', () => {
+    test('Parse architecture', () => {
+        expect(getArchitecture({ name: 'test', architecture: 'x86' })).to.eq('x86');
+        expect(getArchitecture({ name: 'test', architecture: 'amd64' })).to.eq('amd64');
+        expect(getArchitecture({ name: 'test', architecture: { value: 'arm', strategy: 'set' } })).to.eq('arm');
+        expect(getArchitecture({ name: 'test', architecture: { value: 'arm64', strategy: 'external' } })).to.eq('arm64');
+        expect(getArchitecture({ name: 'test' })).to.eq('x86');
+        expect(getArchitecture({ name: 'test', architecture: 'bogus' })).to.eq('bogus');
+    });
+
+    test('Parse toolset', () => {
+        let toolset = getToolset({ name: 'test', toolset: 'v141' });
+        expect(toolset).to.not.contain.keys('name');
+        expect(toolset.version).to.eq('14.16');
+
+        toolset = getToolset({ name: 'test', toolset: 'v142' });
+        expect(toolset).to.not.contain.keys('name');
+        expect(toolset.version).to.eq('14.29');
+
+        toolset = getToolset({ name: 'test', toolset: { value: 'v140', strategy: 'set' } });
+        expect(toolset).to.not.contain.keys('name');
+        expect(toolset.version).to.eq('14.0');
+
+        toolset = getToolset({ name: 'test', toolset: { value: 'v143', strategy: 'external' } });
+        expect(toolset.name).to.eq('v143');
+        expect(toolset).to.not.contain.keys('version');
+
+        toolset = getToolset({ name: 'test', toolset: 'host=x86' });
+        expect(toolset.host).to.eq('x86');
+        toolset = getToolset({ name: 'test', toolset: 'host=x64' });
+        expect(toolset.host).to.eq('x64');
+        toolset = getToolset({ name: 'test', toolset: 'host=x86,version=14.31' });
+        expect(toolset.host).to.eq('x86');
+        expect(toolset.version).to.eq('14.31');
+        toolset = getToolset({ name: 'test', toolset: 'host=x64,version=14.0' });
+        expect(toolset.host).to.eq('x64');
+        expect(toolset.version).to.eq('14.0');
+        toolset = getToolset({ name: 'test', toolset: 'host=x64,version=14.31.12345' });
+        expect(toolset.host).to.eq('x64');
+        expect(toolset.version).to.eq('14.31.12345');
+        toolset = getToolset({ name: 'test', toolset: 'v143,host=arm,version=14.31.12345' });
+        expect(toolset.host).to.eq('arm');
+        expect(toolset.version).to.eq('14.31.12345');
+        toolset = getToolset({ name: 'test', toolset: 'v141,host=arm64,version=14.31.12345' }); // bogus, but testing the override
+        expect(toolset.host).to.eq('arm64');
+        expect(toolset.version).to.eq('14.31.12345');
+        toolset = getToolset({ name: 'test', toolset: 'v143,version=14.31.12345,host=x64,cuda=hey,vctargetspath=nope' });
+        expect(toolset.host).to.eq('x64');
+        expect(toolset.version).to.eq('14.31.12345');
+        expect(toolset.cuda).to.eq('hey');
+        expect(toolset.VCTargetsPath).to.eq('nope');
+    });
+
     test('Evaluate condition objects', () => {
         let condition: Condition = { type: 'const', value: true };
         expect(evaluateCondition(condition)).to.eql(true);