Use local package registry when installing packages and dependencies (#680)

#680

Currently, when a package is installing its dependencies and those dependencies are in different subtrees of the monorepo with different yarn registries, the update and sync methods in jazelle are not aware of the difference in registries when it stitches the lockfile back together.

During the stitching process, jazelle will index all of the lockfiles and then as it builds the lockfiles back up, it will take the highest versioned package across all deps and use that which is correct. However, if one of the projects was resolved with a different package registry, jazelle will potentially overwrite it indiscriminately since it has no concept of ensuring that the registry is correct for that package.

This fix uses liberal use of yarn config get registry before installing missing and transitive deps so that the correct registry for the package will be used.

In the case of lockfile syncing, this fix will convert all lockfile resolved paths to a relative path instead to be used as the index. Then when lockfiles are rebuilt, the registry that is local to the package (again using yarn config get registry) will be prepended to the relative path to reconstruct the lockfile.

There is an open issue in the yarn repo that discusses changing the paths for good to relative paths but there seems to be no recent movement on this: yarnpkg/yarn#5892

The end result of this PR is that:

multiple projects in the monorepo can have their own .yarnrc files that point to different registries
jazelle will ensure all dependencies are synced across all projects it controls while respecting the configured registry for each project

Co-authored-by: Derek Ju <[email protected]>
Co-authored-by: UberOpenSourceBot <[email protected]>

Author: UberOpenSourceBot

jazelle/tests/fixtures/lockfile-registry-resolution/a/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "a",
+  "version": "0.0.0",
+  "dependencies": {
+    "b": "0.0.0",
+    "c": "0.0.0"
+  }
+}

jazelle/tests/fixtures/lockfile-registry-resolution/b/.yarnrc

@@ -0,0 +1 @@
+--registry "https://registry.yarnpkg.com"

jazelle/tests/fixtures/lockfile-registry-resolution/b/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "b",
+  "version": "0.0.0",
+  "dependencies": {
+    "function-bind": "1.1.1"
+  }
+}

jazelle/tests/fixtures/lockfile-registry-resolution/c/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "c",
+  "version": "0.0.0",
+  "dependencies": {
+    "function-bind": "1.1.1"
+  }
+}

jazelle/tests/fixtures/lockfile-registry-resolution/manifest.json

@@ -0,0 +1,8 @@
+{
+  "workspace": "sandbox",
+  "projects": [
+    "a",
+    "b",
+    "c"
+  ]
+}

jazelle/tests/fixtures/lockfile-registry-resolution/third_party/jazelle/BUILD.bazel

@@ -0,0 +1,8 @@
+load("@jazelle//:build-rules.bzl", "web_library")
+
+package(default_visibility = ["//visibility:public"])
+
+web_library(
+  name = "node_modules",
+  srcs = glob(["**/*"]),
+)

jazelle/tests/fixtures/lockfile-registry-resolution/third_party/jazelle/scripts/build-file-template.js

@@ -0,0 +1,66 @@
+// @flow
+/*::
+type TemplateArgs = {
+  name: string,
+  path: string,
+  label: string,
+  dependencies: Array<string>,
+}
+type Template = (TemplateArgs) => Promise<string>;
+*/
+const template /*: Template */ = async ({name, path, dependencies}) => `
+package(default_visibility = ["//visibility:public"])
+
+load("@jazelle//:build-rules.bzl", "web_library", "web_binary", "web_executable", "web_test", "flow_test")
+
+web_library(
+    name = "library",
+    deps = [
+        "//third_party/jazelle:node_modules",
+        ${dependencies.map(d => `"${d}",`).join('\n        ')}
+    ],
+    srcs = glob(["**/*"], exclude = ["dist/**"]),
+)
+
+web_binary(
+    name = "${name}",
+    build = "build",
+    command = "start",
+    deps = [
+        "//${path}:library",
+    ],
+    dist = ["dist"],
+)
+
+web_executable(
+    name = "dev",
+    command = "dev",
+    deps = [
+        "//${path}:library",
+    ],
+)
+
+web_test(
+    name = "test",
+    command = "test",
+    deps = [
+        "//${path}:library",
+    ],
+)
+
+web_test(
+    name = "lint",
+    command = "lint",
+    deps = [
+        "//${path}:library",
+    ],
+)
+
+flow_test(
+    name = "flow",
+    deps = [
+        "//${path}:library",
+    ],
+)`;
+
+module.exports = {template};

jazelle/tests/index.js

@@ -40,6 +40,10 @@ const {getRootDir} = require('../utils/get-root-dir.js');
 const {installDeps} = require('../utils/install-deps.js');
 const {isYarnResolution} = require('../utils/is-yarn-resolution.js');
 const {parse, getPassThroughArgs} = require('../utils/parse-argv.js');
+const {absoluteUrlToRelative} = require('../utils/absolute-url-to-relative.js');
+const {
+  prependRegistryToLockfileEntry,
+} = require('../utils/prepend-registry-to-lockfile-entry.js');
 
 const {
   reportMismatchedTopLevelDeps,
@@ -106,6 +110,9 @@ async function runTests() {
     t(testStarlark),
     t(testYarnCommands),
     t(testBin),
+    t(testLockfileRegistryResolution),
+    t(testAbsoluteUrlToRelative),
+    t(testPrependRegistryToLockfileEntry),
   ]);
 
   await exec(`rm -rf ${__dirname}/tmp`);
@@ -1289,3 +1296,99 @@ async function testBin() {
   await exec(`${jazelle} start`, {cwd: `${cwd}/a`}, [startStream]);
   assert((await read(startStreamFile, 'utf8')).includes('\nstart\n'));
 }
+
+async function testLockfileRegistryResolution() {
+  await exec(
+    `cp -r ${__dirname}/fixtures/lockfile-registry-resolution/ ${__dirname}/tmp/lockfile-registry-resolution`
+  );
+  await install({
+    root: `${__dirname}/tmp/lockfile-registry-resolution`,
+    cwd: `${__dirname}/tmp/lockfile-registry-resolution/a`,
+  });
+  assert(
+    (await read(
+      `${__dirname}/tmp/lockfile-registry-resolution/b/yarn.lock`,
+      'utf8'
+    )).includes('registry.yarnpkg.com')
+  );
+  assert(
+    (await read(
+      `${__dirname}/tmp/lockfile-registry-resolution/c/yarn.lock`,
+      'utf8'
+    )).includes('registry.npmjs.org')
+  );
+}
+
+async function testAbsoluteUrlToRelative() {
+  assert(
+    absoluteUrlToRelative(
+      'https://registry.npmjs.org/@babel/cli/-/cli-7.5.5.tgz'
+    ) === '/@babel/cli/-/cli-7.5.5.tgz'
+  );
+  assert(
+    absoluteUrlToRelative(
+      'http://registry.npmjs.org/@babel/cli/-/cli-7.5.5.tgz'
+    ) === '/@babel/cli/-/cli-7.5.5.tgz'
+  );
+  assert(
+    absoluteUrlToRelative(
+      'https://singledomain.com/@babel/cli/-/cli-7.5.5.tgz'
+    ) === '/@babel/cli/-/cli-7.5.5.tgz'
+  );
+  assert(
+    absoluteUrlToRelative(
+      'https://many.more.domains.so.crazy.com/@babel/cli/-/cli-7.5.5.tgz'
+    ) === '/@babel/cli/-/cli-7.5.5.tgz'
+  );
+}
+/*
+export type LockfileEntry = {
+  version: string,
+  resolved: string,
+  dependencies?: {
+    [string]: string,
+  }
+};
+*/
+async function testPrependRegistryToLockfileEntry() {
+  // Test resolved has prefixed /
+  assert(
+    prependRegistryToLockfileEntry(
+      {
+        version: '1.0.0',
+        resolved: '/@babel/cli/-/cli-7.5.5.tgz',
+      },
+      'https://registry.npmjs.org'
+    ).resolved === 'https://registry.npmjs.org/@babel/cli/-/cli-7.5.5.tgz'
+  );
+  // Test registery has suffix /
+  assert(
+    prependRegistryToLockfileEntry(
+      {
+        version: '1.0.0',
+        resolved: '@babel/cli/-/cli-7.5.5.tgz',
+      },
+      'https://registry.npmjs.org/'
+    ).resolved === 'https://registry.npmjs.org/@babel/cli/-/cli-7.5.5.tgz'
+  );
+  // Test none of the inputs have /
+  assert(
+    prependRegistryToLockfileEntry(
+      {
+        version: '1.0.0',
+        resolved: '@babel/cli/-/cli-7.5.5.tgz',
+      },
+      'https://registry.npmjs.org'
+    ).resolved === 'https://registry.npmjs.org/@babel/cli/-/cli-7.5.5.tgz'
+  );
+  // Test invalid registry input
+  assert(
+    prependRegistryToLockfileEntry(
+      {
+        version: '1.0.0',
+        resolved: '@babel/cli/-/cli-7.5.5.tgz',
+      },
+      ''
+    ).resolved === '@babel/cli/-/cli-7.5.5.tgz'
+  );
+}

jazelle/utils/absolute-url-to-relative.js

@@ -0,0 +1,9 @@
+// @flow
+
+/*::
+export type AbsoluteUrlToRelative = (string) => string
+*/
+const absoluteUrlToRelative /*: AbsoluteUrlToRelative */ = url => {
+  return url.replace(/^https?:\/\/.*?\/(.+)$/, '/$1');
+};
+module.exports = {absoluteUrlToRelative};

jazelle/utils/lockfile.js

