Skip to content
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.

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

feat(run): Add engines check before executing scripts. #7021

Merged
merged 2 commits into from Mar 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -44,6 +44,10 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa

[#6983](https://github.com/yarnpkg/yarn/pull/6983) - [**Micha Reiser**](https://github.com/MichaReiser)

- Run the engines check before executing `run` scripts.

[#7013](https://github.com/yarnpkg/yarn/issues/7013) - [**Eloy Durán**](https://github.com/alloy)

## 1.14.0

- Improves PnP compatibility with Node 6
Expand Down
2 changes: 1 addition & 1 deletion src/cli/commands/install.js
Expand Up @@ -744,7 +744,7 @@ export class Install {

async checkCompatibility(): Promise<void> {
const {manifest} = await this.fetchRequestFromCwd();
await compatibility.checkOne({_reference: {}, ...manifest}, this.config, this.flags.ignoreEngines);
await compatibility.checkOne(manifest, this.config, this.flags.ignoreEngines);
Copy link
Contributor

Choose a reason for hiding this comment

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

what does this change do? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was needed to avoid hitting that same invariant that I did.

}

async persistChanges(): Promise<void> {
Expand Down
8 changes: 8 additions & 0 deletions src/cli/commands/run.js
Expand Up @@ -5,6 +5,7 @@ import type Config from '../../config.js';
import {execCommand, makeEnv} from '../../util/execute-lifecycle-script.js';
import {dynamicRequire} from '../../util/dynamic-require.js';
import {MessageError} from '../../errors.js';
import {checkOne as checkCompatibility} from '../../package-compatibility.js';
import * as fs from '../../util/fs.js';
import * as constants from '../../constants.js';

Expand Down Expand Up @@ -117,6 +118,13 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
}

if (cmds.length) {
const ignoreEngines = !!(flags.ignoreEngines || config.getOption('ignore-engines'));
try {
await checkCompatibility(pkg, config, ignoreEngines);
} catch (err) {
throw err instanceof MessageError ? new MessageError(reporter.lang('cannotRunWithIncompatibleEnv')) : err;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This refines a, in this context, vague error.

}

// Disable wrapper in executed commands
process.env.YARN_WRAP_OUTPUT = 'false';
for (const [stage, cmd] of cmds) {
Expand Down
4 changes: 1 addition & 3 deletions src/package-compatibility.js
Expand Up @@ -8,7 +8,6 @@ import {entries} from './util/misc.js';
import {version as yarnVersion} from './util/yarn-version.js';
import {satisfiesWithPrereleases} from './util/semver.js';

const invariant = require('invariant');
const semver = require('semver');

const VERSIONS = Object.assign({}, process.versions, {
Expand Down Expand Up @@ -111,9 +110,8 @@ export function checkOne(info: Manifest, config: Config, ignoreEngines: boolean)

const pushError = msg => {
const ref = info._reference;
invariant(ref, 'expected package reference');

if (ref.optional) {
if (ref && ref.optional) {
ref.ignore = true;
ref.incompatible = true;

Expand Down
3 changes: 2 additions & 1 deletion src/reporters/lang/en.js
Expand Up @@ -229,11 +229,12 @@ const messages = {
nodeGypAutoInstallFailed:
'Failed to auto-install node-gyp. Please run "yarn global add node-gyp" manually. Error: $0',

foundIncompatible: 'Found incompatible module',
foundIncompatible: 'Found incompatible module.',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this was the odd one out, so added some punctuation 🤓

incompatibleEngine: 'The engine $0 is incompatible with this module. Expected version $1. Got $2',
incompatibleCPU: 'The CPU architecture $0 is incompatible with this module.',
incompatibleOS: 'The platform $0 is incompatible with this module.',
invalidEngine: 'The engine $0 appears to be invalid.',
cannotRunWithIncompatibleEnv: 'Commands cannot run with an incompatible environment.',

optionalCompatibilityExcluded:
'$0 is an optional dependency and failed compatibility check. Excluding it from installation.',
Expand Down