mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-11 07:29:00 +02:00
fix: version-bump honors the .gstack/version-path pin in versionRel (#2462)
cmdClassify's current-version read already resolved the .gstack/version-path pin, but versionRel — the repo-relative path fed to `git show origin/<base>:<path>` — was derived from the CLI flag alone (`argVal(args, "--version-path") ?? "VERSION"`). In a pinned repo with no explicit flag, base and current therefore read DIFFERENT files: current from the pinned file, base from the root VERSION. On a repo with no root VERSION, the base always read 0.0.0.0 — and the pinned-JSON handling never engaged, so a pinned package.json was read as raw text (currentVersion 0.0.0.0) and `write` would have overwritten the manifest with a bare version string. New resolveVersionRel() resolves the pin's REPO-RELATIVE form once (flag → .gstack/version-path first line → "VERSION"); classify, write, and repair all derive both the relative and absolute paths from it, so base and current reads can no longer diverge. The old resolveVersionPath (which returned an absolute path `git show` cannot use) is folded in. Unit tests (the ENG-OV6 spec case plus write/repair coverage): pin set + no flag → classify reads base AND current from the SAME pinned file (plain-text sub/VERSION and pinned frontend/package.json, both against a real git base with NO root VERSION anywhere), write updates the pinned manifest in place without inventing a root VERSION, repair treats the pinned JSON as single-source, and the explicit flag still overrides the pin (38 pass in test/gstack-version-bump.test.ts). Re-spec'd per ENG-OV6 from the report in #2462 (the originally-filed classify-read hypothesis was already handled; the live bug was the :138 versionRel derivation). Same fix shape independently identified in PR #2501 by @YiftahR. Fixes #2462 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
da0e28e686
commit
909a9e9577
+26
-13
@@ -73,17 +73,30 @@ function argVal(args: string[], flag: string): string | undefined {
|
||||
return i >= 0 && i + 1 < args.length ? args[i + 1] : undefined;
|
||||
}
|
||||
|
||||
/** Resolve the VERSION file path: --version-path, else .gstack/version-path, else "VERSION". */
|
||||
function resolveVersionPath(cwd: string, explicit?: string): string {
|
||||
if (explicit) return join(cwd, explicit);
|
||||
/**
|
||||
* Resolve the version file's path RELATIVE to the repo root: --version-path,
|
||||
* else .gstack/version-path, else "VERSION".
|
||||
*
|
||||
* The relative form is what matters — `git show origin/<base>:<rel>` needs it
|
||||
* (an absolute path is unusable there). Callers used to
|
||||
* derive versionRel from the CLI flag alone (#2462), so a repo using the
|
||||
* .gstack/version-path pin had its local (pinned) version compared against
|
||||
* the BASE's root VERSION file: two different files. On a repo with no root
|
||||
* VERSION the base then always read 0.0.0.0, making every branch look FRESH —
|
||||
* and the pinned-JSON handling never engaged without the explicit flag.
|
||||
* Resolving once, here, keeps base and current reads in step.
|
||||
*/
|
||||
function resolveVersionRel(cwd: string, explicit?: string): string {
|
||||
if (explicit) return explicit.trim();
|
||||
const pin = join(cwd, ".gstack", "version-path");
|
||||
if (existsSync(pin)) {
|
||||
const p = readFileSync(pin, "utf-8").trim();
|
||||
if (p) return join(cwd, p);
|
||||
const p = readFileSync(pin, "utf-8").split("\n")[0]?.trim() ?? "";
|
||||
if (p) return p;
|
||||
}
|
||||
return join(cwd, "VERSION");
|
||||
return "VERSION";
|
||||
}
|
||||
|
||||
|
||||
function readVersionFile(p: string, versionRel = "VERSION"): string {
|
||||
try {
|
||||
// extractVersion (#2501): a .json version-path is read as JSON (.version),
|
||||
@@ -98,7 +111,7 @@ function readVersionFile(p: string, versionRel = "VERSION"): string {
|
||||
/**
|
||||
* Resolve the manifest path: --package-json-path, else
|
||||
* .gstack/package-json-path, else "package.json" (#2531, mirrors
|
||||
* resolveVersionPath). A repo whose only Node package lives in a
|
||||
* resolveVersionRel). A repo whose only Node package lives in a
|
||||
* subdirectory (web/, app/, frontend/) has no ROOT package.json, so the
|
||||
* old join(cwd, "package.json") reported pkgExists:false and every bump
|
||||
* silently wrote VERSION alone — leaving the manifest to be edited by
|
||||
@@ -219,8 +232,8 @@ function classifyState(
|
||||
function cmdClassify(args: string[], cwd: string): void {
|
||||
const base = argVal(args, "--base");
|
||||
if (!base) fail("classify requires --base <branch>", 2);
|
||||
const versionPath = resolveVersionPath(cwd, argVal(args, "--version-path"));
|
||||
const versionRel = argVal(args, "--version-path") ?? "VERSION";
|
||||
const versionRel = resolveVersionRel(cwd, argVal(args, "--version-path"));
|
||||
const versionPath = join(cwd, versionRel);
|
||||
const current = readVersionFile(versionPath, versionRel);
|
||||
const baseV = baseVersion(cwd, base!, versionRel);
|
||||
// When the version-path IS a package.json (#2501), that file is the single
|
||||
@@ -261,8 +274,8 @@ function cmdWrite(args: string[], cwd: string): void {
|
||||
if (!VERSION_RE.test(version!)) {
|
||||
fail(`NEW_VERSION (${version}) does not match MAJOR.MINOR.PATCH.MICRO. Aborting.`, 2);
|
||||
}
|
||||
const versionPath = resolveVersionPath(cwd, argVal(args, "--version-path"));
|
||||
const versionRel = argVal(args, "--version-path") ?? "VERSION";
|
||||
const versionRel = resolveVersionRel(cwd, argVal(args, "--version-path"));
|
||||
const versionPath = join(cwd, versionRel);
|
||||
|
||||
// A package.json version-path (#2501) is written in place, keeping the rest
|
||||
// of the file intact — and it is the ONLY file written. Also syncing a root
|
||||
@@ -325,8 +338,8 @@ function cmdWrite(args: string[], cwd: string): void {
|
||||
}
|
||||
|
||||
function cmdRepair(args: string[], cwd: string): void {
|
||||
const versionPath = resolveVersionPath(cwd, argVal(args, "--version-path"));
|
||||
const versionRel = argVal(args, "--version-path") ?? "VERSION";
|
||||
const versionRel = resolveVersionRel(cwd, argVal(args, "--version-path"));
|
||||
const versionPath = join(cwd, versionRel);
|
||||
// Nothing to repair when the version lives in a package.json (#2501): there
|
||||
// is no second file to drift from, and classify never reports DRIFT_* for
|
||||
// that shape.
|
||||
|
||||
@@ -296,6 +296,104 @@ describe('package.json as the version source (monorepo, 3-digit, #2501)', () =>
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* #2462: cmdClassify's current-version read resolved the .gstack/version-path
|
||||
* pin, but versionRel — the repo-relative path fed to `git show
|
||||
* origin/<base>:<path>` — came from the CLI flag alone. In a pinned repo with
|
||||
* no --version-path flag, base and current therefore read DIFFERENT files:
|
||||
* current from the pin, base from the root VERSION (which may not exist, so
|
||||
* base always read 0.0.0.0 and every branch looked FRESH). The pin's
|
||||
* repo-relative form now drives all three subcommands.
|
||||
*/
|
||||
describe('.gstack/version-path pin, no --version-path flag (#2462)', () => {
|
||||
const mkPinned = (pinRel: string): string => {
|
||||
const d = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-pin-'));
|
||||
fs.mkdirSync(path.dirname(path.join(d, pinRel)), { recursive: true });
|
||||
fs.mkdirSync(path.join(d, '.gstack'), { recursive: true });
|
||||
fs.writeFileSync(path.join(d, '.gstack', 'version-path'), pinRel + '\n');
|
||||
return d;
|
||||
};
|
||||
|
||||
const commitBase = (d: string): void => {
|
||||
execFileSync('git', ['init', '-q', '-b', 'main'], { cwd: d });
|
||||
execFileSync('git', ['config', 'user.email', 't@e.com'], { cwd: d });
|
||||
execFileSync('git', ['config', 'user.name', 't'], { cwd: d });
|
||||
execFileSync('git', ['add', '-A'], { cwd: d });
|
||||
execFileSync('git', ['commit', '-qm', 'base'], { cwd: d });
|
||||
const head = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: d }).toString().trim();
|
||||
fs.mkdirSync(path.join(d, '.git', 'refs', 'remotes', 'origin'), { recursive: true });
|
||||
fs.writeFileSync(path.join(d, '.git', 'refs', 'remotes', 'origin', 'main'), head + '\n');
|
||||
};
|
||||
|
||||
test('classify reads base AND current from the SAME pinned plain-text file', () => {
|
||||
const pinRel = 'sub/VERSION';
|
||||
const d = mkPinned(pinRel);
|
||||
fs.writeFileSync(path.join(d, pinRel), '1.4.0.0\n');
|
||||
commitBase(d);
|
||||
// Move the pinned file past base — NO root VERSION file exists at all.
|
||||
fs.writeFileSync(path.join(d, pinRel), '1.5.0.0\n');
|
||||
const out = JSON.parse(execFileSync('bun', [BIN, 'classify', '--base', 'main'], { cwd: d }).toString());
|
||||
// Before the fix: baseVersion read root VERSION → "0.0.0.0" and the
|
||||
// branch misclassified as... current 1.5.0.0 vs base 0.0.0.0. The REAL
|
||||
// base is the pinned file's committed value.
|
||||
expect(out.baseVersion).toBe('1.4.0.0');
|
||||
expect(out.currentVersion).toBe('1.5.0.0');
|
||||
expect(out.state).toBe('ALREADY_BUMPED');
|
||||
fs.rmSync(d, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('classify engages the pinned package.json JSON handling without a flag', () => {
|
||||
const pinRel = 'frontend/package.json';
|
||||
const d = mkPinned(pinRel);
|
||||
fs.writeFileSync(path.join(d, pinRel), JSON.stringify({ name: 'f', version: '0.99.2' }, null, 2) + '\n');
|
||||
commitBase(d);
|
||||
const out = JSON.parse(execFileSync('bun', [BIN, 'classify', '--base', 'main'], { cwd: d }).toString());
|
||||
// Before the fix: versionRel="VERSION" → the pinned JSON was read as raw
|
||||
// text → currentVersion "0.0.0.0", pkgExists false, base from a
|
||||
// nonexistent root VERSION.
|
||||
expect(out.state).toBe('FRESH');
|
||||
expect(out.baseVersion).toBe('0.99.2');
|
||||
expect(out.currentVersion).toBe('0.99.2');
|
||||
expect(out.pkgExists).toBe(true);
|
||||
fs.rmSync(d, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('write honors the pin: updates the pinned package.json in place, no root VERSION invented', () => {
|
||||
const pinRel = 'frontend/package.json';
|
||||
const d = mkPinned(pinRel);
|
||||
fs.writeFileSync(path.join(d, pinRel), JSON.stringify({ name: 'f', version: '0.99.2' }, null, 2) + '\n');
|
||||
const out = JSON.parse(execFileSync('bun', [BIN, 'write', '--version', '0.99.3'], { cwd: d }).toString());
|
||||
expect(out).toEqual({ wrote: '0.99.3', versionPath: pinRel, packageJson: true, packageLock: false });
|
||||
expect(JSON.parse(fs.readFileSync(path.join(d, pinRel), 'utf-8')).version).toBe('0.99.3');
|
||||
// Before the fix, write treated versionRel as "VERSION" and overwrote the
|
||||
// pinned JSON file with a bare "0.99.3\n", destroying the manifest.
|
||||
expect(fs.existsSync(path.join(d, 'VERSION'))).toBe(false);
|
||||
fs.rmSync(d, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('repair honors the pin: pinned package.json is a no-op single source', () => {
|
||||
const pinRel = 'frontend/package.json';
|
||||
const d = mkPinned(pinRel);
|
||||
fs.writeFileSync(path.join(d, pinRel), JSON.stringify({ name: 'f', version: '0.99.2' }, null, 2) + '\n');
|
||||
const out = JSON.parse(execFileSync('bun', [BIN, 'repair'], { cwd: d }).toString());
|
||||
expect(out.repaired).toBeNull();
|
||||
fs.rmSync(d, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('--version-path flag still overrides the pin', () => {
|
||||
const d = mkPinned('sub/VERSION');
|
||||
fs.writeFileSync(path.join(d, 'sub', 'VERSION'), '1.0.0.0\n');
|
||||
fs.writeFileSync(path.join(d, 'OTHER_VERSION'), '2.0.0.0\n');
|
||||
commitBase(d);
|
||||
const out = JSON.parse(
|
||||
execFileSync('bun', [BIN, 'classify', '--base', 'main', '--version-path', 'OTHER_VERSION'], { cwd: d }).toString(),
|
||||
);
|
||||
expect(out.currentVersion).toBe('2.0.0.0');
|
||||
expect(out.baseVersion).toBe('2.0.0.0');
|
||||
fs.rmSync(d, { recursive: true, force: true });
|
||||
});
|
||||
});
|
||||
|
||||
describe('subdirectory manifest (no root package.json, #2531)', () => {
|
||||
/**
|
||||
* The layout this tool used to silently no-op on: the only Node package
|
||||
|
||||
Reference in New Issue
Block a user