Skip to content

Commit

Permalink
Bugfix 3752 - Artifacts disappearing when missing integrity file (#4185)
Browse files Browse the repository at this point in the history
* Add method to update force property

* Update install to force script installs when integrity file missing

* Update add to force script installs when integrity file missing

* Add test case for retaining artifacts after add when missing integrity

* Add test case for retaining artifacts after install when missing integrity

* Add test fixture data for missing integrity tests

* Fix lint errors
  • Loading branch information
jamsinclair authored and arcanis committed Aug 17, 2017
1 parent 034f461 commit 6bfaac3
Show file tree
Hide file tree
Showing 13 changed files with 107 additions and 2 deletions.
31 changes: 31 additions & 0 deletions __tests__/commands/add.js
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,37 @@ test.concurrent('should only refer to root to satisfy peer dependency', (): Prom
);
});

test.concurrent('should retain build artifacts after add when missing integrity file', (): Promise<void> => {
return buildRun(
reporters.BufferReporter,
fixturesLoc,
async (args, flags, config, reporter): Promise<void> => {
const lockfile = await createLockfile(config.cwd);

const addA = new Add(args, flags, config, reporter, lockfile);
await addA.init();

const expectedArtifacts = ['foo.txt'];
const integrityLoc = path.join(config.cwd, 'node_modules', constants.INTEGRITY_FILENAME);

const beforeIntegrity = await fs.readJson(integrityLoc);
expect(beforeIntegrity.artifacts['[email protected]']).toEqual(expectedArtifacts);

await fs.unlink(integrityLoc);

const lockfileAfterPreviousAdd = await Lockfile.fromDirectory(config.cwd);
const addB = new Add(['file:b'], flags, config, reporter, lockfileAfterPreviousAdd);
await addB.init();

const afterIntegrity = await fs.readJson(integrityLoc);
expect(afterIntegrity.artifacts['[email protected]']).toEqual(expectedArtifacts);
},
['file:a'],
{},
'retain-build-artifacts-missing-integrity',
);
});

test.concurrent('should retain build artifacts after add', (): Promise<void> => {
return buildRun(
reporters.BufferReporter,
Expand Down
18 changes: 18 additions & 0 deletions __tests__/commands/install/lockfiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,24 @@ test.concurrent('install should write and read integrity file based on lockfile
});
});

test.concurrent('install should retain artifacts when missing integrity file', (): Promise<void> => {
return runInstall({}, 'install-should-retain-artifacts-when-missing-integrity', async (config, reporter) => {
const expectedArtifacts = ['foo.txt'];
const integrityLoc = path.join(config.cwd, 'node_modules', constants.INTEGRITY_FILENAME);

const beforeIntegrity = await fs.readJson(integrityLoc);
expect(beforeIntegrity.artifacts['[email protected]']).toEqual(expectedArtifacts);

await fs.unlink(integrityLoc);

const reinstall = new Install({}, config, reporter, await Lockfile.fromDirectory(config.cwd));
await reinstall.init();

const afterIntegrity = await fs.readJson(integrityLoc);
expect(afterIntegrity.artifacts['[email protected]']).toEqual(expectedArtifacts);
});
});

test.concurrent('install should not continue if integrity check passes', (): Promise<void> => {
return runInstall({}, 'lockfile-stability', async (config, reporter) => {
await fs.writeFile(path.join(config.cwd, 'node_modules', 'yarn.test'), 'YARN TEST');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
require('fs').writeFileSync('foo.txt', 'foobar');
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "a",
"version": "0.0.0",
"scripts": {
"postinstall": "node install.js"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "b",
"version": "0.0.0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
require('fs').writeFileSync('foo.txt', 'foobar');
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "a",
"version": "0.0.0",
"scripts": {
"postinstall": "node install.js"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"a": "file:a"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


"a@file:a":
version "0.0.0"
18 changes: 16 additions & 2 deletions src/cli/commands/add.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@ import type Config from '../../config.js';
import type {ListOptions} from './list.js';
import Lockfile from '../../lockfile/wrapper.js';
import PackageRequest from '../../package-request.js';
import WorkspaceLayout from '../../workspace-layout.js';
import {getExoticResolver} from '../../resolvers/index.js';
import {buildTree} from './list.js';
import {wrapLifecycle, Install} from './install.js';
import {MessageError} from '../../errors.js';
import * as constants from '../../constants.js';
import * as fs from '../../util/fs.js';

import invariant from 'invariant';
import path from 'path';
import semver from 'semver';

export class Add extends Install {
Expand Down Expand Up @@ -97,8 +101,18 @@ export class Add extends Install {
return preparedPatterns;
}

bailout(patterns: Array<string>): Promise<boolean> {
return Promise.resolve(false);
async bailout(patterns: Array<string>, workspaceLayout: ?WorkspaceLayout): Promise<boolean> {
const lockfileCache = this.lockfile.cache;
if (!lockfileCache) {
return false;
}
const match = await this.integrityChecker.check(patterns, lockfileCache, this.flags, workspaceLayout);
const haveLockfile = await fs.exists(path.join(this.config.lockfileFolder, constants.LOCKFILE_FILENAME));
if (match.integrityFileMissing && haveLockfile) {
// Integrity file missing, force script installations
this.scripts.setForce(true);
}
return false;
}

/**
Expand Down
6 changes: 6 additions & 0 deletions src/cli/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,12 @@ export class Install {
return true;
}

if (match.integrityFileMissing && haveLockfile) {
// Integrity file missing, force script installations
this.scripts.setForce(true);
return false;
}

if (!patterns.length && !match.integrityFileMissing) {
this.reporter.success(this.reporter.lang('nothingToInstall'));
await this.createEmptyManifestFolders();
Expand Down
4 changes: 4 additions & 0 deletions src/package-install-scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ export default class PackageInstallScripts {
force: boolean;
artifacts: InstallArtifacts;

setForce(force: boolean) {
this.force = force;
}

setArtifacts(artifacts: InstallArtifacts) {
this.artifacts = artifacts;
}
Expand Down

0 comments on commit 6bfaac3

Please sign in to comment.