mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 15:27:56 +01:00
fix(ship-receipt): never leave out a path with a compared path beneath it (review fix, GST-1214)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
22b9772776
commit
4e8b1de503
2 files changed
+20
-3
No files matched your search
@@ -670,7 +670,10 @@ function cmdCarry(rest: string[], json: boolean): number {
|
||||
if (!listed[0] || !listed[1] || !listed[2]) return `git could not list the paths the branch touched against ${base}; run /ship on ${head.slice(0, 12)}`;
|
||||
// From a stack parent, a path whose reviewed content is the base's (an earlier base merge brought it in) is base
|
||||
// context, not reviewed product, unless HEAD changed it against the base (GST-1214).
|
||||
const baseMerged = rangeBase === fromBase ? [] : [...listed[0]].filter(p => !listed[2]!.has(p) && !listed[1]!.has(p));
|
||||
// A pathspec also names everything beneath a path, so a path with any compared path under it (a file the base turned
|
||||
// into a directory) stays in.
|
||||
const compared = [...listed[0], ...listed[1]];
|
||||
const baseMerged = rangeBase === fromBase ? [] : [...listed[0]].filter(p => !listed[2]!.has(p) && !listed[1]!.has(p) && !compared.some(q => q.startsWith(`${p}/`)));
|
||||
const touched = new Set([...listed[0], ...listed[1]].filter(p => !baseMerged.includes(p)));
|
||||
const found = productSpec(repoRoot, rules, from, head, { touched, sides: [[rangeBase, from], [headBase, head]], own: branchSides(repoRoot, baseRef, from, '') });
|
||||
if (!found) return `git diff ${from.slice(0, 12)} ${head.slice(0, 12)} failed`;
|
||||
|
||||
@@ -243,7 +243,7 @@ describe('carry after a stack parent squash-merges, receipt written against main
|
||||
|
||||
describe('carry after a stack parent lands, the child having merged main before its receipt (GST-1214)', () => {
|
||||
/** main <- parent (lib/util.js) <- feature (lib/retry.js), which merges main (wiki/) once before its receipt is written against main. */
|
||||
function mergedStack(repo: FixtureRepo): { parent: string; reviewed: string; body: string } {
|
||||
function mergedStack(repo: FixtureRepo, mainFiles: Record<string, string | null> = { 'wiki/a.md': '# a\n', 'wiki/b.md': '# b\n' }): { parent: string; reviewed: string; body: string } {
|
||||
repo.git('checkout', '-q', '-b', 'parent');
|
||||
const parent = commit(repo, { 'lib/util.js': SUB }, 'parent: sub');
|
||||
repo.git('push', '-q', 'origin', 'parent');
|
||||
@@ -251,7 +251,7 @@ describe('carry after a stack parent lands, the child having merged main before
|
||||
repo.git('reset', '-q', '--hard', parent);
|
||||
commit(repo, { 'lib/retry.js': RETRY4 }, 'child: four retries');
|
||||
repo.git('checkout', '-q', 'main');
|
||||
commit(repo, { 'wiki/a.md': '# a\n', 'wiki/b.md': '# b\n' }, 'main: wiki');
|
||||
commit(repo, mainFiles, 'main: wiki');
|
||||
repo.git('push', '-q', 'origin', 'main');
|
||||
repo.git('checkout', '-q', 'feature');
|
||||
repo.git('merge', '-q', '--no-edit', 'origin/main');
|
||||
@@ -290,4 +290,18 @@ describe('carry after a stack parent lands, the child having merged main before
|
||||
expect(out.err).toContain('wiki/a.md');
|
||||
expect(receiptIn(body).head).toBe(reviewed);
|
||||
}, 90_000);
|
||||
|
||||
// Value: protects=fail closed when the earlier base merge turned a file into a directory: leaving out the old file must not
|
||||
// hide an edit beneath it (a pathspec names a path and everything under it); fails_when=the exclusion covers a changed
|
||||
// descendant; seam=none; no_claim=-
|
||||
test('a base merge that replaced a file with a directory never hides an edit beneath it', () => {
|
||||
const repo = makeFixtureRepo({ baseFiles: { services: 'old\n' } }); repos.push(repo);
|
||||
const { reviewed, body } = mergedStack(repo, { services: null, 'services/worker.js': 'exports.allowed = false;\n' });
|
||||
landOnMain(repo, { 'lib/util.js': SUB });
|
||||
commit(repo, { 'services/worker.js': 'exports.allowed = true;\n' }, 'child: allow');
|
||||
const out = carry(repo, body);
|
||||
expect(out.status).toBe(EXIT.refused);
|
||||
expect(out.out).toContain('CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)');
|
||||
expect(receiptIn(body).head).toBe(reviewed);
|
||||
}, 90_000);
|
||||
});
|
||||
Reference in new issue
Block a user