Skip to content

Commit fb39269

Browse files
UberOpenSourceBotderekju
UberOpenSourceBot
and
derekju
committed
Use local package registry when installing packages and dependencies (#466)
https://github.com/uber/fusionjs/pull/466 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]>
1 parent 75978ac commit fb39269

File tree

11 files changed

+297
-15
lines changed

11 files changed

+297
-15
lines changed

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

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "a",
3+
"version": "0.0.0",
4+
"dependencies": {
5+
"b": "0.0.0",
6+
"c": "0.0.0"
7+
}
8+
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
--registry "https://registry.yarnpkg.com"

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

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "b",
3+
"version": "0.0.0",
4+
"dependencies": {
5+
"function-bind": "1.1.1"
6+
}
7+
}

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

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "c",
3+
"version": "0.0.0",
4+
"dependencies": {
5+
"function-bind": "1.1.1"
6+
}
7+
}

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

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"workspace": "sandbox",
3+
"projects": [
4+
"a",
5+
"b",
6+
"c"
7+
]
8+
}

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

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
load("@jazelle//:build-rules.bzl", "web_library")
2+
3+
package(default_visibility = ["//visibility:public"])
4+
5+
web_library(
6+
name = "node_modules",
7+
srcs = glob(["**/*"]),
8+
)

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

+66
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// @flow
2+
/*::
3+
type TemplateArgs = {
4+
name: string,
5+
path: string,
6+
label: string,
7+
dependencies: Array<string>,
8+
}
9+
type Template = (TemplateArgs) => Promise<string>;
10+
*/
11+
const template /*: Template */ = async ({name, path, dependencies}) => `
12+
package(default_visibility = ["//visibility:public"])
13+
14+
load("@jazelle//:build-rules.bzl", "web_library", "web_binary", "web_executable", "web_test", "flow_test")
15+
16+
web_library(
17+
name = "library",
18+
deps = [
19+
"//third_party/jazelle:node_modules",
20+
${dependencies.map(d => `"${d}",`).join('\n ')}
21+
],
22+
srcs = glob(["**/*"], exclude = ["dist/**"]),
23+
)
24+
25+
web_binary(
26+
name = "${name}",
27+
build = "build",
28+
command = "start",
29+
deps = [
30+
"//${path}:library",
31+
],
32+
dist = ["dist"],
33+
)
34+
35+
web_executable(
36+
name = "dev",
37+
command = "dev",
38+
deps = [
39+
"//${path}:library",
40+
],
41+
)
42+
43+
web_test(
44+
name = "test",
45+
command = "test",
46+
deps = [
47+
"//${path}:library",
48+
],
49+
)
50+
51+
web_test(
52+
name = "lint",
53+
command = "lint",
54+
deps = [
55+
"//${path}:library",
56+
],
57+
)
58+
59+
flow_test(
60+
name = "flow",
61+
deps = [
62+
"//${path}:library",
63+
],
64+
)`;
65+
66+
module.exports = {template};

‎tests/index.js

+103
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ const {getRootDir} = require('../utils/get-root-dir.js');
4040
const {installDeps} = require('../utils/install-deps.js');
4141
const {isYarnResolution} = require('../utils/is-yarn-resolution.js');
4242
const {parse, getPassThroughArgs} = require('../utils/parse-argv.js');
43+
const {absoluteUrlToRelative} = require('../utils/absolute-url-to-relative.js');
44+
const {
45+
prependRegistryToLockfileEntry,
46+
} = require('../utils/prepend-registry-to-lockfile-entry.js');
4347

4448
const {
4549
reportMismatchedTopLevelDeps,
@@ -106,6 +110,9 @@ async function runTests() {
106110
t(testStarlark),
107111
t(testYarnCommands),
108112
t(testBin),
113+
t(testLockfileRegistryResolution),
114+
t(testAbsoluteUrlToRelative),
115+
t(testPrependRegistryToLockfileEntry),
109116
]);
110117

