Skip to content

Use npm's node-gyp if available, otherwise automatically install node-gyp #3240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 24, 2017
Merged

Use npm's node-gyp if available, otherwise automatically install node-gyp #3240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4abbac4
Attempt to install node-gyp if it's required but missing
Daniel15 Apr 23, 2017
49b8be2
Try to use node-gyp version from npm
Daniel15 Apr 23, 2017
7694063
Trailing commas
Daniel15 Apr 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions bin/node-gyp-bin/node-gyp
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions bin/node-gyp-bin/node-gyp.cmd
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
"minimatch": "^3.0.3",
"mkdirp": "^0.5.1",
"node-emoji": "^1.0.4",
"node-gyp": "^3.2.1",
"object-path": "^0.11.2",
"proper-lockfile": "^2.0.0",
"read": "^1.0.7",
Expand Down
1 change: 0 additions & 1 deletion scripts/build-dist.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ cp LICENSE dist/
cp artifacts/yarn-legacy-*.js dist/lib/yarn-cli.js
cp bin/yarn-bundle-entry.js dist/bin/yarn.js
cp bin/{yarn,yarnpkg,*.cmd} dist/bin/
cp -r bin/node-gyp-bin dist/bin/
# We cannot bundle v8-compile-cache as it must be loaded separately to be effective.
cp node_modules/v8-compile-cache/v8-compile-cache.js dist/lib/v8-compile-cache.js

