Skip to content

Install daemon updates with the npm CLI that ships with its Node #6083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alexandre-vl wants to merge 1 commit into from
Closed

Install daemon updates with the npm CLI that ships with its Node #6083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
219 changes: 147 additions & 72 deletions apps/host-daemon/src/protocol-self-update.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
import { createHash } from "node:crypto";
import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
import {
mkdir,
mkdtemp,
readFile,
rm,
symlink,
writeFile,
} from "node:fs/promises";
import { tmpdir } from "node:os";
import { delimiter, dirname, join } from "node:path";
import { HOST_DAEMON_PROTOCOL_VERSION } from "@bb/host-daemon-contract";
import { afterEach, describe, expect, it, vi } from "vitest";
import type { HostDaemonLogger } from "./logger.js";
import {
createProtocolSelfUpdater,
defaultInstallTarball,
defaultRunProcess,
SELF_UPDATE_INITIAL_RETRY_DELAY_MS,
SELF_UPDATE_MAX_RETRY_DELAY_MS,
} from "./protocol-self-update.js";
Expand All @@ -29,15 +38,13 @@ async function createFixture(
installFailure?: Error;
now?: () => number;
serverUrl?: string;
useDefaultInstaller?: boolean;
} = {},
) {
const dataDir = await mkdtemp(join(tmpdir(), "bb-self-update-test-"));
roots.push(dataDir);
const installTarball = vi.fn(async () => {
if (args.installFailure) throw args.installFailure;
});
const runProcess = vi.fn(async () => undefined);
const fetchFn = vi.fn(async (input: RequestInfo | URL) => {
const url = String(input);
if (url.endsWith("/install/version")) {
Expand All @@ -57,7 +64,7 @@ async function createFixture(
dataDir,
enabled: args.enabled ?? true,
fetchFn,
...(args.useDefaultInstaller ? { runProcess } : { installTarball }),
installTarball,
logger: testLogger,
now: args.now,
serverUrl: args.serverUrl ?? "https://server.example.test",
Expand All @@ -67,11 +74,33 @@ async function createFixture(
fetchFn,
installTarball,
logger: testLogger,
runProcess,
updater,
};
}

async function createNodeInstall(args: { npmCli: boolean }) {
const root = await mkdtemp(join(tmpdir(), "bb-self-update-node-"));
roots.push(root);
const nodeExecPath =
process.platform === "win32"
? join(root, "node.exe")
: join(root, "bin", "node");
const npmCliPath =
process.platform === "win32"
? join(root, "node_modules", "npm", "bin", "npm-cli.js")
: join(root, "lib", "node_modules", "npm", "bin", "npm-cli.js");
await mkdir(dirname(nodeExecPath), { recursive: true });
if (args.npmCli) {
await mkdir(dirname(npmCliPath), { recursive: true });
await writeFile(npmCliPath, "");
}
return { nodeExecPath, npmCliPath, root };
}

const ALLOW_SCRIPTS_ARG =
"--allow-scripts=better-sqlite3,node-pty,@parcel/watcher";
const TARBALL_PATH = join(tmpdir(), "bb-app-update-test.tgz");

afterEach(async () => {
vi.unstubAllEnvs();
await Promise.all(
Expand Down Expand Up @@ -168,73 +197,6 @@ describe("protocol self-update", () => {
);
});

it("finds npm beside the running Node executable when the service PATH omits it", async () => {
vi.stubEnv("PATH", "/usr/bin:/bin");
const test = await createFixture({ useDefaultInstaller: true });

await expect(test.updater.handleProtocolMismatch()).resolves.toBe(
"updated",
);

expect(test.runProcess).toHaveBeenCalledOnce();
expect(test.runProcess).toHaveBeenCalledWith(
"npm",
[
"install",
"-g",
"--allow-scripts=better-sqlite3,node-pty,@parcel/watcher",
expect.stringContaining("bb-app-update-"),
],
{
env: expect.objectContaining({
PATH: `${dirname(process.execPath)}${delimiter}/usr/bin:/bin`,
}),
},
);
});

it("updates an installer-managed bb-app inside its machine-specific prefix", async () => {
vi.stubEnv("BB_APP_NPM_PREFIX", "/machine-data/npm");
const test = await createFixture({ useDefaultInstaller: true });

await expect(test.updater.handleProtocolMismatch()).resolves.toBe(
"updated",
);

expect(test.runProcess).toHaveBeenCalledWith(
"npm",
[
"install",
"-g",
"--allow-scripts=better-sqlite3,node-pty,@parcel/watcher",
"--prefix",
"/machine-data/npm",
expect.stringContaining("bb-app-update-"),
],
expect.any(Object),
);
});

it("keeps legacy global updates when the installer prefix is blank", async () => {
vi.stubEnv("BB_APP_NPM_PREFIX", " ");
const test = await createFixture({ useDefaultInstaller: true });

await expect(test.updater.handleProtocolMismatch()).resolves.toBe(
"updated",
);

expect(test.runProcess).toHaveBeenCalledWith(
"npm",
[
"install",
"-g",
"--allow-scripts=better-sqlite3,node-pty,@parcel/watcher",
expect.stringContaining("bb-app-update-"),
],
expect.any(Object),
);
});

it("does nothing when auto-update is disabled", async () => {
const test = await createFixture({ enabled: false });
await expect(test.updater.handleProtocolMismatch()).resolves.toBe(
Expand Down Expand Up @@ -371,3 +333,116 @@ describe("protocol self-update", () => {
expect(test.installTarball).toHaveBeenCalledTimes(2);
});
});

describe("default tarball installer", () => {
it("runs the npm CLI installed with the running Node, not an npm found on PATH", async () => {
vi.stubEnv("PATH", "/usr/bin:/bin");
vi.stubEnv("BB_APP_NPM_PREFIX", "");
const node = await createNodeInstall({ npmCli: true });
const runProcess = vi.fn(async () => undefined);

await defaultInstallTarball(TARBALL_PATH, runProcess, node.nodeExecPath);

expect(runProcess).toHaveBeenCalledOnce();
expect(runProcess).toHaveBeenCalledWith(
node.nodeExecPath,
[node.npmCliPath, "install", "-g", ALLOW_SCRIPTS_ARG, TARBALL_PATH],
{
env: expect.objectContaining({
PATH: `${dirname(node.nodeExecPath)}${delimiter}/usr/bin:/bin`,
}),
},
);
});

it("falls back to npm on PATH, searched beside Node first, when that Node install has no npm CLI", async () => {
vi.stubEnv("PATH", "/usr/bin:/bin");
vi.stubEnv("BB_APP_NPM_PREFIX", "");
const node = await createNodeInstall({ npmCli: false });
const runProcess = vi.fn(async () => undefined);

await defaultInstallTarball(TARBALL_PATH, runProcess, node.nodeExecPath);

expect(runProcess).toHaveBeenCalledWith(
"npm",
["install", "-g", ALLOW_SCRIPTS_ARG, TARBALL_PATH],
{
env: expect.objectContaining({
PATH: `${dirname(node.nodeExecPath)}${delimiter}/usr/bin:/bin`,
}),
},
);
});

it("updates an installer-managed bb-app inside its machine-specific prefix", async () => {
vi.stubEnv("BB_APP_NPM_PREFIX", "/machine-data/npm");
const node = await createNodeInstall({ npmCli: true });
const runProcess = vi.fn(async () => undefined);

await defaultInstallTarball(TARBALL_PATH, runProcess, node.nodeExecPath);

expect(runProcess).toHaveBeenCalledWith(
node.nodeExecPath,
[
node.npmCliPath,
"install",
"-g",
ALLOW_SCRIPTS_ARG,
"--prefix",
"/machine-data/npm",
TARBALL_PATH,
],
expect.any(Object),
);
});

it("keeps legacy global updates when the installer prefix is blank", async () => {
vi.stubEnv("BB_APP_NPM_PREFIX", " ");
const node = await createNodeInstall({ npmCli: true });
const runProcess = vi.fn(async () => undefined);

await defaultInstallTarball(TARBALL_PATH, runProcess, node.nodeExecPath);

expect(runProcess).toHaveBeenCalledWith(
node.nodeExecPath,
[node.npmCliPath, "install", "-g", ALLOW_SCRIPTS_ARG, TARBALL_PATH],
expect.any(Object),
);
});

it.skipIf(process.platform === "win32")(
"installs when the npm wrapper beside Node fails after installing, as mise's does without mise on PATH",
async () => {
const node = await createNodeInstall({ npmCli: true });
const prefix = join(node.root, "prefix");
vi.stubEnv("PATH", "/usr/bin:/bin");
vi.stubEnv("BB_APP_NPM_PREFIX", prefix);
const argvPath = join(node.root, "npm-argv.json");
await symlink(process.execPath, node.nodeExecPath);
await writeFile(
join(dirname(node.nodeExecPath), "npm"),
"#!/bin/sh\necho 'mise: command not found' >&2\nexit 127\n",
{ mode: 0o755 },
);
await writeFile(
node.npmCliPath,
`require("node:fs").writeFileSync(${JSON.stringify(argvPath)}, JSON.stringify(process.argv.slice(2)));\n`,
);

await defaultInstallTarball(
TARBALL_PATH,
defaultRunProcess,
node.nodeExecPath,
);

expect(JSON.parse(await readFile(argvPath, "utf8"))).toEqual([
"install",
"-g",
ALLOW_SCRIPTS_ARG,
"--prefix",
prefix,
TARBALL_PATH,
]);
},
);
});
39 changes: 31 additions & 8 deletions apps/host-daemon/src/protocol-self-update.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { execFile } from "node:child_process";
import { existsSync } from "node:fs";
import { mkdir, readFile, rename, rm, writeFile } from "node:fs/promises";
import { delimiter, dirname, isAbsolute, join } from "node:path";
import { promisify } from "node:util";
Expand Down Expand Up @@ -56,7 +57,6 @@ interface CreateProtocolSelfUpdaterOptions {
serverUrl: string;
fetchFn?: FetchFn;
installTarball?: ProtocolSelfUpdateInstaller;
runProcess?: SelfUpdateProcessRunner;
now?: () => number;
}

Expand Down Expand Up @@ -164,11 +164,29 @@ export const defaultRunProcess: SelfUpdateProcessRunner = async (
const BB_APP_ALLOW_SCRIPTS_ARG =
"--allow-scripts=better-sqlite3,node-pty,@parcel/watcher";

function resolveNodeNpmCliPath(nodeExecPath: string): string | null {
const nodeDirectory = dirname(nodeExecPath);
const npmCliPath =
process.platform === "win32"
? join(nodeDirectory, "node_modules", "npm", "bin", "npm-cli.js")
: join(
nodeDirectory,
"..",
"lib",
"node_modules",
"npm",
"bin",
"npm-cli.js",
);
return existsSync(npmCliPath) ? npmCliPath : null;
}

export async function defaultInstallTarball(
tarballPath: string,
runProcess: SelfUpdateProcessRunner,
nodeExecPath: string,
): Promise<void> {
const executableDirectory = dirname(process.execPath);
const executableDirectory = dirname(nodeExecPath);
const inheritedPath = process.env.PATH;
const path = inheritedPath
? `${executableDirectory}${delimiter}${inheritedPath}`
Expand All @@ -181,9 +199,17 @@ export async function defaultInstallTarball(
}
const prefixArgs =
configuredPrefix === undefined ? [] : ["--prefix", configuredPrefix];
const npmArgs = [
"install",
"-g",
BB_APP_ALLOW_SCRIPTS_ARG,
...prefixArgs,
tarballPath,
];
const npmCliPath = resolveNodeNpmCliPath(nodeExecPath);
await runProcess(
"npm",
["install", "-g", BB_APP_ALLOW_SCRIPTS_ARG, ...prefixArgs, tarballPath],
npmCliPath === null ? "npm" : nodeExecPath,
npmCliPath === null ? npmArgs : [npmCliPath, ...npmArgs],
{
env: { ...process.env, PATH: path },
},
Expand All @@ -197,10 +223,7 @@ export function createProtocolSelfUpdater(
const installTarball =
options.installTarball ??
((tarballPath) =>
defaultInstallTarball(
tarballPath,
options.runProcess ?? defaultRunProcess,
));
defaultInstallTarball(tarballPath, defaultRunProcess, process.execPath));
const now = options.now ?? Date.now;
const attemptPath = join(options.dataDir, ATTEMPT_FILE_NAME);
const installedArtifactDigestPath = join(
Expand Down
2 changes: 1 addition & 1 deletion apps/host-daemon/src/server-move/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ export function defaultServerMoveServiceOptions() {
defaultDetachedProcessSpawner,
),
installBbApp: (tarballPath: string) =>
defaultInstallTarball(tarballPath, defaultRunProcess),
defaultInstallTarball(tarballPath, defaultRunProcess, process.execPath),
checkGhAuthenticated: defaultGhAuthenticationCheck,
checkPortAvailable: defaultPortAvailabilityCheck,
processOps: createNodeVerifiedProcessOps(),
Expand Down
Loading