111118
await exec(`rm -rf ${__dirname}/tmp`);
@@ -1289,3 +1296,99 @@ async function testBin() {
12891296
await exec(`${jazelle} start`, {cwd: `${cwd}/a`}, [startStream]);
12901297
assert((await read(startStreamFile, 'utf8')).includes('\nstart\n'));
12911298
}
1299+
1300+
async function testLockfileRegistryResolution() {
1301+
await exec(
1302+
`cp -r ${__dirname}/fixtures/lockfile-registry-resolution/ ${__dirname}/tmp/lockfile-registry-resolution`
1303+
);
1304+
await install({
1305+
root: `${__dirname}/tmp/lockfile-registry-resolution`,
1306+
cwd: `${__dirname}/tmp/lockfile-registry-resolution/a`,
1307+
});
1308+
assert(
1309+
(await read(
1310+
`${__dirname}/tmp/lockfile-registry-resolution/b/yarn.lock`,
1311+
'utf8'
1312+
)).includes('registry.yarnpkg.com')
1313+
);
1314+
assert(
1315+
(await read(
1316+
`${__dirname}/tmp/lockfile-registry-resolution/c/yarn.lock`,
1317+
'utf8'
1318+
)).includes('registry.npmjs.org')
1319+
);
1320+
}
1321+
1322+
async function testAbsoluteUrlToRelative() {
1323+
assert(
1324+
absoluteUrlToRelative(
1325+
'https://registry.npmjs.org/@babel/cli/-/cli-7.5.5.tgz'
1326+
) === '/@babel/cli/-/cli-7.5.5.tgz'
1327+
);
1328+
assert(
1329+
absoluteUrlToRelative(
1330+
'http://registry.npmjs.org/@babel/cli/-/cli-7.5.5.tgz'
1331+
) === '/@babel/cli/-/cli-7.5.5.tgz'
1332+
);
1333+
assert(
1334+
absoluteUrlToRelative(
1335+
'https://singledomain.com/@babel/cli/-/cli-7.5.5.tgz'
1336+
) === '/@babel/cli/-/cli-7.5.5.tgz'
1337+
);
1338+
assert(
1339+
absoluteUrlToRelative(
1340+
'https://many.more.domains.so.crazy.com/@babel/cli/-/cli-7.5.5.tgz'
1341+
) === '/@babel/cli/-/cli-7.5.5.tgz'
1342+
);
1343+
}
1344+
/*
1345+
export type LockfileEntry = {
1346+
version: string,
1347+
resolved: string,
1348+
dependencies?: {
1349+
[string]: string,
1350+
}
1351+
};
1352+
*/
1353+
async function testPrependRegistryToLockfileEntry() {
1354+
// Test resolved has prefixed /
1355+
assert(
1356+
prependRegistryToLockfileEntry(
1357+
{
1358+
version: '1.0.0',
1359+
resolved: '/@babel/cli/-/cli-7.5.5.tgz',
1360+
},
1361+
'https://registry.npmjs.org'
1362+
).resolved === 'https://registry.npmjs.org/@babel/cli/-/cli-7.5.5.tgz'
1363+
);
1364+
// Test registery has suffix /
1365+
assert(
1366+
prependRegistryToLockfileEntry(
1367+
{
1368+
version: '1.0.0',
1369+
resolved: '@babel/cli/-/cli-7.5.5.tgz',
1370+
},
1371+
'https://registry.npmjs.org/'
1372+
).resolved === 'https://registry.npmjs.org/@babel/cli/-/cli-7.5.5.tgz'
1373+
);
1374+
// Test none of the inputs have /
1375+
assert(
1376+
prependRegistryToLockfileEntry(
1377+
{
1378+
version: '1.0.0',
1379+
resolved: '@babel/cli/-/cli-7.5.5.tgz',
1380+
},
1381+
'https://registry.npmjs.org'
1382+
).resolved === 'https://registry.npmjs.org/@babel/cli/-/cli-7.5.5.tgz'
1383+
);
1384+
// Test invalid registry input
1385+
assert(
1386+
prependRegistryToLockfileEntry(
1387+
{
1388+
version: '1.0.0',
1389+
resolved: '@babel/cli/-/cli-7.5.5.tgz',
1390+
},
1391+
''
1392+
).resolved === '@babel/cli/-/cli-7.5.5.tgz'
1393+
);
1394+
}

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

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// @flow
2+
3+
/*::
4+
export type AbsoluteUrlToRelative = (string) => string
5+
*/
6+
const absoluteUrlToRelative /*: AbsoluteUrlToRelative */ = url => {
7+
return url.replace(/^https?:\/\/.*?\/(.+)$/, '/$1');
8+
};
9+
module.exports = {absoluteUrlToRelative};

‎utils/lockfile.js

+55-15
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ const {parse, stringify} = require('@yarnpkg/lockfile');
44
const {read, exec, write} = require('./node-helpers.js');
55
const {node, yarn} = require('./binary-paths.js');
66
const {isYarnResolution} = require('./is-yarn-resolution.js');
7+
const {absoluteUrlToRelative} = require('./absolute-url-to-relative.js');
8+
const {
9+
prependRegistryToLockfileEntry,
10+
} = require('./prepend-registry-to-lockfile-entry.js');
711

