From ebc23a1e05198dc1f919700aa0bd77e07809374a Mon Sep 17 00:00:00 2001 From: Kelly Selden <602423+kellyselden@users.noreply.github.com> Date: Fri, 19 Dec 2025 14:09:38 -0500 Subject: [PATCH 1/5] chore: rename variable cwd --- test/git-test.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/git-test.js b/test/git-test.js index d9971209..0047802b 100644 --- a/test/git-test.js +++ b/test/git-test.js @@ -9,10 +9,10 @@ const path = require('path'); const fs = require('fs'); describe(function() { - let tmpPath; + let cwd; beforeEach(async function() { - tmpPath = await gitInit({ + cwd = await gitInit({ defaultBranchName: 'master', }); }); @@ -22,19 +22,19 @@ describe(function() { describe('in memory', function () { it('works', async function() { let oldSha = await git(['rev-parse', 'HEAD'], { - cwd: tmpPath, + cwd, cached: true, }); - await execa('git', ['commit', '-m', 'test', '--allow-empty'], { cwd: tmpPath }); + await execa('git', ['commit', '-m', 'test', '--allow-empty'], { cwd }); let cachedSha = await git(['rev-parse', 'HEAD'], { - cwd: tmpPath, + cwd, cached: true, }); let newSha = await execa('git', ['rev-parse', 'HEAD'], { - cwd: tmpPath, + cwd, }); expect(cachedSha).to.equal(oldSha); @@ -48,19 +48,19 @@ describe(function() { it('works', async function() { let oldSha = await git(['rev-parse', 'HEAD'], { - cwd: tmpPath, + cwd, cached: this.tmpPath, }); - await execa('git', ['commit', '-m', 'test', '--allow-empty'], { cwd: tmpPath }); + await execa('git', ['commit', '-m', 'test', '--allow-empty'], { cwd }); let cachedSha = await git(['rev-parse', 'HEAD'], { - cwd: tmpPath, + cwd, cached: this.tmpPath, }); let newSha = await execa('git', ['rev-parse', 'HEAD'], { - cwd: tmpPath, + cwd, }); expect(cachedSha).to.equal(oldSha); From ff0bab44067b5e4bcb35441f86870706c4aaa363 Mon Sep 17 00:00:00 2001 From: Kelly Selden <602423+kellyselden@users.noreply.github.com> Date: Fri, 19 Dec 2025 14:09:38 -0500 Subject: [PATCH 2/5] feat: expose getCacheKey used in tests to assert the cache file name --- src/git.js | 1 + test/git-test.js | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/git.js b/src/git.js index b335c22c..29379461 100644 --- a/src/git.js +++ b/src/git.js @@ -164,6 +164,7 @@ async function getCurrentCommit(cwd) { } module.exports = { + getCacheKey, git, getCurrentBranch, getWorkspaceCwd, diff --git a/test/git-test.js b/test/git-test.js index 0047802b..4a5f3504 100644 --- a/test/git-test.js +++ b/test/git-test.js @@ -2,7 +2,7 @@ const { describe, it, setUpTmpDir } = require('./helpers/mocha'); const { expect } = require('./helpers/chai'); -const { git } = require('../src/git'); +const { git, getCacheKey } = require('../src/git'); const execa = require('execa'); const { gitInit } = require('git-fixtures'); const path = require('path'); @@ -68,7 +68,7 @@ describe(function() { let [cachedFilePath] = await fs.promises.readdir(this.tmpPath); - expect(cachedFilePath).to.not.be.undefined; + expect(cachedFilePath).to.equal(getCacheKey(['rev-parse', 'HEAD'], cwd)); expect(path.join(this.tmpPath, cachedFilePath)).to.be.a.file().with.content(oldSha); }); }); From d4c8c1764bd895f19df8fc41b95fed984ec05a2f Mon Sep 17 00:00:00 2001 From: Kelly Selden <602423+kellyselden@users.noreply.github.com> Date: Fri, 19 Dec 2025 14:09:38 -0500 Subject: [PATCH 3/5] refactor: move debug log up --- src/git.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/git.js b/src/git.js index 29379461..529ace7e 100644 --- a/src/git.js +++ b/src/git.js @@ -74,6 +74,8 @@ async function git(args, options) { cwd, }); + debug(stdout); + if (cached) { cache[cacheKey] = stdout; @@ -82,8 +84,6 @@ async function git(args, options) { } } - debug(stdout); - return stdout; } finally { if (lockFilePath) { From 44b33932006aa9878b703641e49ab7b2b3cdb851 Mon Sep 17 00:00:00 2001 From: Kelly Selden <602423+kellyselden@users.noreply.github.com> Date: Fri, 19 Dec 2025 14:09:38 -0500 Subject: [PATCH 4/5] feat: log stderr too --- src/git.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/git.js b/src/git.js index 529ace7e..e3746744 100644 --- a/src/git.js +++ b/src/git.js @@ -70,11 +70,14 @@ async function git(args, options) { debug(args, options); - let { stdout } = await execa('git', args, { + let { + stdout, + all, + } = await execa('git', args, { cwd, }); - debug(stdout); + debug(all); if (cached) { cache[cacheKey] = stdout; From 95f22c509976e4fc137e1396d1e37330fdf4bd4e Mon Sep 17 00:00:00 2001 From: Kelly Selden <602423+kellyselden@users.noreply.github.com> Date: Fri, 19 Dec 2025 14:09:38 -0500 Subject: [PATCH 5/5] fix: handle git error codes Certain boolean commands like `git merge-base --is-ancestor` always throws and uses exit codes instead. We need to refactor this so that any throwing is also cacheable. Before, everything worked but the throwing wasn't caching and slowing things down. --- src/git.js | 51 +++++++++++----- test/git-test.js | 148 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 184 insertions(+), 15 deletions(-) diff --git a/src/git.js b/src/git.js index e3746744..c116f46e 100644 --- a/src/git.js +++ b/src/git.js @@ -20,8 +20,17 @@ async function git(args, options) { let { cwd, cached, + shouldUseExitCode, } = options; + /** + * Git is inverted. A zero exit code is true. + * @param {number} exitCode + */ + function toBool(exitCode) { + return exitCode === 0; + } + let cacheKey; let lockFilePath; @@ -59,9 +68,17 @@ async function git(args, options) { if (_cache !== null) { debug('Git cache hit.'); - cache[cacheKey] = _cache; + let normalized = (() => { + if (shouldUseExitCode) { + return _cache === 'true'; + } + + return _cache; + })(); - return _cache; + cache[cacheKey] = normalized; + + return normalized; } } @@ -73,21 +90,31 @@ async function git(args, options) { let { stdout, all, + exitCode, } = await execa('git', args, { cwd, + reject: !shouldUseExitCode, }); debug(all); + let result = (() => { + if (shouldUseExitCode) { + return toBool(exitCode); + } + + return stdout; + })(); + if (cached) { - cache[cacheKey] = stdout; + cache[cacheKey] = result; if (cached !== true) { - await ensureWriteFile(cachedFilePath, stdout); + await ensureWriteFile(cachedFilePath, result.toString()); } } - return stdout; + return result; } finally { if (lockFilePath) { await unlock(lockFilePath); @@ -120,16 +147,10 @@ function getLinesFromOutput(output) { } async function isCommitAncestorOf(ancestorCommit, descendantCommit, options) { - try { - await git(['merge-base', '--is-ancestor', ancestorCommit, descendantCommit], options); - } catch (err) { - let missingCommit = 128; - if (![1, missingCommit].includes(err.exitCode)) { - throw err; - } - return false; - } - return true; + return await git(['merge-base', '--is-ancestor', ancestorCommit, descendantCommit], { + shouldUseExitCode: true, + ...options, + }); } async function getCommonAncestor(commit1, commit2, options) { diff --git a/test/git-test.js b/test/git-test.js index 4a5f3504..fc52d7c6 100644 --- a/test/git-test.js +++ b/test/git-test.js @@ -40,6 +40,81 @@ describe(function() { expect(cachedSha).to.equal(oldSha); expect(newSha).to.not.equal(oldSha); }); + + describe('exit code commands', function () { + const shouldUseExitCode = true; + + it('true', async function() { + let oldSha = await git(['rev-parse', 'HEAD'], { + cwd, + }); + + await execa('git', ['commit', '-m', 'test', '--allow-empty'], { cwd }); + + let newSha = await execa('git', ['rev-parse', 'HEAD'], { + cwd, + }); + + let cachedArgs = ['merge-base', '--is-ancestor', oldSha, newSha.stdout]; + + let result = await git(cachedArgs, { + cwd, + cached: true, + shouldUseExitCode, + }); + + expect(result).to.equal(true); + + await execa('git', ['reset', '--hard', oldSha], { cwd }); + await execa('git', ['reflog', 'expire', '--expire=now', '--all'], { cwd }); + await execa('git', ['gc', '--prune=now'], { cwd }); + + result = await git(cachedArgs, { + cwd, + cached: true, + shouldUseExitCode, + }); + + expect(result).to.equal(true); + + result = await git(cachedArgs, { + cwd, + shouldUseExitCode, + }); + + expect(result).to.equal(false); + + }); + + it('false', async function() { + let cachedArgs = ['rev-parse', 'non-existent-branch']; + + let result = await git(cachedArgs, { + cwd, + cached: true, + shouldUseExitCode, + }); + + expect(result).to.equal(false); + + await execa('git', ['branch', 'non-existent-branch'], { cwd }); + + result = await git(cachedArgs, { + cwd, + cached: true, + shouldUseExitCode, + }); + + expect(result).to.equal(false); + + result = await git(cachedArgs, { + cwd, + shouldUseExitCode, + }); + + expect(result).to.equal(true); + }); + }); }); describe('on disk', function () { @@ -71,6 +146,79 @@ describe(function() { expect(cachedFilePath).to.equal(getCacheKey(['rev-parse', 'HEAD'], cwd)); expect(path.join(this.tmpPath, cachedFilePath)).to.be.a.file().with.content(oldSha); }); + + describe('exit code commands', function () { + const shouldUseExitCode = true; + + it('true', async function() { + let oldSha = await git(['rev-parse', 'HEAD'], { + cwd, + }); + + await execa('git', ['commit', '-m', 'test', '--allow-empty'], { cwd }); + + let newSha = await execa('git', ['rev-parse', 'HEAD'], { + cwd, + }); + + let args = ['merge-base', '--is-ancestor', oldSha, newSha.stdout]; + + await git(args, { + cwd, + cached: this.tmpPath, + shouldUseExitCode, + }); + + let [cachedFilePath] = await fs.promises.readdir(this.tmpPath); + + expect(cachedFilePath).to.equal(getCacheKey(args, cwd)); + expect(path.join(this.tmpPath, cachedFilePath)).to.be.a.file().with.content('true'); + }); + + it('false', async function() { + let oldSha = await git(['rev-parse', 'HEAD'], { + cwd, + }); + + await execa('git', ['commit', '-m', 'test', '--allow-empty'], { cwd }); + + let newSha = await execa('git', ['rev-parse', 'HEAD'], { + cwd, + }); + + let args = ['merge-base', '--is-ancestor', newSha.stdout, oldSha]; + + await git(args, { + cwd, + cached: this.tmpPath, + shouldUseExitCode, + }); + + let [cachedFilePath] = await fs.promises.readdir(this.tmpPath); + + expect(cachedFilePath).to.equal(getCacheKey(args, cwd)); + expect(path.join(this.tmpPath, cachedFilePath)).to.be.a.file().with.content('false'); + }); + + it('error', async function() { + let sha = await git(['rev-parse', 'HEAD'], { + cwd, + }); + + let args = ['merge-base', '--is-ancestor', 'missing-commit', sha]; + + await git(args, { + cwd, + cached: this.tmpPath, + shouldUseExitCode, + }); + + let [cachedFilePath] = await fs.promises.readdir(this.tmpPath); + + expect(cachedFilePath).to.equal(getCacheKey(args, cwd)); + expect(path.join(this.tmpPath, cachedFilePath)).to.be.a.file().with.content('false'); + }); + }); }); }); });