Fixes for GitHub issues 964 and 1084

.vscode/launch.json

@@ -207,41 +207,6 @@
                 }
             }
         },
-        {
-            "name": "Launch Extension Tests (vs-preferred-gen)",
-            "type": "extensionHost",
-            "request": "launch",
-            "runtimeExecutable": "${execPath}",
-            "args": [
-                "${workspaceFolder}/test/extension-tests/vs-preferred-gen/project-folder",
-                "--extensionDevelopmentPath=${workspaceFolder}",
-                "--extensionTestsPath=${workspaceFolder}/out/test/extension-tests/vs-preferred-gen"
-            ],
-            "stopOnEntry": false,
-            "sourceMaps": true,
-            "outFiles": [
-                "${workspaceFolder}/out/*",
-                "${workspaceFolder}/out/src/*",
-                "${workspaceFolder}/out/test/*",
-                "${workspaceFolder}/out/test/extension-tests/vs-preferred-gen/*",
-                "${workspaceFolder}/out/test/extension-tests/vs-preferred-gen/test/*"
-            ],
-            "preLaunchTask": "build-tests-with-tsc-watch",
-            "windows": {
-                "env": {
-                    "CMT_TESTING": "1",
-                    "CMT_QUIET_CONSOLE": "1",
-                    "HasVs": "true"
-                }
-            },
-            "linux": {
-                "env": {
-                    "CMT_TESTING": "1",
-                    "CMT_QUIET_CONSOLE": "1",
-                    "HasVs": "false"
-                }
-            }
-        },
         {
             "type": "node",
             "request": "launch",

docs/kits.rst

@@ -113,9 +113,10 @@ command palette. The following process occurs to find available kits:
     CMake tools includes a bundled ``vswhere.exe`` which it uses to ask about
     existing Visual Studio instances installed on the system.
 
-    For each of ``x86``, ``amd64``, ``x86_amd64``, ``x86_arm``, ``amd64_arm``,
-    and ``amd64_x86``, CMake Tools will check for installed Visual C++
-    environments. A kit is generated for each existing MSVC toolchain.
+    For each of ``x86``, ``amd64``, ``x86_amd64``, ``x86_arm``, ``x86_arm64``,
+    ``amd64_x86``, ``amd64_arm``, and ``amd64_arm64``, CMake Tools will check
+    // for installed Visual C++ environments.
+    // A kit is generated for each existing MSVC toolchain.
 
 #. **Save results to the user-local kits file**
 

package.json

@@ -734,10 +734,7 @@
         },
         "cmake.preferredGenerators": {
           "type": "array",
-          "default": [
-            "Ninja",
-            "Unix Makefiles"
-          ],
+          "default": [],
           "description": "%cmake-tools.configuration.cmake.preferredGenerators.description%",
           "scope": "resource"
         },
@@ -1244,11 +1241,11 @@
   "devDependencies": {
     "@octokit/rest": "^16.28.9",
     "@types/ajv": "^0.0.3",
-    "@types/chai": "^4.0.4",
+    "@types/chai": "^4.2.11",
     "@types/chai-as-promised": "^7.1.0",
     "@types/chai-string": "^1.4.1",
     "@types/es6-promisify": "~5.0.0",
-    "@types/js-yaml": "^3.5.28",
+    "@types/js-yaml": "^3.12.3",
     "@types/json5": "~0.0.29",
     "@types/mocha": "~2.2.41",
     "@types/node": "~8.9.3",
@@ -1272,21 +1269,21 @@
     "mocha": "^4.1.0",
     "module-alias": "^2.2.1",
     "sinon": "~5.0.7",
-    "ts-loader": "^6.0.4",
-    "ts-node": "^6.0.0",
+    "ts-loader": "^6.2.2",
+    "ts-node": "^8.8.1",
     "tslint": "^5.9.1",
     "typedoc": "^0.15.3",
     "typescript": "^3.7.2",
     "vscode": "^1.1.36",
     "vscode-nls-dev": "^3.2.6",
-    "webpack": "^4.39.2",
+    "webpack": "^4.42.1",
     "webpack-cli": "^3.3.7"
   },
   "dependencies": {
     "ajv": "^4.7.5",
     "chokidar": "^3.3.0",
     "es6-promisify": "~5.0.0",
-    "handlebars": "^4.5.3",
+    "handlebars": "^4.7.5",
     "iconv-lite": "^0.4.21",
     "js-yaml": "^3.6.1",
     "json5": "^0.5.1",
@@ -1295,7 +1292,7 @@
     "tmp": "^0.0.33",
     "vscode-cpptools": "^3.0.1",
     "vscode-extension-telemetry": "^0.1.2",
-    "vscode-nls": "^4.1.1",
+    "vscode-nls": "^4.1.2",
     "which": "~1.3.0",
     "xml2js": "^0.4.17"
   },

