Skip to content

Commit

Permalink
feat(install): Add --update-checksums to cli install (yarnpkg#4860)
Browse files Browse the repository at this point in the history
**Summary**

Fixes yarnpkg#4817.
When the `--update-checksums` flag is set, yarn would know to ignore a checksum mismatch between `yarn.lock` and the repository, and instead update the yarn.lock file with the proper checksum(s).

**Test plan**

Added new tests.

To manually check this:
1. Change one or more of the package checksums in `yarn.lock`
2. Delete node_modules (optionally also run `yarn cache clean`)
3. Run `yarn` => checksum mismatch error will be received.
4. Run `yarn --update-checksums` => will install successfully and fix the damaged checksums in `yarn.lock`
  • Loading branch information
imsnif authored and terra-incognita committed Nov 9, 2017
1 parent 1146b25 commit 5e3552a
Show file tree
Hide file tree
Showing 13 changed files with 101 additions and 8 deletions.
1 change: 1 addition & 0 deletions __tests__/commands/_helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export function makeConfigFromDirectory(cwd: string, reporter: Reporter, flags:
linkFolder: flags.linkFolder || path.join(cwd, '.yarn-link'),
prefix: flags.prefix,
production: flags.production,
updateChecksums: !!flags.updateChecksums,
},
reporter,
);
Expand Down
15 changes: 15 additions & 0 deletions __tests__/commands/install/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,21 @@ test.concurrent('install should add missing deps to yarn and mirror (PR import s
});
});

