Skip to content
This repository was archived by the owner on May 17, 2025. It is now read-only.

Commit 5554809

Browse files
guybedfordguybedford
authored
fix: install racing (#373)
1 parent e44f0e4 commit 5554809

File tree

5 files changed

+52
-117
lines changed

5 files changed

+52
-117
lines changed

‎package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/generator.ts‎

Lines changed: 20 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -358,11 +358,6 @@ export class Generator {
358358
log: Log;
359359
integrity: boolean;
360360

361-
/**
362-
* The number of concurrent installs the generator is busy processing.
363-
*/
364-
installCnt = 0;
365-
366361
/**
367362
* Constructs a new Generator instance.
368363
*
@@ -577,7 +572,6 @@ export class Generator {
577572
): Promise<{ staticDeps: string[]; dynamicDeps: string[] }> {
578573
if (typeof specifier === "string") specifier = [specifier];
579574
let error = false;
580-
if (this.installCnt++ === 0) this.traceMap.startInstall();
581575
await this.traceMap.processInputMap;
582576
specifier = specifier.map((specifier) => specifier.replace(/\\/g, "/"));
583577
try {
@@ -600,12 +594,9 @@ export class Generator {
600594
error = true;
601595
throw e;
602596
} finally {
603-
if (--this.installCnt === 0) {
604-
const { map, staticDeps, dynamicDeps } =
605-
await this.traceMap.finishInstall(this.traceMap.pins, this.integrity);
606-
this.map = map;
607-
if (!error) return { staticDeps, dynamicDeps };
608-
}
597+
const { map, staticDeps, dynamicDeps } = await this.traceMap.extractMap(this.traceMap.pins, this.integrity);
598+
this.map = map;
599+
if (!error) return { staticDeps, dynamicDeps };
609600
}
610601
}
611602

@@ -679,8 +670,6 @@ export class Generator {
679670
comment =
680671
" Generated by @jspm/generator - https://github.com/jspm/generator ";
681672
if (typeof htmlUrl === "string") htmlUrl = new URL(htmlUrl);
682-
if (this.installCnt !== 0)
683-
throw new JspmError("htmlInject cannot run alongside other install ops");
684673

685674
const analysis = analyzeHtml(html, htmlUrl);
686675

@@ -907,8 +896,7 @@ export class Generator {
907896
// Split the case of multiple install targets:
908897
if (Array.isArray(install)) {
909898
if (install.length === 0) {
910-
const { map, staticDeps, dynamicDeps } =
911-
await this.traceMap.finishInstall(this.traceMap.pins, this.integrity);
899+
const { map, staticDeps, dynamicDeps } = await this.traceMap.extractMap(this.traceMap.pins, this.integrity);
912900
this.map = map;
913901
return { staticDeps, dynamicDeps };
914902
}
@@ -945,7 +933,6 @@ export class Generator {
945933

946934
// Handle case of a single install target with at most one subpath:
947935
let error = false;
948-
if (this.installCnt++ === 0) this.traceMap.startInstall();
949936
await this.traceMap.processInputMap; // don't race input processing
950937
try {
951938
// Resolve input information to a target package:
@@ -987,12 +974,9 @@ export class Generator {
987974
error = true;
988975
throw e;
989976
} finally {
990-
if (--this.installCnt === 0) {
991-
const { map, staticDeps, dynamicDeps } =
992-
await this.traceMap.finishInstall(this.traceMap.pins, this.integrity);
993-
this.map = map;
994-
if (!error) return { staticDeps, dynamicDeps };
995-
}
977+
const { map, staticDeps, dynamicDeps } = await this.traceMap.extractMap(this.traceMap.pins, this.integrity);
978+
this.map = map;
979+
if (!error) return { staticDeps, dynamicDeps };
996980
}
997981
}
998982

@@ -1001,14 +985,11 @@ export class Generator {
1001985
* versions of anything (similar to "npm ci").
1002986
*/
1003987
async reinstall() {
1004-
if (this.installCnt++ === 0) this.traceMap.startInstall();
1005988
await this.traceMap.processInputMap;
1006-
if (--this.installCnt === 0) {
1007-
const { map, staticDeps, dynamicDeps } =
1008-
await this.traceMap.finishInstall(this.traceMap.pins, this.integrity);
1009-
this.map = map;
1010-
return { staticDeps, dynamicDeps };
1011-
}
989+
const { map, staticDeps, dynamicDeps } =
990+
await this.traceMap.extractMap(this.traceMap.pins, this.integrity);
991+
this.map = map;
992+
return { staticDeps, dynamicDeps };
1012993
}
1013994

1014995
/**
@@ -1021,7 +1002,6 @@ export class Generator {
10211002
*/
10221003
async update(pkgNames?: string | string[]) {
10231004
if (typeof pkgNames === "string") pkgNames = [pkgNames];
1024-
if (this.installCnt++ === 0) this.traceMap.startInstall();
10251005
await this.traceMap.processInputMap;
10261006

10271007
const primaryResolutions = this.traceMap.installer.installs.primary;
@@ -1038,7 +1018,6 @@ export class Generator {
10381018
for (const name of pkgNames) {
10391019
const resolution = primaryResolutions[name];
10401020
if (!resolution) {
1041-
this.installCnt--;
10421021
throw new JspmError(
10431022
`No "imports" package entry for "${name}" to update. Note update takes package names not package specifiers.`
10441023
);
@@ -1079,19 +1058,14 @@ export class Generator {
10791058
}
10801059

10811060
await this._install(installs, mode);
1082-
if (--this.installCnt === 0) {
1083-
const { map, staticDeps, dynamicDeps } =
1084-
await this.traceMap.finishInstall(this.traceMap.pins, this.integrity);
1085-
this.map = map;
1086-
return { staticDeps, dynamicDeps };
1087-
}
1061+
const { map, staticDeps, dynamicDeps } =
1062+
await this.traceMap.extractMap(this.traceMap.pins, this.integrity);
1063+
this.map = map;
1064+
return { staticDeps, dynamicDeps };
10881065
}
10891066

10901067
async uninstall(names: string | string[]) {
10911068
if (typeof names === "string") names = [names];
1092-
if (this.installCnt++ === 0) {
1093-
this.traceMap.startInstall();
1094-
}
10951069
await this.traceMap.processInputMap;
10961070
let pins = this.traceMap.pins;
10971071
const unusedNames = new Set([...names]);
@@ -1106,18 +1080,15 @@ export class Generator {
11061080
}
11071081
}
11081082
if (unusedNames.size) {
1109-
this.installCnt--;
11101083
throw new JspmError(
11111084
`No "imports" entry for "${[...unusedNames][0]}" to uninstall.`
11121085
);
11131086
}
11141087
this.traceMap.pins = pins;
1115-
if (--this.installCnt === 0) {
1116-
const { staticDeps, dynamicDeps, map } =
1117-
await this.traceMap.finishInstall(this.traceMap.pins, this.integrity);
1118-
this.map = map;
1119-
return { staticDeps, dynamicDeps };
1120-
}
1088+
const { staticDeps, dynamicDeps, map } =
1089+
await this.traceMap.extractMap(this.traceMap.pins, this.integrity);
1090+
this.map = map;
1091+
return { staticDeps, dynamicDeps };
11211092
}
11221093

11231094
async extractMap(
@@ -1130,13 +1101,8 @@ export class Generator {
11301101
if (typeof rootUrl === "string") rootUrl = new URL(rootUrl, this.baseUrl);
11311102
if (!Array.isArray(pins)) pins = [pins];
11321103
if (typeof integrity !== "boolean") integrity = this.integrity;
1133-
if (this.installCnt++ !== 0)
1134-
throw new JspmError(`Cannot run extract map during installs`);
1135-
this.traceMap.startInstall();
11361104
await this.traceMap.processInputMap;
1137-
if (--this.installCnt !== 0)
1138-
throw new JspmError(`Another install was started during extract map.`);
1139-
const { map, staticDeps, dynamicDeps } = await this.traceMap.finishInstall(
1105+
const { map, staticDeps, dynamicDeps } = await this.traceMap.extractMap(
11401106
pins,
11411107
integrity
11421108
);

‎src/install/installer.ts‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,9 @@ export class Installer {
100100
opts: InstallerOptions;
101101
installs: LockResolutions;
102102
constraints: VersionConstraints;
103-
installing = false;
104103
newInstalls = false;
105104
// @ts-ignore
106105
installBaseUrl: `${string}/`;
107-
added = new Map<string, InstallTarget>();
108106
hasLock = false;
109107
defaultProvider = { provider: "jspm.io", layer: "default" };
110108
defaultRegistry = "npm";
@@ -161,17 +159,6 @@ export class Installer {
161159
}
162160
}
163161

164-
startInstall() {
165-
if (this.installing) throw new Error("Internal error: already installing");
166-
this.installing = true;
167-
this.newInstalls = false;
168-
this.added = new Map<string, InstallTarget>();
169-
}
170-
171-
finishInstall() {
172-
this.installing = false;
173-
}
174-
175162
getProvider(target: PackageTarget) {
176163
let provider = this.defaultProvider;
177164
for (const name of Object.keys(this.providers)) {
@@ -366,7 +353,6 @@ export class Installer {
366353
"installer/install",
367354
`installing ${pkgName} from ${parentUrl} in scope ${pkgScope}`
368355
);
369-
if (!this.installing) throwInternalError("Not installing");
370356

371357
// Anything installed in the scope of the installer's base URL is treated
372358
// as top-level, and hits the primary locks. Anything else is treated as

‎src/trace/tracemap.ts‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -378,19 +378,6 @@ export default class TraceMap {
378378
};
379379
}
380380

381-
startInstall() {
382-
this.installer.startInstall();
383-
}
384-
385-
async finishInstall(
386-
modules: string[],
387-
integrity: boolean
388-
): Promise<{ map: ImportMap; staticDeps: string[]; dynamicDeps: string[] }> {
389-
const result = await this.extractMap(modules, integrity);
390-
this.installer.finishInstall();
391-
return result;
392-
}
393-
394381
async add(
395382
name: string,
396383
target: InstallTarget,

‎test/test.html‎

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"scopes": {
1111
"../": {
1212
"#fetch": "../dist/fetch-native.js",
13-
"@babel/core": "https://ga.jspm.io/npm:@babel/core@7.24.7/lib/index.js",
13+
"@babel/core": "https://ga.jspm.io/npm:@babel/core@7.25.2/lib/index.js",
1414
"@babel/plugin-syntax-import-attributes": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
1515
"@babel/preset-typescript": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
1616
"@jspm/import-map": "https://ga.jspm.io/npm:@jspm/[email protected]/dist/map.js",
@@ -24,59 +24,55 @@
2424
"url": "https://ga.jspm.io/npm:@jspm/[email protected]/nodelibs/browser/url.js"
2525
},
2626
"https://ga.jspm.io/": {
27-
"#lib/config/files/index.js": "https://ga.jspm.io/npm:@babel/core@7.24.7/lib/config/files/index-browser.js",
28-
"#lib/config/resolve-targets.js": "https://ga.jspm.io/npm:@babel/core@7.24.7/lib/config/resolve-targets-browser.js",
29-
"#lib/transform-file.js": "https://ga.jspm.io/npm:@babel/core@7.24.7/lib/transform-file-browser.js",
30-
"#node.js": "https://ga.jspm.io/npm:[email protected].2/browser.js",
27+
"#lib/config/files/index.js": "https://ga.jspm.io/npm:@babel/core@7.25.2/lib/config/files/index-browser.js",
28+
"#lib/config/resolve-targets.js": "https://ga.jspm.io/npm:@babel/core@7.25.2/lib/config/resolve-targets-browser.js",
29+
"#lib/transform-file.js": "https://ga.jspm.io/npm:@babel/core@7.25.2/lib/transform-file-browser.js",
30+
"#node.js": "https://ga.jspm.io/npm:[email protected].3/browser.js",
3131
"@ampproject/remapping": "https://ga.jspm.io/npm:@ampproject/[email protected]/dist/remapping.umd.js",
3232
"@babel/code-frame": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
33-
"@babel/compat-data/native-modules": "https://ga.jspm.io/npm:@babel/compat-data@7.24.7/native-modules.js",
34-
"@babel/compat-data/plugins": "https://ga.jspm.io/npm:@babel/compat-data@7.24.7/plugins.js",
35-
"@babel/core": "https://ga.jspm.io/npm:@babel/core@7.24.7/lib/index.js",
36-
"@babel/generator": "https://ga.jspm.io/npm:@babel/generator@7.24.7/lib/index.js",
33+
"@babel/compat-data/native-modules": "https://ga.jspm.io/npm:@babel/compat-data@7.25.2/native-modules.js",
34+
"@babel/compat-data/plugins": "https://ga.jspm.io/npm:@babel/compat-data@7.25.2/plugins.js",
35+
"@babel/core": "https://ga.jspm.io/npm:@babel/core@7.25.2/lib/index.js",
36+
"@babel/generator": "https://ga.jspm.io/npm:@babel/generator@7.25.0/lib/index.js",
3737
"@babel/helper-annotate-as-pure": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
38-
"@babel/helper-compilation-targets": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
39-
"@babel/helper-create-class-features-plugin": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
40-
"@babel/helper-environment-visitor": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
41-
"@babel/helper-function-name": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
42-
"@babel/helper-hoist-variables": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
43-
"@babel/helper-member-expression-to-functions": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
38+
"@babel/helper-compilation-targets": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
39+
"@babel/helper-create-class-features-plugin": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
40+
"@babel/helper-member-expression-to-functions": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
4441
"@babel/helper-module-imports": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
45-
"@babel/helper-module-transforms": "https://ga.jspm.io/npm:@babel/helper-module-transforms@7.24.7/lib/index.js",
42+
"@babel/helper-module-transforms": "https://ga.jspm.io/npm:@babel/helper-module-transforms@7.25.2/lib/index.js",
4643
"@babel/helper-optimise-call-expression": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
47-
"@babel/helper-plugin-utils": "https://ga.jspm.io/npm:@babel/[email protected].7/lib/index.js",
48-
"@babel/helper-replace-supers": "https://ga.jspm.io/npm:@babel/helper-replace-supers@7.24.7/lib/index.js",
44+
"@babel/helper-plugin-utils": "https://ga.jspm.io/npm:@babel/[email protected].8/lib/index.js",
45+
"@babel/helper-replace-supers": "https://ga.jspm.io/npm:@babel/helper-replace-supers@7.25.0/lib/index.js",
4946
"@babel/helper-simple-access": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
5047
"@babel/helper-skip-transparent-expression-wrappers": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
51-
"@babel/helper-split-export-declaration": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
52-
"@babel/helper-string-parser": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
48+
"@babel/helper-string-parser": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
5349
"@babel/helper-validator-identifier": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
54-
"@babel/helper-validator-option": "https://ga.jspm.io/npm:@babel/[email protected].7/lib/index.js",
55-
"@babel/helpers": "https://ga.jspm.io/npm:@babel/helpers@7.24.7/lib/index.js",
50+
"@babel/helper-validator-option": "https://ga.jspm.io/npm:@babel/[email protected].8/lib/index.js",
51+
"@babel/helpers": "https://ga.jspm.io/npm:@babel/helpers@7.25.0/lib/index.js",
5652
"@babel/highlight": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
57-
"@babel/parser": "https://ga.jspm.io/npm:@babel/parser@7.24.7/lib/index.js",
53+
"@babel/parser": "https://ga.jspm.io/npm:@babel/parser@7.25.3/lib/index.js",
5854
"@babel/plugin-syntax-jsx": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
5955
"@babel/plugin-syntax-typescript": "https://ga.jspm.io/npm:@babel/[email protected]/lib/index.js",
60-
"@babel/plugin-transform-modules-commonjs": "https://ga.jspm.io/npm:@babel/[email protected].7/lib/index.js",
61-
"@babel/plugin-transform-typescript": "https://ga.jspm.io/npm:@babel/plugin-transform-typescript@7.24.7/lib/index.js",
62-
"@babel/template": "https://ga.jspm.io/npm:@babel/template@7.24.7/lib/index.js",
63-
"@babel/traverse": "https://ga.jspm.io/npm:@babel/traverse@7.24.7/lib/index.js",
64-
"@babel/types": "https://ga.jspm.io/npm:@babel/types@7.24.7/lib/index.js",
56+
"@babel/plugin-transform-modules-commonjs": "https://ga.jspm.io/npm:@babel/[email protected].8/lib/index.js",
57+
"@babel/plugin-transform-typescript": "https://ga.jspm.io/npm:@babel/plugin-transform-typescript@7.25.2/lib/index.js",
58+
"@babel/template": "https://ga.jspm.io/npm:@babel/template@7.25.0/lib/index.js",
59+
"@babel/traverse": "https://ga.jspm.io/npm:@babel/traverse@7.25.3/lib/index.js",
60+
"@babel/types": "https://ga.jspm.io/npm:@babel/types@7.25.2/lib/index.js",
6561
"@jridgewell/gen-mapping": "https://ga.jspm.io/npm:@jridgewell/[email protected]/dist/gen-mapping.umd.js",
6662
"@jridgewell/resolve-uri": "https://ga.jspm.io/npm:@jridgewell/[email protected]/dist/resolve-uri.umd.js",
6763
"@jridgewell/set-array": "https://ga.jspm.io/npm:@jridgewell/[email protected]/dist/set-array.umd.js",
6864
"@jridgewell/sourcemap-codec": "https://ga.jspm.io/npm:@jridgewell/[email protected]/dist/sourcemap-codec.umd.js",
6965
"@jridgewell/trace-mapping": "https://ga.jspm.io/npm:@jridgewell/[email protected]/dist/trace-mapping.umd.js",
7066
"ansi-styles": "https://ga.jspm.io/npm:[email protected]/index.js",
71-
"browserslist": "https://ga.jspm.io/npm:[email protected].2/index.js",
67+
"browserslist": "https://ga.jspm.io/npm:[email protected].3/index.js",
7268
"buffer": "https://ga.jspm.io/npm:@jspm/[email protected]/nodelibs/browser/buffer.js",
73-
"caniuse-lite/dist/unpacker/agents": "https://ga.jspm.io/npm:[email protected].30001641/dist/unpacker/agents.js",
69+
"caniuse-lite/dist/unpacker/agents": "https://ga.jspm.io/npm:[email protected].30001649/dist/unpacker/agents.js",
7470
"chalk": "https://ga.jspm.io/npm:[email protected]/index.js",
7571
"color-convert": "https://ga.jspm.io/npm:[email protected]/index.js",
7672
"color-name": "https://ga.jspm.io/npm:[email protected]/index.js",
7773
"convert-source-map": "https://ga.jspm.io/npm:[email protected]/index.js",
78-
"debug": "https://ga.jspm.io/npm:[email protected].5/src/browser.js",
79-
"electron-to-chromium/versions": "https://ga.jspm.io/npm:electron-to-chromium@1.4.823/versions.js",
74+
"debug": "https://ga.jspm.io/npm:[email protected].6/src/browser.js",
75+
"electron-to-chromium/versions": "https://ga.jspm.io/npm:electron-to-chromium@1.5.4/versions.js",
8076
"escape-string-regexp": "https://ga.jspm.io/npm:[email protected]/index.js",
8177
"fs": "https://ga.jspm.io/npm:@jspm/[email protected]/nodelibs/browser/fs.js",
8278
"gensync": "https://ga.jspm.io/npm:[email protected]/index.js",
@@ -85,8 +81,8 @@
8581
"jsesc": "https://ga.jspm.io/npm:[email protected]/jsesc.js",
8682
"lru-cache": "https://ga.jspm.io/npm:[email protected]/index.js",
8783
"ms": "https://ga.jspm.io/npm:[email protected]/index.js",
88-
"node-releases/data/processed/envs.json": "https://ga.jspm.io/npm:[email protected].14/data/processed/envs.json.js",
89-
"node-releases/data/release-schedule/release-schedule.json": "https://ga.jspm.io/npm:[email protected].14/data/release-schedule/release-schedule.json.js",
84+
"node-releases/data/processed/envs.json": "https://ga.jspm.io/npm:[email protected].18/data/processed/envs.json.js",
85+
"node-releases/data/release-schedule/release-schedule.json": "https://ga.jspm.io/npm:[email protected].18/data/release-schedule/release-schedule.json.js",
9086
"path": "https://ga.jspm.io/npm:@jspm/[email protected]/nodelibs/browser/path.js",
9187
"picocolors": "https://ga.jspm.io/npm:[email protected]/picocolors.browser.js",
9288
"process": "https://ga.jspm.io/npm:@jspm/[email protected]/nodelibs/browser/process-production.js",

0 commit comments

Comments
 (0)