src/drivers/driver.ts

@@ -331,6 +331,13 @@ export abstract class CMakeDriver implements vscode.Disposable {
     if (kit.preferredGenerator)
       preferredGenerators.push(kit.preferredGenerator);
 
+    // If no preferred generator is defined by the current kit or the user settings,
+    // it's time to consider the defaults.
+    if (preferredGenerators.length === 0) {
+      preferredGenerators.push({name: "Ninja"});
+      preferredGenerators.push({name: "Unix Makefiles"});
+    }
+
     // Use the "best generator" selection logic only if the user did not define already
     // in settings (via "cmake.generator") a particular generator to be used.
     if (this.config.generator) {
@@ -541,6 +548,7 @@ export abstract class CMakeDriver implements vscode.Disposable {
   async findBestGenerator(preferredGenerators: CMakeGenerator[]): Promise<CMakeGenerator|null> {
     log.debug(localize('trying.to.detect.generator', 'Trying to detect generator supported by system'));
     const platform = process.platform;
+
     for (const gen of preferredGenerators) {
       const gen_name = gen.name;
       const generator_present = await (async(): Promise<boolean> => {

src/kit.ts

@@ -430,8 +430,32 @@ export function vsDisplayName(inst: VSInstallation): string {
  * @param inst The VSInstallation to use
  * @param arch The desired architecture (e.g. x86, amd64)
  */
-function kitName(inst: VSInstallation, arch: string): string {
-  return `${vsDisplayName(inst)} - ${arch}`;
+function kitName(inst: VSInstallation, hostArch: string, targetArch: string): string {
+  // We still keep the amd64 alias for x64, only in the name of the detected VS kits,
+  // for compatibility reasons. Switching to 'x64' means leaving
+  // orphaned 'amd64' kits around ("Scan for kits" does not delete them yet)
+  // and also it may require a new kit selection.
+  // VS toolsets paths on disk, vcvarsall.bat parameters and CMake arguments are all x64 now.
+  // We can revise later whether to change to 'x64' in the VS kit name as well and how to mitigate it.
+  if (hostArch === "x64") {
+    hostArch = "amd64";
+  }
+
+  if (targetArch === "x64") {
+    targetArch = "amd64";
+  }
+
+  return `${vsDisplayName(inst)} - ${kitHostTargetArch(hostArch, targetArch)}`;
+}
+
+/**
+ * Create the host-target arch specification of a VS install,
+ * from the VS kit architecture (host) and generator platform (target).
+ * @param hostArch The architecture of the host toolset
+ * @param targetArch The architecture of the target
+ */
+function kitHostTargetArch(hostArch: string, targetArch: string): string {
+  return hostArch === targetArch ? hostArch : hostArch.concat("_").concat(targetArch);
 }
 
 /**
@@ -525,12 +549,12 @@ async function collectDevBatVars(devbat: string, args: string[], major_version:
 }
 
 /**
- * Platform arguments for VS Generators
+ * Platform arguments for VS Generators.
+ * Currently, there is a mismatch only between x86 and win32.
+ * For example, VS kits x86 and amd64_x86 will generate -A win32
  */
-const VsArchitectures: {[key: string]: string} = {
-  amd64: 'x64',
-  arm: 'ARM',
-  amd64_arm: 'ARM',
+const genPlatformFromVsHostTargetArchs: {[key: string]: string} = {
+  x86: 'win32'
 };
 
 /**
@@ -547,18 +571,38 @@ const VsGenerators: {[key: string]: string} = {
   16: 'Visual Studio 16 2019'
 };
 
-async function varsForVSInstallation(inst: VSInstallation, arch: string): Promise<Map<string, string>|null> {
-  console.log(`varsForVSInstallation path:'${inst.installationPath}' version:${inst.installationVersion} arch:${arch}`);
+async function varsForVSInstallation(inst: VSInstallation, hostArch: string, targetArch: string): Promise<Map<string, string>|null> {
+  console.log(`varsForVSInstallation path:'${inst.installationPath}' version:${inst.installationVersion} host arch:${hostArch} - target arch:${targetArch}`);
+
+  // TODO: if needed, cover also the scenario of VS older than 2017,
+  // which has vcvarsall.bat in a different location (Program Files x86\Microsoft Visual Studio [ver]\VC)
+  // and which is not recognizing x64 alias for amd64.
   const common_dir = path.join(inst.installationPath, 'Common7', 'Tools');
   let devbat = path.join(inst.installationPath, 'VC', 'Auxiliary', 'Build', 'vcvarsall.bat');
   const majorVersion = parseInt(inst.installationVersion);
   if (majorVersion < 15) {
     devbat = path.join(inst.installationPath, 'VC', 'vcvarsall.bat');
   }
-  const variables = await collectDevBatVars(devbat, [`${arch}`], majorVersion, common_dir);
+  const variables = await collectDevBatVars(devbat, [`${kitHostTargetArch(hostArch, targetArch)}`], majorVersion, common_dir);
   if (!variables) {
     return null;
   } else {
+    // When invoked for arm or arm64, the vcvarsall.bat script creates an x86 environment,
+    // when the arm(64) components are not installed.
+    // The way to differentiate between the two cases is to check that the first path
+    // in the PATH environment variable ends with the target architecture.
+    // The logic applies to any new future target architectures and also to x86/amd64,
+    // even if they are always installed by default.
+    let pathStr: string | undefined = variables.get('PATH');
+    if (!pathStr) {
+      return null;
+    }
+
+    pathStr = pathStr.split(";")[0].toLowerCase();
+    if (!pathStr.endsWith(targetArch)) {
+      return null;
+    }
+
     // This is a very *hacky* and sub-optimal solution, but it
     // works for now. This *helps* CMake make the right decision
     // when you have the release and pre-release edition of the same
@@ -597,22 +641,23 @@ async function varsForVSInstallation(inst: VSInstallation, arch: string): Promis
 /**
  * Try to get a VSKit from a VS installation and architecture
  * @param inst A VS installation from vswhere
- * @param arch The architecture to try
+ * @param hostTargetArch The host-target architecture combination to try
  */
-async function tryCreateNewVCEnvironment(inst: VSInstallation, arch: string, pr?: ProgressReporter): Promise<Kit|null> {
-  const name = kitName(inst, arch);
+async function tryCreateNewVCEnvironment(inst: VSInstallation, hostArch: string, targetArch: string, pr?: ProgressReporter): Promise<Kit|null> {
+  const name = kitName(inst, hostArch, targetArch);
   log.debug(localize('checking.for.kit', 'Checking for kit: {0}', name));
   if (pr) {
     pr.report({message: localize('checking', 'Checking {0}', name)});
   }
-  const variables = await varsForVSInstallation(inst, arch);
-  if (!variables)
+  const variables = await varsForVSInstallation(inst, hostArch, targetArch);
+  if (!variables) {
     return null;
+  }
 
   const kit: Kit = {
     name,
     visualStudio: kitVSName(inst),
-    visualStudioArchitecture: arch,
+    visualStudioArchitecture: hostArch
   };
 
   const version = /^(\d+)+./.exec(inst.installationVersion);
@@ -626,7 +671,9 @@ async function tryCreateNewVCEnvironment(inst: VSInstallation, arch: string, pr?
       log.debug(` ${localize('generator.present', 'Generator Present: {0}', generatorName)}`);
       kit.preferredGenerator = {
         name: generatorName,
-        platform: VsArchitectures[arch] as string || undefined,
+        platform: genPlatformFromVsHostTargetArchs[targetArch] as string || targetArch,
+        // CMake generator toolsets support also different versions (via -T version=).
+        toolset: "host=" + hostArch
       };
     }
     log.debug(` ${localize('selected.preferred.generator.name', 'Selected Preferred Generator Name: {0} {1}', generatorName, JSON.stringify(kit.preferredGenerator))}`);
@@ -642,12 +689,24 @@ export async function scanForVSKits(pr?: ProgressReporter): Promise<Kit[]> {
   const installs = await vsInstallations();
   const prs = installs.map(async(inst): Promise<Kit[]> => {
     const ret = [] as Kit[];
-    const arches = ['x86', 'amd64', 'x86_amd64', 'x86_arm', 'amd64_arm', 'amd64_x86'];
-    const sub_prs = arches.map(arch => tryCreateNewVCEnvironment(inst, arch, pr));
+    const hostArches: string[] = ['x86', 'x64'];
+    const targetArches: string[] = ['x86', 'x64', 'arm', 'arm64'];
+
+    const sub_prs: Promise<Kit | null>[] = [];
+    hostArches.forEach(hostArch => {
+      targetArches.forEach(targetArch => {
+        const kit: Promise<Kit | null> = tryCreateNewVCEnvironment(inst, hostArch, targetArch, pr);
+        if (kit) {
+          sub_prs.push(kit);
+        }
+      });
+    });
+
     const maybe_kits = await Promise.all(sub_prs);
     maybe_kits.map(k => k ? ret.push(k) : null);
     return ret;
-  });
+});
+
   const vs_kits = await Promise.all(prs);
   return ([] as Kit[]).concat(...vs_kits);
 }
@@ -709,7 +768,8 @@ export async function getVSKitEnvironment(kit: Kit): Promise<Map<string, string>
   if (!requested) {
     return null;
   }
-  return varsForVSInstallation(requested, kit.visualStudioArchitecture!);
+
+  return varsForVSInstallation(requested, kit.visualStudioArchitecture!, kit.preferredGenerator?.platform!);
 }
 
 export async function effectiveKitEnvironment(kit: Kit, opts?: expand.ExpansionOptions): Promise<Map<string, string>> {
@@ -851,7 +911,8 @@ export async function descriptionForKit(kit: Kit): Promise<string> {
   if (kit.visualStudio) {
     const inst = await getVSInstallForKit(kit);
     if (inst) {
-      return localize('using.compilers.for', 'Using compilers for {0} ({1} architecture)', vsVersionName(inst), kit.visualStudioArchitecture);
+      const hostTargetArch = kitHostTargetArch(kit.visualStudioArchitecture!, kit.preferredGenerator?.platform!);
+      return localize('using.compilers.for', 'Using compilers for {0} ({1} architecture)', vsVersionName(inst), hostTargetArch);
     }
     return '';
   }

test/extension-tests/successful-build/test/configure-and-build.test.ts

@@ -179,15 +179,29 @@ suite('Build', async () => {
     // Run Configure kit without preferred generator
     testEnv.kitSelection.defaultKitLabel = compiler[1].kitLabel;
     await cmt.setKit(await getMatchingProjectKit(compiler[1].kitLabel, testEnv.projectFolder.location));
-    expect(cmt.activeKit).to.be.null;
+    await cmt.build();
+    // Keep result1 for a later comparison
+    const result1 = await testEnv.result.getResultAsJson();
 
-    // Test return to workin kit
+    // Test return to previous kit
     testEnv.kitSelection.defaultKitLabel = compiler[0].kitLabel;
     await cmt.setKit(await getMatchingProjectKit(compiler[0].kitLabel, testEnv.projectFolder.location));
     await cmt.build();
 
-    const result1 = await testEnv.result.getResultAsJson();
-    expect(result1['cmake-generator']).to.eql(compiler[0].generator);
+    const result2 = await testEnv.result.getResultAsJson();
+    expect(result2['cmake-generator']).to.eql(compiler[0].generator);
+
+    // result1 (for no preferred generator given) should be the same as
+    // a list of default preferred generators: Ninja + Unix Makefiles.
+    // These defaults take effect only when no other preferred generator
+    // is deduced from other sources: settings (cmake.generator, cmake.preferredGenerators)
+    // or kits preferred generator in cmake-tools-kits.json.
+    testEnv.config.updatePartial({preferredGenerators: ["Ninja", "Unix Makefiles"]});
+    testEnv.kitSelection.defaultKitLabel = compiler[1].kitLabel;
+    await cmt.setKit(await getMatchingProjectKit(compiler[1].kitLabel, testEnv.projectFolder.location));
+    await cmt.build();
+    const result3 = await testEnv.result.getResultAsJson();
+    expect(result1['cmake-generator']).to.eql(result3['cmake-generator']);
   }).timeout(100000);
 
   test('Test kit switch between different preferred generators and compilers',

test/extension-tests/successful-build/test/preferred-generators.test.ts

@@ -310,19 +310,19 @@ KITS_BY_PLATFORM[workername].forEach(buildSystem => {
                .to.be.contains('Unable to determine what CMake generator to use.');
          });
 
-    // This test is NOT valid for kits which have any preferred generator defined
-    // since we expect CMT to reject the build.
-    test(`Reject if all \'preferredGenerators\' fields are empty (${buildSystem.defaultKit})`,
+    // This test verifies that kits without any preferred generator defined
+    // are still able to pick one preferred generator from the default list
+    // (Ninja + Unix Makefiles)
+    test(`Select default if all \'preferredGenerators\' fields are empty (${buildSystem.defaultKit})`,
          async function(this: ITestCallbackContext) {
            skipTestIf({preferredGeneratorIsAvailable: true}, this, context);
            this.timeout(BUILD_TIMEOUT);
-
            context.testEnv.config.updatePartial({preferredGenerators: []});
-           await expect(context.cmt.build()).to.eventually.be.rejected;
-
-           expect(context.testEnv.errorMessagesQueue.length).to.gte(1);
-           expect(context.testEnv.errorMessagesQueue[0])
-               .to.be.contains('Unable to determine what CMake generator to use.');
+           expect(await context.cmt.build()).to.eql(0);
+           const result = await context.testEnv.result.getResultAsJson();
+           expect(result['cmake-generator']).to.match(buildSystem.expectedDefaultGenerator);
+           expect(context.testEnv.errorMessagesQueue.length)
+               .to.eql(0, 'Wrong message ' + context.testEnv.errorMessagesQueue[0]);
          });
 
     test(`Use preferred generator from settings or kit file (${buildSystem.defaultKit})`,