mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
fix(restamp): review pass 2 — positions counted through joined-hunk context
parseUnifiedDiff records each added line's new-side position (addedAt), counting context lines a hunk carries, and branchAddedLines pins --inter-hunk-context=0, so a diff.interHunkContext setting that joins two hunks around a base row no longer shifts the branch's second row onto the base row. Test with interHunkContext=1 (red on the pass-1 tree). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
6469472483
commit
8f3f6867cd
3 files changed
+30
-9
No files matched your search
+1
-1
@@ -7,7 +7,7 @@
|
||||
### Fixed
|
||||
- **Quoted versions are stamp forms** in both the mention scanner (`tentativeMentions`) and the exact rewrite table (`rewriteStamps`) of `lib/restamp.ts`: `since: 'X'`, `since: "X"`, `(was 'X')` and `(was "X")` match like the bare forms and keep their quote style, and an opened quote must close right after X, so `since: '1.2.3-beta'` and a mismatched `since: '1.2.3"` are not the form (the tree receipt and the gate-reuse normalizers share `rewriteStamps`, so a quoted `since:` rewrite classifies as a stamp without a template, and a quoted prerelease change never does); `vX` already accepted a quote before the `v`. The scanner also reads the policy's `stamp_patterns` templates, so a template-only form on a branch-added line is a candidate too.
|
||||
- **The equal-version rule.** When the branch has no stamp of its own (its VERSION equals the base's and no `--was` is given), a stamp-form mention of the base version on a line the branch added is its stale tentative stamp and moves to the allocated version; the same version on a line the base already carries is the base's release and stays. The `stamp_paths` stage detail, printed in the stage table and kept in the journal, names both counts per file (`VERSION equals base X, so X moved only on lines this branch added and stayed on the base's own lines (<file>: N moved, M kept)`), so the stamp commit's tool output shows which lines moved and why; when an explicit stamp matched nothing the skipped detail now names the version it searched for. The rule reads line ownership, not meaning: a branch-added `Requires vX or later` inside `stamp_paths` on a branch forked at X moves too, visible in `--dry-run` and the counts; `docs/ship-policy.md` says how to write such a reference. The explicit `--was X` path is unchanged and still rewrites X on every line; its help line now says so and names the CHANGELOG re-heading it also drives.
|
||||
- **Branch-added lines are known by position, not text.** `branchAddedLines` records each added line's position from the `--unified=0` hunk headers beside its text, and the stage rewrites by position, so a base line whose text equals a branch-added line stays while the added copy moves, and a CRLF checkout (`core.autocrlf`, `eol=crlf`), whose diff is LF-normalized while the file carries CR, still finds the branch's row instead of reporting `0 moved` and shipping it stale.
|
||||
- **Branch-added lines are known by position, not text.** `branchAddedLines` records each added line's new-side position beside its text (`parseUnifiedDiff` counts it through any context line a hunk carries, and the diff pins `--inter-hunk-context=0` so a `diff.interHunkContext` setting cannot join hunks), and the stage rewrites by position, so a base line whose text equals a branch-added line stays while the added copy moves, and a CRLF checkout (`core.autocrlf`, `eol=crlf`), whose diff is LF-normalized while the file carries CR, still finds the branch's row instead of reporting `0 moved` and shipping it stale.
|
||||
- **A stamp interrupted after VERSION was written retries as a stamp.** The journal recovery restored the branch's own stamp but kept the already-written VERSION as a stamp when the original was the base version, so the retry rewrote nothing and returned `noop` with the row still stale; it now restores the equal-base provenance (no branch stamp) and the retry completes the stamp.
|
||||
|
||||
### Tests
|
||||
|
||||
+13
-8
@@ -209,18 +209,21 @@ function allowedOutput(plan: Plan, root: string, rel: string): boolean {
|
||||
return matchesAny(p.stamp_paths, rel) || matchesAny(p.stamp_files, rel) || matchesAny(p.release_outputs, rel);
|
||||
}
|
||||
|
||||
interface Hunk { removed: string[]; added: string[]; at: number }
|
||||
interface Hunk { removed: string[]; added: string[]; at: number; addedAt: number[] }
|
||||
export interface FileDiff { path: string; binary: boolean; hunks: Hunk[]; status: 'M' | 'A' | 'D' | 'R'; modeChanged?: boolean }
|
||||
|
||||
/**
|
||||
* Parse `git diff --unified=0` into per-file hunks of removed/added lines (shared with lib/tree-receipt.ts).
|
||||
* File headers come only before a file's first hunk: inside a hunk every `-`/`+` line is content, so a
|
||||
* body line that begins `--` or `++` (`---`/`+++` in the diff) is never dropped as a header.
|
||||
* `addedAt` holds each added line's 1-based position on the new side, counted through any context
|
||||
* line the hunk carries (`diff.interHunkContext` can join hunks under `--unified=0`).
|
||||
*/
|
||||
export function parseUnifiedDiff(text: string): FileDiff[] {
|
||||
const files: FileDiff[] = [];
|
||||
let cur: FileDiff | null = null;
|
||||
let hunk: Hunk | null = null;
|
||||
let cursor = 0;
|
||||
for (const line of text.split('\n')) {
|
||||
if (line.startsWith('diff --git ')) {
|
||||
const m = /^diff --git a\/(.+?) b\/(.+)$/.exec(line);
|
||||
@@ -233,9 +236,10 @@ export function parseUnifiedDiff(text: string): FileDiff[] {
|
||||
else if (!hunk && (line.startsWith('old mode ') || line.startsWith('new mode '))) cur.modeChanged = true;
|
||||
else if (line.startsWith('rename to ')) cur.status = 'R';
|
||||
else if (line.startsWith('Binary files')) cur.binary = true;
|
||||
else if (line.startsWith('@@')) { hunk = { removed: [], added: [], at: Number(/^@@ -\d+(?:,\d+)? \+(\d+)/.exec(line)?.[1] ?? 0) }; cur.hunks.push(hunk); }
|
||||
else if (line.startsWith('@@')) { hunk = { removed: [], added: [], at: Number(/^@@ -\d+(?:,\d+)? \+(\d+)/.exec(line)?.[1] ?? 0), addedAt: [] }; cursor = hunk.at; cur.hunks.push(hunk); }
|
||||
else if (hunk && line.startsWith('-')) hunk.removed.push(line.slice(1));
|
||||
else if (hunk && line.startsWith('+')) hunk.added.push(line.slice(1));
|
||||
else if (hunk && line.startsWith('+')) { hunk.added.push(line.slice(1)); hunk.addedAt.push(cursor); cursor += 1; }
|
||||
else if (hunk && line.startsWith(' ')) cursor += 1;
|
||||
}
|
||||
return files;
|
||||
}
|
||||
@@ -245,17 +249,18 @@ export interface AddedLines { text: Set<string>; at: Set<number> }
|
||||
|
||||
/**
|
||||
* The lines a branch itself added, per stamp file: `git diff --unified=0 <since> -- <files>` in `cwd`
|
||||
* against its working tree. Positions come from each hunk's new-side start (no context under
|
||||
* `--unified=0`, so a hunk's added lines are consecutive from there), so a base line whose text
|
||||
* equals an added line, or a CRLF checkout whose diff is LF-normalized, is told apart by position.
|
||||
* against its working tree. Positions are the parser's per-line count on the new side (context
|
||||
* lines included, and `--inter-hunk-context=0` keeps a `diff.interHunkContext` setting from joining
|
||||
* hunks), so a base line whose text equals an added line, or a CRLF checkout whose diff is
|
||||
* LF-normalized, is told apart by position.
|
||||
*/
|
||||
export function branchAddedLines(cwd: string, since: string, files: string[]): Map<string, AddedLines> {
|
||||
const out = new Map<string, AddedLines>();
|
||||
if (files.length === 0) return out;
|
||||
const diff = git(cwd, ['diff', '--unified=0', '--no-color', '--no-ext-diff', since, '--', ...files]);
|
||||
const diff = git(cwd, ['diff', '--unified=0', '--inter-hunk-context=0', '--no-color', '--no-ext-diff', since, '--', ...files]);
|
||||
for (const f of parseUnifiedDiff(diff.out)) {
|
||||
const own = out.get(f.path) ?? { text: new Set<string>(), at: new Set<number>() };
|
||||
for (const h of f.hunks) h.added.forEach((a, i) => { own.text.add(a); own.at.add(h.at + i); });
|
||||
for (const h of f.hunks) h.added.forEach((a, i) => { own.text.add(a); own.at.add(h.addedAt[i]!); });
|
||||
out.set(f.path, own);
|
||||
}
|
||||
return out;
|
||||
|
||||
@@ -181,6 +181,22 @@ describe('a PR whose VERSION already equals the base (gbrain #7640)', () => {
|
||||
expect(readFile(repo, NOTICE)).toBe(notice([row('1.2.3.0', 'dup'), '', '', '', row('1.2.4.0', 'dup')]));
|
||||
});
|
||||
|
||||
test('diff.interHunkContext joining two hunks around a base row still places each branch row by position', () => {
|
||||
const repo = makeFixtureRepo({ prefix: 'restamp-mentions-ihc-', policy: policy(true) }); repos.push(repo);
|
||||
write(repo, NOTICE, notice([' { since: "0.0.0.0", id: "first_placeholder" },', row('1.2.3.0', 'base_row'), ' { since: "0.0.0.0", id: "second_placeholder" },']));
|
||||
repo.git('add', '-A'); repo.git('commit', '-qm', 'notice rows'); repo.git('push', '-q', 'origin', 'main');
|
||||
repo.git('config', 'diff.interHunkContext', '1');
|
||||
makePr(repo, 'pr-mine');
|
||||
write(repo, NOTICE, notice([row('1.2.3.0', 'first'), row('1.2.3.0', 'base_row'), row('1.2.3.0', 'second')]));
|
||||
repo.git('commit', '-qam', 'two rows around the base row');
|
||||
// The joined hunk carries the base row as context between the two added lines.
|
||||
expect(repo.git('diff', '--unified=0', 'origin/main', '--', NOTICE)).toMatch(/\n\+ \{ since: '1\.2\.3\.0', id: 'first'.*\n \{ since: '1\.2\.3\.0', id: 'base_row'.*\n-.*\n\+ \{ since: '1\.2\.3\.0', id: 'second'/);
|
||||
const r = restamp(repo, ['--next']);
|
||||
expect(r.status).toBe(EXIT.ok);
|
||||
expect(stageDetail(r.out, 'stamp_paths')![2]).toContain(`${NOTICE}: 2 moved, 1 kept`);
|
||||
expect(readFile(repo, NOTICE)).toBe(notice([row('1.2.4.0', 'first'), row('1.2.3.0', 'base_row'), row('1.2.4.0', 'second')]));
|
||||
});
|
||||
|
||||
test('a CRLF checkout (core.autocrlf, eol=crlf) whose diff is LF-normalized still finds the branch-added row', () => {
|
||||
const repo = makeFixtureRepo({ prefix: 'restamp-mentions-crlf-', policy: policy(true) }); repos.push(repo);
|
||||
write(repo, '.gitattributes', `${NOTICE} text eol=crlf\n`);
|
||||
|
||||
Reference in new issue
Block a user