From f53c59874c2a325dc8bac31da9864e6df639bfca Mon Sep 17 00:00:00 2001 From: Patrick Cheung Date: Fri, 25 Sep 2026 17:36:50 +0800 Subject: [PATCH] feat(lfs): allow disabling background LFS lock polling (#70) --- README.md | 1 + package.json | 5 ++ src/git/__tests__/git-service.extra.test.ts | 17 +++++ src/git/git-service.ts | 14 ++++ src/panels/MainPanel.ts | 30 ++++++--- src/panels/__tests__/MainPanel.test.ts | 72 ++++++++++++++++++++- src/utils/__tests__/config.test.ts | 24 ++++++- src/utils/config.ts | 10 +++ 8 files changed, 160 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 395532d5..0448993c 100644 --- a/README.md +++ b/README.md @@ -184,6 +184,7 @@ A modern, full-featured Git GUI for VS Code. Visualize your commit history, mana | Setting | Default | Description | | -------------------------------------- | ------------- | -------------------------------------------------------- | | `gitGraphPlus.autoRefresh` | `true` | Auto-refresh on repository changes | +| `gitGraphPlus.lfsLocks` | `true` | Fetch LFS lock status from the origin server (off for hosts without LFS locking) | | `gitGraphPlus.timeout` | `60` | Max time (seconds) to wait for a Git command before abort | | `gitGraphPlus.initialCommitCount` | `200` | Commits loaded on first render / refresh (lower = faster in huge repos) | | `gitGraphPlus.loadMoreCommitCount` | `50` | Extra commits fetched per **Load more commits** click | diff --git a/package.json b/package.json index 8a1c2932..b1c1f312 100644 --- a/package.json +++ b/package.json @@ -419,6 +419,11 @@ "default": "ui", "markdownDescription": "How **Interactive Rebase** opens: the built-in GUI editor (`ui`), or the classic `git rebase -i` flow in the integrated terminal (`classic`), which uses your configured Git editor (e.g. vim, nano, `code --wait`)." }, + "gitGraphPlus.lfsLocks": { + "type": "boolean", + "default": true, + "markdownDescription": "Fetch Git LFS lock status from the origin server when commit details are shown. Disable this on hosts that do not support LFS locking (e.g. some on-premise Git servers) to stop repeated lock requests and their error pop-ups. Local LFS file status is still shown. Lock fetching is also skipped when the repository sets `lfs.locksverify=false`." + }, "gitGraphPlus.graphSortOrder": { "type": "string", "enum": [ diff --git a/src/git/__tests__/git-service.extra.test.ts b/src/git/__tests__/git-service.extra.test.ts index 3b0ee028..4d60bdd2 100644 --- a/src/git/__tests__/git-service.extra.test.ts +++ b/src/git/__tests__/git-service.extra.test.ts @@ -122,6 +122,23 @@ describe('GitService — LFS', () => { expect(calls[0]).toEqual(['lfs', 'unlock', '--force', '--', 'a.bin']); }); + it('isLfsLocksVerifyEnabled reads lfs.locksverify as a bool', async () => { + const calls: string[][] = []; + mockExec(service, async (args) => { calls.push(args); return 'true\n'; }); + expect(await service.isLfsLocksVerifyEnabled()).toBe(true); + expect(calls[0]).toEqual(['config', '--bool', '--get', 'lfs.locksverify']); + }); + + it('isLfsLocksVerifyEnabled returns false only for an explicit false', async () => { + mockExec(service, async () => 'false\n'); + expect(await service.isLfsLocksVerifyEnabled()).toBe(false); + }); + + it('isLfsLocksVerifyEnabled defaults to true when the key is unset or unreadable', async () => { + mockExec(service, async (args) => { throw new GitError('missing key', 1, args); }); + expect(await service.isLfsLocksVerifyEnabled()).toBe(true); + }); + it('lfsLock rejects an option-like file name', async () => { await expect(service.lfsLock('-evil')).rejects.toThrow(); }); diff --git a/src/git/git-service.ts b/src/git/git-service.ts index 71e65760..df1b876d 100644 --- a/src/git/git-service.ts +++ b/src/git/git-service.ts @@ -2104,6 +2104,20 @@ export class GitService { return this.exec(args); } + /** + * Reads the repository's `lfs.locksverify` config. Returns false only when + * the value parses to false; an unset (exit 1) or unreadable value keeps lock + * fetching enabled, matching git-lfs' own default. + */ + async isLfsLocksVerifyEnabled(): Promise { + try { + const raw = (await this.exec(['config', '--bool', '--get', 'lfs.locksverify'], { silent: true })).trim(); + return raw !== 'false'; + } catch { + return true; + } + } + async lfsLocks(): Promise> { try { const raw = await this.exec(['lfs', 'locks']); diff --git a/src/panels/MainPanel.ts b/src/panels/MainPanel.ts index 5869905d..a0836a9d 100644 --- a/src/panels/MainPanel.ts +++ b/src/panels/MainPanel.ts @@ -5,7 +5,7 @@ import { GitService, GitError } from '../git/git-service'; import { formatGitError, isAuthFailure, transportFromRemoteUrl } from '../git/git-error-formatter'; import { splitUpstreamRef } from '../git/git-parser'; import { samePath } from '../utils/path'; -import { readTimeoutMs, readInitialCommitCount, readLoadMoreCommitCount, readInteractiveRebaseMode } from '../utils/config'; +import { readTimeoutMs, readInitialCommitCount, readLoadMoreCommitCount, readInteractiveRebaseMode, readLfsLocksEnabled } from '../utils/config'; import { buildClassicRebaseCommand } from '../git/classic-rebase'; import { buildFullGraph } from '../git/git-graph-builder'; import { compileBranchColorRules, makeBranchColorResolver } from '../git/branch-color-resolver'; @@ -76,6 +76,19 @@ export class MainPanel { } } + /** + * Loads LFS file status (local, always) and lock status (origin server). + * Lock polling is skipped — and an empty list posted — when the + * `gitGraphPlus.lfsLocks` setting is off or the repository sets + * `lfs.locksverify=false` (issue #70). + */ + private async postLfsData(): Promise { + const files = await this.gitService.lfsLsFiles(); + const fetchLocks = readLfsLocksEnabled() && (await this.gitService.isLfsLocksVerifyEnabled()); + const locks = fetchLocks ? await this.gitService.lfsLocks() : []; + this.post({ type: 'lfsData', payload: { files, locks } }); + } + public static setExtraEnv(env: Record): void { this.extraEnv = env; if (this.currentPanel) { @@ -241,6 +254,9 @@ export class MainPanel { if (e.affectsConfiguration('gitGraphPlus.timeout')) { this.gitService.setDefaultTimeout(readTimeoutMs()); } + if (e.affectsConfiguration('gitGraphPlus.lfsLocks')) { + void this.postLfsData(); + } }) ); @@ -1381,27 +1397,21 @@ export class MainPanel { } // --- LFS --- case 'getLfsFiles': { - const lfsFiles = await this.gitService.lfsLsFiles(); - const lfsLocks = await this.gitService.lfsLocks(); - this.post({ type: 'lfsData', payload: { files: lfsFiles, locks: lfsLocks } }); + await this.postLfsData(); break; } case 'lfsLock': { await this.gitService.lfsLock(message.payload.file); this.post({ type: 'operationComplete', payload: { operation: 'lfsLock', success: true } }); // Refresh LFS data - const lfsFiles = await this.gitService.lfsLsFiles(); - const lfsLocks = await this.gitService.lfsLocks(); - this.post({ type: 'lfsData', payload: { files: lfsFiles, locks: lfsLocks } }); + await this.postLfsData(); break; } case 'lfsUnlock': { await this.gitService.lfsUnlock(message.payload.file, message.payload.force); this.post({ type: 'operationComplete', payload: { operation: 'lfsUnlock', success: true } }); // Refresh LFS data - const lfsFiles2 = await this.gitService.lfsLsFiles(); - const lfsLocks2 = await this.gitService.lfsLocks(); - this.post({ type: 'lfsData', payload: { files: lfsFiles2, locks: lfsLocks2 } }); + await this.postLfsData(); break; } // --- Worktree --- diff --git a/src/panels/__tests__/MainPanel.test.ts b/src/panels/__tests__/MainPanel.test.ts index 76051ee3..f40feeb1 100644 --- a/src/panels/__tests__/MainPanel.test.ts +++ b/src/panels/__tests__/MainPanel.test.ts @@ -31,9 +31,16 @@ const H = vi.hoisted(() => { setAuthRetryHandler: vi.fn(), setExtraEnv: vi.fn(), setDefaultTimeout: vi.fn(), + lfsLsFiles: vi.fn(async () => []), + lfsLocks: vi.fn(async () => []), + lfsLock: vi.fn(async () => ''), + lfsUnlock: vi.fn(async () => ''), + isLfsLocksVerifyEnabled: vi.fn(async () => true), }; return { git, + config: {} as Record, + configChangeHandler: null as null | ((e: { affectsConfiguration: (s: string) => boolean }) => void), messageHandler: null as null | ((m: unknown) => unknown), panel: null as null | { webview: { postMessage: ReturnType } }, repos: [] as Array<{ path: string; name: string; type: string }>, @@ -70,10 +77,13 @@ vi.mock('vscode', () => { showSaveDialog: vi.fn(async () => undefined), }, workspace: { - getConfiguration: () => ({ get: (_k: string, d?: unknown) => d }), + getConfiguration: () => ({ get: (k: string, d?: unknown) => (H.config[k] === undefined ? d : H.config[k]) }), getWorkspaceFolder: () => ({ uri: { fsPath: '/repo' } }), workspaceFolders: [{ uri: { fsPath: '/repo' } }], - onDidChangeConfiguration: () => ({ dispose() {} }), + onDidChangeConfiguration: (cb: (e: { affectsConfiguration: (s: string) => boolean }) => void) => { + H.configChangeHandler = cb; + return { dispose() {} }; + }, fs: { writeFile: vi.fn(async () => {}) }, }, commands: { executeCommand: vi.fn() }, @@ -127,6 +137,10 @@ beforeEach(() => { H.git.showCommitDiff.mockResolvedValue([]); H.git.fileExistsAtRef.mockResolvedValue(true); H.git.getEmptyTreeRef.mockResolvedValue('4b825dc642cb6eb9a060e54bf8d69288fbee4904'); + H.git.lfsLsFiles.mockResolvedValue([]); + H.git.lfsLocks.mockResolvedValue([]); + H.git.isLfsLocksVerifyEnabled.mockResolvedValue(true); + H.config = {}; H.repos = [{ path: '/repo', name: 'repo', type: 'root' }]; (MainPanel as unknown as { currentPanel: unknown }).currentPanel = undefined; MainPanel.createOrShow(extUri, '/repo'); @@ -289,6 +303,60 @@ describe('MainPanel message routing', () => { }); }); +describe('MainPanel LFS lock polling', () => { + it('getLfsFiles calls lfsLocks and posts the locks by default', async () => { + H.git.lfsLsFiles.mockResolvedValue([{ oid: 'o1', path: 'a.bin' }]); + H.git.lfsLocks.mockResolvedValue([{ path: 'a.bin', owner: 'alice', id: 'L1' }]); + + await dispatch({ type: 'getLfsFiles' }); + + expect(H.git.isLfsLocksVerifyEnabled).toHaveBeenCalled(); + expect(H.git.lfsLocks).toHaveBeenCalled(); + const data = postedOfType('lfsData').at(-1)!; + expect(data.payload!.files).toEqual([{ oid: 'o1', path: 'a.bin' }]); + expect(data.payload!.locks).toEqual([{ path: 'a.bin', owner: 'alice', id: 'L1' }]); + }); + + it('getLfsFiles skips lfsLocks and posts locks: [] when gitGraphPlus.lfsLocks is disabled', async () => { + H.config.lfsLocks = false; + H.git.lfsLsFiles.mockResolvedValue([{ oid: 'o1', path: 'a.bin' }]); + + await dispatch({ type: 'getLfsFiles' }); + + expect(H.git.lfsLocks).not.toHaveBeenCalled(); + expect(H.git.isLfsLocksVerifyEnabled).not.toHaveBeenCalled(); + const data = postedOfType('lfsData').at(-1)!; + expect(data.payload!.files).toEqual([{ oid: 'o1', path: 'a.bin' }]); + expect(data.payload!.locks).toEqual([]); + }); + + it('getLfsFiles skips lfsLocks when the repo sets lfs.locksverify=false', async () => { + H.git.isLfsLocksVerifyEnabled.mockResolvedValue(false); + H.git.lfsLsFiles.mockResolvedValue([{ oid: 'o1', path: 'a.bin' }]); + + await dispatch({ type: 'getLfsFiles' }); + + expect(H.git.lfsLocks).not.toHaveBeenCalled(); + const data = postedOfType('lfsData').at(-1)!; + expect(data.payload!.locks).toEqual([]); + }); + + it('toggling gitGraphPlus.lfsLocks refreshes LFS data without a reload', async () => { + H.config.lfsLocks = false; + H.git.lfsLsFiles.mockResolvedValue([{ oid: 'o1', path: 'a.bin' }]); + + H.configChangeHandler!({ affectsConfiguration: (s: string) => s === 'gitGraphPlus.lfsLocks' }); + + await vi.waitFor(() => { + const data = postedOfType('lfsData').at(-1); + expect(data).toBeDefined(); + expect(data!.payload!.files).toEqual([{ oid: 'o1', path: 'a.bin' }]); + expect(data!.payload!.locks).toEqual([]); + }); + expect(H.git.lfsLocks).not.toHaveBeenCalled(); + }); +}); + describe('MainPanel error handling', () => { it('posts notGitRepo when git reports "not a git repository"', async () => { H.git.log.mockRejectedValue(new GitError('fatal: not a git repository', 128, ['log'])); diff --git a/src/utils/__tests__/config.test.ts b/src/utils/__tests__/config.test.ts index 63667d34..01722c69 100644 --- a/src/utils/__tests__/config.test.ts +++ b/src/utils/__tests__/config.test.ts @@ -13,7 +13,7 @@ vi.mock('vscode', () => ({ }, })); -import { readTimeoutMs, readInitialCommitCount, readLoadMoreCommitCount } from '../config'; +import { readTimeoutMs, readInitialCommitCount, readLoadMoreCommitCount, readLfsLocksEnabled } from '../config'; describe('readTimeoutMs', () => { // Back-compat alias so the existing timeout cases below read naturally. @@ -101,3 +101,25 @@ describe('readLoadMoreCommitCount', () => { expect(readLoadMoreCommitCount()).toBe(50); }); }); + +describe('readLfsLocksEnabled', () => { + beforeEach(() => { h.values = {}; }); + + it('defaults to true when unset', () => { + expect(readLfsLocksEnabled()).toBe(true); + }); + + it('returns the configured boolean as-is', () => { + h.values.lfsLocks = true; + expect(readLfsLocksEnabled()).toBe(true); + h.values.lfsLocks = false; + expect(readLfsLocksEnabled()).toBe(false); + }); + + it('falls back to true for non-boolean values', () => { + h.values.lfsLocks = 'no'; + expect(readLfsLocksEnabled()).toBe(true); + h.values.lfsLocks = 0; + expect(readLfsLocksEnabled()).toBe(true); + }); +}); diff --git a/src/utils/config.ts b/src/utils/config.ts index fa8fabed..7d7f4bf1 100644 --- a/src/utils/config.ts +++ b/src/utils/config.ts @@ -37,6 +37,16 @@ export function readLoadMoreCommitCount(): number { return readPositiveIntSetting('loadMoreCommitCount', DEFAULT_LOAD_MORE_COMMIT_COUNT); } +/** + * Reads `gitGraphPlus.lfsLocks` — whether to poll the origin server for Git LFS + * lock status. Falls back to `true` (the previous behaviour) when unset or + * non-boolean. + */ +export function readLfsLocksEnabled(): boolean { + const enabled = vscode.workspace.getConfiguration('gitGraphPlus').get('lfsLocks', true); + return typeof enabled === 'boolean' ? enabled : true; +} + /** * Reads `gitGraphPlus.interactiveRebase.mode` — whether interactive rebase * opens the GUI editor (`ui`, default) or runs classic `git rebase -i` in the