Put build artifact tracking in integrity file (#3224)

* Put build artifact tracking in integrity file

* fix lint

Author: Sebastian McKenzie

__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<void> => {
     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'],
     );
 

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)

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<string>,
+  artifacts?: ?InstallArtifacts,
 };
 
 type IntegrityHashLocation = {
@@ -33,6 +34,7 @@ type IntegrityFile = {
     [key: string]: string
   },
   files: Array<string>,
+  artifacts: ?InstallArtifacts,
 }
 
 type IntegrityFlags = {
@@ -125,6 +127,7 @@ export default class InstallationIntegrityChecker {
     patterns: Array<string>,
     flags: IntegrityFlags,
     modulesFolder: string,
+    artifacts?: InstallArtifacts,
   ): Promise<IntegrityFile> {
 
     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<string>,
     lockfile: {[key: string]: LockManifest},
     flags: IntegrityFlags,
-    usedRegistries?: Set<RegistryNames>): Promise<void> {
+    usedRegistries?: Set<RegistryNames>,
+    artifacts: InstallArtifacts): Promise<void> {
     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));
   }
 

src/package-install-scripts.js

@@ -14,13 +14,18 @@ const path = require('path');
 
 const INSTALL_STAGES = ['preinstall', 'install', 'postinstall'];
 
+export type InstallArtifacts = {
+  [pattern: string]: Array<string>,
+};
+
 export default class PackageInstallScripts {
   constructor(config: Config, resolver: PackageResolver, force: boolean) {
     this.installed = 0;
     this.resolver = resolver;
     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<void> {

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<void> {
     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) {