Avoid optional Git locks during status checks (#2330)
* Squashed commits - WIP: uncommitted changes before rebase - ci - Show inline PR check details in task drawer - Add a Checks tab that opens from the PR checks cell and expands runs inline - Fetch check output, annotations, and workflow job steps through IPC/RPC - Improve markdown/comment wrapping so long PR content stays within the drawer * Use app-styled confirmations for PR actions (#2324) Co-authored-by: Orca <help@stably.ai> * fix: pr-bug-scan validated finding from #2274 (#2296) Co-authored-by: orca-bug-scan-bot <orca-bug-scan-bot@stably.ai> * fix: avoid optional git locks during status checks --------- Co-authored-by: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Co-authored-by: Orca <help@stably.ai> Co-authored-by: buf0-bot[bot] <252831055+buf0-bot[bot]@users.noreply.github.com> Co-authored-by: orca-bug-scan-bot <orca-bug-scan-bot@stably.ai>
This commit is contained in:
parent
63b11f11a4
commit
5b1d84232c
|
|
@ -212,6 +212,15 @@ type GitExecOptions = {
|
|||
env?: NodeJS.ProcessEnv
|
||||
}
|
||||
|
||||
export function gitOptionalLocksDisabledEnv(
|
||||
env: NodeJS.ProcessEnv = process.env
|
||||
): NodeJS.ProcessEnv {
|
||||
return {
|
||||
...env,
|
||||
GIT_OPTIONAL_LOCKS: '0'
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Async git command execution. Drop-in replacement for
|
||||
* `execFileAsync('git', args, { cwd, encoding, ... })`.
|
||||
|
|
|
|||
|
|
@ -13,7 +13,11 @@ const { gitExecFileAsyncMock, gitExecFileAsyncBufferMock, readFileMock, rmMock,
|
|||
|
||||
vi.mock('./runner', () => ({
|
||||
gitExecFileAsync: gitExecFileAsyncMock,
|
||||
gitExecFileAsyncBuffer: gitExecFileAsyncBufferMock
|
||||
gitExecFileAsyncBuffer: gitExecFileAsyncBufferMock,
|
||||
gitOptionalLocksDisabledEnv: (env: NodeJS.ProcessEnv = process.env) => ({
|
||||
...env,
|
||||
GIT_OPTIONAL_LOCKS: '0'
|
||||
})
|
||||
}))
|
||||
|
||||
vi.mock('fs/promises', () => ({
|
||||
|
|
@ -363,7 +367,7 @@ describe('getStatus', () => {
|
|||
'--branch',
|
||||
'--untracked-files=all'
|
||||
],
|
||||
{ cwd: '/repo' }
|
||||
{ cwd: '/repo', env: expect.objectContaining({ GIT_OPTIONAL_LOCKS: '0' }) }
|
||||
)
|
||||
expect(result.entries).toEqual([
|
||||
{ path: 'docs/日本語/sample.md', status: 'modified', area: 'unstaged' }
|
||||
|
|
@ -389,7 +393,7 @@ describe('getStatus', () => {
|
|||
'--untracked-files=all',
|
||||
'--ignored=matching'
|
||||
],
|
||||
{ cwd: '/repo' }
|
||||
{ cwd: '/repo', env: expect.objectContaining({ GIT_OPTIONAL_LOCKS: '0' }) }
|
||||
)
|
||||
expect(result.ignoredPaths).toEqual(['dist/', 'generated/file.js'])
|
||||
})
|
||||
|
|
@ -458,7 +462,7 @@ describe('getStatus', () => {
|
|||
'--branch',
|
||||
'--untracked-files=all'
|
||||
],
|
||||
{ cwd: '/repo' }
|
||||
{ cwd: '/repo', env: expect.objectContaining({ GIT_OPTIONAL_LOCKS: '0' }) }
|
||||
)
|
||||
expect('ignoredPaths' in result).toBe(false)
|
||||
})
|
||||
|
|
@ -482,7 +486,7 @@ describe('getStatus', () => {
|
|||
'--untracked-files=all',
|
||||
'--ignored=matching'
|
||||
],
|
||||
{ cwd: '/repo' }
|
||||
{ cwd: '/repo', env: expect.objectContaining({ GIT_OPTIONAL_LOCKS: '0' }) }
|
||||
)
|
||||
expect(result.ignoredPaths).toEqual(['dist/', '.env', 'coverage/'])
|
||||
expect(result.entries).toEqual([])
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ import type {
|
|||
GitStatusResult
|
||||
} from '../../shared/types'
|
||||
import type { CommitMessageDraftContext } from '../../shared/commit-message-generation'
|
||||
import { gitExecFileAsync, gitExecFileAsyncBuffer } from './runner'
|
||||
import { gitExecFileAsync, gitExecFileAsyncBuffer, gitOptionalLocksDisabledEnv } from './runner'
|
||||
|
||||
const MAX_GIT_SHOW_BYTES = 10 * 1024 * 1024
|
||||
const MAX_STAGED_COMMIT_CONTEXT_BYTES = MAX_GIT_SHOW_BYTES
|
||||
|
|
@ -59,7 +59,12 @@ export async function getStatus(
|
|||
if (options.includeIgnored) {
|
||||
statusArgs.push('--ignored=matching')
|
||||
}
|
||||
const statusPromise = gitExecFileAsync(statusArgs, { cwd: worktreePath })
|
||||
const statusPromise = gitExecFileAsync(statusArgs, {
|
||||
cwd: worktreePath,
|
||||
// Why: status polling is read-like; avoid refreshing the index and racing
|
||||
// terminal Git commands on `.git/worktrees/*/index.lock`.
|
||||
env: gitOptionalLocksDisabledEnv()
|
||||
})
|
||||
const conflictOperation = await conflictPromise
|
||||
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import { getBitbucketRepoSlug } from '../bitbucket/client'
|
|||
import { getGiteaRepoSlug } from '../gitea/client'
|
||||
import { createGitHubPullRequest, getRepoSlug } from '../github/client'
|
||||
import { acquire, ghExecFileAsync, gitExecFileAsync, release } from '../github/gh-utils'
|
||||
import { gitOptionalLocksDisabledEnv } from '../git/runner'
|
||||
import { resolveDefaultBaseRefViaExec } from '../git/repo'
|
||||
import { getUpstreamStatus } from '../git/upstream'
|
||||
import { getProjectSlug } from '../gitlab/client'
|
||||
|
|
@ -104,7 +105,12 @@ async function getCurrentBranch(repoPath: string): Promise<string> {
|
|||
}
|
||||
|
||||
async function hasUncommittedChanges(repoPath: string): Promise<boolean> {
|
||||
const { stdout } = await gitExecFileAsync(['status', '--porcelain'], { cwd: repoPath })
|
||||
const { stdout } = await gitExecFileAsync(['status', '--porcelain'], {
|
||||
cwd: repoPath,
|
||||
// Why: create-PR validation should not take Git's optional index lock while
|
||||
// the user may be running fetch/pull/rebase from a terminal.
|
||||
env: gitOptionalLocksDisabledEnv()
|
||||
})
|
||||
return stdout.trim().length > 0
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -11,7 +11,11 @@ import { bufferToBlob, buildDiffResult, parseBranchDiff } from './git-handler-ut
|
|||
|
||||
// ─── Executor types ──────────────────────────────────────────────────
|
||||
|
||||
export type GitExec = (args: string[], cwd: string) => Promise<{ stdout: string; stderr: string }>
|
||||
export type GitExec = (
|
||||
args: string[],
|
||||
cwd: string,
|
||||
opts?: { maxBuffer?: number; disableOptionalLocks?: boolean }
|
||||
) => Promise<{ stdout: string; stderr: string }>
|
||||
|
||||
export type GitBufferExec = (args: string[], cwd: string) => Promise<Buffer>
|
||||
|
||||
|
|
|
|||
|
|
@ -94,7 +94,11 @@ export async function getStatusOp(
|
|||
if (includeIgnored) {
|
||||
statusArgs.push('--ignored=matching')
|
||||
}
|
||||
const { stdout } = await git(statusArgs, worktreePath)
|
||||
const { stdout } = await git(statusArgs, worktreePath, {
|
||||
// Why: status polling is read-like; avoid refreshing the index and racing
|
||||
// terminal Git commands on `.git/worktrees/*/index.lock`.
|
||||
disableOptionalLocks: true
|
||||
})
|
||||
const parsed = parseStatusOutput(stdout)
|
||||
entries.push(...parsed.entries)
|
||||
head = parsed.head
|
||||
|
|
|
|||
|
|
@ -67,11 +67,15 @@ export class GitHandler {
|
|||
private async git(
|
||||
args: string[],
|
||||
cwd: string,
|
||||
opts?: { maxBuffer?: number }
|
||||
opts?: { maxBuffer?: number; disableOptionalLocks?: boolean }
|
||||
): Promise<{ stdout: string; stderr: string }> {
|
||||
const env = buildRelayCommandEnv()
|
||||
if (opts?.disableOptionalLocks) {
|
||||
env.GIT_OPTIONAL_LOCKS = '0'
|
||||
}
|
||||
return execFileAsync('git', args, {
|
||||
cwd: expandTilde(cwd),
|
||||
env: buildRelayCommandEnv(),
|
||||
env,
|
||||
encoding: 'utf-8',
|
||||
maxBuffer: opts?.maxBuffer ?? MAX_GIT_BUFFER
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue