Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 7 additions & 30 deletions apps/desktop/electron/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ import {
MIN_WIDTH as WINDOW_MIN_WIDTH
} from './window-state'
import { hiddenWindowsChildOptions } from './windows-child-options'
import { resolveGitBinary as resolveGitBinaryImpl } from './windows-git-binary'
import {
buildPathExtCandidates,
chooseUpdaterArgs,
Expand Down Expand Up @@ -1831,37 +1832,13 @@ function makeDashboardReadyFile() {
// PATH), so a bare spawn('git') ENOENTs and self-update checks fail with
// "Couldn't check for updates". Mirror findGitBash: PortableGit first, then
// standard Git-for-Windows locations, then PATH. Cached after first probe.
let _gitBinaryCache = null

function resolveGitBinary() {
if (_gitBinaryCache) {
return _gitBinaryCache
}

if (!IS_WINDOWS) {
_gitBinaryCache = findOnPath('git') || 'git'

return _gitBinaryCache
}

const localAppData = process.env.LOCALAPPDATA || ''
const candidates = []

if (localAppData) {
candidates.push(path.join(localAppData, 'hermes', 'git', 'cmd', 'git.exe'))
candidates.push(path.join(localAppData, 'hermes', 'git', 'bin', 'git.exe'))
}

candidates.push(path.join(process.env['ProgramFiles'] || 'C:\\Program Files', 'Git', 'cmd', 'git.exe'))
candidates.push(path.join(process.env['ProgramFiles(x86)'] || 'C:\\Program Files (x86)', 'Git', 'cmd', 'git.exe'))

if (localAppData) {
candidates.push(path.join(localAppData, 'Programs', 'Git', 'cmd', 'git.exe'))
}

_gitBinaryCache = candidates.find(fileExists) || findOnPath('git') || 'git'

return _gitBinaryCache
return resolveGitBinaryImpl({
isWindows: IS_WINDOWS,
fileExists,
findOnPath,
env: process.env
})
}

// resolveGhBinary β€” locate the GitHub CLI. GUI-launched apps get a minimal PATH
Expand Down
111 changes: 111 additions & 0 deletions apps/desktop/electron/windows-git-binary.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// Real-behavior tests for the Windows git binary resolver.
//
// These exercise the actual resolveGitBinary() logic by injecting fake
// filesystem and PATH probes, so they run on any OS.

import assert from 'node:assert/strict'
import path from 'node:path'

import { beforeEach, test } from 'vitest'

import { resetGitBinaryCache, resolveGitBinary } from './windows-git-binary'

beforeEach(() => {
resetGitBinaryCache()
})

function makeDeps(overrides: Partial<Parameters<typeof resolveGitBinary>[0]> = {}) {
return {
isWindows: false,
fileExists: () => false,
findOnPath: () => null,
toShortPath: (p: string) => p,
env: {},
...overrides
}
}

test('non-Windows: returns the PATH result', () => {
const deps = makeDeps({
isWindows: false,
findOnPath: (cmd: string) => (cmd === 'git' ? '/usr/local/bin/git' : null)
})

assert.equal(resolveGitBinary(deps), '/usr/local/bin/git')
})

test('non-Windows: falls back to the bare git command', () => {
const deps = makeDeps({ isWindows: false })

assert.equal(resolveGitBinary(deps), 'git')
})

test('Windows: returns the short path when a candidate exists and short-name conversion succeeds', () => {
const env = { ProgramFiles: 'C:\\Program Files' }
const longPath = path.win32.join(env.ProgramFiles, 'Git', 'cmd', 'git.exe')
const shortPath = path.win32.join('C:\\PROGRA~1', 'Git', 'cmd', 'git.exe')

const deps = makeDeps({
isWindows: true,
env,
fileExists: (p: string) => p === longPath,
toShortPath: (p: string) => (p === longPath ? shortPath : p)
})

assert.equal(resolveGitBinary(deps), shortPath)
})

test('Windows: keeps the long path when 8.3 short-name conversion returns the same path', () => {
const env = { ProgramFiles: 'C:\\Program Files' }
const longPath = path.win32.join(env.ProgramFiles, 'Git', 'cmd', 'git.exe')

const deps = makeDeps({
isWindows: true,
env,
fileExists: (p: string) => p === longPath,
toShortPath: (p: string) => p
})

assert.equal(resolveGitBinary(deps), longPath)
})

test('Windows: falls back to PATH when no candidate exists', () => {
const pathGit = path.win32.join('C:\\Users', 'Dev', 'bin', 'git.exe')
const shortGit = path.win32.join('C:\\Users', 'DEV', 'bin', 'git.exe')

const deps = makeDeps({
isWindows: true,
fileExists: () => false,
findOnPath: (cmd: string) => (cmd === 'git' ? pathGit : null),
toShortPath: (p: string) => (p === pathGit ? shortGit : p)
})

assert.equal(resolveGitBinary(deps), shortGit)
})

test('Windows: falls back to bare git command when nothing is found', () => {
const deps = makeDeps({
isWindows: true,
fileExists: () => false,
findOnPath: () => null
})

assert.equal(resolveGitBinary(deps), 'git')
})

test('caches the resolved binary so subsequent calls do not re-probe', () => {
let probes = 0

const deps = makeDeps({
isWindows: false,
findOnPath: (cmd: string) => {
probes++

return cmd === 'git' ? '/usr/bin/git' : null
}
})

assert.equal(resolveGitBinary(deps), '/usr/bin/git')
assert.equal(resolveGitBinary(deps), '/usr/bin/git')
assert.equal(probes, 1)
})
77 changes: 77 additions & 0 deletions apps/desktop/electron/windows-git-binary.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/**
* windows-git-binary.ts
*
* Resolve the git binary used by the Desktop review pane, with a Windows-specific
* short-path conversion so simple-git's customBinaryPlugin accepts the path.
*/

import path from 'node:path'

import { toShortPath as defaultToShortPath } from './windows-short-path'

export interface ResolveGitBinaryDeps {
isWindows: boolean
fileExists: (filePath: string) => boolean
findOnPath: (command: string) => string | null
toShortPath?: (filePath: string) => string
env?: {
LOCALAPPDATA?: string | undefined
ProgramFiles?: string | undefined
'ProgramFiles(x86)'?: string | undefined
}
}

let _gitBinaryCache: string | null = null

export function resetGitBinaryCache(): void {
_gitBinaryCache = null
}

/**
* Locate git.exe. On Windows, convert an absolute resolved path to its 8.3
* short form before caching so simple-git's argument validation accepts it.
*/
export function resolveGitBinary(deps: ResolveGitBinaryDeps): string {
if (_gitBinaryCache !== null) {
return _gitBinaryCache
}

const {
isWindows,
fileExists,
findOnPath,
toShortPath = defaultToShortPath,
env = process.env
} = deps

if (!isWindows) {
_gitBinaryCache = findOnPath('git') || 'git'

return _gitBinaryCache
}

const localAppData = env.LOCALAPPDATA || ''
const candidates: string[] = []

if (localAppData) {
candidates.push(path.win32.join(localAppData, 'hermes', 'git', 'cmd', 'git.exe'))
candidates.push(path.win32.join(localAppData, 'hermes', 'git', 'bin', 'git.exe'))
}

candidates.push(
path.win32.join(env.ProgramFiles || 'C:\\Program Files', 'Git', 'cmd', 'git.exe'),
path.win32.join(env['ProgramFiles(x86)'] || 'C:\\Program Files (x86)', 'Git', 'cmd', 'git.exe')
)

if (localAppData) {
candidates.push(path.win32.join(localAppData, 'Programs', 'Git', 'cmd', 'git.exe'))
}

_gitBinaryCache = candidates.find(fileExists) || findOnPath('git') || 'git'

if (_gitBinaryCache !== 'git' && path.win32.isAbsolute(_gitBinaryCache)) {
_gitBinaryCache = toShortPath(_gitBinaryCache)
}

return _gitBinaryCache
}
98 changes: 98 additions & 0 deletions apps/desktop/electron/windows-short-path.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// Real-behavior tests for the Windows 8.3 short-path helper.
//
// These exercise the exported toShortPath() function by stubbing
// node:child_process, so they run on any OS and verify the actual logic
// rather than checking source text.

import assert from 'node:assert/strict'
import type { execFileSync } from 'node:child_process'

import { beforeEach, test } from 'vitest'

import { toShortPath } from './windows-short-path'

let stubExecFileSync: typeof execFileSync
let captured: { file: string; args: string[]; options: Record<string, unknown> } | null

beforeEach(() => {
stubExecFileSync = ((() => {
throw new Error('unexpected execFileSync call')
}) as unknown) as typeof execFileSync
captured = null
})

function withExecFileSync(value: string | Error, fn: () => void) {
stubExecFileSync = ((...args: Parameters<typeof execFileSync>) => {
captured = { file: args[0], args: args[1] as string[], options: (args[2] as Record<string, unknown>) ?? {} }

if (value instanceof Error) {throw value}

return value
}) as typeof execFileSync

fn()
}

test('toShortPath returns the short path when cmd expansion succeeds', () => {
const original = 'C:\\Program Files\\Git\\cmd\\git.exe'
const short = 'C:\\PROGRA~1\\Git\\cmd\\git.exe'

let result = original

withExecFileSync(short, () => {
result = toShortPath(original, { execFileSync: stubExecFileSync })
})

assert.equal(result, short)
})

test('toShortPath falls back to the original path when cmd expansion throws', () => {
const original = 'C:\\Program Files\\Git\\cmd\\git.exe'

let result = ''

withExecFileSync(new Error('cmd not found'), () => {
result = toShortPath(original, { execFileSync: stubExecFileSync })
})

assert.equal(result, original)
})

test('toShortPath falls back to the original path when expansion returns the same path', () => {
const original = 'C:\\some\\path\\git.exe'

let result = ''

withExecFileSync(original, () => {
result = toShortPath(original, { execFileSync: stubExecFileSync })
})

assert.equal(result, original)
})

test('toShortPath falls back to the original path when expansion returns empty', () => {
const original = 'C:\\Program Files\\Git\\cmd\\git.exe'

let result = ''

withExecFileSync('', () => {
result = toShortPath(original, { execFileSync: stubExecFileSync })
})

assert.equal(result, original)
})

test('toShortPath invokes cmd.exe with the correct for-variable short-path expansion', () => {
const original = 'C:\\Program Files\\Git\\cmd\\git.exe'
const short = 'C:\\PROGRA~1\\Git\\cmd\\git.exe'

withExecFileSync(short, () => {
toShortPath(original, { execFileSync: stubExecFileSync })
})

assert.equal(captured?.file, 'cmd.exe')
assert.deepEqual(captured?.args, ['/c', `for %A in ("${original}") do @echo %~sA`])
assert.equal(captured?.options.timeout, 5000)
assert.equal(captured?.options.windowsHide, true)
assert.equal(captured?.options.encoding, 'utf8')
})
45 changes: 45 additions & 0 deletions apps/desktop/electron/windows-short-path.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* windows-short-path.ts
*
* Convert a Windows path to its 8.3 short-path form.
*
* simple-git's customBinaryPlugin validates the binary path with a regex that
* does not allow spaces. Git-for-Windows is installed under "C:\Program Files"
* by default, so the resolved absolute path must be converted to its short form
* before being passed to simple-git.
*/

import { execFileSync } from 'node:child_process'

export interface ToShortPathDeps {
execFileSync?: typeof execFileSync
}

/**
* Convert a Windows path to its 8.3 short-path form.
*
* Uses cmd.exe's for-variable expansion (`%~sA`) so the result contains no
* spaces and passes simple-git's isBadArgument regex. Falls back to the
* original path if the lookup fails or returns the same path.
*/
export function toShortPath(filePath: string, deps: ToShortPathDeps = {}): string {
const run = deps.execFileSync ?? execFileSync

try {
const result = String(
run(
'cmd.exe',
['/c', `for %A in ("${filePath}") do @echo %~sA`],
{ timeout: 5000, windowsHide: true, encoding: 'utf8' }
)
).trim()

if (result && result !== filePath) {
return result
}
} catch {
// fall through to the original path
}

return filePath
}