Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
63 changes: 40 additions & 23 deletions src/install/PackageManager/PackageManagerEnqueue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ pub fn enqueue_dependency_with_main_and_success_fn(
if let Some(fail) = fail_fn {
fail(this, dependency, id, err);
} else if dependency.behavior.is_peer() {
warn_unmet_peer_dependency(this, name, &version);
warn_unmet_peer_dependency(this, name, &version, err);
} else {
this.log_mut()
.add_error_fmt(
Expand All @@ -822,7 +822,7 @@ pub fn enqueue_dependency_with_main_and_success_fn(
if let Some(fail) = fail_fn {
fail(this, dependency, id, err);
} else if dependency.behavior.is_peer() {
warn_unmet_peer_dependency(this, name, &version);
warn_unmet_peer_dependency(this, name, &version, err);
} else {
bun_ast::add_error_pretty!(
this.log_mut(),
Expand All @@ -839,6 +839,8 @@ pub fn enqueue_dependency_with_main_and_success_fn(
if dependency.behavior.is_required() {
if let Some(fail) = fail_fn {
fail(this, dependency, id, err);
} else if dependency.behavior.is_peer() {
warn_unmet_peer_dependency(this, name, &version, err);
} else {
let age_gate_ms =
this.options.minimum_release_age_ms.unwrap_or(0.0);
Expand Down Expand Up @@ -1078,19 +1080,13 @@ pub fn enqueue_dependency_with_main_and_success_fn(
find_result.package, min_age_ms,
)
{
let package_name = this.lockfile.str(&name);
let min_age_seconds = min_age_ms / MS_PER_S;
let _ = this.log_mut().add_error_fmt(
None,
bun_ast::Loc::EMPTY,
format_args!(
"Version \"{}@{}\" was published within minimum release age of {} seconds",
bstr::BStr::new(package_name),
find_result.version.fmt(this.lockfile.buffers.string_bytes.as_slice()),
min_age_seconds,
),
);
return Ok(());
// Reported by the `TooRecentVersion` arm above, like a fresh manifest would be.
resolve_result_ =
Err(crate::Error::TooRecentVersion);
let _ = this
.network_dedupe_map
.remove(&task_id);
continue 'retry_with_new_resolve_result;
}
}
// reshaped for borrowck — `find_result`
Expand Down Expand Up @@ -1643,21 +1639,37 @@ pub fn enqueue_dependency_with_main_and_success_fn(
}

/// Unmet peers stay unresolved instead of failing the install; see `may_stay_unresolved`.
/// `err` is the `NoMatchingVersion` / `DistTagNotFound` / `TooRecentVersion` the lookup returned.
#[cold]
#[inline(never)]
fn warn_unmet_peer_dependency(
this: &PackageManager,
name: SemverString,
version: &dependency::Version,
err: crate::Error,
) {
bun_ast::add_warning_pretty!(
this.log_mut(),
None,
bun_ast::Loc::EMPTY,
"No version matching \"{}\" found for peer dependency \"{}\"<r> <d>(but package exists)<r>",
bstr::BStr::new(this.lockfile.str(&version.literal)),
bstr::BStr::new(this.lockfile.str(&name)),
);
let literal = bstr::BStr::new(this.lockfile.str(&version.literal));
let name = bstr::BStr::new(this.lockfile.str(&name));
if err == crate::Error::TooRecentVersion {
bun_ast::add_warning_pretty!(
this.log_mut(),
None,
bun_ast::Loc::EMPTY,
"No version matching \"{}\" found for peer dependency \"{}\"<r> <d>(blocked by minimum-release-age: {} seconds)<r>",
literal,
name,
this.options.minimum_release_age_ms.unwrap_or(0.0) / MS_PER_S,
);
} else {
bun_ast::add_warning_pretty!(
this.log_mut(),
None,
bun_ast::Loc::EMPTY,
"No version matching \"{}\" found for peer dependency \"{}\"<r> <d>(but package exists)<r>",
literal,
name,
);
}
}

/// Allocate and initialise an `.extract` Task for an npm tarball.
Expand Down Expand Up @@ -2552,6 +2564,11 @@ fn get_or_put_resolved_package(
Npm::FindVersionResult::Err(err_type) => match err_type {
Npm::FindVersionError::TooRecent
| Npm::FindVersionError::AllVersionsTooRecent => {
// Deferred like `NotFound` below: the peer pass may still bind the
// peer to a same-named package that is already in the tree.
Comment thread
robobun marked this conversation as resolved.
Resolved
if behavior.is_peer() && !install_peer {
return Ok(None);
}
return Err(crate::Error::TooRecentVersion);
}
Npm::FindVersionError::NotFound => None, // Handle below with existing logic
Expand Down
226 changes: 226 additions & 0 deletions test/cli/install/minimum-release-age.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Server } from "bun";
import { afterAll, beforeAll, describe, expect, setDefaultTimeout, test } from "bun:test";
import { bunEnv, bunExe, normalizeBunSnapshot, tempDir } from "harness";
import { rm } from "node:fs/promises";

// These tests drive real `bun install` runs against a mock registry, which is
// slow under the debug/ASAN build — give them the same generous timeout the
Expand Down Expand Up @@ -2354,6 +2355,231 @@
});
});

// A required peer that nothing in the tree provides and that no published
// version satisfies only warns and stays unresolved (bun-lock.test.ts, "peer
// no published version satisfies"). A peer whose only satisfying versions
// are blocked by the age gate has to end up the same way, on every path that
// can report the blocked lookup.
describe("peer dependencies", () => {
const AGE_GATE = 3 * SECONDS_PER_DAY;
const blockedPeerWarning = (range: string, name = "gated") =>
`warn: No version matching "${range}" found for peer dependency "${name}" (blocked by minimum-release-age: ${AGE_GATE} seconds)`;

// gated@2.0.0 is the only version the peers below accept and it is inside
// the age window; every version of `fresh` is inside it.
const registryPackages: Record<
string,
{ time: Record<string, string>; peerDependencies?: Record<string, string> }
> = {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
gated: { time: { "1.0.0": daysAgo(30), "2.0.0": daysAgo(1) } },
fresh: { time: { "1.0.0": daysAgo(1) } },
"needs-gated": { time: { "1.0.0": daysAgo(30) }, peerDependencies: { gated: "^2.0.0" } },
};

function serveRegistry() {
const requests: string[] = [];
const server = Bun.serve({
port: 0,
fetch(req) {
const { origin, pathname } = new URL(req.url);
requests.push(pathname);
const tarball = pathname.match(/^\/([^/]+)\/-\/\1-(\d+\.\d+\.\d+)\.tgz$/);
if (tarball) return new Response(createTarball(tarball[1], tarball[2]));
const name = pathname.slice(1);
const pkg = registryPackages[name];
if (!pkg) return new Response("Not Found", { status: 404 });
const versions: Record<string, unknown> = {};
for (const version of Object.keys(pkg.time)) {
versions[version] = {
name,
version,
dist: { tarball: `${origin}/${name}/-/${name}-${version}.tgz` },
peerDependencies: pkg.peerDependencies,
};
}
return Response.json({
name,
"dist-tags": { latest: Object.keys(pkg.time).at(-1) },
versions,
time: pkg.time,
});
},
});
return {
origin: server.url.origin,
requests,
[Symbol.dispose]() {
server.stop(true);
},
};
}

function createProject(registryOrigin: string, files: Record<string, string>) {
return tempDir("age-gated-peer", { ...files, ".npmrc": `registry=${registryOrigin}` });
}

async function install(dir: string, { args = [] as string[], env = {} as Record<string, string> } = {}) {
await using proc = Bun.spawn({
cmd: [bunExe(), "install", "--minimum-release-age", `${AGE_GATE}`, ...args],
cwd: dir,
// One manifest/tarball cache per project, so the second install of a
// test sees exactly what its first install cached.
env: { ...bunEnv, BUN_INSTALL_CACHE_DIR: `${dir}/.bun-cache`, ...env },
stdout: "pipe",
stderr: "pipe",
});
const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);

Check warning on line 2431 in test/cli/install/minimum-release-age.test.ts

View check run for this annotation

Claude / Claude Code Review

install() test helper pipes stdout but never drains it

The `install()` helper sets `stdout: "pipe"` but only awaits `proc.stderr.text()` and `proc.exited` — stdout is never drained, which REVIEW.md flags as the pipe-buffer-deadlock shape. These installs produce far too little output to actually deadlock, so this is just a convention nit: either add `proc.stdout.text()` to the `Promise.all`, or drop `stdout: "pipe"` since nothing reads it.
Comment thread
claude[bot] marked this conversation as resolved.
Resolved
return { stderr, exitCode };
}

async function reResolve(dir: string) {
await rm(`${dir}/bun.lock`, { force: true });
await rm(`${dir}/node_modules`, { recursive: true, force: true });
}

test.concurrent("declared by a registry package", async () => {
using registry = serveRegistry();
using dir = createProject(registry.origin, {
"package.json": JSON.stringify({ name: "app", dependencies: { "needs-gated": "1.0.0" } }),
});

let { stderr, exitCode } = await install(String(dir));
expect(stderr).toContain(blockedPeerWarning("^2.0.0"));
expect(stderr).not.toContain("error:");
expect(exitCode).toBe(0);
expect(await Bun.file(`${dir}/node_modules/needs-gated/package.json`).exists()).toBeTrue();
expect(await Bun.file(`${dir}/node_modules/gated/package.json`).exists()).toBeFalse();
const lockfile = await Bun.file(`${dir}/bun.lock`).text();
expect(lockfile.replaceAll(registry.origin, "<registry>")).toMatchInlineSnapshot(`
"{
"lockfileVersion": 1,
"configVersion": 1,
"workspaces": {
"": {
"name": "app",
"dependencies": {
"needs-gated": "1.0.0",
},
},
},
"packages": {
"needs-gated": ["needs-gated@1.0.0", "<registry>/needs-gated/-/needs-gated-1.0.0.tgz", { "peerDependencies": { "gated": "^2.0.0" } }, ""],
}
}
"
`);

({ stderr, exitCode } = await install(String(dir), { args: ["--frozen-lockfile"] }));
expect(stderr).not.toContain("error:");
expect(exitCode).toBe(0);
expect(await Bun.file(`${dir}/bun.lock`).text()).toBe(lockfile);
});

test.concurrent("declared by the root package and a workspace (range, exact pin, dist-tag)", async () => {
using registry = serveRegistry();
using dir = createProject(registry.origin, {
"package.json": JSON.stringify({
name: "app",
workspaces: ["packages/*"],
peerDependencies: { gated: "^2.0.0", fresh: "latest" },
}),
"packages/ws/package.json": JSON.stringify({ name: "ws", peerDependencies: { gated: "2.0.0" } }),
});

let { stderr, exitCode } = await install(String(dir));
expect(stderr).toContain(blockedPeerWarning("^2.0.0"));
expect(stderr).toContain(blockedPeerWarning("2.0.0"));
expect(stderr).toContain(blockedPeerWarning("latest", "fresh"));
expect(stderr).not.toContain("error:");
expect(exitCode).toBe(0);
const lockfile = await Bun.file(`${dir}/bun.lock`).text();
expect(lockfile).toMatchInlineSnapshot(`
"{
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
"name": "app",
"peerDependencies": {
"fresh": "latest",
"gated": "^2.0.0",
},
},
"packages/ws": {
"name": "ws",
"peerDependencies": {
"gated": "2.0.0",
},
},
},
"packages": {
"ws": ["ws@workspace:packages/ws"],
}
}
"
`);

({ stderr, exitCode } = await install(String(dir), { args: ["--frozen-lockfile"] }));
expect(stderr).not.toContain("error:");
expect(exitCode).toBe(0);
expect(await Bun.file(`${dir}/bun.lock`).text()).toBe(lockfile);
});

test.concurrent("exact pin answered from an expired cached manifest", async () => {
using registry = serveRegistry();
using dir = createProject(registry.origin, {
"package.json": JSON.stringify({ name: "app", peerDependencies: { gated: "2.0.0" } }),
});
// Keeps writing manifests to the cache but treats every cached one as
// expired, which is how an exact pin ends up being answered from the
// stale manifest instead of from a fresh lookup.
const staleManifests = { env: { BUN_MANIFEST_CACHE: "1" } };

let { stderr, exitCode } = await install(String(dir), staleManifests);
expect(stderr).toContain(blockedPeerWarning("2.0.0"));
expect(exitCode).toBe(0);
expect(registry.requests).toEqual(["/gated"]);

await reResolve(String(dir));
registry.requests.length = 0;
({ stderr, exitCode } = await install(String(dir), staleManifests));
expect(stderr).toContain(blockedPeerWarning("2.0.0"));
expect(stderr).not.toContain("error:");
expect(exitCode).toBe(0);
expect(registry.requests).toEqual([]);
});

test.concurrent("bound to the version already in the tree, with and without a warm manifest cache", async () => {
using registry = serveRegistry();
using dir = createProject(registry.origin, {
"package.json": JSON.stringify({
name: "app",
dependencies: { gated: "^1.0.0", "needs-gated": "1.0.0" },
}),
});

const expectBoundToInstalledVersion = ({ stderr, exitCode }: { stderr: string; exitCode: number }) => {
expect(stderr).toContain('warn: incorrect peer dependency "gated@1.0.0"');
expect(stderr).not.toContain("blocked by minimum-release-age");
expect(stderr).not.toContain("error:");
expect(exitCode).toBe(0);
};

expectBoundToInstalledVersion(await install(String(dir)));
const lockfile = await Bun.file(`${dir}/bun.lock`).text();
expect(lockfile).toContain('"gated@1.0.0"');

// With both manifests in the cache the peer is looked up synchronously
// while regular dependencies are still being resolved, before the pass
// that binds peers to what the tree contains.
await reResolve(String(dir));
registry.requests.length = 0;
expectBoundToInstalledVersion(await install(String(dir)));
expect(registry.requests).toEqual([]);
expect(await Bun.file(`${dir}/bun.lock`).text()).toBe(lockfile);
});
});

describe("clock skew scenarios", () => {
test("handles packages with future timestamps", async () => {
using dir = tempDir("future-timestamp", {
Expand Down
Loading