Skip to content

Commit 6c49a68

Browse files
Copilotpelikhangithub-actions[bot]
authored
Optimize incremental bundle transport for stale PR branch sync workflows (#34753)
* Initial plan * Reduce incremental bundle size by excluding base branch objects Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Polish incremental bundle test naming Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Address review feedback for incremental bundle optimization Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Simplify bundle test artifact cleanup Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Harden incremental bundle semantic assertions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Refine bundle test assertion clarity Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Remove redundant bundle head assertion Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * test: add fallback integration coverage for incremental bundle exclusions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * test: verify fallback incremental bundle behavior when base ref missing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Fix incremental base-ref fetch warning and remove redundant probe Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * chore: plan dry solid git audit Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Refactor remote-ref checks for DRY while preserving git bundle behavior Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Harden helper error handling in bundle ref probe refactor Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent db2cb7f commit 6c49a68

3 files changed

Lines changed: 212 additions & 15 deletions

File tree

.changeset/patch-incremental-bundle-base-exclusion.md

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

actions/setup/js/generate_git_bundle.cjs

Lines changed: 68 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,41 @@ function debugLog(message) {
2424
}
2525
}
2626

27+
/**
28+
* Ensure refs/remotes/origin/<branch> is available locally.
29+
* Returns whether the ref exists and whether a fetch was required.
30+
*
31+
* @param {string} branch - Branch name (without origin/ prefix)
32+
* @param {Object} options
33+
* @param {string} options.cwd - Working directory for git commands
34+
* @param {string} [options.token] - Optional auth token used for fetch
35+
* @param {boolean} [options.suppressLogs=false] - Whether to suppress execGitSync error logs
36+
* @returns {{ exists: boolean, fetched: boolean, fetchError?: Error }}
37+
* fetchError is populated only when exists=false after a failed fetch attempt.
38+
*/
39+
function ensureOriginRemoteTrackingRef(branch, options) {
40+
const ref = `refs/remotes/origin/${branch}`;
41+
try {
42+
execGitSync(["show-ref", "--verify", "--quiet", ref], {
43+
cwd: options.cwd,
44+
suppressLogs: options.suppressLogs || false,
45+
});
46+
return { exists: true, fetched: false };
47+
} catch {
48+
try {
49+
const fetchEnv = { ...process.env, ...getGitAuthEnv(options.token) };
50+
execGitSync(["fetch", "origin", "--", branch], {
51+
cwd: options.cwd,
52+
env: fetchEnv,
53+
suppressLogs: options.suppressLogs || false,
54+
});
55+
return { exists: true, fetched: true };
56+
} catch (fetchError) {
57+
return { exists: false, fetched: false, fetchError };
58+
}
59+
}
60+
}
61+
2762
/**
2863
* Sanitize a string for use as a bundle filename component.
2964
* Replaces path separators and special characters with dashes.
@@ -182,21 +217,17 @@ async function generateGitBundle(branchName, baseBranch, options = {}) {
182217
debugLog(`Strategy 1 (full): Using existing origin/${branchName} as baseRef`);
183218
} catch {
184219
debugLog(`Strategy 1 (full): origin/${branchName} not found, trying merge-base with ${defaultBranch}`);
185-
let hasLocalDefaultBranch = false;
186-
try {
187-
execGitSync(["show-ref", "--verify", "--quiet", `refs/remotes/origin/${defaultBranch}`], { cwd });
188-
hasLocalDefaultBranch = true;
189-
debugLog(`Strategy 1 (full): origin/${defaultBranch} exists locally`);
190-
} catch {
191-
debugLog(`Strategy 1 (full): origin/${defaultBranch} not found locally, attempting fetch`);
192-
try {
193-
const fullFetchEnv = { ...process.env, ...getGitAuthEnv(options.token) };
194-
execGitSync(["fetch", "origin", "--", defaultBranch], { cwd, env: fullFetchEnv });
195-
hasLocalDefaultBranch = true;
220+
const defaultBranchRefResult = ensureOriginRemoteTrackingRef(defaultBranch, { cwd, token: options.token });
221+
const hasLocalDefaultBranch = defaultBranchRefResult.exists;
222+
if (hasLocalDefaultBranch) {
223+
if (defaultBranchRefResult.fetched) {
196224
debugLog(`Strategy 1 (full): Successfully fetched origin/${defaultBranch}`);
197-
} catch (fetchErr) {
198-
debugLog(`Strategy 1 (full): Fetch failed - ${getErrorMessage(fetchErr)} (will try other strategies)`);
225+
} else {
226+
debugLog(`Strategy 1 (full): origin/${defaultBranch} exists locally`);
199227
}
228+
} else {
229+
debugLog(`Strategy 1 (full): origin/${defaultBranch} not found locally, attempting fetch`);
230+
debugLog(`Strategy 1 (full): Fetch failed - ${getErrorMessage(defaultBranchRefResult.fetchError || new Error("Unknown fetch error"))} (will try other strategies)`);
200231
}
201232

202233
if (hasLocalDefaultBranch) {
@@ -219,8 +250,30 @@ async function generateGitBundle(branchName, baseBranch, options = {}) {
219250

220251
if (commitCount > 0) {
221252
// Generate bundle from the determined base to the branch
222-
// git bundle create <file> <range> creates a bundle with the commit range
223-
execGitSync(["bundle", "create", bundlePath, `${baseRef}..${branchName}`], { cwd });
253+
// git bundle create <file> <range> creates a bundle with the commit range.
254+
// In incremental mode, also exclude origin/<defaultBranch> when present so
255+
// a "merge base branch into PR branch" workflow does not re-embed upstream
256+
// commits that the remote already has.
257+
const bundleCreateArgs = ["bundle", "create", bundlePath, `${baseRef}..${branchName}`];
258+
if (mode === "incremental") {
259+
const defaultBranchRefResult = ensureOriginRemoteTrackingRef(defaultBranch, {
260+
cwd,
261+
token: options.token,
262+
suppressLogs: true,
263+
});
264+
if (defaultBranchRefResult.exists) {
265+
if (defaultBranchRefResult.fetched) {
266+
debugLog(`Strategy 1 (incremental): fetched origin/${defaultBranch} for bundle exclusions`);
267+
}
268+
bundleCreateArgs.push(`^origin/${defaultBranch}`);
269+
debugLog(`Strategy 1 (incremental): excluding origin/${defaultBranch} from bundle prerequisites`);
270+
} else {
271+
const warningMessage = `Strategy 1 (incremental): could not fetch origin/${defaultBranch} for exclusions - ${getErrorMessage(defaultBranchRefResult.fetchError || new Error("Unknown fetch error"))}. Bundle will include base-branch history.`;
272+
debugLog(warningMessage);
273+
core.warning(warningMessage);
274+
}
275+
}
276+
execGitSync(bundleCreateArgs, { cwd });
224277

225278
if (fs.existsSync(bundlePath)) {
226279
const stat = fs.statSync(bundlePath);
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import { describe, it, expect, afterEach } from "vitest";
2+
import { createRequire } from "module";
3+
import fs from "fs";
4+
import os from "os";
5+
import path from "path";
6+
import { spawnSync } from "child_process";
7+
8+
const require = createRequire(import.meta.url);
9+
10+
global.core = {
11+
debug: () => {},
12+
error: () => {},
13+
info: () => {},
14+
warning: () => {},
15+
};
16+
17+
function execGit(args, options = {}) {
18+
const result = spawnSync("git", args, {
19+
encoding: "utf8",
20+
...options,
21+
});
22+
if (result.error) {
23+
throw result.error;
24+
}
25+
if (result.status !== 0 && !options.allowFailure) {
26+
throw new Error(`git ${args.join(" ")} failed: ${result.stderr}`);
27+
}
28+
return result;
29+
}
30+
31+
describe("generateGitBundle (incremental)", () => {
32+
const tempDirs = [];
33+
const bundlePaths = [];
34+
35+
afterEach(() => {
36+
for (const tempDir of tempDirs.splice(0)) {
37+
fs.rmSync(tempDir, { recursive: true, force: true });
38+
}
39+
for (const bundlePath of bundlePaths.splice(0)) {
40+
fs.rmSync(bundlePath, { force: true });
41+
}
42+
});
43+
44+
it("reduces bundle size by excluding origin/base branch objects already on remote", async () => {
45+
const remoteDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-bundle-remote-"));
46+
const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-bundle-work-"));
47+
tempDirs.push(remoteDir, workDir);
48+
49+
execGit(["init", "--bare"], { cwd: remoteDir });
50+
execGit(["clone", remoteDir, workDir]);
51+
execGit(["config", "user.name", "Test User"], { cwd: workDir });
52+
execGit(["config", "user.email", "test@example.com"], { cwd: workDir });
53+
54+
fs.writeFileSync(path.join(workDir, "base.txt"), "base\n");
55+
execGit(["add", "base.txt"], { cwd: workDir });
56+
execGit(["commit", "-m", "base"], { cwd: workDir });
57+
execGit(["branch", "-M", "main"], { cwd: workDir });
58+
execGit(["push", "-u", "origin", "main"], { cwd: workDir });
59+
60+
execGit(["checkout", "-b", "pr-branch"], { cwd: workDir });
61+
fs.writeFileSync(path.join(workDir, "pr.txt"), "pr start\n");
62+
execGit(["add", "pr.txt"], { cwd: workDir });
63+
execGit(["commit", "-m", "pr start"], { cwd: workDir });
64+
execGit(["push", "-u", "origin", "pr-branch"], { cwd: workDir });
65+
66+
execGit(["checkout", "main"], { cwd: workDir });
67+
for (let commitIndex = 0; commitIndex < 4; commitIndex++) {
68+
fs.writeFileSync(path.join(workDir, `upstream-${commitIndex}.txt`), `upstream ${commitIndex}\n`);
69+
execGit(["add", `upstream-${commitIndex}.txt`], { cwd: workDir });
70+
execGit(["commit", "-m", `upstream ${commitIndex}`], { cwd: workDir });
71+
}
72+
execGit(["push", "origin", "main"], { cwd: workDir });
73+
74+
execGit(["checkout", "pr-branch"], { cwd: workDir });
75+
execGit(["merge", "--no-ff", "origin/main", "-m", "merge main into pr"], { cwd: workDir });
76+
fs.writeFileSync(path.join(workDir, "resolution.txt"), "resolved\n");
77+
execGit(["add", "resolution.txt"], { cwd: workDir });
78+
execGit(["commit", "-m", "resolution commit"], { cwd: workDir });
79+
80+
const { generateGitBundle } = require("./generate_git_bundle.cjs");
81+
const result = await generateGitBundle("pr-branch", "main", { mode: "incremental", cwd: workDir });
82+
expect(result.success).toBe(true);
83+
expect(result.bundlePath).toBeTruthy();
84+
bundlePaths.push(result.bundlePath);
85+
86+
const naiveBundlePath = path.join(workDir, "naive.bundle");
87+
const optimizedBundlePath = path.join(workDir, "optimized.bundle");
88+
execGit(["bundle", "create", naiveBundlePath, "origin/pr-branch..pr-branch"], { cwd: workDir });
89+
execGit(["bundle", "create", optimizedBundlePath, "origin/pr-branch..pr-branch", "^origin/main"], { cwd: workDir });
90+
91+
const prBranchHeadSha = execGit(["rev-parse", "pr-branch"], { cwd: workDir }).stdout.trim();
92+
const generatedBundleHeads = execGit(["bundle", "list-heads", result.bundlePath], { cwd: workDir }).stdout.trim();
93+
const optimizedBundleHeads = execGit(["bundle", "list-heads", optimizedBundlePath], { cwd: workDir }).stdout.trim();
94+
const generatedSize = fs.statSync(result.bundlePath).size;
95+
const naiveSize = fs.statSync(naiveBundlePath).size;
96+
97+
expect(prBranchHeadSha).toBeTruthy();
98+
expect(prBranchHeadSha).toMatch(/^[a-f0-9]{40}$/);
99+
expect(optimizedBundleHeads).toContain(prBranchHeadSha);
100+
expect(generatedBundleHeads).toBe(optimizedBundleHeads);
101+
expect(generatedSize).toBeLessThan(naiveSize);
102+
});
103+
104+
it("falls back to non-exclusion bundle generation when origin/base branch is unavailable", async () => {
105+
const remoteDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-bundle-remote-"));
106+
const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-bundle-work-"));
107+
tempDirs.push(remoteDir, workDir);
108+
109+
execGit(["init", "--bare"], { cwd: remoteDir });
110+
execGit(["clone", remoteDir, workDir]);
111+
execGit(["config", "user.name", "Test User"], { cwd: workDir });
112+
execGit(["config", "user.email", "test@example.com"], { cwd: workDir });
113+
114+
execGit(["checkout", "-b", "pr-branch"], { cwd: workDir });
115+
fs.writeFileSync(path.join(workDir, "pr.txt"), "pr start\n");
116+
execGit(["add", "pr.txt"], { cwd: workDir });
117+
execGit(["commit", "-m", "pr start"], { cwd: workDir });
118+
execGit(["push", "-u", "origin", "pr-branch"], { cwd: workDir });
119+
120+
fs.writeFileSync(path.join(workDir, "pr-2.txt"), "pr second\n");
121+
execGit(["add", "pr-2.txt"], { cwd: workDir });
122+
execGit(["commit", "-m", "pr second"], { cwd: workDir });
123+
124+
const { generateGitBundle } = require("./generate_git_bundle.cjs");
125+
const result = await generateGitBundle("pr-branch", "main", { mode: "incremental", cwd: workDir });
126+
expect(result.success).toBe(true);
127+
expect(result.bundlePath).toBeTruthy();
128+
bundlePaths.push(result.bundlePath);
129+
130+
const naiveBundlePath = path.join(workDir, "naive.bundle");
131+
execGit(["bundle", "create", naiveBundlePath, "origin/pr-branch..pr-branch"], { cwd: workDir });
132+
expect(fs.existsSync(naiveBundlePath)).toBe(true);
133+
134+
const generatedBundleHeads = execGit(["bundle", "list-heads", result.bundlePath], { cwd: workDir }).stdout.trim();
135+
const naiveBundleHeads = execGit(["bundle", "list-heads", naiveBundlePath], { cwd: workDir }).stdout.trim();
136+
137+
expect(generatedBundleHeads).toBe(naiveBundleHeads);
138+
});
139+
});

0 commit comments

Comments
 (0)