Skip to content

Commit

Permalink
Improvements to Homebrew support (#345)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fnesveda authored Mar 9, 2023
1 parent 5a6dbcd commit 0b8adb0
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 29 deletions.
32 changes: 23 additions & 9 deletions src/commands/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
39 changes: 22 additions & 17 deletions src/commands/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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) {
Expand Down
27 changes: 26 additions & 1 deletion src/lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const https = require('https');
const { ApifyClient } = require('apify-client');
const {
execSync,
spawnSync,
} = require('child_process');
const semver = require('semver');
const {
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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,
Expand Down Expand Up @@ -558,4 +581,6 @@ module.exports = {
detectPythonVersion,
isPythonVersionSupported,
getPythonCommand,
detectNodeVersion,
isNodeVersionSupported,
};
4 changes: 2 additions & 2 deletions src/lib/version_check.js
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 0b8adb0

Please sign in to comment.