test.concurrent('install should update checksums in yarn.lock (--update-checksums)', (): Promise<void> => {
const packageRealHash = '5faad9c2c07f60dd76770f71cf025b62a63cfd4e';
const packageCacheName = `npm-abab-1.0.4-${packageRealHash}`;
return runInstall({updateChecksums: true}, 'install-update-checksums', async config => {
const lockFileContent = await fs.readFile(path.join(config.cwd, 'yarn.lock'));
const lockFileLines = explodeLockfile(lockFileContent);
const packageHashInLockfile = lockFileLines[2].replace(/(^.*#)|("$)/g, '');
const installedPackageJson = path.resolve(config.cwd, 'node_modules', 'abab', 'package.json');
const cachePackageJson = path.resolve(config.cwd, '.yarn-cache/v1/', packageCacheName, 'package.json');
expect(packageHashInLockfile).toEqual(packageRealHash);
expect(await fs.exists(installedPackageJson)).toBe(true);
expect(await fs.exists(cachePackageJson)).toBe(true);
});
});

test.concurrent('install should update a dependency to yarn and mirror (PR import scenario 2)', (): Promise<void> => {
// [email protected] is gets updated to [email protected] via
// a change in package.json,
Expand Down
22 changes: 22 additions & 0 deletions __tests__/fetchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,28 @@ test('TarballFetcher.fetch throws on invalid hash', async () => {
expect(readdirSync(path.join(offlineMirrorDir))).toEqual([]);
});

test('TarballFetcher.fetch fixes hash if updateChecksums flag is true', async () => {
const wrongHash = 'foo';
const dir = await mkdir(`tarball-fetcher-${wrongHash}`);
const config = await Config.create({}, new Reporter());
config.updateChecksums = true;
const url = 'https://github.com/sindresorhus/beeper/archive/master.tar.gz';
const fetcher = new TarballFetcher(
dir,
{
type: 'tarball',
hash: wrongHash,
reference: url,
registry: 'npm',
},
config,
);
await fetcher.fetch();
const dirWithProperHash = dir.replace(wrongHash, fetcher.hash);
const name = (await fs.readJson(path.join(dirWithProperHash, 'package.json'))).name;
expect(name).toBe('beeper');
});

test('TarballFetcher.fetch supports local ungzipped tarball', async () => {
const dir = await mkdir('tarball-fetcher');
const fetcher = new LocalTarballFetcher(
Expand Down
15 changes: 15 additions & 0 deletions __tests__/fixtures/install/install-update-checksums/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"name": "install-update-checksums",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"author": "",
"license": "ISC",
"dependencies": {
"abab": "^1.0.4",
"leftpad": "^0.0.1"
}
}
11 changes: 11 additions & 0 deletions __tests__/fixtures/install/install-update-checksums/yarn.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


abab@^1.0.4:
version "1.0.4"
resolved "https://registry.yarnpkg.com/abab/-/abab-1.0.4.tgz#foo"

leftpad@^0.0.1:
version "0.0.1"
resolved "https://registry.yarnpkg.com/leftpad/-/leftpad-0.0.1.tgz#86b1a4de4face180ac545a83f1503523d8fed115"
Binary file not shown.
Binary file not shown.
1 change: 1 addition & 0 deletions src/cli/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ function normalizeFlags(config: Config, rawFlags: Object): Flags {
flat: !!rawFlags.flat,
lockfile: rawFlags.lockfile !== false,
pureLockfile: !!rawFlags.pureLockfile,
updateChecksums: !!rawFlags.updateChecksums,
skipIntegrityCheck: !!rawFlags.skipIntegrityCheck,
frozenLockfile: !!rawFlags.frozenLockfile,
linkDuplicates: !!rawFlags.linkDuplicates,
Expand Down
2 changes: 2 additions & 0 deletions src/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export function main({
commander.option('--no-lockfile', "don't read or generate a lockfile");
commander.option('--pure-lockfile', "don't generate a lockfile");
commander.option('--frozen-lockfile', "don't generate a lockfile and fail if an update is needed");
commander.option('--update-checksums', 'update package checksums from current repository');
commander.option('--link-duplicates', 'create hardlinks to the repeated modules in node_modules');
commander.option('--link-folder <path>', 'specify a custom folder to store global links');
commander.option('--global-folder <path>', 'specify a custom folder to store global packages');
Expand Down Expand Up @@ -488,6 +489,7 @@ export function main({
networkTimeout: commander.networkTimeout,
nonInteractive: commander.nonInteractive,
scriptsPrependNodePath: commander.scriptsPrependNodePath,
updateChecksums: commander.updateChecksums,
})
.then(() => {
// lockfile check must happen after config.init sets lockfileFolder
Expand Down
4 changes: 4 additions & 0 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ export type ConfigOptions = {

commandName?: ?string,
registry?: ?string,

updateChecksums?: boolean,
};

type PackageMetadata = {
Expand Down Expand Up @@ -102,6 +104,7 @@ export default class Config {
linkFileDependencies: boolean;
ignorePlatform: boolean;
binLinks: boolean;
updateChecksums: boolean;

//
linkedModules: Array<string>;
Expand Down Expand Up @@ -364,6 +367,7 @@ export default class Config {
this.linkFolder = opts.linkFolder || constants.LINK_REGISTRY_DIRECTORY;
this.offline = !!opts.offline;
this.binLinks = !!opts.binLinks;
this.updateChecksums = !!opts.updateChecksums;

this.ignorePlatform = !!opts.ignorePlatform;
this.ignoreScripts = !!opts.ignoreScripts;
Expand Down
13 changes: 6 additions & 7 deletions src/fetchers/base-fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,28 +43,27 @@ export default class BaseFetcher {
}

fetch(defaultManifest: ?Object): Promise<FetchedMetadata> {
const {dest} = this;
return fs.lockQueue.push(dest, async (): Promise<FetchedMetadata> => {
await fs.mkdirp(dest);
return fs.lockQueue.push(this.dest, async (): Promise<FetchedMetadata> => {
await fs.mkdirp(this.dest);

// fetch package and get the hash
const {hash} = await this._fetch();

const pkg = await (async () => {
// load the new normalized manifest
try {
return await this.config.readManifest(dest, this.registry);
return await this.config.readManifest(this.dest, this.registry);
} catch (e) {
if (e.code === 'ENOENT' && defaultManifest) {
return normalizeManifest(defaultManifest, dest, this.config, false);
return normalizeManifest(defaultManifest, this.dest, this.config, false);
} else {
throw e;
}
}
})();

await fs.writeFile(
path.join(dest, constants.METADATA_FILENAME),
path.join(this.dest, constants.METADATA_FILENAME),
JSON.stringify(
{
manifest: pkg,
Expand All @@ -80,7 +79,7 @@ export default class BaseFetcher {

return {
hash,
dest,
dest: this.dest,
package: pkg,
cached: false,
};
Expand Down
13 changes: 12 additions & 1 deletion src/fetchers/tarball-fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,25 @@ export default class TarballFetcher extends BaseFetcher {
error.message = `${error.message}${tarballPath ? ` (${tarballPath})` : ''}`;
reject(error);
})
.on('finish', () => {
.on('finish', async () => {
const expectHash = this.hash;
const actualHash = validateStream.getHash();

if (!expectHash || expectHash === actualHash) {
resolve({
hash: actualHash,
});
} else if (this.config.updateChecksums) {
// checksums differ and should be updated
// update hash, destination and cached package
const destUpdatedHash = this.dest.replace(this.hash || '', actualHash);
await fsUtil.unlink(destUpdatedHash);
await fsUtil.rename(this.dest, destUpdatedHash);
this.dest = this.dest.replace(this.hash || '', actualHash);
this.hash = actualHash;
resolve({
hash: actualHash,
});
} else {
reject(
new SecurityError(
Expand Down
12 changes: 12 additions & 0 deletions src/package-fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,18 @@ export function fetch(pkgs: Array<Manifest>, config: Config): Promise<Array<Mani
// update with new remote
// but only if there was a hash previously as the tarball fetcher does not provide a hash.
if (ref.remote.hash) {
// if the checksum was updated, also update resolved and cache
if (ref.remote.hash !== res.hash && config.updateChecksums) {
const oldHash = ref.remote.hash;
if (ref.remote.resolved) {
ref.remote.resolved = ref.remote.resolved.replace(oldHash, res.hash);
}
ref.config.cache = Object.keys(ref.config.cache).reduce((cache, entry) => {
const entryWithNewHash = entry.replace(oldHash, res.hash);
cache[entryWithNewHash] = ref.config.cache[entry];
return cache;
}, {});
}
ref.remote.hash = res.hash;
}
}
Expand Down

0 comments on commit 5e3552a

Please sign in to comment.