Expand Down
2 changes: 2 additions & 0 deletions src/reporters/lang/en.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ const messages = {
possibleCommands: 'Project commands',
commandQuestion: 'Which command would you like to run?',
commandFailed: 'Command failed with exit code $0.',
packageRequiresNodeGyp: 'This package requires node-gyp, which is not currently installed. Yarn will attempt to automatically install it. If this fails, you can run "yarn global add node-gyp" to manually install it.',
nodeGypAutoInstallFailed: 'Failed to auto-install node-gyp. Please run "yarn global add node-gyp" manually. Error: $0',

foundIncompatible: 'Found incompatible module',
incompatibleEngine: 'The engine $0 is incompatible with this module. Expected version $1.',
Expand Down
76 changes: 74 additions & 2 deletions src/util/execute-lifecycle-script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,13 @@ import type Config from '../config.js';
import {MessageError, SpawnError} from '../errors.js';
import * as constants from '../constants.js';
import * as child from './child.js';
import {exists} from './fs.js';
import {registries} from '../resolvers/index.js';
import {fixCmdWinSlashes} from './fix-cmd-win-slashes.js';
import {
run as globalRun,
getBinFolder as getGlobalBinFolder,
} from '../cli/commands/global.js';

const path = require('path');

Expand Down Expand Up @@ -116,8 +121,22 @@ export async function executeLifecycleScript(
// split up the path
const pathParts = (env[constants.ENV_PATH_KEY] || '').split(path.delimiter);

// add node-gyp
pathParts.unshift(path.join(__dirname, '..', '..', 'bin', 'node-gyp-bin'));
// Include node-gyp version that was bundled with the current Node.js version,
// if available.
pathParts.unshift(
path.join(
path.dirname(process.execPath), 'node_modules', 'npm', 'bin', 'node-gyp-bin',
),
);
pathParts.unshift(
path.join(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this stuff can use path.resolve which gets rid of the .. from the resulting string, and you can pass a single arg with /. Yes, that works on windows.

$ node -pe "require('path').win32.resolve('/root', 'some/path/../slashes')"
\root\some\slashes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this can be path.resolve(path.dirname(process.execPath), '../lib/node_modules/npm/bin/node-gyp-bin')

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path.join gets rid of the .. too 😃

I used path.join with all the pieces rather than hard-coding a string with / since it seemed cleaner. I guess either way is fine.

Copy link
Contributor

@SimenB SimenB Apr 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path.join gets rid of the .. too

Didn't know! Nice

$ node -pe "require('path').win32.join('/root', 'cool', 'some/path/../slashes', '..')"
\root\cool\some

Would you look at that, it also fixes the slashes! You learn something every day

path.dirname(process.execPath), '..', 'lib', 'node_modules', 'npm', 'bin', 'node-gyp-bin',
),
);

// Add global bin folder, as some packages depend on a globally-installed
// version of node-gyp.
pathParts.unshift(getGlobalBinFolder(config, {}));

// add .bin folders to PATH
for (const registry of Object.keys(registries)) {
Expand All @@ -126,6 +145,8 @@ export async function executeLifecycleScript(
pathParts.unshift(path.join(cwd, binFolder));
}

await checkForGypIfNeeded(config, cmd, pathParts);

// join path back together
env[constants.ENV_PATH_KEY] = pathParts.join(path.delimiter);

Expand Down Expand Up @@ -168,6 +189,57 @@ export async function executeLifecycleScript(

export default executeLifecycleScript;

let checkGypPromise: ?Promise<void> = null;
/**
* Special case: Some packages depend on node-gyp, but don't specify this in
* their package.json dependencies. They assume that node-gyp is available
* globally. We need to detect this case and show an error message.
*/
function checkForGypIfNeeded(
config: Config,
cmd: string,
paths: Array<string>,
): Promise<void> {
if (cmd.substr(0, cmd.indexOf(' ')) !== 'node-gyp') {
return Promise.resolve();
}

// Ensure this only runs once, rather than multiple times in parallel.
if (!checkGypPromise) {
checkGypPromise = _checkForGyp(config, paths);
}
return checkGypPromise;
}

async function _checkForGyp(
config: Config,
paths: Array<string>,
): Promise<void> {
const {reporter} = config;

// Check every directory in the PATH
const allChecks = await Promise.all(
paths.map((dir) => exists(path.join(dir, 'node-gyp'))),
);
if (allChecks.some(Boolean)) {
// node-gyp is available somewhere
return;
}

reporter.info(reporter.lang('packageRequiresNodeGyp'));

try {
await globalRun(
config,
reporter,
{},
['add', 'node-gyp'],
);
} catch (e) {
throw new MessageError(reporter.lang('nodeGypAutoInstallFailed', e.message));
}
}

export async function execFromManifest(config: Config, commandName: string, cwd: string): Promise<void> {
const pkg = await config.maybeReadManifest(cwd);
if (!pkg || !pkg.scripts) {
Expand Down
37 changes: 6 additions & 31 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3295,24 +3295,6 @@ node-emoji@^1.0.4:
dependencies:
string.prototype.codepointat "^0.2.0"

node-gyp@^3.2.1:
version "3.5.0"
resolved "https://registry.yarnpkg.com/node-gyp/-/node-gyp-3.5.0.tgz#a8fe5e611d079ec16348a3eb960e78e11c85274a"
dependencies:
fstream "^1.0.0"
glob "^7.0.3"
graceful-fs "^4.1.2"
minimatch "^3.0.2"
mkdirp "^0.5.0"
nopt "2 || 3"
npmlog "0 || 1 || 2 || 3 || 4"
osenv "0"
request "2"
rimraf "2"
semver "2.x || 3.x || 4 || 5"
tar "^2.0.0"
which "1"

node-int64@^0.4.0:
version "0.4.0"
resolved "https://registry.yarnpkg.com/node-int64/-/node-int64-0.4.0.tgz#87a9065cdb355d3182d8f94ce11188b825c68a3b"
Expand Down Expand Up @@ -3368,7 +3350,7 @@ node-pre-gyp@^0.6.29:
tar "~2.2.1"
tar-pack "~3.3.0"

"nopt@2 || 3", nopt@~3.0.6:
nopt@~3.0.6:
version "3.0.6"
resolved "https://registry.yarnpkg.com/nopt/-/nopt-3.0.6.tgz#c6465dbf08abcd4db359317f79ac68a646b28ff9"
dependencies:
Expand All @@ -3387,7 +3369,7 @@ normalize-path@^2.0.1:
version "2.0.1"
resolved "https://registry.yarnpkg.com/normalize-path/-/normalize-path-2.0.1.tgz#47886ac1662760d4261b7d979d241709d3ce3f7a"

"npmlog@0 || 1 || 2 || 3 || 4", npmlog@^4.0.1:
npmlog@^4.0.1:
version "4.0.2"
resolved "https://registry.yarnpkg.com/npmlog/-/npmlog-4.0.2.tgz#d03950e0e78ce1527ba26d2a7592e9348ac3e75f"
dependencies:
Expand Down Expand Up @@ -3501,13 +3483,6 @@ os-tmpdir@^1.0.0, os-tmpdir@^1.0.1, os-tmpdir@~1.0.1:
version "1.0.2"
resolved "https://registry.yarnpkg.com/os-tmpdir/-/os-tmpdir-1.0.2.tgz#bbe67406c79aa85c5cfec766fe5734555dfa1274"

osenv@0:
version "0.1.4"
resolved "https://registry.yarnpkg.com/osenv/-/osenv-0.1.4.tgz#42fe6d5953df06c8064be6f176c3d05aaaa34644"
dependencies:
os-homedir "^1.0.0"
os-tmpdir "^1.0.0"

p-limit@^1.1.0:
version "1.1.0"
resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-1.1.0.tgz#b07ff2d9a5d88bec806035895a2bab66a27988bc"
Expand Down Expand Up @@ -3903,7 +3878,7 @@ request-capture-har@^1.2.2:
version "1.2.2"
resolved "https://registry.yarnpkg.com/request-capture-har/-/request-capture-har-1.2.2.tgz#cd692cfb2cc744fd84a3358aac6ee51528cf720d"

request@2, request@^2.79.0, request@^2.81.0:
request@^2.79.0, request@^2.81.0:
version "2.81.0"
resolved "https://registry.yarnpkg.com/request/-/request-2.81.0.tgz#c6928946a0e06c5f8d6f8a9333469ffda46298a0"
dependencies:
Expand Down Expand Up @@ -4054,7 +4029,7 @@ sax@^1.2.1:
version "1.2.2"
resolved "https://registry.yarnpkg.com/sax/-/sax-1.2.2.tgz#fd8631a23bc7826bef5d871bdb87378c95647828"

"semver@2 || 3 || 4 || 5", "[email protected] || 3.x || 4 || 5", semver@^5.1.0, semver@^5.3.0, semver@~5.3.0:
"semver@2 || 3 || 4 || 5", semver@^5.1.0, semver@^5.3.0, semver@~5.3.0:
version "5.3.0"
resolved "https://registry.yarnpkg.com/semver/-/semver-5.3.0.tgz#9b2ce5d3de02d17c6012ad326aa6b4d0cf54f94f"

Expand Down Expand Up @@ -4352,7 +4327,7 @@ tar-stream@^1.1.2, tar-stream@^1.5.2:
readable-stream "^2.0.0"
xtend "^4.0.0"

tar@^2.0.0, tar@~2.2.1:
tar@~2.2.1:
version "2.2.1"
resolved "https://registry.yarnpkg.com/tar/-/tar-2.2.1.tgz#8e4d2a256c0e2185c6b18ad694aec968b83cb1d1"
dependencies:
Expand Down Expand Up @@ -4705,7 +4680,7 @@ which-module@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/which-module/-/which-module-1.0.0.tgz#bba63ca861948994ff307736089e3b96026c2a4f"

which@1, which@^1.1.1, which@^1.2.12:
which@^1.1.1, which@^1.2.12:
version "1.2.12"
resolved "https://registry.yarnpkg.com/which/-/which-1.2.12.tgz#de67b5e450269f194909ef23ece4ebe416fa1192"
dependencies:
Expand Down