Skip to content

Conversation

@nektro
Copy link
Member

@nektro nektro commented Nov 9, 2025

cc #1524

testing with [build images] to ensure nothing existing breaks but no need to [publish images] upon merge yet

@robobun
Copy link
Collaborator

robobun commented Nov 9, 2025

Updated 4:36 AM PT - Nov 9th, 2025

@nektro, your commit 35183e8 has 2 failures in Build #31386 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 24534

That installs a local version of the PR into your bun-24534 executable, so you can run:

bun-24534 --bun

@nektro nektro marked this pull request as ready for review November 9, 2025 19:47
@coderabbitai

This comment was marked as duplicate.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
scripts/machine.mjs (1)

965-1000: Don’t force legacy SCP everywhere

Pushing -O unconditionally forces the legacy SCP/rcp protocol and bypasses the SFTP transport that modern OpenSSH uses by default. Many hardened installations deliberately expose only SFTP (for example via ForceCommand internal-sftp), so this change will now fail against those hosts and roll back the security improvements OpenSSH 9.0 introduced.(openssh.com)

Please preserve the existing behaviour and only retry with -O when the default transfer fails with the specific “subsystem request failed” / “sftp-server not found” errors, rather than forcing the downgrade up‑front.

-  command.push("-O"); // use SCP instead of SFTP
+  let forceLegacyScp = false;
+  if (identityPaths?.length === 0 && password) {
+    forceLegacyScp = true;
+  }
+  if (forceLegacyScp) {
+    command.push("-O"); // use SCP instead of SFTP
+  }
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between b4f85c8 and 35183e8.

📒 Files selected for processing (5)
  • .buildkite/ci.mjs (4 hunks)
  • package.json (1 hunks)
  • scripts/bootstrap.sh (25 hunks)
  • scripts/machine.mjs (3 hunks)
  • scripts/utils.mjs (1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add new V8 API mangled symbols (without leading underscore) to src/symbols.txt

Applied to files:

  • scripts/utils.mjs
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use process.platform and process.arch for platform detection (rely on inlining/dead-code elimination)

Applied to files:

  • .buildkite/ci.mjs
  • package.json
  • scripts/machine.mjs
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Run bun bd after changes

Applied to files:

  • scripts/bootstrap.sh
📚 Learning: 2025-10-08T03:11:45.286Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-10-08T03:11:45.286Z
Learning: Build the debug version using `bun bd` or `bun run build:debug`; the build outputs to `./build/debug/bun-debug` and takes ~2.5 minutes

Applied to files:

  • scripts/bootstrap.sh
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/bun/**/*.{ts,js} : Place Bun-specific modules (e.g., bun:ffi, bun:sqlite) under bun/

Applied to files:

  • scripts/bootstrap.sh
📚 Learning: 2025-10-08T03:11:45.286Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-10-08T03:11:45.286Z
Learning: To run a file, use `bun bd <file> <...args>`; never use `bun <file>` directly

Applied to files:

  • scripts/bootstrap.sh
📚 Learning: 2025-10-08T03:11:45.286Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-10-08T03:11:45.286Z
Learning: Run any command with the debug build via `bun bd <command>`

Applied to files:

  • scripts/bootstrap.sh
🧬 Code graph analysis (3)
.buildkite/ci.mjs (2)
scripts/utils.mjs (2)
  • os (1679-1679)
  • profile (1683-1683)
scripts/runner.node.mjs (1)
  • os (353-353)
scripts/bootstrap.sh (1)
scripts/utils.mjs (1)
  • sh (2044-2044)
scripts/machine.mjs (2)
packages/bun-release/src/fs.ts (1)
  • join (7-9)
scripts/utils.mjs (4)
  • name (3009-3009)
  • options (2240-2240)
  • options (3092-3092)
  • waitForPort (2239-2271)

@lin72h
Copy link

lin72h commented Nov 9, 2025

This is like an early Christmas gift for me. Thanks for giving FreeBSD some love!

clipboard: ["📋", "clipboard"],
rocket: ["🚀", "rocket"],
freebsd: ["😈", "freebsd"],
openbsd: ["🐠", "openbsd"],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
openbsd: ["🐠", "openbsd"],
openbsd: ["🐡", "openbsd"],

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about freebsd: ["👹", "freebsd"],

rocket: ["🚀", "rocket"],
freebsd: ["😈", "freebsd"],
openbsd: ["🐠", "openbsd"],
netbsd: ["🏴", "netbsd"],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
netbsd: ["🏴", "netbsd"],
netbsd: ["🚩", "netbsd"],

@nektro nektro requested a review from taylordotfish November 10, 2025 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants