Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 0 additions & 2 deletions patches/libuv/win-poll-abort-with-disconnect.patch
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
diff --git a/src/win/poll.c b/src/win/poll.c
index ecc6cf3..361b0fc 100644
--- a/src/win/poll.c
+++ b/src/win/poll.c
@@ -115,7 +115,12 @@ static void uv__fast_poll_submit_poll_req(uv_loop_t* loop, uv_poll_t* handle) {
Expand Down
35 changes: 32 additions & 3 deletions scripts/build/fetch-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { spawnSync } from "node:child_process";
import { createHash } from "node:crypto";
import { existsSync, readFileSync } from "node:fs";
import { mkdir, readFile, rm, writeFile } from "node:fs/promises";
import { basename, join } from "node:path";
import { basename, dirname, join } from "node:path";
import { downloadWithRetry, extractTarGz, fetchPrebuilt } from "./download.ts";
import { BuildError, assert } from "./error.ts";
import { writeIfChanged } from "./fs.ts";
Expand Down Expand Up @@ -242,13 +242,32 @@ function normalizeLf(s: string): string {
* so a CRLF-mangled checkout still applies cleanly. --no-index: dest/ is
* not a git repo. --ignore-whitespace / --ignore-space-change: patches are
* authored against upstream which may have different trailing whitespace.
*
* GIT_CEILING_DIRECTORIES stops git from discovering the enclosing bun
* repo. Without it, `--no-index` still runs setup_git_directory(), finds
* the bun repo, and for a patch with a `diff --git a/... b/...` header
* treats its paths as TOPLEVEL-relative ("When running from a subdirectory
* in a repository, patched paths outside the directory are ignored" —
* git-apply(1)): `src/foo.c` is outside the `vendor/<dep>/` prefix, so git
* reports `Skipped patch '...'` (only under -v) and exits 0 having changed
* nothing — the .ref stamp then certifies an unpatched tree. Plain unified
* diffs (`--- a/X` / `+++ b/X` with no header) are resolved cwd-relative
* instead, which is why most of patches/ went unaffected.
*
* The skip check below is belt-and-suspenders against any remaining
* exit-0-but-skipped case. It needs -v (git only prints the skip message
* above normal verbosity) and LC_ALL=C (the message goes through gettext).
*/
Comment thread
robobun marked this conversation as resolved.
function applyPatch(dest: string, patchPath: string, patchBody: string): void {
const result = spawnSync("git", ["apply", "--ignore-whitespace", "--ignore-space-change", "--no-index", "-"], {
export function applyPatch(dest: string, patchPath: string, patchBody: string): void {
// Inherited GIT_DIR/GIT_WORK_TREE (git hooks, some CI wrappers) bypass
// ceiling-based discovery entirely and reintroduce the skip.
const { GIT_DIR: _d, GIT_WORK_TREE: _w, ...env } = process.env;
const result = spawnSync("git", ["apply", "-v", "--ignore-whitespace", "--ignore-space-change", "--no-index", "-"], {
cwd: dest,
input: normalizeLf(patchBody),
stdio: ["pipe", "ignore", "pipe"],
encoding: "utf8",
env: { ...env, GIT_CEILING_DIRECTORIES: dirname(dest), LC_ALL: "C" },
});
Comment thread
claude[bot] marked this conversation as resolved.

if (result.error) {
Expand All @@ -264,6 +283,16 @@ function applyPatch(dest: string, patchPath: string, patchBody: string): void {
hint: "The patch may be out of date with the pinned commit",
});
}

// Defense-in-depth: a file git decides is outside the worktree is
// skipped with exit 0. Nothing under patches/ is ever meant to be
// skipped — if it fires, GIT_CEILING_DIRECTORIES above didn't take.
if (result.stderr.includes("Skipped patch")) {
throw new BuildError(`git apply silently skipped a file:\n${result.stderr.trim()}`, {
file: patchPath,
hint: "The patch path resolved outside the dep source dir; see applyPatch() in fetch-cli.ts",
});
}
}

// Only run if this file is the entry point (not imported as a module).
Expand Down
98 changes: 98 additions & 0 deletions test/internal/dep-patch-apply.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/**
* Regression tests for scripts/build/fetch-cli.ts::applyPatch: `git apply`
* from a repo subdirectory treats `diff --git` patch paths as toplevel-
* relative and silently skips them with exit 0 (git-apply(1)). See the
* doc comment on applyPatch for the full mechanism.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
*/
import { describe, expect, test } from "bun:test";
import { tempDir } from "harness";
import { spawnSync } from "node:child_process";
import { readFileSync } from "node:fs";
import { join } from "node:path";

import { applyPatch } from "../../scripts/build/fetch-cli.ts";

const ORIGINAL = "line one\nline two\nline three\n";
const PATCHED = "line one\nline two (patched)\nline three\n";

/** A patch with a `diff --git` header, the form that tripped the bug. */
const GIT_HEADER_PATCH = `diff --git a/src/file.c b/src/file.c
index 1111111..2222222 100644
--- a/src/file.c
+++ b/src/file.c
@@ -1,3 +1,3 @@
line one
-line two
+line two (patched)
line three
`;

/** The same change as a plain unified diff (what most of patches/ uses). */
const PLAIN_PATCH = `--- a/src/file.c
+++ b/src/file.c
@@ -1,3 +1,3 @@
line one
-line two
+line two (patched)
line three
`;

/** git repo → vendor/<dep>/src/file.c; returns the `dest` fetch-cli passes. */
function makeDepTree(): { dir: ReturnType<typeof tempDir>; dest: string; target: string } {
const dir = tempDir("apply-patch", {
"vendor/mydep/src/file.c": ORIGINAL,
});
// Repo above dest is the precondition for the toplevel-relative rewrite.
const init = spawnSync("git", ["init", "-q"], { cwd: String(dir), encoding: "utf8" });
if (init.status !== 0) throw new Error(`git init failed: ${init.stderr}`);
const dest = join(String(dir), "vendor", "mydep");
return { dir, dest, target: join(dest, "src", "file.c") };
}

describe("applyPatch (scripts/build/fetch-cli.ts)", () => {
test("applies a diff --git patch from a repo subdirectory", () => {
const { dir, dest, target } = makeDepTree();
using _ = dir;

// Pre-fix: "Skipped patch 'src/file.c'." (only under -v), exit 0, file untouched.
applyPatch(dest, "test.patch", GIT_HEADER_PATCH);

expect(readFileSync(target, "utf8")).toBe(PATCHED);
});

test("applies a plain unified diff from a repo subdirectory", () => {
const { dir, dest, target } = makeDepTree();
using _ = dir;

applyPatch(dest, "test.patch", PLAIN_PATCH);

expect(readFileSync(target, "utf8")).toBe(PATCHED);
});

test("a patch that does not apply is reported as an error", () => {
const { dir, dest } = makeDepTree();
using _ = dir;

const bad = GIT_HEADER_PATCH.replace("-line two\n", "-does not exist\n");
expect(() => applyPatch(dest, "test.patch", bad)).toThrow(/Patch failed/);
});

test("ignores inherited GIT_DIR / GIT_WORK_TREE", () => {
// These (set by git hooks) bypass GIT_CEILING_DIRECTORIES entirely.
const { dir, dest, target } = makeDepTree();
using _ = dir;

const saved = { GIT_DIR: process.env.GIT_DIR, GIT_WORK_TREE: process.env.GIT_WORK_TREE };
process.env.GIT_DIR = join(String(dir), ".git");
process.env.GIT_WORK_TREE = String(dir);
try {
applyPatch(dest, "test.patch", GIT_HEADER_PATCH);
expect(readFileSync(target, "utf8")).toBe(PATCHED);
} finally {
for (const [k, v] of Object.entries(saved)) {
if (v === undefined) delete process.env[k];
else process.env[k] = v;
}
}
});
});
Loading