From 0b8adb071d9a33cd4123e5f5fc1b1add35f19f0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franti=C5=A1ek=20Nesveda?= Date: Thu, 9 Mar 2023 10:07:55 +0100 Subject: [PATCH] Improvements to Homebrew support (#345) This PR adds two improvements to running the CLI on Homebrew: - we now check the supported Node.js version based on `node --version`, not by `process.versions.node` - this makes sure that if someone installs the CLI via Homebrew and does not have Node.js available globally, it shows him a nice error message rather than a weird error - we now check where the symlink to the executable leads via `fs.realpathSync(...)`, not via `execSync('realpath ...')` - this is because `realpath` is not available on macOS 12 and earlier by default, and also not in some Linux distributions - also because why should we run an external command when the same thing is built in to Node.js --- src/commands/create.js | 32 +++++++++++++++++++++++--------- src/commands/run.js | 39 ++++++++++++++++++++++----------------- src/lib/utils.js | 27 ++++++++++++++++++++++++++- src/lib/version_check.js | 4 ++-- 4 files changed, 73 insertions(+), 29 deletions(-) diff --git a/src/commands/create.js b/src/commands/create.js index 6bce0c1f..5d21ae7f 100644 --- a/src/commands/create.js +++ b/src/commands/create.js @@ -3,6 +3,7 @@ const fs = require('fs'); const path = require('path'); const actorTemplates = require('@apify/actor-templates'); const unzipper = require('unzipper'); +const semver = require('semver'); const { ApifyCommand } = require('../lib/apify_command'); const execWithLog = require('../lib/exec'); const outputs = require('../lib/outputs'); @@ -15,8 +16,10 @@ const { detectPythonVersion, isPythonVersionSupported, getPythonCommand, + detectNodeVersion, + isNodeVersionSupported, } = require('../lib/utils'); -const { EMPTY_LOCAL_CONFIG, LOCAL_CONFIG_PATH, PYTHON_VENV_PATH } = require('../lib/consts'); +const { EMPTY_LOCAL_CONFIG, LOCAL_CONFIG_PATH, PYTHON_VENV_PATH, SUPPORTED_NODEJS_VERSION } = require('../lib/consts'); const { httpsGet, ensureValidActorName, getTemplateDefinition } = require('../lib/create-utils'); class CreateCommand extends ApifyCommand { @@ -76,14 +79,25 @@ class CreateCommand extends ApifyCommand { let dependenciesInstalled = false; if (!skipDependencyInstall) { if (fs.existsSync(packageJsonPath)) { - // If the actor is a Node.js actor (has package.json), run `npm install` - await updateLocalJson(packageJsonPath, { name: actorName }); - // Run npm install in actor dir. - // For efficiency, don't install Puppeteer for templates that don't use it - const cmdArgs = ['install']; - if (skipOptionalDeps) cmdArgs.push('--no-optional'); - await execWithLog(getNpmCmd(), cmdArgs, { cwd: actFolderDir }); - dependenciesInstalled = true; + const currentNodeVersion = detectNodeVersion(); + const minimumSupportedNodeVersion = semver.minVersion(SUPPORTED_NODEJS_VERSION); + if (currentNodeVersion) { + if (!isNodeVersionSupported(currentNodeVersion)) { + outputs.warning(`You are running Node.js version ${currentNodeVersion}, which is no longer supported. ` + + `Please upgrade to Node.js version ${minimumSupportedNodeVersion} or later.`); + } + // If the actor is a Node.js actor (has package.json), run `npm install` + await updateLocalJson(packageJsonPath, { name: actorName }); + // Run npm install in actor dir. + // For efficiency, don't install Puppeteer for templates that don't use it + const cmdArgs = ['install']; + if (skipOptionalDeps) cmdArgs.push('--no-optional'); + await execWithLog(getNpmCmd(), cmdArgs, { cwd: actFolderDir }); + dependenciesInstalled = true; + } else { + outputs.error(`No Node.js detected! Please install Node.js ${minimumSupportedNodeVersion} or higher` + + ' to be able to run Node.js actors locally.'); + } } else if (fs.existsSync(requirementsTxtPath)) { const pythonVersion = detectPythonVersion(actFolderDir); if (pythonVersion) { diff --git a/src/commands/run.js b/src/commands/run.js index 08925b17..8118e7d3 100644 --- a/src/commands/run.js +++ b/src/commands/run.js @@ -11,6 +11,7 @@ const { getLocalUserInfo, purgeDefaultQueue, purgeDefaultKeyValueStore, purgeDefaultDataset, getLocalConfigOrThrow, getNpmCmd, checkIfStorageIsEmpty, detectPythonVersion, isPythonVersionSupported, getPythonCommand, + detectNodeVersion, isNodeVersionSupported, } = require('../lib/utils'); const { error, info, warning } = require('../lib/outputs'); const { replaceSecretsValue } = require('../lib/secrets'); @@ -86,26 +87,30 @@ class RunCommand extends ApifyCommand { } if (packageJsonExists) { // Actor is written in Node.js - const serverJsFile = path.join(cwd, 'server.js'); - const packageJson = await loadJson(packageJsonPath); - if ((!packageJson.scripts || !packageJson.scripts.start) && !fs.existsSync(serverJsFile)) { - throw new Error('The "npm start" script was not found in package.json. Please set it up for your project. ' - + 'For more information about that call "apify help run".'); - } + const currentNodeVersion = detectNodeVersion(); + const minimumSupportedNodeVersion = semver.minVersion(SUPPORTED_NODEJS_VERSION); + if (currentNodeVersion) { + const serverJsFile = path.join(cwd, 'server.js'); + const packageJson = await loadJson(packageJsonPath); + if ((!packageJson.scripts || !packageJson.scripts.start) && !fs.existsSync(serverJsFile)) { + throw new Error('The "npm start" script was not found in package.json. Please set it up for your project. ' + + 'For more information about that call "apify help run".'); + } - // --max-http-header-size=80000 - // Increases default size of headers. The original limit was 80kb, but from node 10+ they decided to lower it to 8kb. - // However they did not think about all the sites there with large headers, - // so we put back the old limit of 80kb, which seems to work just fine. - const currentNodeVersion = process.versions.node; - const lastSupportedVersion = semver.minVersion(SUPPORTED_NODEJS_VERSION); - if (semver.gte(currentNodeVersion, lastSupportedVersion)) { - env.NODE_OPTIONS = env.NODE_OPTIONS ? `${env.NODE_OPTIONS} --max-http-header-size=80000` : '--max-http-header-size=80000'; + // --max-http-header-size=80000 + // Increases default size of headers. The original limit was 80kb, but from node 10+ they decided to lower it to 8kb. + // However they did not think about all the sites there with large headers, + // so we put back the old limit of 80kb, which seems to work just fine. + if (isNodeVersionSupported(currentNodeVersion)) { + env.NODE_OPTIONS = env.NODE_OPTIONS ? `${env.NODE_OPTIONS} --max-http-header-size=80000` : '--max-http-header-size=80000'; + } else { + warning(`You are running Node.js version ${currentNodeVersion}, which is no longer supported. ` + + `Please upgrade to Node.js version ${minimumSupportedNodeVersion} or later.`); + } + await execWithLog(getNpmCmd(), ['start'], { env }); } else { - warning(`You are running Node.js version ${currentNodeVersion}, which is no longer supported. ` - + `Please upgrade to Node.js version ${lastSupportedVersion} or later.`); + error(`No Node.js detected! Please install Node.js ${minimumSupportedNodeVersion} or higher to be able to run Node.js actors locally.`); } - await execWithLog(getNpmCmd(), ['start'], { env }); } else if (mainPyExists) { const pythonVersion = detectPythonVersion(cwd); if (pythonVersion) { diff --git a/src/lib/utils.js b/src/lib/utils.js index 4e4ee3f3..d3d298ca 100644 --- a/src/lib/utils.js +++ b/src/lib/utils.js @@ -20,6 +20,7 @@ const https = require('https'); const { ApifyClient } = require('apify-client'); const { execSync, + spawnSync, } = require('child_process'); const semver = require('semver'); const { @@ -31,6 +32,7 @@ const { DEPRECATED_LOCAL_CONFIG_NAME, ACTOR_SPECIFICATION_VERSION, APIFY_CLIENT_DEFAULT_HEADERS, + SUPPORTED_NODEJS_VERSION, MINIMUM_SUPPORTED_PYTHON_VERSION, } = require('./consts'); const { @@ -518,7 +520,10 @@ const getPythonCommand = (directory) => { const detectPythonVersion = (directory) => { const pythonCommand = getPythonCommand(directory); try { - return execSync(`${pythonCommand} -c "import platform; print(platform.python_version(), end='')"`, { encoding: 'utf-8' }); + const spawnResult = spawnSync(pythonCommand, ['-c', 'import platform; print(platform.python_version())'], { encoding: 'utf-8' }); + if (!spawnResult.error && spawnResult.stdout) { + return spawnResult.stdout.trim(); + } } catch { return undefined; } @@ -528,6 +533,24 @@ const isPythonVersionSupported = (installedPythonVersion) => { return semver.satisfies(installedPythonVersion, `^${MINIMUM_SUPPORTED_PYTHON_VERSION}`); }; +const detectNodeVersion = () => { + try { + const spawnResult = spawnSync('node', ['--version'], { encoding: 'utf-8' }); + if (!spawnResult.error && spawnResult.stdout) { + return spawnResult.stdout.trim().replace(/^v/, ''); + } + } catch { + return undefined; + } +}; + +const isNodeVersionSupported = (installedNodeVersion) => { + // SUPPORTED_NODEJS_VERSION can be a version range, + // we need to get the minimum supported version from that range to be able to compare them + const minimumSupportedNodeVersion = semver.minVersion(SUPPORTED_NODEJS_VERSION); + return semver.gte(installedNodeVersion, minimumSupportedNodeVersion); +}; + module.exports = { getLoggedClientOrThrow, getLocalConfig, @@ -558,4 +581,6 @@ module.exports = { detectPythonVersion, isPythonVersionSupported, getPythonCommand, + detectNodeVersion, + isNodeVersionSupported, }; diff --git a/src/lib/version_check.js b/src/lib/version_check.js index 100ab79d..6f7f8d0b 100644 --- a/src/lib/version_check.js +++ b/src/lib/version_check.js @@ -1,4 +1,4 @@ -const { execSync } = require('child_process'); +const fs = require('fs'); const process = require('process'); const axios = require('axios'); const chalk = require('chalk'); @@ -37,7 +37,7 @@ const detectInstallationType = () => { // If the real command path is like `/opt/homebrew/Cellar/apify-cli/...` or `/home/linuxbrew/.linuxbrew/Cellar/apify-cli/...`, // then the CLI is installed via Homebrew if (process.platform === 'linux' || process.platform === 'darwin') { - const realCommandPath = execSync(`realpath "${commandPath}"`); + const realCommandPath = fs.realpathSync(commandPath); if (realCommandPath.includes('homebrew/Cellar') || realCommandPath.includes('linuxbrew/Cellar')) { return INSTALLATION_TYPE.HOMEBREW; }