RegisterQt: Check if Ninja exists on the system

* If the system has Ninja, use it instead of ninja inside qtFolder * Check existence of Ninja in qtFolder before adding to kits * Reduce code repetition * Add `command-exists` module * Remove CMAKE_MAKE_PROGRAM because it causes a bug when `cmake.generator` is selected different than `Ninja`. Adding `Ninja` path in `qtFolder` is enough for that case. Change-Id: If7c25d54530373dcfd873f2ee0b3b65d7988028e Reviewed-by: Marcus Tillmanns <[email protected]>
qt-labs · Mar 4, 2024 · fc4ad41 · fc4ad41
1 parent f613f7f
commit fc4ad41
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 90 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -281,6 +281,7 @@
     "@types/mocha": "^10.0.1",
     "@types/node": "^20.2.5",
     "@types/vscode": "^1.78.0",
+    "@types/command-exists": "^1.2.3",
     "@typescript-eslint/eslint-plugin": "^6.7.3",
     "@typescript-eslint/parser": "^6.7.3",
     "@vscode/l10n-dev": "^0.0.30",
@@ -297,6 +298,7 @@
     "typescript": "^5.2.2"
   },
   "dependencies": {
+    "command-exists": "^1.2.3",
     "@vscode/l10n": "^0.0.16",
     "command-exists": "^1.2.3",
     "ts-sinon": "^2.0.2",

diff --git a/src/commands/detect-qt-cmake.ts b/src/commands/detect-qt-cmake.ts
@@ -8,6 +8,7 @@ import * as fs from 'fs/promises';
 import * as vscode from 'vscode';
 import { CMakeKitFiles as cmake, Kit } from '../util/cmake-kit-files';
 import * as qtpath from '../util/get-qt-paths';
+import commandExists = require('command-exists');
 
 export let QtCMakeKits: cmake;
 
@@ -25,15 +26,15 @@ async function* generateCMakeKitsOfQtInstallationPath(
 
   const qtRootDir = qtpath.qtRootByQtInstallation(installation);
   const promiseMingwPath = qtpath.locateMingwBinDirPath(qtRootDir);
-  const promiseNinjaExecutable = qtpath.locateNinjaExecutable(qtRootDir);
-
-  const toolchain = path.basename(installation);
-
-  const ninjaExePath = await promiseNinjaExecutable;
-  const qtPathEnv = qtpath.envPathForQtInstallationWithNinja(
-    installation,
-    ninjaExePath
-  );
+  let qtPathEnv = qtpath.generateEnvPathForQtInstallation(installation);
+  let locatedNinjaExePath = '';
+  if (!commandExists.sync('ninja')) {
+    const promiseNinjaExecutable = qtpath.locateNinjaExecutable(qtRootDir);
+    locatedNinjaExePath = await promiseNinjaExecutable;
+  }
+  if (locatedNinjaExePath) {
+    qtPathEnv += path.delimiter + path.dirname(locatedNinjaExePath);
+  }
 
   let newKit: Kit = {
     name: qtpath.mangleQtInstallation(installation),
@@ -43,17 +44,14 @@ async function* generateCMakeKitsOfQtInstallationPath(
     isTrusted: true,
     preferredGenerator: {
       name: cmake.CMakeDefaultGenerator
-    },
-    cmakeSettings: {
-      CMAKE_MAKE_PROGRAM: ninjaExePath
     }
   };
 
   const toolchainFilePath = await promiseCmakeQtToolchainPath;
   if (toolchainFilePath) {
     newKit.toolchainFile = toolchainFilePath;
   }
-
+  const toolchain = path.basename(installation);
   const tokens = toolchain.split('_');
   let platform = tokens[0];
   if (platform != 'android') {

diff --git a/src/commands/register-qt-path.ts b/src/commands/register-qt-path.ts
@@ -181,9 +181,9 @@ export async function getSelectedQtInstallationPath(): Promise<string> {
   if (selectedQtKit.environmentVariables.PATH === undefined) {
     throw new Error('Selected Qt installation path not found');
   }
-  const pathSeperator = IsWindows ? ';' : ':';
-  selectedQtKitPath =
-    selectedQtKit.environmentVariables.PATH.split(pathSeperator)[0];
+  selectedQtKitPath = selectedQtKit.environmentVariables.PATH.split(
+    path.delimiter
+  )[0];
 
   if (!fs.existsSync(selectedQtKitPath)) {
     throw new Error('Selected Qt installation path does not exist');

diff --git a/src/util/cmake-kit-files.ts b/src/util/cmake-kit-files.ts
@@ -240,8 +240,7 @@ export class CMakeKitFiles {
   static async loadCMakeKitsFileJSON(): Promise<Kit[]> {
     const data = await fs.readFile(this.CMAKE_KITS_FILEPATH);
     const stringData = data.toString();
-    const json: unknown = JSON.parse(stringData);
-    const kits = json as Kit[];
+    const kits = JSON.parse(stringData) as Kit[];
     return kits;
   }
 

diff --git a/src/util/get-qt-paths.ts b/src/util/get-qt-paths.ts
@@ -7,6 +7,7 @@ import { Home, IsMacOS, IsWindows } from './os';
 import * as path from 'path';
 import * as fsutil from './fs';
 import * as vscode from 'vscode';
+import commandExists = require('command-exists');
 
 export const PlatformExecutableExtension = IsWindows ? '.exe' : '';
 export const QmakeFileName = 'qmake' + PlatformExecutableExtension;
@@ -152,51 +153,32 @@ export async function locateQmakeExeFilePath(selectedQtPath: string) {
 }
 
 export async function locateNinjaExecutable(qtRootDir: string) {
-  const ninjaDirPath = path.join(qtToolsDirByQtRootDir(qtRootDir), 'Ninja');
-  const ninjaExePath = path.join(ninjaDirPath, NinjaFileName);
-  try {
-    await fs.access(ninjaExePath);
-    return ninjaExePath;
-  } catch (err) {
-    // Do nothing
-  }
-
+  const pathsToCheck = [
+    path.join(qtToolsDirByQtRootDir(qtRootDir), 'Ninja', NinjaFileName)
+  ];
   const vs2022dir = process.env.VS2022INSTALLDIR;
   if (vs2022dir) {
-    const vsNinjaExecutable = path.join(
-      vs2022dir,
-      'Common7',
-      'IDE',
-      'CommonExtensions',
-      'Microsoft',
-      'CMake',
-      'Ninja',
-      NinjaFileName
-    );
-    try {
-      await fs.access(vsNinjaExecutable);
-      return vsNinjaExecutable;
-    } catch (err) {
-      // Do nothing
-    }
-
-    const visualStudioAndroidNinjaExecutable = path.join(
-      vs2022dir,
-      'MSBuild',
-      'Google',
-      'Android',
-      'bin',
-      NinjaFileName
+    pathsToCheck.push(
+      path.join(
+        vs2022dir,
+        'Common7',
+        'IDE',
+        'CommonExtensions',
+        'Microsoft',
+        'CMake',
+        'Ninja',
+        NinjaFileName
+      ),
+      path.join(vs2022dir, 'MSBuild', 'Google', 'Android', 'bin', NinjaFileName)
     );
-    try {
-      await fs.access(visualStudioAndroidNinjaExecutable);
-      return visualStudioAndroidNinjaExecutable;
-    } catch (err) {
-      // Do nothing
+  }
+  for (const path of pathsToCheck) {
+    if (await fsutil.exists(path)) {
+      return path;
     }
   }
 
-  return ninjaExePath;
+  return '';
 }
 
 export async function locateMingwBinDirPath(qtRootDir: string) {
@@ -247,19 +229,14 @@ export function qtRootByQtInstallation(installation: string) {
   return path.normalize(path.join(installation, '..', '..'));
 }
 
-export function envPathForQtInstallationWithNinja(
-  installation: string,
-  ninjaExePath: string
-) {
+export function generateEnvPathForQtInstallation(installation: string) {
   const qtRootDir = qtRootByQtInstallation(installation);
   const cmakeDirPath = locateCMakeExecutableDirectoryPath(qtRootDir);
-  const ninjaDirPath = path.dirname(ninjaExePath);
   const installationBinDir = path.join(installation, 'bin');
   const QtPathAddition = [
     installation,
     installationBinDir,
     '${env:PATH}',
-    ninjaDirPath,
     cmakeDirPath
   ].join(path.delimiter);
   return QtPathAddition;
@@ -270,32 +247,29 @@ export async function locateJomExecutable(qtRootDir: string) {
   const jomDirPath = path.join(qtToolsDir, 'QtCreator', 'bin', 'jom');
   const jomFileName = 'jom' + PlatformExecutableExtension;
   const jomExePath = path.join(jomDirPath, jomFileName);
-  try {
-    await fs.access(jomExePath);
-    return jomExePath;
-  } catch (err) {
-    // Do nothing
-  }
-  try {
-    await fs.access(jomExePath);
+
+  if (await fsutil.exists(jomExePath)) {
     return jomExePath;
-  } catch (err) {
-    return '';
   }
+  return '';
 }
 
 export async function envPathForQtInstallation(installation: string) {
   const qtRootDir = qtRootByQtInstallation(installation);
-  const promiseNinjaPath = locateNinjaExecutable(qtRootDir);
   const promiseJomPath = locateJomExecutable(qtRootDir);
   const isMingwInstallation = path.basename(installation).startsWith('mingw');
   const promiseMingwPath = isMingwInstallation
     ? locateMingwBinDirPath(qtRootDir)
     : undefined;
-  let qtPathEnv = envPathForQtInstallationWithNinja(
-    installation,
-    await promiseNinjaPath
-  );
+  let qtPathEnv = generateEnvPathForQtInstallation(installation);
+
+  if (!commandExists.sync('ninja')) {
+    const ninjaPath = await locateNinjaExecutable(qtRootDir);
+    if (ninjaPath) {
+      qtPathEnv = `${ninjaPath}${path.delimiter}${qtPathEnv}`;
+    }
+  }
+
   const jomExePath = await promiseJomPath;
   if (jomExePath) {
     qtPathEnv = `${jomExePath}${path.delimiter}${qtPathEnv}`;
@@ -353,29 +327,20 @@ export async function locateQtDesignerExePath(selectedQtPath: string) {
         DesignerExeName
       )
     : path.join(selectedQtPath, 'bin', DesignerExeName);
-  try {
-    await fs.access(designerExePath);
+  if (await fsutil.exists(designerExePath)) {
     return designerExePath;
-  } catch (err) {
-    // Do nothing
   }
 
   const hostBinDir = await queryHostBinDirPath(selectedQtPath);
   designerExePath = path.join(hostBinDir, DesignerExeName);
-  try {
-    await fs.access(designerExePath);
+  if (await fsutil.exists(designerExePath)) {
     return designerExePath;
-  } catch (err) {
-    // Do nothing
   }
 
   if (!IsWindows) {
     designerExePath = '/usr/bin/designer';
-    try {
-      await fs.access(designerExePath);
+    if (await fsutil.exists(designerExePath)) {
       return designerExePath;
-    } catch (err) {
-      // Do nothing
     }
   }