From bb927d0a8318dc7825f7ab1f464a79c24a15ebee Mon Sep 17 00:00:00 2001 From: Sebastian McKenzie Date: Sat, 22 Apr 2017 16:45:03 +0100 Subject: [PATCH] Put build artifact tracking in integrity file (#3224) * Put build artifact tracking in integrity file * fix lint --- __tests__/commands/install/integration.js | 6 ++--- src/cli/commands/install.js | 14 +++++++++- src/integrity-checker.js | 12 ++++++--- src/package-install-scripts.js | 32 ++++++++++++----------- src/package-linker.js | 17 ++++++++++-- 5 files changed, 57 insertions(+), 24 deletions(-) diff --git a/__tests__/commands/install/integration.js b/__tests__/commands/install/integration.js index e83b993eac..5fbc469f89 100644 --- a/__tests__/commands/install/integration.js +++ b/__tests__/commands/install/integration.js @@ -52,9 +52,9 @@ test.concurrent('properly find and save build artifacts', async () => { await runInstall({}, 'artifacts-finds-and-saves', async (config): Promise => { const cacheFolder = path.join(config.cacheFolder, 'npm-dummy-0.0.0'); - expect( - (await fs.readJson(path.join(cacheFolder, constants.METADATA_FILENAME))).artifacts, - ).toEqual( + const integrity = await fs.readJson(path.join(config.cwd, 'node_modules', constants.INTEGRITY_FILENAME)); + + expect(integrity.artifacts['dummy@0.0.0']).toEqual( ['dummy', path.join('dummy', 'dummy.txt'), 'dummy.txt'], ); diff --git a/src/cli/commands/install.js b/src/cli/commands/install.js index 2e8e3541d5..a83fc819a7 100644 --- a/src/cli/commands/install.js +++ b/src/cli/commands/install.js @@ -329,6 +329,12 @@ export class Install { return true; } + const {artifacts} = match; + if (artifacts) { + this.linker.setArtifacts(artifacts); + this.scripts.setArtifacts(artifacts); + } + return false; } @@ -603,7 +609,13 @@ export class Install { } // write integrity hash - await this.integrityChecker.save(patterns, lockfileBasedOnResolver, this.flags, this.resolver.usedRegistries); + await this.integrityChecker.save( + patterns, + lockfileBasedOnResolver, + this.flags, + this.resolver.usedRegistries, + this.scripts.getArtifacts(), + ); const lockFileHasAllPatterns = patterns.filter((p) => !this.lockfile.getLocked(p)).length === 0; const resolverPatternsAreSameAsInLockfile = Object.keys(lockfileBasedOnResolver) diff --git a/src/integrity-checker.js b/src/integrity-checker.js index f21d2bc9aa..99abb24ac2 100644 --- a/src/integrity-checker.js +++ b/src/integrity-checker.js @@ -8,7 +8,7 @@ import * as constants from './constants.js'; import {registryNames} from './registries/index.js'; import * as fs from './util/fs.js'; import {sortAlpha, compareSortedArrays} from './util/misc.js'; - +import type {InstallArtifacts} from './package-install-scripts.js'; const invariant = require('invariant'); const path = require('path'); @@ -17,6 +17,7 @@ export type IntegrityCheckResult = { integrityFileMissing: boolean, integrityMatches?: boolean, missingPatterns: Array, + artifacts?: ?InstallArtifacts, }; type IntegrityHashLocation = { @@ -33,6 +34,7 @@ type IntegrityFile = { [key: string]: string }, files: Array, + artifacts: ?InstallArtifacts, } type IntegrityFlags = { @@ -125,6 +127,7 @@ export default class InstallationIntegrityChecker { patterns: Array, flags: IntegrityFlags, modulesFolder: string, + artifacts?: InstallArtifacts, ): Promise { const result: IntegrityFile = { @@ -133,6 +136,7 @@ export default class InstallationIntegrityChecker { topLevelPatters: [], lockfileEntries: {}, files: [], + artifacts, }; result.topLevelPatters = patterns.sort(sortAlpha); @@ -247,6 +251,7 @@ export default class InstallationIntegrityChecker { integrityFileMissing: false, integrityMatches, missingPatterns, + artifacts: expected ? expected.artifacts : null, }; } @@ -257,11 +262,12 @@ export default class InstallationIntegrityChecker { patterns: Array, lockfile: {[key: string]: LockManifest}, flags: IntegrityFlags, - usedRegistries?: Set): Promise { + usedRegistries?: Set, + artifacts: InstallArtifacts): Promise { const loc = await this._getIntegrityHashLocation(usedRegistries); invariant(loc.locationPath, 'expected integrity hash location'); await fs.mkdirp(path.dirname(loc.locationPath)); - const integrityFile = await this._generateIntegrityFile(lockfile, patterns, flags, loc.locationFolder); + const integrityFile = await this._generateIntegrityFile(lockfile, patterns, flags, loc.locationFolder, artifacts); await fs.writeFile(loc.locationPath, JSON.stringify(integrityFile, null, 2)); } diff --git a/src/package-install-scripts.js b/src/package-install-scripts.js index e7d0f56548..2f9e21e34e 100644 --- a/src/package-install-scripts.js +++ b/src/package-install-scripts.js @@ -14,6 +14,10 @@ const path = require('path'); const INSTALL_STAGES = ['preinstall', 'install', 'postinstall']; +export type InstallArtifacts = { + [pattern: string]: Array, +}; + export default class PackageInstallScripts { constructor(config: Config, resolver: PackageResolver, force: boolean) { this.installed = 0; @@ -21,6 +25,7 @@ export default class PackageInstallScripts { this.reporter = config.reporter; this.config = config; this.force = force; + this.artifacts = {}; } needsPermission: boolean; @@ -29,7 +34,15 @@ export default class PackageInstallScripts { installed: number; config: Config; force: boolean; + artifacts: InstallArtifacts; + + setArtifacts(artifacts: InstallArtifacts) { + this.artifacts = artifacts; + } + getArtifacts(): InstallArtifacts { + return this.artifacts; + } getInstallCommands(pkg: Manifest): Array<[string, string]> { const scripts = pkg.scripts; @@ -77,21 +90,10 @@ export default class PackageInstallScripts { return; } - // if the process is killed while copying over build artifacts then we'll leave - // the cache in a bad state. remove the metadata file and add it back once we've - // done our copies to ensure cache integrity. - const cachedLoc = this.config.generateHardModulePath(pkg._reference, true); - const metadata = await this.config.readPackageMetadata(cachedLoc); - metadata.artifacts = buildArtifacts; - - const metadataLoc = path.join(cachedLoc, constants.METADATA_FILENAME); - await fs.writeFile(metadataLoc, JSON.stringify({ - ...metadata, - - // config.readPackageMetadata also returns the package manifest but that's not in the original - // metadata json - package: undefined, - }, null, ' ')); + // set build artifacts + const ref = pkg._reference; + invariant(ref, 'expected reference'); + this.artifacts[`${pkg.name}@${pkg.version}`] = buildArtifacts; } async install(cmds: Array<[string, string]>, pkg: Manifest, spinner: ReporterSetSpinner): Promise { diff --git a/src/package-linker.js b/src/package-linker.js index ecb4f1501c..e7259d8eb1 100644 --- a/src/package-linker.js +++ b/src/package-linker.js @@ -6,6 +6,7 @@ import type {Reporter} from './reporters/index.js'; import type Config from './config.js'; import type {HoistManifestTuples} from './package-hoister.js'; import type {CopyQueueItem} from './util/fs.js'; +import type {InstallArtifacts} from './package-install-scripts.js'; import PackageHoister from './package-hoister.js'; import * as constants from './constants.js'; import * as promise from './util/promise.js'; @@ -43,12 +44,18 @@ export default class PackageLinker { this.resolver = resolver; this.reporter = config.reporter; this.config = config; + this.artifacts = {}; } + artifacts: InstallArtifacts; reporter: Reporter; resolver: PackageResolver; config: Config; + setArtifacts(artifacts: InstallArtifacts) { + this.artifacts = artifacts; + } + async linkSelfDependencies(pkg: Manifest, pkgLoc: string, targetBinLoc: string): Promise { targetBinLoc = await fs.realpath(targetBinLoc); pkgLoc = await fs.realpath(pkgLoc); @@ -139,13 +146,19 @@ export default class PackageLinker { invariant(ref, 'expected package reference'); ref.setLocation(dest); - // get a list of build artifacts contained in this module so we can prevent them from being marked as - // extraneous + // backwards compatibility: get build artifacts from metadata const metadata = await this.config.readPackageMetadata(src); for (const file of metadata.artifacts) { artifactFiles.push(path.join(dest, file)); } + const integrityArtifacts = this.artifacts[`${pkg.name}@${pkg.version}`]; + if (integrityArtifacts) { + for (const file of integrityArtifacts) { + artifactFiles.push(path.join(dest, file)); + } + } + const copiedDest = copiedSrcs.get(src); if (!copiedDest) { if (hardlinksEnabled) {