Skip to content

Commit

Permalink
fix(bootstrap): Bail out of hoisted recursive lifecycles
Browse files Browse the repository at this point in the history
Fixes #1125
  • Loading branch information
evocateur committed Jan 2, 2019
1 parent 00a372e commit 169c943
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 20 deletions.
16 changes: 16 additions & 0 deletions commands/bootstrap/__tests__/bootstrap-command.test.js
Expand Up @@ -87,6 +87,11 @@ describe("BootstrapCommand", () => {
createSymlink.mockResolvedValue();

describe("lifecycle scripts", () => {
afterEach(() => {
delete process.env.LERNA_EXEC_PATH;
delete process.env.LERNA_ROOT_PATH;
});

it("should run preinstall, postinstall and prepublish scripts", async () => {
const testDir = await initFixture("lifecycle-scripts");

Expand All @@ -103,6 +108,17 @@ describe("BootstrapCommand", () => {

expect(ranScriptsInDirectories(testDir)).toMatchSnapshot();
});

it("should not recurse from hoisted root postinstall", async () => {
const testDir = await initFixture("lifecycle-scripts");

process.env.LERNA_EXEC_PATH = testDir;
process.env.LERNA_ROOT_PATH = testDir;

await lernaBootstrap(testDir)();

expect(runLifecycle).not.toHaveBeenCalled();
});
});

describe("with hoisting", () => {
Expand Down
11 changes: 10 additions & 1 deletion commands/bootstrap/index.js
Expand Up @@ -64,6 +64,16 @@ class BootstrapCommand extends Command {
);
}

// postinstall and prepare are commonly used to call `lerna bootstrap`,
// but we need to avoid recursive execution when `--hoist` is enabled
const { LERNA_EXEC_PATH = "leaf", LERNA_ROOT_PATH = "root" } = process.env;

if (LERNA_EXEC_PATH === LERNA_ROOT_PATH) {
this.logger.warn("bootstrap", "Skipping recursive execution");

return false;
}

this.runPackageLifecycle = createRunner({ registry });
this.npmConfig = {
registry,
Expand Down Expand Up @@ -181,7 +191,6 @@ class BootstrapCommand extends Command {

const mapPackageWithScript = pkg =>
this.runPackageLifecycle(pkg, stage).then(() => {
tracker.silly("lifecycle", "finished %j in %s", stage, pkg.name);
tracker.completeWork(1);
});

Expand Down
14 changes: 14 additions & 0 deletions integration/lerna-bootstrap-hoist.test.js
Expand Up @@ -65,3 +65,17 @@ package-3 cli2 package-2 OK
});
expect(rootLock).not.toHaveProperty("dependencies.tiny-tarball.optional");
});

test("postinstall does not recurse", async () => {
const cwd = await initFixture("lerna-bootstrap");
const lerna = cliRunner(cwd, {
LERNA_EXEC_PATH: cwd,
LERNA_ROOT_PATH: cwd,
});

const { stderr } = await lerna("bootstrap", "--no-ci", "--hoist");
expect(stderr).toMatchInlineSnapshot(`
lerna notice cli __TEST_VERSION__
lerna WARN bootstrap Skipping recursive execution
`);
});
48 changes: 29 additions & 19 deletions utils/npm-install/__tests__/npm-install.test.js
Expand Up @@ -30,7 +30,8 @@ describe("npm-install", () => {
{
name: "test-npm-install",
},
path.normalize("/test/npm-install-deps")
path.normalize("/test/npm-install-promise"),
path.normalize("/test")
);

await npmInstall(pkg, {
Expand All @@ -42,7 +43,15 @@ describe("npm-install", () => {
expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith(
"yarn",
["install", "--mutex", "file:foo", "--non-interactive", "--no-optional"],
{ cwd: pkg.location, env: {}, pkg, stdio: "pipe" }
{
cwd: pkg.location,
env: {
LERNA_EXEC_PATH: pkg.location,
LERNA_ROOT_PATH: pkg.rootPath,
},
pkg,
stdio: "pipe",
}
);
});

Expand All @@ -53,19 +62,20 @@ describe("npm-install", () => {
{
name: "test-npm-install",
},
path.normalize("/test/npm-install-deps")
path.normalize("/test/npm-install-stdio")
);

await npmInstall(pkg, {
stdio: "inherit",
});

expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith("npm", ["install"], {
cwd: pkg.location,
env: {},
pkg,
stdio: "inherit",
});
expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith(
"npm",
["install"],
expect.objectContaining({
stdio: "inherit",
})
);
});

it("does not swallow errors", async () => {
Expand All @@ -77,7 +87,7 @@ describe("npm-install", () => {
{
name: "test-npm-install-error",
},
path.normalize("/test/npm/install/error")
path.normalize("/test/npm-install-error")
);

try {
Expand All @@ -92,7 +102,7 @@ describe("npm-install", () => {
["install", "--non-interactive"],
{
cwd: pkg.location,
env: {},
env: expect.any(Object),
pkg,
stdio: "pipe",
}
Expand Down Expand Up @@ -169,7 +179,7 @@ describe("npm-install", () => {
});
expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith("npm", ["install"], {
cwd: pkg.location,
env: {},
env: expect.any(Object),
pkg,
stdio: "pipe",
});
Expand Down Expand Up @@ -210,9 +220,9 @@ describe("npm-install", () => {
});
expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith("npm", ["install"], {
cwd: pkg.location,
env: {
env: expect.objectContaining({
npm_config_registry: config.registry,
},
}),
pkg,
stdio: "pipe",
});
Expand Down Expand Up @@ -252,7 +262,7 @@ describe("npm-install", () => {
});
expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith("npm", ["install", "--global-style"], {
cwd: pkg.location,
env: {},
env: expect.any(Object),
pkg,
stdio: "pipe",
});
Expand Down Expand Up @@ -288,7 +298,7 @@ describe("npm-install", () => {
expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith(
"yarn",
["install", "--mutex", "network:12345", "--non-interactive"],
{ cwd: pkg.location, env: {}, pkg, stdio: "pipe" }
{ cwd: pkg.location, env: expect.any(Object), pkg, stdio: "pipe" }
);
});

Expand Down Expand Up @@ -329,7 +339,7 @@ describe("npm-install", () => {
["install", "--production", "--no-optional"],
{
cwd: pkg.location,
env: {},
env: expect.any(Object),
pkg,
stdio: "pipe",
}
Expand Down Expand Up @@ -372,7 +382,7 @@ describe("npm-install", () => {
});
expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith("npm", ["install", "--global-style"], {
cwd: pkg.location,
env: {},
env: expect.any(Object),
pkg,
stdio: "pipe",
});
Expand Down Expand Up @@ -412,7 +422,7 @@ describe("npm-install", () => {
});
expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith("npm", ["ci"], {
cwd: pkg.location,
env: {},
env: expect.any(Object),
pkg,
stdio: "pipe",
});
Expand Down
4 changes: 4 additions & 0 deletions utils/npm-install/npm-install.js
Expand Up @@ -41,6 +41,10 @@ function npmInstall(
// potential override, e.g. "inherit" in root-only bootstrap
opts.stdio = stdio;

// provide env sentinels to avoid recursive execution from scripts
opts.env.LERNA_EXEC_PATH = pkg.location;
opts.env.LERNA_ROOT_PATH = pkg.rootPath;

log.silly("npmInstall", [cmd, args]);
return ChildProcessUtilities.exec(cmd, args, opts);
}
Expand Down

0 comments on commit 169c943

Please sign in to comment.