Add step in CI to upload link artifacts #25448
Open
+127
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
|
Updated 10:24 PM PT - Dec 9th, 2025
❌ @dylan-conway, your commit c67effa has 3 failures in
🧪 To try this PR locally: bunx bun-pr 25448That installs a local version of the PR into your bun-25448 --bun |
Contributor
WalkthroughAdds optional link-artifact creation and upload: a pipeline option enables an environment flag, CMake registers a post-build step to run a new script, and a new script collects and compresses link outputs into Changes
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (21)📓 Common learnings📚 Learning: 2025-11-24T18:37:11.466ZApplied to files:
📚 Learning: 2025-11-14T16:07:01.064ZApplied to files:
📚 Learning: 2025-11-24T18:34:55.173ZApplied to files:
📚 Learning: 2025-11-24T18:36:08.558ZApplied to files:
📚 Learning: 2025-11-24T18:34:55.173ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-12-02T05:59:51.485ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-12-02T05:59:51.485ZApplied to files:
📚 Learning: 2025-11-24T18:36:08.558ZApplied to files:
📚 Learning: 2025-11-24T18:34:55.173ZApplied to files:
📚 Learning: 2025-11-24T18:34:55.173ZApplied to files:
📚 Learning: 2025-11-24T18:35:39.205ZApplied to files:
📚 Learning: 2025-11-11T22:08:38.015ZApplied to files:
📚 Learning: 2025-11-24T18:35:39.205ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:35:50.422ZApplied to files:
📚 Learning: 2025-11-24T18:37:11.466ZApplied to files:
🧬 Code graph analysis (1)scripts/create-link-artifacts.mjs (2)
🔇 Additional comments (6)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.buildkite/ci.mjs (1)
762-774: UpdatePipelineOptionsJSDoc to includeuploadLinkArtifacts
getPipelineOptions()now returns anuploadLinkArtifactsfield (from the[upload link artifacts]commit tag), andgetBuildEnvreads it, but thePipelineOptionstypedef doesn’t list this property. Please add it to keep tooling and editor hints in sync./** * @typedef {Object} PipelineOptions @@ * @property {string | boolean} [buildImages] * @property {string | boolean} [publishImages] + * @property {string | boolean} [uploadLinkArtifacts] * @property {number} [canary]Also applies to: 1046-1048
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.buildkite/ci.mjs(3 hunks)cmake/targets/BuildBun.cmake(1 hunks)scripts/create-link-artifacts.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (19)
📚 Learning: 2025-11-24T18:34:55.173Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-11-24T18:34:55.173Z
Learning: Applies to src/**/*.{cpp,zig} : Use `bun bd` or `bun run build:debug` to build debug versions for C++ and Zig source files; creates debug build at `./build/debug/bun-debug`
Applied to files:
cmake/targets/BuildBun.cmakescripts/create-link-artifacts.mjs
📚 Learning: 2025-11-24T18:34:55.173Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-11-24T18:34:55.173Z
Learning: Applies to src/**/*.{cpp,zig} : Execute files using `bun bd <file> <...args>`; never use `bun <file>` directly as it will not include your changes
Applied to files:
cmake/targets/BuildBun.cmakescripts/create-link-artifacts.mjs
📚 Learning: 2025-11-24T18:34:55.173Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-11-24T18:34:55.173Z
Learning: Applies to src/**/*.{cpp,zig} : Run tests using `bun bd test <test-file>` with the debug build; never use `bun test` directly as it will not include your changes
Applied to files:
cmake/targets/BuildBun.cmake
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Add BUN_EXPORT visibility attribute to all public V8 API functions to ensure proper symbol export across platforms
Applied to files:
cmake/targets/BuildBun.cmake
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Only push changes after running `bun bd test <file>` and ensuring tests pass
Applied to files:
cmake/targets/BuildBun.cmake
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
cmake/targets/BuildBun.cmakescripts/create-link-artifacts.mjs.buildkite/ci.mjs
📚 Learning: 2025-11-11T22:08:38.015Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 24607
File: packages/bun-usockets/src/crypto/root_certs.h:3589-3618
Timestamp: 2025-11-11T22:08:38.015Z
Learning: For PRs that update certificate data (e.g., packages/bun-usockets/src/crypto/root_certs.h or packages/bun-usockets/certdata.txt), treat these files as generated artifacts and avoid review comments unless there is a clear build/runtime breakage.
Applied to files:
cmake/targets/BuildBun.cmake
📚 Learning: 2025-11-24T18:34:55.173Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-11-24T18:34:55.173Z
Learning: Applies to src/**/*.{cpp,zig} : Code generation happens automatically as part of the build process; no manual code generation commands are required
Applied to files:
cmake/targets/BuildBun.cmake
📚 Learning: 2025-09-04T04:16:49.322Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22391
File: src/bun.js/modules/NodeModuleModule.cpp:911-913
Timestamp: 2025-09-04T04:16:49.322Z
Learning: In the Bun codebase, LUT header files (like NodeModuleModule.lut.h) are automatically regenerated by the CI system during the build process, so developers do not need to manually regenerate them when editing LUT source blocks.
Applied to files:
cmake/targets/BuildBun.cmake
📚 Learning: 2025-11-24T18:36:08.558Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Include new class bindings in `src/bun.js/bindings/generated_classes_list.zig` to register them with the code generator
Applied to files:
cmake/targets/BuildBun.cmakescripts/create-link-artifacts.mjs
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Run `bun run zig:check-all` to compile Zig code on all platforms when making platform-specific changes
Applied to files:
cmake/targets/BuildBun.cmakescripts/create-link-artifacts.mjs
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
scripts/create-link-artifacts.mjs
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` with files that end in `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
scripts/create-link-artifacts.mjs
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
scripts/create-link-artifacts.mjs
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : For each new V8 C++ method, add both GCC/Clang and MSVC mangled symbol names to the V8API struct in src/napi/napi.zig using extern fn declarations
Applied to files:
scripts/create-link-artifacts.mjs
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Applied to files:
scripts/create-link-artifacts.mjs
📚 Learning: 2025-10-18T23:43:42.502Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23817
File: src/js/node/test.ts:282-282
Timestamp: 2025-10-18T23:43:42.502Z
Learning: In the Bun repository, the error code generation script (generate-node-errors.ts) always runs during the build process. When reviewing code that uses error code intrinsics like $ERR_TEST_FAILURE, $ERR_INVALID_ARG_TYPE, etc., do not ask to verify whether the generation script has been run or will run, as it is automatically executed.
Applied to files:
scripts/create-link-artifacts.mjs
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr
Applied to files:
scripts/create-link-artifacts.mjs
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: The build process converts source TS/JS through preprocessing, bundling, and C++ header generation; assign numeric IDs in A-Z sorted order
Applied to files:
scripts/create-link-artifacts.mjs
🧬 Code graph analysis (2)
scripts/create-link-artifacts.mjs (2)
scripts/utils.mjs (1)
$(143-161)packages/bun-types/bun.d.ts (1)
Glob(6292-6347)
.buildkite/ci.mjs (1)
scripts/utils.mjs (1)
isMainBranch(692-694)
🔇 Additional comments (2)
.buildkite/ci.mjs (1)
423-436: Link‑artifact flag behavior looks correct for CI semanticsUsing
isMainBranch() || uploadLinkArtifactsto driveENABLE_LINK_ARTIFACTS_UPLOADmakes sense: main‑branch builds always produce link artifacts, and other builds can opt in via the commit tag. This will propagate consistently to all build steps that callgetBuildEnv.cmake/targets/BuildBun.cmake (1)
805-822: EnsureENABLE_LINK_ARTIFACTS_UPLOADis actually set as a CMake variableThe new POST_BUILD
register_commandis fine, butif(ENABLE_LINK_ARTIFACTS_UPLOAD)will only ever be true if this name is passed into CMake as a cache/regular variable (e.g. via-DENABLE_LINK_ARTIFACTS_UPLOAD=ONoroptionx(...)), not just as an environment variable.Given the CI change only sets
ENABLE_LINK_ARTIFACTS_UPLOADin the Buildkite env, please double‑check that your build wrapper already maps that env into a CMake variable like other flags (DEBUG, CI, etc.), otherwise this block will stay disabled even when the CI flag is set.#!/bin/bash # Simple check: search build scripts for where ENABLE_LINK_ARTIFACTS_UPLOAD # is turned into a -D flag or CMake variable. rg -n "ENABLE_LINK_ARTIFACTS_UPLOAD" scripts cmake .buildkite || true
Comment on lines
+1
to
+8
| #!/usr/bin/env bun | ||
| /** | ||
| * Extracts link command and creates bun-link-artifacts.tar.gz | ||
| * Only includes object and library files needed for linking (.o/.a on Unix, .obj/.lib on Windows) | ||
| * Files are placed under an artifacts/ prefix in the tarball | ||
| * | ||
| * Usage: bun scripts/create-link-artifacts.mjs <build-path> <bun-target> | ||
| */ |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Align header comment and make bun-zig.o assumption explicit
Two small clarity issues:
- The header says the script creates
bun-link-artifacts.tar.gz, but the code actually emitsbun-link-artifacts.tar.zst. - Direct files include
"bun-zig.o", which assumes a single Zig object with that exact name. That matches current CI, but will need updating if the Zig build ever emits multiple objects (e.g.bun-zig.0.o,bun-zig.1.o) or different names.
-/**
- * Extracts link command and creates bun-link-artifacts.tar.gz
+/**
+ * Extracts link command and creates bun-link-artifacts.tar.zst
@@
-// Direct files to copy
-const directFiles = ["link-command.txt", "bun-zig.o", libName];
+// Direct files to copy
+// NOTE: "bun-zig.o" assumes a single Zig object output; update if Zig
+// ever emits multiple per-target object files.
+const directFiles = ["link-command.txt", "bun-zig.o", libName];Also applies to: 44-45
Comment on lines
33
to
39
| const artifactsDir = join(buildPath, "artifacts"); | ||
| const tarballPath = join(buildPath, "bun-link-artifacts.tar.zst"); | ||
|
|
||
| // Step 3: Create artifacts directory | ||
| rmSync(artifactsDir, { recursive: true }); | ||
| mkdirSync(artifactsDir, { recursive: true }); | ||
|
|
Contributor
There was a problem hiding this comment.
rmSync will throw if artifacts/ does not exist (first run) – add force or existence guard
rmSync(artifactsDir, { recursive: true }); is called before mkdirSync, but artifactsDir will typically not exist on the first invocation. With force defaulting to false, this throws ENOENT and aborts the script. The same applies to the final cleanup if the directory was never created due to earlier failure.
Use force: true (or an existsSync check) to make this idempotent:
-const artifactsDir = join(buildPath, "artifacts");
+const artifactsDir = join(buildPath, "artifacts");
@@
-// Step 3: Create artifacts directory
-rmSync(artifactsDir, { recursive: true });
+// Step 3: Create artifacts directory
+rmSync(artifactsDir, { recursive: true, force: true });
mkdirSync(artifactsDir, { recursive: true });
@@
-// Step 6: Cleanup
-rmSync(artifactsDir, { recursive: true });
+// Step 6: Cleanup
+rmSync(artifactsDir, { recursive: true, force: true });Also applies to: 101-103
🤖 Prompt for AI Agents
In scripts/create-link-artifacts.mjs around lines 33 to 39 (and also apply the
same change at lines 101-103), the current rmSync(artifactsDir, { recursive:
true }) will throw ENOENT if the artifacts directory does not exist; update both
removals to be idempotent by either adding { force: true, recursive: true } to
rmSync or wrap the rmSync call in an existsSync(artifactsDir) check before
calling it so the script no longer fails on first run or when the directory was
never created.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
How did you verify your code works?