812
/*::
913
export type Report = {
@@ -218,14 +222,16 @@ const getDepEntries = meta => {
218222
/*::
219223
import type {PackageJson} from './get-local-dependencies.js';
220224
221-
export type Lockfile = {
222-
[string]: {
223-
version: string,
224-
dependencies?: {
225-
[string]: string,
226-
}
225+
export type LockfileEntry = {
226+
version: string,
227+
resolved: string,
228+
dependencies?: {
229+
[string]: string,
227230
}
228-
}
231+
};
232+
export type Lockfile = {
233+
[string]: LockfileEntry,
234+
};
229235
export type VersionSet = {
230236
dir: string,
231237
meta: PackageJson,
@@ -266,7 +272,7 @@ const update /*: Update */ = async ({
266272
);
267273
}
268274

269-
for (const {meta, lockfile} of sets) {
275+
for (const {dir, meta, lockfile} of sets) {
270276
// handle removals
271277
if (removals.length > 0) {
272278
for (const name of removals) {
@@ -327,8 +333,7 @@ const update /*: Update */ = async ({
327333
const data = JSON.stringify(missing, null, 2);
328334
await exec(`mkdir -p ${cwd}`);
329335
await write(`${cwd}/package.json`, data, 'utf8');
330-
const yarnrc = '"--install.frozen-lockfile" false';
331-
await write(`${cwd}/.yarnrc`, yarnrc, 'utf8');
336+
await writeYarnRc(cwd, dir);
332337
const install = `${node} ${yarn} install --ignore-scripts --ignore-engines`;
333338
await exec(install, {cwd}, [process.stdout, process.stderr]);
334339

@@ -356,8 +361,7 @@ const update /*: Update */ = async ({
356361
if (missingTransitives.length > 0) {
357362
const cwd = `${tmp}/yarn-utils-${Math.random() * 1e17}`;
358363
await exec(`mkdir -p ${cwd}`);
359-
const yarnrc = '"--add.frozen-lockfile" false';
360-
await write(`${cwd}/.yarnrc`, yarnrc, 'utf8');
364+
await writeYarnRc(cwd, dir);
361365
await write(`${cwd}/package.json`, '{}', 'utf8');
362366
const deps = missingTransitives.join(' ');
363367
const add = `yarn add ${deps} --ignore-engines`;
@@ -382,7 +386,13 @@ const update /*: Update */ = async ({
382386
const id = `${key}|${lockfile[key].version}`;
383387
if (!index[name]) index[name] = [];
384388
if (!ids.has(id)) {
385-
index[name].push({lockfile, key, isAlias});
389+
// Before pushing the lockfile to the index, convert the registry to a relative path
390+
// When we sync the dependencies later, we'll add them back
391+
index[name].push({
392+
lockfile: convertToRelativeDomain(lockfile),
393+
key,
394+
isAlias,
395+
});
386396
ids.add(id);
387397
}
388398
}
@@ -397,7 +407,8 @@ const update /*: Update */ = async ({
397407

398408
// sync
399409
for (const item of sets) {
400-
const {meta} = item;
410+
const {dir, meta} = item;
411+
const registry = await getRegistry(dir);
401412
const graph = {};
402413
for (const {name, range} of getDepEntries(meta)) {
403414
const ignored = ignore.find(dep => dep === name);
@@ -414,7 +425,10 @@ const update /*: Update */ = async ({
414425
}
415426
for (const key in graph) {
416427
const [, name] = key.match(/(.+?)@(.+)/) || [];
417-
lockfile[key] = map[`${name}@${graph[key].version}`];
428+
lockfile[key] = prependRegistryToLockfileEntry(
429+
map[`${name}@${graph[key].version}`],
430+
registry
431+
);
418432
}
419433

420434
if (frozenLockfile) {
@@ -467,4 +481,30 @@ const isBetterVersion = (version, range, graph, key) => {
467481
);
468482
};
469483

484+
const getRegistry = async cwd => {
485+
const getRegistry = `${node} ${yarn} config get registry`;
486+
const registry = await exec(getRegistry, {cwd});
487+
return registry ? registry.trim() : '';
488+
};
489+
490+
const writeYarnRc = async (cwd, packageDir) => {
491+
const yarnrc = ['"--install.frozen-lockfile" false'];
492+
const registry = await getRegistry(packageDir);
493+
if (registry) {
494+
yarnrc.push(`--registry "${registry}"`);
495+
}
496+
await write(`${cwd}/.yarnrc`, yarnrc.join('\n'), 'utf8');
497+
};
498+
499+
const convertToRelativeDomain = lockfile => {
500+
const converted = {};
501+
for (const key of Object.keys(lockfile)) {
502+
converted[key] = {
503+
...lockfile[key],
504+
resolved: absoluteUrlToRelative(lockfile[key].resolved),
505+
};
506+
}
507+
return converted;
508+
};
509+
470510
module.exports = {check, add, remove, upgrade, sync, merge};

0 commit comments

Comments
 (0)