From fc4ad419390c398acd70ce079d09b34ec6db2de5 Mon Sep 17 00:00:00 2001 From: Orkun Tokdemir Date: Thu, 29 Feb 2024 18:15:25 +0100 Subject: [PATCH] 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 --- package-lock.json | 13 ++++ package.json | 2 + src/commands/detect-qt-cmake.ts | 24 ++++--- src/commands/register-qt-path.ts | 6 +- src/util/cmake-kit-files.ts | 3 +- src/util/get-qt-paths.ts | 109 +++++++++++-------------------- 6 files changed, 67 insertions(+), 90 deletions(-) diff --git a/package-lock.json b/package-lock.json index cce5a91..0c8ce4e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,11 +10,13 @@ "hasInstallScript": true, "dependencies": { "@vscode/l10n": "^0.0.16", + "command-exists": "^1.2.3", "ts-sinon": "^2.0.2", "typescript": "^5.2.2" }, "devDependencies": { "@types/chai": "^4.3.10", + "@types/command-exists": "^1.2.3", "@types/glob": "^8.1.0", "@types/mocha": "^10.0.1", "@types/node": "^20.2.5", @@ -743,6 +745,12 @@ "resolved": "https://registry.npmjs.org/@types/chai/-/chai-4.3.12.tgz", "integrity": "sha512-zNKDHG/1yxm8Il6uCCVsm+dRdEsJlFoDu73X17y09bId6UwoYww+vFBsAcRzl8knM1sab3Dp1VRikFQwDOtDDw==" }, + "node_modules/@types/command-exists": { + "version": "1.2.3", + "resolved": "https://registry.npmjs.org/@types/command-exists/-/command-exists-1.2.3.tgz", + "integrity": "sha512-PpbaE2XWLaWYboXD6k70TcXO/OdOyyRFq5TVpmlUELNxdkkmXU9fkImNosmXU1DtsNrqdUgWd/nJQYXgwmtdXQ==", + "dev": true + }, "node_modules/@types/glob": { "version": "8.1.0", "resolved": "https://registry.npmjs.org/@types/glob/-/glob-8.1.0.tgz", @@ -1674,6 +1682,11 @@ "integrity": "sha512-72fSenhMw2HZMTVHeCA9KCmpEIbzWiQsjN+BHcBbS9vr1mtt+vJjPdksIBNUmKAW8TFUDPJK5SUU3QhE9NEXDw==", "dev": true }, + "node_modules/command-exists": { + "version": "1.2.9", + "resolved": "https://registry.npmjs.org/command-exists/-/command-exists-1.2.9.tgz", + "integrity": "sha512-LTQ/SGc+s0Xc0Fu5WaKnR0YiygZkm9eKFvyS+fRsU7/ZWFF8ykFM6Pc9aCVf1+xasOOZpO3BAVgVrKvsqKHV7w==" + }, "node_modules/commander": { "version": "11.1.0", "resolved": "https://registry.npmjs.org/commander/-/commander-11.1.0.tgz", diff --git a/package.json b/package.json index bc47522..56bce64 100644 --- 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 index b9946de..19517d3 100644 --- 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,9 +44,6 @@ async function* generateCMakeKitsOfQtInstallationPath( isTrusted: true, preferredGenerator: { name: cmake.CMakeDefaultGenerator - }, - cmakeSettings: { - CMAKE_MAKE_PROGRAM: ninjaExePath } }; @@ -53,7 +51,7 @@ async function* generateCMakeKitsOfQtInstallationPath( 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 index 3f211a8..9b129c3 100644 --- a/src/commands/register-qt-path.ts +++ b/src/commands/register-qt-path.ts @@ -181,9 +181,9 @@ export async function getSelectedQtInstallationPath(): Promise { 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 index 4fe16e0..6c00606 100644 --- 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 { 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 index 2960c0e..ac37bc6 100644 --- 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 } }