Skip to content

Commit e291e82

Browse files
committed
fix: make applyPRDiff robust with multiple strategies
git apply was failing on large/complex PR diffs with 'corrupt patch' errors. Replace single git apply call with a cascade of strategies: 1. git apply --whitespace=nowarn --ignore-whitespace (most lenient real apply) 2. git apply --3way --whitespace=nowarn (3-way merge fallback) 3. patch -p1 --no-backup-if-mismatch (most lenient, handles edge cases) 4. git apply --whitespace=fix (auto-fix whitespace issues) Also normalize line endings (CRLF/CR → LF) before writing diff file to prevent cross-platform line-ending corruption.
1 parent 284c87c commit e291e82

1 file changed

Lines changed: 50 additions & 16 deletions

File tree

scripts/github-code-review.php

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -252,34 +252,68 @@ function applyPRDiff(string $checkoutPath, ?string $diff): bool
252252
return false;
253253
}
254254

255-
// Write diff to a temporary file
255+
// Write diff to a temporary file (use binary mode to preserve all bytes)
256256
$diffFile = tempnam(sys_get_temp_dir(), 'pr_diff_');
257257
if ($diffFile === false) {
258258
verbose_log("applyPRDiff: failed to create temp file", 1);
259259
return false;
260260
}
261261

262262
try {
263-
file_put_contents($diffFile, $diff);
263+
// Write with explicit LF line endings (not CRLF) for cross-platform compat
264+
$diffNormalized = str_replace(["\r\n", "\r"], "\n", $diff);
265+
file_put_contents($diffFile, $diffNormalized);
266+
267+
// Try multiple strategies in order of preference
268+
$strategies = [
269+
// Strategy 1: Strict apply with ignore-whitespace (covers most real diffs)
270+
[
271+
'cmd' => 'git apply --whitespace=nowarn --ignore-whitespace %s 2>&1',
272+
'label' => 'ignore-whitespace',
273+
],
274+
// Strategy 2: 3-way merge fallback - applies partial diff using index info
275+
[
276+
'cmd' => 'git apply --3way --whitespace=nowarn %s 2>&1',
277+
'label' => '3way-merge',
278+
],
279+
// Strategy 3: patch command (more lenient than git apply for complex diffs)
280+
[
281+
'cmd' => 'patch -p1 --no-backup-if-mismatch --dry-run < %s 2>&1 && patch -p1 --no-backup-if-mismatch < %s',
282+
'label' => 'patch-dryrun-then-apply',
283+
'doubleFile' => true,
284+
],
285+
// Strategy 4: git apply with --whitespace=fix (auto-fix whitespace)
286+
[
287+
'cmd' => 'git apply --whitespace=fix %s 2>&1',
288+
'label' => 'whitespace-fix',
289+
],
290+
];
291+
292+
$lastOutput = '';
293+
294+
foreach ($strategies as $strategy) {
295+
$cmd = sprintf(
296+
'cd %s && ' . $strategy['cmd'],
297+
escapeshellarg($checkoutPath),
298+
escapeshellarg($diffFile),
299+
escapeshellarg($diffFile)
300+
);
264301

265-
// Apply the diff using git apply
266-
$cmd = sprintf(
267-
'cd %s && git apply --verbose %s 2>&1',
268-
escapeshellarg($checkoutPath),
269-
escapeshellarg($diffFile)
270-
);
302+
$output = [];
303+
$ret = 0;
304+
exec($cmd, $output, $ret);
305+
$lastOutput = implode("\n", $output);
271306

272-
$output = [];
273-
$ret = 0;
274-
exec($cmd, $output, $ret);
307+
if ($ret === 0) {
308+
verbose_log("applyPRDiff: diff applied successfully (strategy: {$strategy['label']})", 3);
309+
return true;
310+
}
275311

276-
if ($ret !== 0) {
277-
verbose_log("applyPRDiff: git apply failed: " . implode("\n", $output), 2);
278-
return false;
312+
verbose_log("applyPRDiff: strategy {$strategy['label']} failed: " . substr($lastOutput, 0, 200), 3);
279313
}
280314

281-
verbose_log("applyPRDiff: diff applied successfully", 3);
282-
return true;
315+
verbose_log("applyPRDiff: all strategies failed - last output: " . substr($lastOutput, 0, 500), 2);
316+
return false;
283317
} finally {
284318
// Always clean up temp file
285319
if ($diffFile !== false && file_exists($diffFile)) {

0 commit comments

Comments
 (0)