@@ -4,6 +4,10 @@ const {parse, stringify} = require('@yarnpkg/lockfile');
 const {read, exec, write} = require('./node-helpers.js');
 const {node, yarn} = require('./binary-paths.js');
 const {isYarnResolution} = require('./is-yarn-resolution.js');
+const {absoluteUrlToRelative} = require('./absolute-url-to-relative.js');
+const {
+  prependRegistryToLockfileEntry,
+} = require('./prepend-registry-to-lockfile-entry.js');
 
 /*::
 export type Report = {
@@ -218,14 +222,16 @@ const getDepEntries = meta => {
 /*::
 import type {PackageJson} from './get-local-dependencies.js';
 
-export type Lockfile = {
-  [string]: {
-    version: string,
-    dependencies?: {
-      [string]: string,
-    }
+export type LockfileEntry = {
+  version: string,
+  resolved: string,
+  dependencies?: {
+    [string]: string,
   }
-}
+};
+export type Lockfile = {
+  [string]: LockfileEntry,
+};
 export type VersionSet = {
   dir: string,
   meta: PackageJson,
@@ -266,7 +272,7 @@ const update /*: Update */ = async ({
     );
   }
 
-  for (const {meta, lockfile} of sets) {
+  for (const {dir, meta, lockfile} of sets) {
     // handle removals
     if (removals.length > 0) {
       for (const name of removals) {
@@ -327,8 +333,7 @@ const update /*: Update */ = async ({
       const data = JSON.stringify(missing, null, 2);
       await exec(`mkdir -p ${cwd}`);
       await write(`${cwd}/package.json`, data, 'utf8');
-      const yarnrc = '"--install.frozen-lockfile" false';
-      await write(`${cwd}/.yarnrc`, yarnrc, 'utf8');
+      await writeYarnRc(cwd, dir);
       const install = `${node} ${yarn} install --ignore-scripts --ignore-engines`;
       await exec(install, {cwd}, [process.stdout, process.stderr]);
 
@@ -356,8 +361,7 @@ const update /*: Update */ = async ({
     if (missingTransitives.length > 0) {
       const cwd = `${tmp}/yarn-utils-${Math.random() * 1e17}`;
       await exec(`mkdir -p ${cwd}`);
-      const yarnrc = '"--add.frozen-lockfile" false';
-      await write(`${cwd}/.yarnrc`, yarnrc, 'utf8');
+      await writeYarnRc(cwd, dir);
       await write(`${cwd}/package.json`, '{}', 'utf8');
       const deps = missingTransitives.join(' ');
       const add = `yarn add ${deps} --ignore-engines`;
@@ -382,7 +386,13 @@ const update /*: Update */ = async ({
       const id = `${key}|${lockfile[key].version}`;
       if (!index[name]) index[name] = [];
       if (!ids.has(id)) {
-        index[name].push({lockfile, key, isAlias});
+        // Before pushing the lockfile to the index, convert the registry to a relative path
+        // When we sync the dependencies later, we'll add them back
+        index[name].push({
+          lockfile: convertToRelativeDomain(lockfile),
+          key,
+          isAlias,
+        });
         ids.add(id);
       }
     }
@@ -397,7 +407,8 @@ const update /*: Update */ = async ({
 
   // sync
   for (const item of sets) {
-    const {meta} = item;
+    const {dir, meta} = item;
+    const registry = await getRegistry(dir);
     const graph = {};
     for (const {name, range} of getDepEntries(meta)) {
       const ignored = ignore.find(dep => dep === name);
@@ -414,7 +425,10 @@ const update /*: Update */ = async ({
     }
     for (const key in graph) {
       const [, name] = key.match(/(.+?)@(.+)/) || [];
-      lockfile[key] = map[`${name}@${graph[key].version}`];
+      lockfile[key] = prependRegistryToLockfileEntry(
+        map[`${name}@${graph[key].version}`],
+        registry
+      );
     }
 
     if (frozenLockfile) {
@@ -467,4 +481,30 @@ const isBetterVersion = (version, range, graph, key) => {
   );
 };
 
+const getRegistry = async cwd => {
+  const getRegistry = `${node} ${yarn} config get registry`;
+  const registry = await exec(getRegistry, {cwd});
+  return registry ? registry.trim() : '';
+};
+
+const writeYarnRc = async (cwd, packageDir) => {
+  const yarnrc = ['"--install.frozen-lockfile" false'];
+  const registry = await getRegistry(packageDir);
+  if (registry) {
+    yarnrc.push(`--registry "${registry}"`);
+  }
+  await write(`${cwd}/.yarnrc`, yarnrc.join('\n'), 'utf8');
+};
+
+const convertToRelativeDomain = lockfile => {
+  const converted = {};
+  for (const key of Object.keys(lockfile)) {
+    converted[key] = {
+      ...lockfile[key],
+      resolved: absoluteUrlToRelative(lockfile[key].resolved),
+    };
+  }
+  return converted;
+};
+
 module.exports = {check, add, remove, upgrade, sync, merge};