Fix perf regression (#6204)

* Revert "fix(util/fs): use file content instead of mtime to determine equality (#6010)"

This reverts commit c53d039.

* Bumps the cache version

* Forces the mtime to be based on the time the archives got fetched

* Fixes subseconds time assignment (nodejs/node#22070)

* Adds regression tests for #5723

* Fixes tests

* Fixes test on architectures that don't support subsecond precision

__tests__/commands/install/integration.js

@@ -10,6 +10,7 @@ import * as reporters from '../../../src/reporters/index.js';
 import {Install, run as install} from '../../../src/cli/commands/install.js';
 import Lockfile from '../../../src/lockfile';
 import * as fs from '../../../src/util/fs.js';
+import * as misc from '../../../src/util/misc.js';
 import {getPackageVersion, explodeLockfile, runInstall, runLink, createLockfile, run as buildRun} from '../_helpers.js';
 
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 150000;
@@ -111,6 +112,20 @@ test('installing a package with a renamed file should not delete it', () =>
     expect(await fs.exists(`${config.cwd}/node_modules/pkg/state.js`)).toEqual(true);
   }));
 
+test("installing a new package should correctly update it, even if the files mtime didn't change", () =>
+  runInstall({}, 'mtime-same', async (config, reporter): Promise<void> => {
+    await misc.sleep(2000);
+
+    const pkgJson = await fs.readJson(`${config.cwd}/package.json`);
+    pkgJson.dependencies['pkg'] = 'file:./pkg-b.tgz';
+    await fs.writeFile(`${config.cwd}/package.json`, JSON.stringify(pkgJson));
+
+    const reInstall = new Install({}, config, reporter, await Lockfile.fromDirectory(config.cwd));
+    await reInstall.init();
+
+    expect(await fs.readJson(`${config.cwd}/node_modules/pkg/package.json`)).toMatchObject({version: '2.0.0'});
+  }));
+
 test('properly find and save build artifacts', () =>
   runInstall({}, 'artifacts-finds-and-saves', async config => {
     const integrity = await fs.readJson(path.join(config.cwd, 'node_modules', constants.INTEGRITY_FILENAME));
@@ -217,7 +232,7 @@ test('changes the cache path when bumping the cache version', () =>
     const reporter = new reporters.JSONReporter({stdout: inOut});
 
     await cache(config, reporter, {}, ['dir']);
-    expect((JSON.parse(String(inOut.read())): any).data).toMatch(/[\\\/]v1[\\\/]?$/);
+    expect((JSON.parse(String(inOut.read())): any).data).toMatch(/[\\\/]v(?!42[\\\/]?$)[0-9]+[\\\/]?$/);
 
     await mockConstants(config, {CACHE_VERSION: 42}, async config => {
       await cache(config, reporter, {}, ['dir']);

__tests__/fixtures/install/mtime-same/package.json

@@ -0,0 +1,5 @@
+{
+    "dependencies": {
+        "pkg": "file:./pkg-a.tgz"
+    }
+}

src/constants.js

@@ -25,7 +25,7 @@ export const YARN_INSTALLER_MSI = 'https://yarnpkg.com/latest.msi';
 export const SELF_UPDATE_VERSION_URL = 'https://yarnpkg.com/latest-version';
 
 // cache version, bump whenever we make backwards incompatible changes
-export const CACHE_VERSION = 1;
+export const CACHE_VERSION = 2;
 
 // lockfile version, bump whenever we make backwards incompatible changes
 export const LOCKFILE_VERSION = 1;

src/fetchers/tarball-fetcher.js

@@ -93,12 +93,48 @@ export default class TarballFetcher extends BaseFetcher {
   } {
     const integrityInfo = this._supportedIntegrity();
 
+    const now = new Date();
+
+    const fs = require('fs');
+    const patchedFs = Object.assign({}, fs, {
+      utimes: (path, atime, mtime, cb) => {
+        fs.stat(path, (err, stat) => {
+          if (err) {
+            cb(err);
+            return;
+          }
+          if (stat.isDirectory()) {
+            fs.utimes(path, atime, mtime, cb);
+            return;
+          }
+          fs.open(path, 'a', (err, fd) => {
+            if (err) {
+              cb(err);
+              return;
+            }
+            fs.futimes(fd, atime, mtime, err => {
+              if (err) {
+                fs.close(fd, () => cb(err));
+              } else {
+                fs.close(fd, err => cb(err));
+              }
+            });
+          });
+        });
+      },
+    });
+
     const validateStream = new ssri.integrityStream(integrityInfo);
     const untarStream = tarFs.extract(this.dest, {
       strip: 1,
       dmode: 0o755, // all dirs should be readable
       fmode: 0o644, // all files should be readable
       chown: false, // don't chown. just leave as it is
+      map: header => {
+        header.mtime = now;
+        return header;
+      },
+      fs: patchedFs,
     });
     const extractorStream = gunzip();
 

src/reporters/lang/en.js

@@ -46,7 +46,7 @@ const messages = {
   verboseFileCopy: 'Copying $0 to $1.',
   verboseFileLink: 'Creating hardlink at $0 to $1.',
   verboseFileSymlink: 'Creating symlink at $0 to $1.',
-  verboseFileSkip: 'Skipping copying of file $0 as the file at $1 is the same size ($2) and has the same content.',
+  verboseFileSkip: 'Skipping copying of file $0 as the file at $1 is the same size ($2) and mtime ($3).',
   verboseFileSkipSymlink: 'Skipping copying of $0 as the file at $1 is the same symlink ($2).',
   verboseFileSkipHardlink: 'Skipping copying of $0 as the file at $1 is the same hardlink ($2).',
   verboseFileRemoveExtraneous: 'Removing extraneous file $0.',

src/util/fs.js

@@ -13,7 +13,7 @@ import BlockingQueue from './blocking-queue.js';
 import * as promise from './promise.js';
 import {promisify} from './promise.js';
 import map from './map.js';
-import {copyFile, unlink} from './fs-normalized.js';
+import {copyFile, fileDatesEqual, unlink} from './fs-normalized.js';
 
 export const constants =
   typeof fs.constants !== 'undefined'
@@ -222,10 +222,10 @@ async function buildActionsForCopy(
         return;
       }
 
-      if (bothFiles && srcStat.size === destStat.size && fs.readFileSync(src).equals(fs.readFileSync(dest))) {
+      if (bothFiles && srcStat.size === destStat.size && fileDatesEqual(srcStat.mtime, destStat.mtime)) {
         // we can safely assume this is the same file
         onDone();
-        reporter.verbose(reporter.lang('verboseFileSkip', src, dest, srcStat.size));
+        reporter.verbose(reporter.lang('verboseFileSkip', src, dest, srcStat.size, +srcStat.mtime));
         return;
       }