Fix: Don't do range collapsing when using a frozen lockfile (yarnpkg#…

…3604)

**Summary**

Fixes yarnpkg#3313.

Yarn automatically optimizes less-than-ideal `yarn.lock` files, usually from older versions. That said when run with the `--frozen-lockfile` argument, it should neither touch the lockfile nor throw an exception if the lockfile satisfies all the needs, even if it can be optimized.

**Test plan**

Existing unit tests plus a new unit test which fails without the fix applied.

__tests__/commands/install/integration.js

@@ -144,7 +144,7 @@ test('changes the cache directory when bumping the cache version', async () => {
     const lockfile = await Lockfile.fromDirectory(config.cwd);
 
     const resolver = new PackageResolver(config, lockfile);
-    await resolver.init([{pattern: 'is-array', registry: 'npm'}]);
+    await resolver.init([{pattern: 'is-array', registry: 'npm'}], {});
 
     const ref = resolver.getManifests()[0]._reference;
     const cachePath = config.generateHardModulePath(ref, true);

__tests__/commands/install/lockfiles.js

@@ -71,6 +71,20 @@ test.concurrent("throws an error if existing lockfile isn't satisfied with --fro
   expect(thrown).toEqual(true);
 });
 
+test.concurrent(
+  "doesn't write new lockfile if existing one satisfied but not fully optimized with --frozen-lockfile",
+  (): Promise<void> => {
+    return runInstall(
+      {frozenLockfile: true},
+      'install-should-not-write-lockfile-if-not-optimized-and-frozen',
+      async (config): Promise<void> => {
+        const lockfile = await fs.readFile(path.join(config.cwd, 'yarn.lock'));
+        expect(lockfile.indexOf('left-pad@1.1.3:')).toBeGreaterThanOrEqual(0);
+      },
+    );
+  },
+);
+
 test.concurrent('install transitive optional dependency from lockfile', (): Promise<void> => {
   return runInstall({}, 'install-optional-dep-from-lockfile', (config, reporter, install) => {
     expect(install && install.resolver && install.resolver.patterns['fsevents@^1.0.0']).toBeTruthy();

__tests__/fixtures/install/install-should-not-write-lockfile-if-not-optimized-and-frozen/package.json

@@ -0,0 +1,6 @@
+{
+  "dependencies": {
+    "left-pad": "1.1.3",
+    "node.date-time": "1.2.2"
+  }
+}

__tests__/fixtures/install/install-should-not-write-lockfile-if-not-optimized-and-frozen/yarn.lock

@@ -0,0 +1,17 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+# yarn lockfile v1
+
+
+left-pad@1.1.3:
+  version "1.1.3"
+  resolved "https://registry.yarnpkg.com/left-pad/-/left-pad-1.1.3.tgz#612f61c033f3a9e08e939f1caebeea41b6f3199a"
+
+left-pad@^1.1.0:
+  version "1.1.1"
+  resolved "https://registry.yarnpkg.com/left-pad/-/left-pad-1.1.1.tgz#ca566bbdd84b90cc5969ac1726fda51f9d936a3c"
+
+node.date-time@1.2.2:
+  version "1.2.2"
+  resolved "https://registry.yarnpkg.com/node.date-time/-/node.date-time-1.2.2.tgz#2fc553b0520f1c75625b33fa5a1835c637ad9856"
+  dependencies:
+    left-pad "^1.1.0"

__tests__/package-resolver.js

@@ -42,7 +42,7 @@ function addTest(pattern, registry = 'npm', init: ?(cacheFolder: string) => Prom
     }
 
     const resolver = new PackageResolver(config, lockfile);
-    await resolver.init([{pattern, registry}]);
+    await resolver.init([{pattern, registry}], {});
 
     const ref = resolver.getManifests()[0]._reference;
     const cachePath = config.generateHardModulePath(ref, true);

src/cli/commands/import.js

@@ -1,5 +1,6 @@
 /* @flow */
 
+import type {ResolverOptions} from '../../package-resolver.js';
 import type {Manifest, DependencyRequestPattern, DependencyRequestPatterns} from '../../types.js';
 import type {Reporter} from '../../reporters/index.js';
 import type Config from '../../config.js';
@@ -227,7 +228,7 @@ class ImportPackageResolver extends PackageResolver {
       this.activity.tick(req.pattern);
     }
     const request = new ImportPackageRequest(req, this);
-    await request.find(false);
+    await request.find({fresh: false});
   }
 
   async findAll(deps: DependencyRequestPatterns): Promise<void> {
@@ -254,8 +255,11 @@ class ImportPackageResolver extends PackageResolver {
     }
   }
 
-  async init(deps: DependencyRequestPatterns, isFlat: boolean): Promise<void> {
-    this.flat = isFlat;
+  async init(
+    deps: DependencyRequestPatterns,
+    {isFlat, isFrozen, workspaceLayout}: ResolverOptions = {isFlat: false, isFrozen: false, workspaceLayout: undefined},
+  ): Promise<void> {
+    this.flat = Boolean(isFlat);
     const activity = (this.activity = this.reporter.activity());
     await this.findAll(deps);
     this.resetOptional();
@@ -280,7 +284,7 @@ export class Import extends Install {
     if (manifest.name && this.resolver instanceof ImportPackageResolver) {
       this.resolver.rootName = manifest.name;
     }
-    await this.resolver.init(requests, this.flags.flat);
+    await this.resolver.init(requests, {isFlat: this.flags.flat, isFrozen: this.flags.frozenLockfile});
     const manifests: Array<Manifest> = await fetcher.fetch(this.resolver.getManifests(), this.config);
     this.resolver.updateManifests(manifests);
     await compatibility.check(this.resolver.getManifests(), this.config, this.flags.ignoreEngines);

src/cli/commands/install.js

@@ -55,6 +55,7 @@ type Flags = {
   flat: boolean,
   lockfile: boolean,
   pureLockfile: boolean,
+  frozenLockfile: boolean,
   skipIntegrityCheck: boolean,
   checkFiles: boolean,
 
@@ -422,7 +423,11 @@ export class Install {
 
     steps.push(async (curr: number, total: number) => {
       this.reporter.step(curr, total, this.reporter.lang('resolvingPackages'), emoji.get('mag'));
-      await this.resolver.init(this.prepareRequests(depRequests), this.flags.flat, workspaceLayout);
+      await this.resolver.init(this.prepareRequests(depRequests), {
+        isFlat: this.flags.flat,
+        isFrozen: this.flags.frozenLockfile,
+        workspaceLayout,
+      });
       topLevelPatterns = this.preparePatterns(rawPatterns);
       flattenedTopLevelPatterns = await this.flatten(topLevelPatterns);
       return {bailout: await this.bailout(topLevelPatterns, workspaceLayout)};
@@ -693,7 +698,11 @@ export class Install {
     const request = await this.fetchRequestFromCwd([], ignoreUnusedPatterns);
     const {requests: depRequests, patterns: rawPatterns, ignorePatterns, workspaceLayout} = request;
 
-    await this.resolver.init(depRequests, this.flags.flat, workspaceLayout);
+    await this.resolver.init(depRequests, {
+      isFlat: this.flags.flat,
+      isFrozen: this.flags.frozenLockfile,
+      workspaceLayout,
+    });
     await this.flatten(rawPatterns);
     this.markIgnored(ignorePatterns);
 

src/cli/commands/list.js

@@ -175,7 +175,7 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
   const lockfile = await Lockfile.fromDirectory(config.lockfileFolder, reporter);
   const install = new Install(flags, config, reporter, lockfile);
   const {requests: depRequests, patterns} = await install.fetchRequestFromCwd();
-  await install.resolver.init(depRequests, install.flags.flat);
+  await install.resolver.init(depRequests, {isFlat: install.flags.flat, isFrozen: install.flags.frozenLockfile});
 
   const opts: ListOptions = {
     reqDepth: getReqDepth(flags.depth),

src/cli/commands/upgrade.js

@@ -53,8 +53,7 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
       throw new MessageError(reporter.lang('scopeNotValid'));
     }
   } else if (flags.latest && args.length === 0) {
-    addArgs = Object.keys(allDependencies)
-      .map(dependency => getDependency(allDependencies, dependency));
+    addArgs = Object.keys(allDependencies).map(dependency => getDependency(allDependencies, dependency));
   } else {
     addArgs = args.map(dependency => {
       return getDependency(allDependencies, dependency);

src/cli/commands/why.js

@@ -131,7 +131,7 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
   const lockfile = await Lockfile.fromDirectory(config.lockfileFolder, reporter);
   const install = new Install(flags, config, reporter, lockfile);
   const {requests: depRequests, patterns} = await install.fetchRequestFromCwd();
-  await install.resolver.init(depRequests, install.flags.flat);
+  await install.resolver.init(depRequests, {isFlat: install.flags.flat, isFrozen: install.flags.frozenLockfile});
   const hoisted = await install.linker.getFlatHoistedTree(patterns);
 
   // finding

src/package-request.js

@@ -254,7 +254,7 @@ export default class PackageRequest {
   /**
    * TODO description
    */
-  async find(fresh: boolean): Promise<void> {
+  async find({fresh, frozen}: {fresh: boolean, frozen?: boolean}): Promise<void> {
     // find version info for this package pattern
     const info: ?Manifest = await this.findVersionInfo();
 
@@ -270,7 +270,9 @@ export default class PackageRequest {
     // check if while we were resolving this dep we've already resolved one that satisfies
     // the same range
     const {range, name} = PackageRequest.normalizePattern(this.pattern);
-    const resolved: ?Manifest = this.resolver.getHighestRangeVersionMatch(name, range);
+    const resolved: ?Manifest = frozen
+      ? this.resolver.getExactVersionMatch(name, range)
+      : this.resolver.getHighestRangeVersionMatch(name, range);
     if (resolved) {
       this.resolver.reportPackageWithExistingVersion(this, info);
       return;

src/package-resolver.js

@@ -15,6 +15,12 @@ import WorkspaceLayout from './workspace-layout.js';
 const invariant = require('invariant');
 const semver = require('semver');
 
+export type ResolverOptions = {|
+  isFlat?: boolean,
+  isFrozen?: boolean,
+  workspaceLayout?: WorkspaceLayout,
+|};
+
 export default class PackageResolver {
   constructor(config: Config, lockfile: Lockfile) {
     this.patternsByPackage = map();
@@ -33,6 +39,8 @@ export default class PackageResolver {
   // whether the dependency graph will be flattened
   flat: boolean;
 
+  frozen: boolean;
+
   workspaceLayout: ?WorkspaceLayout;
 
   // list of registries that have been used in this resolution
@@ -448,15 +456,19 @@ export default class PackageResolver {
     }
 
     const request = new PackageRequest(req, this);
-    await request.find(fresh);
+    await request.find({fresh, frozen: this.frozen});
   }
 
   /**
    * TODO description
    */
 
-  async init(deps: DependencyRequestPatterns, isFlat: boolean, workspaceLayout?: WorkspaceLayout): Promise<void> {
-    this.flat = isFlat;
+  async init(
+    deps: DependencyRequestPatterns,
+    {isFlat, isFrozen, workspaceLayout}: ResolverOptions = {isFlat: false, isFrozen: false, workspaceLayout: undefined},
+  ): Promise<void> {
+    this.flat = Boolean(isFlat);
+    this.frozen = Boolean(isFrozen);
     this.workspaceLayout = workspaceLayout;
     const activity = (this.activity = this.reporter.activity());
     await Promise.all(deps.map((req): Promise<void> => this.find(req)));