mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-15 04:47:23 +01:00
fix(stamp): name branch lines by position; carry and finish accept a kept VERSION claim (GST-979, GST-980)
BranchSide carries the positions of the branch-added lines and stampOnlyLine takes the removed line position, so a base line whose text duplicates a branch line keeps the old-version floor. At equal versions only the branch own lines may move. stampOnlyText and stampedFiles accept a kept claim for those lines, and the carry leaves a stamp file out of the product comparison when its only change since the reviewed head is a stamp on the branch own rows and the base did not touch it. Failing-first: all five new tests fail on main fc292e22f.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
40a3170335
commit
fbf83f11ef
12 files changed
+178
-35
No files matched your search
+10
-5
@@ -244,7 +244,7 @@ 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; addedAt: number[] }
|
||||
interface Hunk { removed: string[]; added: string[]; at: number; addedAt: number[]; removedAt: number[] }
|
||||
export interface FileDiff { path: string; binary: boolean; hunks: Hunk[]; status: 'M' | 'A' | 'D' | 'R'; modeChanged?: boolean }
|
||||
|
||||
/**
|
||||
@@ -252,13 +252,15 @@ export interface FileDiff { path: string; binary: boolean; hunks: Hunk[]; status
|
||||
* 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`).
|
||||
* line the hunk carries (`diff.interHunkContext` can join hunks under `--unified=0`), and `removedAt` each removed
|
||||
* line's position on the old side, counted the same way.
|
||||
*/
|
||||
export function parseUnifiedDiff(text: string): FileDiff[] {
|
||||
const files: FileDiff[] = [];
|
||||
let cur: FileDiff | null = null;
|
||||
let hunk: Hunk | null = null;
|
||||
let cursor = 0;
|
||||
let oldCursor = 0;
|
||||
for (const line of text.split('\n')) {
|
||||
if (line.startsWith('diff --git ')) {
|
||||
const m = /^diff --git a\/(.+?) b\/(.+)$/.exec(line);
|
||||
@@ -271,10 +273,13 @@ 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), addedAt: [] }; cursor = hunk.at; cur.hunks.push(hunk); }
|
||||
else if (hunk && line.startsWith('-')) hunk.removed.push(line.slice(1));
|
||||
else if (line.startsWith('@@')) {
|
||||
const m = /^@@ -(\d+)(?:,\d+)? \+(\d+)/.exec(line);
|
||||
hunk = { removed: [], added: [], at: Number(m?.[2] ?? 0), addedAt: [], removedAt: [] }; cursor = hunk.at; oldCursor = Number(m?.[1] ?? 0); cur.hunks.push(hunk);
|
||||
}
|
||||
else if (hunk && line.startsWith('-')) { hunk.removed.push(line.slice(1)); hunk.removedAt.push(oldCursor); oldCursor += 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;
|
||||
else if (hunk && line.startsWith(' ')) { cursor += 1; oldCursor += 1; }
|
||||
}
|
||||
return files;
|
||||
}
|
||||
|
||||
@@ -111,7 +111,8 @@ export function releasePaths(policy: Record<string, any> | null, cwd: string, re
|
||||
* A moved version is floored at `ref`'s VERSION, except on a line the branch added itself (gstack-restamp's own rule:
|
||||
* added since the merge base with origin/<base>, and not added by the base since): after a lost slot and a merge of the
|
||||
* base its stale stamp is older than `ref`'s VERSION, so it must be at or newer than the fork point's version instead,
|
||||
* the bound gstack-restamp's stamp_paths stage uses (GST-895; the fork version itself since GST-963).
|
||||
* the bound gstack-restamp's stamp_paths stage uses (GST-895; the fork version itself since GST-963). A branch line is
|
||||
* named by its position, and with a kept VERSION only such a line may move (GST-979, GST-980).
|
||||
*/
|
||||
export function stampedFiles(policy: Record<string, any> | null, cwd: string, ref: string, base: string): string[] {
|
||||
const globs = Array.isArray(policy?.stamp_files) ? policy.stamp_files.filter((s: unknown): s is string => typeof s === 'string') : [];
|
||||
@@ -120,14 +121,14 @@ export function stampedFiles(policy: Record<string, any> | null, cwd: string, re
|
||||
if (!('version' in before)) return [];
|
||||
let after = '';
|
||||
try { after = extractVersion(fs.readFileSync(path.join(cwd, before.rel), 'utf8'), before.rel); } catch { return []; }
|
||||
if (!after || after === before.version) return [];
|
||||
if (!after) return [];
|
||||
const templates = Array.isArray(policy?.stamp_patterns) ? policy.stamp_patterns.filter((s: unknown): s is string => typeof s === 'string') : [];
|
||||
const diff = git(cwd, ['diff', '--unified=0', '--no-color', '--no-ext-diff', '--no-textconv', '--no-renames', ref]);
|
||||
if (diff.status !== 0) return [];
|
||||
const side = branchSides(cwd, `origin/${base}`, 'HEAD', before.version, ref);
|
||||
const stamp = (rel: string, removed: string, added: string): boolean => stampOnlyLine(removed, added, before.version, after, templates, side(rel));
|
||||
const stamp = (rel: string, removed: string, added: string, at: number): boolean => stampOnlyLine(removed, added, before.version, after, templates, side(rel), at);
|
||||
return parseUnifiedDiff(diff.stdout).filter(f => f.status === 'M' && !f.binary && !f.modeChanged && matchesAny(globs, f.path) && f.hunks.length > 0 &&
|
||||
f.hunks.every(h => h.removed.length === h.added.length && h.removed.every((line, i) => stamp(f.path, line, h.added[i]!)))).map(f => f.path);
|
||||
f.hunks.every(h => h.removed.length === h.added.length && h.removed.every((line, i) => stamp(f.path, line, h.added[i]!, h.removedAt[i]!)))).map(f => f.path);
|
||||
}
|
||||
|
||||
/** Under `changelog_fragments`, the branch's own Step 13 entry `<dir>/<branch-slug>.md` (docs/ship-policy.md#changelog-fragments); null without the key. */
|
||||
|
||||
+12
-7
@@ -178,6 +178,8 @@ export interface CarryPlanInput {
|
||||
retargetedFrom?: string;
|
||||
/** A clean scoped review of product paths whose only difference is base context next to unchanged hunks (lib/ship-patch.ts, GST-384): the carry reason it adds. */
|
||||
scopedReview?: string;
|
||||
/** Stamp files the branch changed as product whose only change since the reviewed head is a stamp on the branch's own rows, the base untouched, and without which the two product patches match (GST-980). */
|
||||
stampedRows?: string[];
|
||||
}
|
||||
export interface CarryRecord { from: string; reason: string; product_patch_id: string; changed_paths: string[]; merge_commits: number; range_diff: string; covers: string[]; reruns: string[]; trees?: string[]; ancestry?: 'linear' | 'rewritten' }
|
||||
export type CarryPlan = { ok: true; carry: CarryRecord } | { ok: false; code: 'RECEIPT_CARRY_PRODUCT_CHANGED' | 'RECEIPT_CARRY_REFUSED' | 'RECEIPT_CARRY_TREE_MISMATCH'; message: string };
|
||||
@@ -195,22 +197,23 @@ export interface CarryChange { before: string | null; after: string | null; oldV
|
||||
* old text once every new version in it is the old one, or when every changed
|
||||
* line is a stamp-form rewrite to the new version (`stampOnlyLine`: the old
|
||||
* version or a newer tentative `*_KEY_VERSION` the slot stamp moved, #3512, and
|
||||
* on a line the branch added itself, `side`, the fork version or newer, GST-963): the rule
|
||||
* on a line the branch added itself, `side`, the fork version or newer, GST-963; with a kept VERSION only such a line,
|
||||
* GST-980): the rule
|
||||
* the tree receipt applies to the same `stamp_paths`, `stamp_files` and
|
||||
* `release_outputs` before the train accepts a stamp commit. No version
|
||||
* change, or a file missing on either side, is not a stamp.
|
||||
* change outside the branch's own lines, or a file missing on either side, is not a stamp.
|
||||
*/
|
||||
export function stampOnlyText(change: CarryChange): boolean {
|
||||
const { before, after, oldVersion, newVersion } = change;
|
||||
if (before === null || after === null || !oldVersion || !newVersion || oldVersion === newVersion) return false;
|
||||
if (before === null || after === null || !oldVersion || !newVersion || (oldVersion === newVersion && !change.side)) return false;
|
||||
// Digit-bounded: replacing 1.2.3 inside 11.2.3 or 1.2.34 would read a different version's stamp as this one's.
|
||||
const escaped = newVersion.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
if (after.replace(new RegExp(`(?<![0-9])${escaped}(?![0-9])`, 'g'), oldVersion) === before) return true;
|
||||
if (oldVersion !== newVersion && after.replace(new RegExp(`(?<![0-9])${escaped}(?![0-9])`, 'g'), oldVersion) === before) return true;
|
||||
const was = before.split('\n');
|
||||
const now = after.split('\n');
|
||||
if (was.length !== now.length) return false;
|
||||
const changed = was.map((line, i) => i).filter(i => was[i] !== now[i]);
|
||||
return changed.length > 0 && changed.every(i => stampOnlyLine(was[i]!, now[i]!, oldVersion, newVersion, change.templates ?? [], change.side));
|
||||
return changed.length > 0 && changed.every(i => stampOnlyLine(was[i]!, now[i]!, oldVersion, newVersion, change.templates ?? [], change.side, i + 1));
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -241,16 +244,18 @@ export function classifyCarryPath(rel: string, extra: { metadata?: readonly stri
|
||||
* conflict-only base merge, a test-only fix or a release-metadata change
|
||||
* leaves behind. A product patch that differs is RECEIPT_CARRY_PRODUCT_CHANGED,
|
||||
* whatever else changed, unless every difference is base context next to unchanged hunks
|
||||
* and a scoped review of those hunks came back clean (`scopedReview`).
|
||||
* and a scoped review of those hunks came back clean (`scopedReview`), or a stamp on the
|
||||
* branch's own rows in a stamp file the branch also changed as product (`stampedRows`, GST-980).
|
||||
*/
|
||||
export function planCarry(input: CarryPlanInput): CarryPlan {
|
||||
if (input.productPatchId.from !== input.productPatchId.head && (input.changedProduct.length || !input.scopedReview)) {
|
||||
if (input.productPatchId.from !== input.productPatchId.head && (input.changedProduct.length || (!input.scopedReview && !input.stampedRows?.length))) {
|
||||
const named = input.changedProduct.slice(0, CARRY_NAMED_PATHS);
|
||||
return { ok: false, code: 'RECEIPT_CARRY_PRODUCT_CHANGED', message: `product diff changed between ${input.from.slice(0, 12)} and ${input.head.slice(0, 12)}: ${named.join(', ') || 'patch-id differs'}${input.changedProduct.length > named.length ? ` (+${input.changedProduct.length - named.length} more)` : ''}` };
|
||||
}
|
||||
const reasons: string[] = input.retargetedFrom ? [`stack parent ${input.retargetedFrom} landed`] : [];
|
||||
if (input.mergeCommits > 0) reasons.push(input.mergeCommits === 1 ? 'base merge' : `${input.mergeCommits} base merges`);
|
||||
if (input.scopedReview && input.productPatchId.from !== input.productPatchId.head) reasons.push(input.scopedReview);
|
||||
if (input.stampedRows?.length && input.productPatchId.from !== input.productPatchId.head) reasons.push(`a stamp on the branch's own rows (${input.stampedRows.join(', ')})`);
|
||||
if (input.testPatchId.from !== input.testPatchId.head) reasons.push('test-only change');
|
||||
if (input.changed.some(c => c.class === 'metadata')) reasons.push('release metadata');
|
||||
if (input.changed.some(c => c.class === 'generated')) reasons.push('regenerated outputs');
|
||||
|
||||
+17
-10
@@ -20,18 +20,25 @@ import { cmpVersion, parseVersion, versionWidth } from './version-source';
|
||||
* one of another width (`tentativeMentions`: a 3-digit dependency pin in a
|
||||
* 4-digit repo is never a candidate, so moving it is a hand edit). A line that
|
||||
* moves nothing, or changes anything beyond those mentions, is not one either.
|
||||
* With `side`, a removed line the branch added itself is bounded by its fork
|
||||
* point's version instead, the stage's bound for its own lines (GST-895,
|
||||
* GST-963): finish's stampedFiles, the tree receipt and the receipt carry all
|
||||
* pass the same side.
|
||||
* With `side`, a removed line the branch added itself, named by its 1-based
|
||||
* position `at` in the side's file (GST-979: a base line with the same text
|
||||
* is not the branch's), is bounded by its fork point's version instead, the
|
||||
* stage's bound for its own lines (GST-895, GST-963): finish's stampedFiles,
|
||||
* the tree receipt and the receipt carry all pass the same side. When the
|
||||
* version does not move (a slot that keeps its claimed VERSION), only such a
|
||||
* branch line may change: the stage moves nothing else then.
|
||||
*/
|
||||
export function stampOnlyLine(removed: string, added: string, oldV: string, newV: string, templates: readonly string[] = [], side?: BranchSide): boolean {
|
||||
export function stampOnlyLine(removed: string, added: string, oldV: string, newV: string, templates: readonly string[] = [], side?: BranchSide, at?: number): boolean {
|
||||
if (removed === added) return false;
|
||||
return stampOnlyWithin(removed, added, oldV, newV, templates) || (!!side?.added.has(removed) && stampOnlyWithin(removed, added, side.fork, newV, templates));
|
||||
if (oldV !== newV && stampOnlyWithin(removed, added, oldV, newV, templates)) return true;
|
||||
return !!side && at !== undefined && side.at.has(at) && side.added.has(removed) && stampOnlyWithin(removed, added, side.fork, newV, templates);
|
||||
}
|
||||
|
||||
/** Lines the branch added to one file (since its merge base with the base, less the lines the base added since) and its fork point's version. */
|
||||
export interface BranchSide { fork: string; added: ReadonlySet<string> }
|
||||
/**
|
||||
* Lines the branch added to one file (since its merge base with the base, less the lines the base added since): their
|
||||
* text and their 1-based positions in the side's file, and its fork point's version.
|
||||
*/
|
||||
export interface BranchSide { fork: string; added: ReadonlySet<string>; at: ReadonlySet<number> }
|
||||
|
||||
/**
|
||||
* Each file's BranchSide for `head` against `baseRef`, read lazily and cached: the lines come from merge-base..`to`
|
||||
@@ -48,8 +55,8 @@ export function branchSides(repoRoot: string, baseRef: string, head: string, fal
|
||||
let side: BranchSide | undefined;
|
||||
if (since) {
|
||||
try {
|
||||
const added = branchAddedLines(repoRoot, since, [rel], to, branchAddedLines(repoRoot, since, [rel], baseRef)).get(rel)?.text;
|
||||
if (added?.size) side = { fork: (fork ??= forkVersion(repoRoot, baseRef, fallback, head)), added };
|
||||
const own = branchAddedLines(repoRoot, since, [rel], to, branchAddedLines(repoRoot, since, [rel], baseRef)).get(rel);
|
||||
if (own?.text.size) side = { fork: (fork ??= forkVersion(repoRoot, baseRef, fallback, head)), added: own.text, at: own.at };
|
||||
} catch {}
|
||||
}
|
||||
sides.set(rel, side);
|
||||
|
||||
+1
-1
@@ -86,7 +86,7 @@ function stampOnlyHunks(hunks: FileDiff['hunks'], oldV: string, newV: string, te
|
||||
for (let i = 0; i < h.removed.length; i++) {
|
||||
const r = h.removed[i]!;
|
||||
const a = h.added[i]!;
|
||||
if (!stampOnlyLine(r, a, oldV, newV, templates, side)) return { why: `${JSON.stringify(r.trim().slice(0, 60))} -> ${JSON.stringify(a.trim().slice(0, 60))}`, line: h.at + i };
|
||||
if (!stampOnlyLine(r, a, oldV, newV, templates, side, h.removedAt[i])) return { why: `${JSON.stringify(r.trim().slice(0, 60))} -> ${JSON.stringify(a.trim().slice(0, 60))}`, line: h.at + i };
|
||||
}
|
||||
}
|
||||
return null;
|
||||
|
||||
Reference in new issue
Block a user