Skip to content

Commit fdf582d

Browse files
The tarball IO honors the sandboxDir it is handed
Per-package sandboxes exposed a latent bug: realTarballIo ignored the sandboxDir argument, always installing into one fixed directory and reading the started package's name back from it. With two sandboxes, startBin looked for node_modules in directories nothing had installed into, and both bins reported exit null. installSandbox now creates the directory it was asked for (fresh each time) and startBin reads the manifest from the same directory it starts in. Also hoists the sandbox name regexes and de-nests a test ternary, per lint. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
1 parent 64f2226 commit fdf582d

3 files changed

Lines changed: 18 additions & 13 deletions

File tree

packages/cli-conformance/src/checks/tarball.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ export interface TarballInput {
9797

9898
const EXACT_VERSION = /^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?$/;
9999

100+
/** Package-name characters that cannot appear in a directory name. */
101+
const SANDBOX_NAME_UNSAFE = /[@/]/g;
102+
const LEADING_DASH = /^-/;
103+
100104
/**
101105
* Check 3: the tarballs a registry would receive. 3a — packed output
102106
* imports only what the packed manifest declares. 3b — the root tarball
@@ -289,7 +293,7 @@ async function sandboxFindings(
289293
): Promise<readonly Finding[]> {
290294
const sandboxDir = join(
291295
input.sandboxDir,
292-
packageName.replace(/[@/]/g, "-").replace(/^-/, ""),
296+
packageName.replace(SANDBOX_NAME_UNSAFE, "-").replace(LEADING_DASH, ""),
293297
);
294298
// Transitive: a sibling reached only through another sibling still
295299
// needs its override, or the install falls back to the registry.

packages/cli-conformance/src/tarball-io.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ export function realTarballIo(
3838
// exact files CI uploads and attaches to the GitHub Release.
3939
const tarballDir = resolve(options.tarballDir ?? join(absWork, "tarballs"));
4040
rmSync(tarballDir, { recursive: true, force: true });
41-
const sandbox = () => join(absWork, "sandbox");
4241

4342
return {
4443
async pack(pkgDir) {
@@ -88,8 +87,9 @@ export function realTarballIo(
8887
return files;
8988
},
9089

91-
async installSandbox({ rootTarball, overrides }) {
92-
const dir = sandbox();
90+
async installSandbox({ sandboxDir, rootTarball, overrides }) {
91+
const dir = resolve(sandboxDir);
92+
rmSync(dir, { recursive: true, force: true });
9393
mkdirSync(dir, { recursive: true });
9494
const rootManifest = await this.readPackedManifest(rootTarball);
9595
const name = manifestName(rootManifest);
@@ -176,7 +176,7 @@ export function realTarballIo(
176176
argv,
177177
timeoutMs,
178178
}) {
179-
const rootManifestPath = join(sandbox(), "package.json");
179+
const rootManifestPath = join(sandboxDir, "package.json");
180180
const rootManifest = JSON.parse(
181181
readFileSync(rootManifestPath, "utf8"),
182182
) as {

packages/cli-conformance/tests/tarball.test.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -570,14 +570,15 @@ describe("sibling manifests and per-package sandboxes (the rc.8 class)", () => {
570570
overrides: Partial<TarballIo> = {},
571571
): TarballIo {
572572
return fakeIo({
573-
readPackedManifest: (tarball) =>
574-
Promise.resolve(
575-
tarball.includes("cli-engine")
576-
? ENGINE_MANIFEST
577-
: tarball.includes("prisma-wrapper")
578-
? wrapperManifest
579-
: SHELL_MANIFEST,
580-
),
573+
readPackedManifest: (tarball) => {
574+
if (tarball.includes("cli-engine")) {
575+
return Promise.resolve(ENGINE_MANIFEST);
576+
}
577+
if (tarball.includes("prisma-wrapper")) {
578+
return Promise.resolve(wrapperManifest);
579+
}
580+
return Promise.resolve(SHELL_MANIFEST);
581+
},
581582
readPackedFiles: (tarball) =>
582583
Promise.resolve(
583584
tarball.includes("prisma-wrapper")

0 commit comments

Comments
 (0)