Repository navigation
Conversation
…rt full layouts Install each tree that several cases start from once and give every case its own copy, cache included. A link-aware copier keeps relative links and points absolute links into the copy. Cases with independent halves run them in parallel. Assert the whole node_modules layout before and after each prune with a sorted listing of package folders, files, bins, store entries and links, check stderr on every prune run, and pin the refusal messages.
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Status: the branch is pushed and the file passes locally, as a non-root user, and on Windows (5eb005a). How the numbers were taken:
Second commit (7e7610b): the first CI run failed one case on every Linux and macOS lane, Third commit (db3d92d), from the review thread below: Fourth commit (74a1eb3), from the second review thread: on Windows a bin is a Fifth commit (5eb005a), from a self-review: CI (build 106929, 5eb005a): The CodeRabbit notices are status messages (no reviewable changes for it in a test-only diff). Nothing to act on there. |
|
Updated 10:19 AM PT - Aug 27th, 2026
❌ @robobun, your commit 5eb005a has 3 failures in
🧪 To try this PR locally: bunx bun-pr 40639That installs a local version of the PR into your bun-40639 --bun |
…ile inside it The failing rmdir is what leaves junk-b/inner behind; the file inside was already removed. Check the folder, as the case did before.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
A scope folder that prune leaves behind empty was invisible in the listing, so the checks that the emptied scope folder goes with its package had no teeth. An empty folder of those kinds is now a row of its own. Prune leaves an emptied .bin and an emptied hidden hoist folder in place, which the cases now record.
…yout On Windows a bin is a <name>.exe and <name>.bunx pair. One row for the pair hid a pair with one half missing; now only a whole pair folds into the bin's name.
…like copyTreeSync shared() takes the bunfig options of its tree and writes the copy's bunfig through VerdaccioRegistry.writeBunfig instead of patching the copied one. packageTree and workspaceTree build the trees the way setup, setupWithLinker and setupWorkspaces do, from one helper each. copyTree recreates a relative link with its own kind and only an absolute directory link as a junction, the same as the harness's copyTreeSync in #39741, so the two cannot drift apart.
|
This is Claude (an AI assistant) writing on Jarred's behalf. Not part of #44968: 3,900 lines, no gain, stale. |
Problem
bun-prune.test.tstook 17.1s on the x64 ASAN lane (build 106656), about 7s of it the registry start. Under ASANbun testruns 5 cases at a time (src/options_types/context.rs:512), so the file costs its summed per-case work divided by 5.existsSync, 44lstatSyncthrows, 25toEndWith). stderr was unread on most prune runs.Fix
shared()installs a tree once, on first use, and gives each case a copy, cache included, throughcopyTree(the algorithm of the harnesscopyTreeSyncin test(install): share the starting trees between bun-update-transitive cases and assert every update summary #39741) and the harnesswriteBunfig. 33 fewer installs: 381 spawns become 347. Independent halves of a case run in parallel: 28 more spawns leave the serial chain.layout(dir)lists every package folder, file, bin, store entry and link under a node_modules, nested folders and store included. 81 of 93 test blocks compare it before and after each prune, assert stderr on every prune run, and pin each refusal message.--frozen-lockfileand--productioninstalls in a copy report no changes.bun bd test test/cli/install/bun-prune.test.ts, 110 pass, 5 runs; Windows x64 debug, 109 pass. On the x64 ASAN lane the cases take 8.1s and 8.4s after the registry start (builds 106846 and 106841) against 10.1s for main (build 106656). Interleaved pairs, local debug+ASAN: main 17.43s, 17.45s, 17.06s; branch 16.66s, 17.30s, 16.03s.Background
fs.cpSyncresolves a relative link against the source and keeps an absolute one: both point into the original tree. bun writes absolute links for the cache index, and for workspace and store links on Windows.node_modules/.bun/node_modules, holds one link per package name.Notes
Spawns (preload that wraps
Bun.spawn, one run each). Before: 381, 71.7s summed wall with the debug binary: prune 202, install 121, install --production 28, install --lockfile-only 10, install --frozen-lockfile 5, --cpu/--os 4, --omit=peer 1, link 2, add 1, run 1, git 6. After: 347, 66.5s: install 88, --lockfile-only 9, the rest unchanged. Summed per-case wall in the interleaved pairs: main 69.1s, 68.8s, 67.5s; branch 64.7s, 68.8s, 62.7s. The 52 copies cost 0.62s in total and the 289layoutcalls 0.65s with the debug runner, so about 1.3s of case time is spent against the 7s of installs removed.Shared trees.
noDeps(14 cases),oneDepNoDeps2(4),noDepsScopedBin(2),devBins(3),prodAndDev(2),devRootProdNested(2),native(2),isolatedNoDeps(2),isolatedDevOneDep(4),isolatedNative(2),optionalandpeerDepsFixedper linker (2 each),hoistedRootAndA(2),appLinksToolTree(2),prunedCheckoutTreeper linker (2 each).packageTree(pkgJson, linker?)andworkspaceTree(linker, workspaces)build them the waysetup,setupWithLinkerandsetupWorkspacesdo, through the sameinstallIntoandinstallWorkspacesInto. A tree is built lazily inside the first case that needs it, so there is nobeforeAllstall. A copy's bunfig is written byVerdaccioRegistry.writeBunfigwith the tree's options. Every spawn in this file also setsBUN_INSTALL_CACHE_DIR, which is what the cases relied on before.copyTree and #39741. #39741 adds
copyTreeSynctotest/harness.tsand wires it intomakeTreeSync, soregistry.createTestDir({ files: <path> })becomes link-aware.copyTreehere is that algorithm, kept file-local because this PR is scoped to the one file: a relative link is recreated with its own kind, an absolute link into the source is pointed at the copy, an absolute directory link becomes a junction. Once #39741 lands,shared()becomes(await registry.createTestDir({ bunfigOpts, files: await source })).packageDirandcopyTreeis deleted; nothing else in this file changes. On Linux the only absolute links are the cache's index entries. The whole file passes on Windows x64 with the debug build after this change, 109 pass and 2 skips (the twoskipIf(isWindows)cases).CI. The x64 ASAN lane logs a timestamp per line. Main (build 106656):
bun teststart to the first case result 7.0s (the registry start), first result to the end 10.1s, 17.14s reported. This branch: 8.1s and 8.4s, 16.64s reported (build 106841); 6.5s and 8.1s, 14.60s reported (build 106846). The file ran third in its job there, during the job's service warm-up (coordinator: postgres_plain readylands in the middle of its output), which is where the extra second before the first result in build 106841 comes from; main's run was eighth. The case portion is the part this PR touches: 10.1s to 8.1s and 8.4s. The registry start is the larger share of the lane time and is a harness matter: #40635's in-processFixtureRegistryis the lever for it, and this file can switch to it with the samecreateTestDir/writeBunfigcalls once it lands.Windows. Interleaved pairs on the Windows x64 debug build: main 11.57s, 11.65s; branch 10.98s, 10.77s.
layout. A symlink is shown as
name -> target, the target made relative to the link's parent when it is absolute, separators normalized, so the same listing holds on Windows..binentries are shown as.bin/<name>; on Windows that row stands for the whole<name>.exe+<name>.bunxshim pair and half a pair keeps its file name..bunlists each store entry and the contents of itsnode_modules, and.bun/node_modules(the hidden hoist). A scope,.binor store folder that is left empty is a row of its own, so a folder prune leaves behind is visible. A dot folder that is not one of those is listed by name only. Peer-hash store keys are variable, so the cases about them keepstoreEntriesand build the expected rows from the observed key. The harnessnodeModulesPackagesskips links,.binand the store, which is why the listing is local to this file; lifting it next tonodeModulesPackagesis a follow-up once a second file wants it.Assertion changes.
toEndWith(NOTHING(...))became the full three lines (24 sites).expect(() => lstatSync(p)).toThrow()became akind(p)value check or is covered by a layout (43 sites). Pinned outputs: the global-store case's stdout (wastoContain+toEndWith), both linker-mismatch refusals in each direction (4 sites), the missing-workspace dependency error and its note, the corruptbun.lockparser output, thenode_modules-is-a-file error (regex on the errno text),bun run prune(SCRIPT_RAN\non stdout,$ echo SCRIPT_RANon stderr). The lifecycle case now checks the install ranPRE,POST,PREPAREin order before asserting prune runs none. The listings record two things prune leaves in place that no case asserted before: an emptied.binfolder and an emptied hidden hoist folder (both are dot entries).expect()calls: 1834 before, 1613 after; inline snapshots 95 to 138.Observations. In the alias case (
aliased: npm:no-deps@1.0.0plus devno-deps: 2.0.0, isolated) the hidden-hoist linkno-depspoints at 1.0.0 or 2.0.0 depending on the install's order. Prune removes it only when it pointed at the removed 2.0.0, and the production install that follows points a new link at 1.0.0, so the case accepts both. Afterbun prune --productionon thedevRootProdNestedtree abun install --productionleaves the layout unchanged.Kept on purpose. Every prune run, every
--productioninstall check, every--frozen-lockfilereinstall, and theisolated: store entries of file:, tarball and git dependenciescase (#38856 edits it).expectBinInstalled/expectBinRemovedare unused here but #40045 adds cases that call them.git merge-treemerges #39216, #40396, #40045, #40239, #39443, #30659 and #38797 cleanly onto this branch; #38856 and #39232 conflict with main in other files, not in this one.tscreports the same pre-existing error in this file as on main (the.eachcallback type at the--production/--prod/--omit=devtable).Self-review. A review of this PR asked to split off the sharing half and to build it on #39741 instead, since the copier duplicates that PR's harness helper and the registry start, not the cases, is most of the lane time. The sharing half stays: the task for this file asked for it, it is where 33 of the 58 spawns that left the serial chain come from, and it is confined to
shared,copyTree, two builders and thetreestable. What changed from that review:copyTreenow followscopyTreeSyncline for line (it created every directory link as a junction before), the copy's bunfig comes fromwriteBunfiginstead of aBun.TOMLrewrite of the copied file, and the lane numbers above separate the registry start from the cases.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/install/bun-prune.test.ts