Repository navigation
fix(git): keep merge-tree's throwaway trees out of the real object store #22356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+1,444
−26
Merged
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
67a866a
fix(git): keep merge-tree's throwaway trees out of the real object store
brennanb2025 70e453e
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 3395892
fix(git): keep partial-clone downloads and clean up stranded scratch …
brennanb2025 500da40
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 610627a
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 58a4bbd
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 0f3ed37
fix(git): quote Windows alternates paths containing ';' and log scrat…
brennanb2025 fec8ad8
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 ed3be7d
fix(git): sweep a merge-tree scratch dir only when its owner is prova…
brennanb2025 d3c2df6
fix(git): cap every merge-tree scratch dir at two weeks, live-looking…
brennanb2025 558f13e
fix(git): sweep merge-tree scratch dirs by age alone, off the check's…
brennanb2025 dbedad1
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 b531686
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 12b19c7
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 3da84cf
fix(git): let quarantined merge-tree see exactly the object stores Gi…
brennanb2025 710b17d
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 3a96c33
fix(git): run merge-tree unquarantined when the repo's store has its …
brennanb2025 3aea8a6
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 14c3c50
refactor(git): drop the stale-folder sweep and pack-move rollback fro…
brennanb2025 2fae54e
fix(git): quarantine merge-tree only once the host is known to suppor…
brennanb2025 5bc549e
fix(git): learn merge-tree support from a conflicting branch-cleanup …
brennanb2025 1a0e383
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 b3ae921
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 f7d0cc0
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 92b4030
Merge remote-tracking branch 'origin/main' into brennanb2025/merge-tr…
brennanb2025 eb5e450
fix(git): put WSL Git's scratch object store on its command line
brennanb2025 240e830
test(git): drop quarantine tests the real-Git and compatibility tests…
brennanb2025 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
44 changes: 44 additions & 0 deletions
44
src/main/git/command-runner/wsl-command-resolution-object-store.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import { runProcess } from '@orca/process-host' | ||
| import { afterEach, beforeEach, describe, expect, it } from 'vitest' | ||
| import { resolveCommand } from './wsl-command-resolution' | ||
|
|
||
| const originalPlatform = process.platform | ||
| const QUARANTINE = { | ||
| GIT_OBJECT_DIRECTORY: '/home/me/repo/.git/objects/tmp_objdir-orca-merge-tree-x', | ||
| GIT_ALTERNATE_OBJECT_DIRECTORIES: '/home/me/repo/.git/objects' | ||
| } | ||
|
|
||
| /** What WSL Git would see when a login profile has set `profileAlternates` before the payload runs. */ | ||
| async function alternatesSeenByWslGit(profileAlternates: string | undefined): Promise<string> { | ||
| const resolved = resolveCommand('git', [], undefined, 'Ubuntu', { | ||
| env: { ...process.env, ...QUARANTINE }, | ||
| wslShellCommand: 'printenv GIT_ALTERNATE_OBJECT_DIRECTORIES' | ||
| }) | ||
| const payload = resolved.args.at(-1) ?? '' | ||
| const profile = | ||
| profileAlternates === undefined | ||
| ? '' | ||
| : `export GIT_ALTERNATE_OBJECT_DIRECTORIES='${profileAlternates}'; ` | ||
| const result = await runProcess({ program: 'sh', args: ['-c', `${profile}${payload}`] }) | ||
| expect(result.code).toBe(0) | ||
| return result.stdout.trim() | ||
| } | ||
|
|
||
| // Why pin this: a WSL login profile that sets GIT_ALTERNATE_OBJECT_DIRECTORIES used to replace a | ||
| // WSLENV-forwarded value, so a quarantined merge-tree lost the real object store and failed. | ||
| describe.skipIf(originalPlatform === 'win32')('WSL Git object-store variables', () => { | ||
| beforeEach(() => { | ||
| Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) | ||
| }) | ||
|
|
||
| it('reach WSL Git after the login profile, appending to its alternates', async () => { | ||
| await expect(alternatesSeenByWslGit(undefined)).resolves.toBe('/home/me/repo/.git/objects') | ||
| await expect(alternatesSeenByWslGit('/shared/objects')).resolves.toBe( | ||
| '/shared/objects:/home/me/repo/.git/objects' | ||
| ) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' | ||
| import type * as GitObjectQuarantineModule from '../../shared/git-object-quarantine' | ||
| import type { GitObjectQuarantine, GitObjectsDirectory } from '../../shared/git-object-quarantine' | ||
| import { | ||
| createLocalGitObjectQuarantine, | ||
| inheritedLocalObjectStore, | ||
| localGitObjectsDirectory | ||
| } from './local-git-object-quarantine' | ||
| import { | ||
| resetWslLinkedWorktreeGitRoutingForTests, | ||
| seedWslLinkedWorktreeGitRoutingForTests | ||
| } from './wsl-linked-worktree-git-routing' | ||
|
|
||
| const { readRepoCommonDirFromGitMock } = vi.hoisted(() => ({ | ||
| readRepoCommonDirFromGitMock: vi.fn() | ||
| })) | ||
|
|
||
| vi.mock('./worktree-list-reader', () => ({ | ||
| readRepoCommonDirFromGit: readRepoCommonDirFromGitMock | ||
| })) | ||
| vi.mock('../../shared/git-object-quarantine', async (importOriginal) => ({ | ||
| ...(await importOriginal<typeof GitObjectQuarantineModule>()), | ||
| // Why: hands the resolved objects dir straight to the command; these tests pin routing, not disk. | ||
| createGitObjectQuarantine: ( | ||
| resolve: () => Promise<GitObjectsDirectory | undefined> | ||
| ): GitObjectQuarantine => ({ | ||
| run: async (command) => { | ||
| const objects = await resolve() | ||
| return command( | ||
| objects | ||
| ? { | ||
| GIT_OBJECT_DIRECTORY: `${objects.gitPath}/scratch`, | ||
| GIT_ALTERNATE_OBJECT_DIRECTORIES: objects.gitPath | ||
| } | ||
| : undefined | ||
| ) | ||
| } | ||
| }) | ||
| })) | ||
|
|
||
| describe('localGitObjectsDirectory', () => { | ||
| it('translates a WSL Git common dir into a path the Windows host can open', () => { | ||
| expect(localGitObjectsDirectory('/home/me/repo/.git', 'Ubuntu')).toEqual({ | ||
| hostPath: '\\\\wsl.localhost\\Ubuntu\\home\\me\\repo\\.git\\objects', | ||
| gitPath: '/home/me/repo/.git/objects' | ||
| }) | ||
| }) | ||
|
|
||
| it('hands WSL Git the drvfs spelling of a drive-path common dir (Git < 2.31 prints it relative)', () => { | ||
| expect(localGitObjectsDirectory('C:\\repo\\.git', 'Ubuntu')).toEqual({ | ||
| hostPath: 'C:\\repo\\.git\\objects', | ||
| gitPath: '/mnt/c/repo/.git/objects' | ||
| }) | ||
| expect(localGitObjectsDirectory('/mnt/c/repo/.git', 'Ubuntu')).toEqual({ | ||
| hostPath: 'C:\\repo\\.git\\objects', | ||
| gitPath: '/mnt/c/repo/.git/objects' | ||
| }) | ||
| }) | ||
|
|
||
| it('uses one spelling for native Git', () => { | ||
| expect(localGitObjectsDirectory('/Users/me/repo/.git', undefined)).toEqual({ | ||
| hostPath: '/Users/me/repo/.git/objects', | ||
| gitPath: '/Users/me/repo/.git/objects' | ||
| }) | ||
| expect(localGitObjectsDirectory('C:\\repo\\.git', undefined)).toEqual({ | ||
| hostPath: 'C:\\repo\\.git\\objects', | ||
| gitPath: 'C:\\repo\\.git\\objects' | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| describe('createLocalGitObjectQuarantine on a Windows host', () => { | ||
| const originalPlatform = process.platform | ||
|
|
||
| beforeEach(() => { | ||
| Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) | ||
| resetWslLinkedWorktreeGitRoutingForTests() | ||
| readRepoCommonDirFromGitMock.mockReset() | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) | ||
| resetWslLinkedWorktreeGitRoutingForTests() | ||
| }) | ||
|
|
||
| const envSeenBy = (repoPath: string, options: { wslDistro?: string } = {}) => | ||
| createLocalGitObjectQuarantine(repoPath, options).run(async (env) => env) | ||
|
|
||
| it('runs unquarantined when a WSL-authored linked worktree routes to Windows Git', async () => { | ||
| readRepoCommonDirFromGitMock.mockResolvedValue('/mnt/c/repo/.git') | ||
| seedWslLinkedWorktreeGitRoutingForTests('C:\\wt') | ||
|
|
||
| await expect(envSeenBy('C:\\wt', { wslDistro: 'Ubuntu' })).resolves.toBeUndefined() | ||
| }) | ||
| }) | ||
|
|
||
| describe('inheritedLocalObjectStore', () => { | ||
| const ALTERNATES = { GIT_ALTERNATE_OBJECT_DIRECTORIES: '/shared/objects' } | ||
|
|
||
| it('passes native Git’s inherited alternates on and refuses an inherited object dir', () => { | ||
| expect(inheritedLocalObjectStore(undefined, ALTERNATES, 'darwin')).toEqual({ | ||
| inheritedAlternates: '/shared/objects' | ||
| }) | ||
| expect( | ||
| inheritedLocalObjectStore(undefined, { GIT_OBJECT_DIRECTORY: 'C:\\objects' }, 'win32') | ||
| ).toBeUndefined() | ||
| }) | ||
|
|
||
| it('ignores Windows values WSLENV does not forward, since WSL Git never saw them', () => { | ||
| expect(inheritedLocalObjectStore('Ubuntu', ALTERNATES, 'win32')).toEqual({}) | ||
| expect( | ||
| inheritedLocalObjectStore( | ||
| 'Ubuntu', | ||
| { WSLENV: 'GIT_OBJECT_DIRECTORY/p:GIT_ALTERNATE_OBJECT_DIRECTORIES' }, | ||
| 'win32' | ||
| ) | ||
| ).toEqual({}) | ||
| expect( | ||
| inheritedLocalObjectStore('Ubuntu', { GIT_OBJECT_DIRECTORY: 'C:\\objects' }, 'win32') | ||
| ).toEqual({}) | ||
| }) | ||
|
|
||
| it.each(['GIT_ALTERNATE_OBJECT_DIRECTORIES', 'GIT_OBJECT_DIRECTORY/p'])( | ||
| 'runs unquarantined when WSLENV forwards %s to WSL Git', | ||
| (token) => { | ||
| const env = { | ||
| GIT_ALTERNATE_OBJECT_DIRECTORIES: 'C:\\shared', | ||
| GIT_OBJECT_DIRECTORY: 'C:\\objects', | ||
| WSLENV: `ORCA_X/u:${token}` | ||
| } | ||
|
|
||
| expect(inheritedLocalObjectStore('Ubuntu', env, 'win32')).toBeUndefined() | ||
| } | ||
| ) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| import { isWindowsAbsolutePathLike } from '../../shared/cross-platform-path' | ||
| import type { GitAdmissionTier } from '../../shared/rpc-contract/git-admission-tier-params' | ||
| import { | ||
| createGitObjectQuarantine, | ||
| inheritedObjectStore, | ||
| pathApiForGitPath, | ||
| type GitObjectsDirectory | ||
| } from '../../shared/git-object-quarantine' | ||
| import { toLinuxPath, toWindowsWslPath } from '../../shared/wsl-paths' | ||
| import { parseWslPath } from '../wsl' | ||
| import { usesHostGitForWslLinkedWorktree } from './wsl-linked-worktree-git-routing' | ||
| import { readRepoCommonDirFromGit } from './worktree-list-reader' | ||
|
|
||
| export type LocalGitObjectQuarantine = { | ||
| /** `env` is a full process env for `gitExecFileAsync`, or undefined to run unquarantined. */ | ||
| run: <T>(command: (env: NodeJS.ProcessEnv | undefined) => Promise<T>) => Promise<T> | ||
| } | ||
|
|
||
| /** `objects/` in Git's spelling and, for WSL Git, translated to one the main process can open. */ | ||
| export function localGitObjectsDirectory( | ||
| commonDir: string, | ||
| wslDistro: string | undefined | ||
| ): GitObjectsDirectory { | ||
| const hostCommonDir = | ||
| wslDistro && !isWindowsAbsolutePathLike(commonDir) | ||
| ? toWindowsWslPath(commonDir, wslDistro) | ||
| : commonDir | ||
| // Why: Git < 2.31 prints a relative common dir, resolved against a `C:\` repo path; WSL Git needs `/mnt/c`. | ||
| const gitCommonDir = wslDistro ? toLinuxPath(commonDir) : commonDir | ||
| return { | ||
| hostPath: pathApiForGitPath(hostCommonDir).join(hostCommonDir, 'objects'), | ||
| gitPath: pathApiForGitPath(gitCommonDir).join(gitCommonDir, 'objects') | ||
| } | ||
| } | ||
|
|
||
| const OBJECT_STORE_VARIABLES = ['GIT_OBJECT_DIRECTORY', 'GIT_ALTERNATE_OBJECT_DIRECTORIES'] | ||
|
|
||
| /** The object-store variables local Git would inherit, or undefined when they cannot be mirrored. */ | ||
| export function inheritedLocalObjectStore( | ||
| wslDistro: string | undefined, | ||
| env: NodeJS.ProcessEnv = process.env, | ||
| platform: NodeJS.Platform = process.platform | ||
| ): Pick<GitObjectsDirectory, 'inheritedAlternates'> | undefined { | ||
| if (platform !== 'win32' || !wslDistro) { | ||
| return inheritedObjectStore(env) | ||
| } | ||
| // Why: wsl.exe forwards only what WSLENV names, maybe path-translated (`/p`); skip rather than guess. | ||
| const forwarded = new Set((env.WSLENV ?? '').split(':').map((token) => token.split('/')[0])) | ||
| return OBJECT_STORE_VARIABLES.some((key) => forwarded.has(key) && env[key] !== undefined) | ||
| ? undefined | ||
| : {} | ||
| } | ||
|
|
||
| /** Scratch object store for throwaway Git writes on the local (native or WSL) host. */ | ||
| export function createLocalGitObjectQuarantine( | ||
| repoPath: string, | ||
| options: { wslDistro?: string; signal?: AbortSignal; admissionTier?: GitAdmissionTier } = {} | ||
| ): LocalGitObjectQuarantine { | ||
| const wslDistro = parseWslPath(repoPath)?.distro ?? options.wslDistro | ||
| const quarantine = createGitObjectQuarantine(async () => { | ||
| const inherited = inheritedLocalObjectStore(wslDistro) | ||
| if (!inherited) { | ||
| return undefined | ||
| } | ||
| const commonDir = await readRepoCommonDirFromGit(repoPath, { | ||
| ...(options.wslDistro ? { wslDistro: options.wslDistro } : {}), | ||
| ...(options.signal ? { signal: options.signal } : {}), | ||
| ...(options.admissionTier ? { admissionTier: options.admissionTier } : {}) | ||
| }) | ||
| // Why after the read: that first Git call is what settles the WSL linked-worktree route, | ||
| // and Windows Git on that route may not share the spelling resolved above. | ||
| if (!commonDir || usesHostGitForWslLinkedWorktree(repoPath, options.wslDistro)) { | ||
| return undefined | ||
| } | ||
| return { ...localGitObjectsDirectory(commonDir, wslDistro), ...inherited } | ||
| }) | ||
| return { | ||
| run: (command) => | ||
| // Why no WSLENV: the WSL runner puts both variables on WSL Git's command line. | ||
| quarantine.run((env) => command(env ? { ...process.env, ...env } : undefined)) | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.