mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-30 01:51:20 +00:00
fix(storage): address third review round on the shared store (#3374)
- clean --gc never collects a slot whose index lock is held: an analyze holds it until it registers the checkout, so a seeded but not yet registered slot is busy, not orphaned. - Reclaim compares absolute paths, so a relative GITNEXUS_HOME does not make live commit graphs look unreferenced. - graphPath is followed only into a published <commit>-<featureKey> dir, never .publish-* staging (TS and all three hook copies). - clean --gc fails on an unreadable stores root instead of reporting nothing to collect. - --no-share help states it is for opted-in clones. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d746675c28
commit
e2a9467171
10 changed files with 85 additions and 12 deletions
|
|
@ -321,9 +321,11 @@ function resolveGraphPath(storagePath, metadata) {
|
||||||
const recorded = metadata && metadata.graphPath;
|
const recorded = metadata && metadata.graphPath;
|
||||||
if (typeof recorded !== 'string' || !path.isAbsolute(recorded)) return own;
|
if (typeof recorded !== 'string' || !path.isAbsolute(recorded)) return own;
|
||||||
const graph = path.resolve(recorded);
|
const graph = path.resolve(recorded);
|
||||||
|
// Only a published `<commit>-<featureKey>` dir, never `.publish-*` staging.
|
||||||
const valid =
|
const valid =
|
||||||
path.basename(graph) === LBUG_DIRECTORY &&
|
path.basename(graph) === LBUG_DIRECTORY &&
|
||||||
isDirectChild(path.join(root, 'commits'), path.dirname(graph));
|
isDirectChild(path.join(root, 'commits'), path.dirname(graph)) &&
|
||||||
|
/^[0-9a-f]{7,64}-[0-9a-f]{8,64}$/.test(path.basename(path.dirname(graph)));
|
||||||
return valid ? graph : own;
|
return valid ? graph : own;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -321,9 +321,11 @@ function resolveGraphPath(storagePath, metadata) {
|
||||||
const recorded = metadata && metadata.graphPath;
|
const recorded = metadata && metadata.graphPath;
|
||||||
if (typeof recorded !== 'string' || !path.isAbsolute(recorded)) return own;
|
if (typeof recorded !== 'string' || !path.isAbsolute(recorded)) return own;
|
||||||
const graph = path.resolve(recorded);
|
const graph = path.resolve(recorded);
|
||||||
|
// Only a published `<commit>-<featureKey>` dir, never `.publish-*` staging.
|
||||||
const valid =
|
const valid =
|
||||||
path.basename(graph) === LBUG_DIRECTORY &&
|
path.basename(graph) === LBUG_DIRECTORY &&
|
||||||
isDirectChild(path.join(root, 'commits'), path.dirname(graph));
|
isDirectChild(path.join(root, 'commits'), path.dirname(graph)) &&
|
||||||
|
/^[0-9a-f]{7,64}-[0-9a-f]{8,64}$/.test(path.basename(path.dirname(graph)));
|
||||||
return valid ? graph : own;
|
return valid ? graph : own;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -321,9 +321,11 @@ function resolveGraphPath(storagePath, metadata) {
|
||||||
const recorded = metadata && metadata.graphPath;
|
const recorded = metadata && metadata.graphPath;
|
||||||
if (typeof recorded !== 'string' || !path.isAbsolute(recorded)) return own;
|
if (typeof recorded !== 'string' || !path.isAbsolute(recorded)) return own;
|
||||||
const graph = path.resolve(recorded);
|
const graph = path.resolve(recorded);
|
||||||
|
// Only a published `<commit>-<featureKey>` dir, never `.publish-*` staging.
|
||||||
const valid =
|
const valid =
|
||||||
path.basename(graph) === LBUG_DIRECTORY &&
|
path.basename(graph) === LBUG_DIRECTORY &&
|
||||||
isDirectChild(path.join(root, 'commits'), path.dirname(graph));
|
isDirectChild(path.join(root, 'commits'), path.dirname(graph)) &&
|
||||||
|
/^[0-9a-f]{7,64}-[0-9a-f]{8,64}$/.test(path.basename(path.dirname(graph)));
|
||||||
return valid ? graph : own;
|
return valid ? graph : own;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -168,7 +168,12 @@ const reportReclaim = (result: ReclaimResult | null): void => {
|
||||||
/** `clean --gc`: collect every shared store under GITNEXUS_HOME (#3352). */
|
/** `clean --gc`: collect every shared store under GITNEXUS_HOME (#3352). */
|
||||||
const collectSharedStores = async (force: boolean): Promise<void> => {
|
const collectSharedStores = async (force: boolean): Promise<void> => {
|
||||||
const storesDir = path.join(getGlobalDir(), STORES_DIR);
|
const storesDir = path.join(getGlobalDir(), STORES_DIR);
|
||||||
const names = await fs.readdir(storesDir).catch(() => [] as string[]);
|
// Only a missing stores root means "nothing to collect"; an unreadable one
|
||||||
|
// must fail loudly rather than report success.
|
||||||
|
const names = await fs.readdir(storesDir).catch((err: NodeJS.ErrnoException) => {
|
||||||
|
if (err.code === 'ENOENT') return [] as string[];
|
||||||
|
throw err;
|
||||||
|
});
|
||||||
if (names.length === 0) {
|
if (names.length === 0) {
|
||||||
console.log(t('clean.gc.none'));
|
console.log(t('clean.gc.none'));
|
||||||
return;
|
return;
|
||||||
|
|
|
||||||
|
|
@ -267,7 +267,8 @@ export const zhCN = {
|
||||||
'即使已有其他路径使用相同 --name 别名,也注册该仓库。会使两个路径的 `-r <name>` 产生歧义;请用 -r <path> 消除歧义。',
|
'即使已有其他路径使用相同 --name 别名,也注册该仓库。会使两个路径的 `-r <name>` 产生歧义;请用 -r <path> 消除歧义。',
|
||||||
'help.option.analyze.shareWith':
|
'help.option.analyze.shareWith':
|
||||||
'加入同一仓库已注册工作树的共享索引存储(名称或路径);远程 URL 必须一致。之后的运行会记住此选择。',
|
'加入同一仓库已注册工作树的共享索引存储(名称或路径);远程 URL 必须一致。之后的运行会记住此选择。',
|
||||||
'help.option.analyze.noShare': '离开共享索引存储,重新索引到 <repo>/.gitnexus',
|
'help.option.analyze.noShare':
|
||||||
|
'仅限已加入的克隆:离开共享索引存储,重新索引到 <repo>/.gitnexus(链接工作树始终共享;请改用 GITNEXUS_SHARED_STORE=off)',
|
||||||
'help.option.verbose': '启用详细输出',
|
'help.option.verbose': '启用详细输出',
|
||||||
'help.option.analyze.maxFileSize':
|
'help.option.analyze.maxFileSize':
|
||||||
'跳过大于该值的文件(KB)。默认:512。硬上限:32768(tree-sitter 限制)。',
|
'跳过大于该值的文件(KB)。默认:512。硬上限:32768(tree-sitter 限制)。',
|
||||||
|
|
|
||||||
|
|
@ -150,7 +150,11 @@ program
|
||||||
'Join the shared index store of a registered worktree of the same repository ' +
|
'Join the shared index store of a registered worktree of the same repository ' +
|
||||||
'(name or path); the remote URL must match. Remembered for later runs.',
|
'(name or path); the remote URL must match. Remembered for later runs.',
|
||||||
)
|
)
|
||||||
.option('--no-share', 'Leave the shared index store and index into <repo>/.gitnexus again')
|
.option(
|
||||||
|
'--no-share',
|
||||||
|
'Opted-in clones only: leave the shared index store and index into <repo>/.gitnexus again ' +
|
||||||
|
'(linked worktrees always share; set GITNEXUS_SHARED_STORE=off instead)',
|
||||||
|
)
|
||||||
.option('-v, --verbose', 'Enable verbose ingestion warnings (default: false)')
|
.option('-v, --verbose', 'Enable verbose ingestion warnings (default: false)')
|
||||||
.option(
|
.option(
|
||||||
'--max-file-size <kb>',
|
'--max-file-size <kb>',
|
||||||
|
|
|
||||||
|
|
@ -12,7 +12,7 @@
|
||||||
import { existsSync } from 'fs';
|
import { existsSync } from 'fs';
|
||||||
import fs from 'fs/promises';
|
import fs from 'fs/promises';
|
||||||
import path from 'path';
|
import path from 'path';
|
||||||
import { acquireIndexLock, requireExclusiveIndexLock } from './index-lock.js';
|
import { acquireIndexLock, requireExclusiveIndexLock, type IndexLockHandle } from './index-lock.js';
|
||||||
import {
|
import {
|
||||||
canonicalizePath,
|
canonicalizePath,
|
||||||
findRegistryEntryByRepoPath,
|
findRegistryEntryByRepoPath,
|
||||||
|
|
@ -94,9 +94,12 @@ const orphanMembers = async (slots: string[]): Promise<Set<string>> => {
|
||||||
* (KTD7), and slot pointers written under the same lock are always counted.
|
* (KTD7), and slot pointers written under the same lock are always counted.
|
||||||
*/
|
*/
|
||||||
export const reclaimSharedStoreLocked = async (
|
export const reclaimSharedStoreLocked = async (
|
||||||
storeRoot: string,
|
storeRootInput: string,
|
||||||
opts: { gc?: boolean; dryRun?: boolean } = {},
|
opts: { gc?: boolean; dryRun?: boolean } = {},
|
||||||
): Promise<ReclaimResult> => {
|
): Promise<ReclaimResult> => {
|
||||||
|
// Absolute, so commit dirs compare equal to the resolved graphPath parents
|
||||||
|
// even when GITNEXUS_HOME is relative.
|
||||||
|
const storeRoot = path.resolve(storeRootInput);
|
||||||
const result: ReclaimResult = { removed: [], kept: [], droppedMembers: [], storeRemoved: false };
|
const result: ReclaimResult = { removed: [], kept: [], droppedMembers: [], storeRemoved: false };
|
||||||
const checkoutsDir = path.join(storeRoot, 'checkouts');
|
const checkoutsDir = path.join(storeRoot, 'checkouts');
|
||||||
const commitsDir = path.join(storeRoot, 'commits');
|
const commitsDir = path.join(storeRoot, 'commits');
|
||||||
|
|
@ -104,9 +107,31 @@ export const reclaimSharedStoreLocked = async (
|
||||||
let slots = (await listDir(checkoutsDir)).map((name) => path.join(checkoutsDir, name));
|
let slots = (await listDir(checkoutsDir)).map((name) => path.join(checkoutsDir, name));
|
||||||
if (opts.gc) {
|
if (opts.gc) {
|
||||||
const orphans = await orphanMembers(slots);
|
const orphans = await orphanMembers(slots);
|
||||||
for (const slot of orphans) {
|
for (const slot of [...orphans]) {
|
||||||
if (!opts.dryRun) await fs.rm(slot, { recursive: true, force: true });
|
if (opts.dryRun) {
|
||||||
result.droppedMembers.push(slot);
|
result.droppedMembers.push(slot);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
// An analyze holds its slot's index lock until it has registered the
|
||||||
|
// checkout, so a slot that is seeded but not yet registered is busy,
|
||||||
|
// not orphaned. Judge and delete only a slot whose lock is free.
|
||||||
|
let lock: IndexLockHandle;
|
||||||
|
try {
|
||||||
|
lock = await acquireIndexLock(slot, { timeoutMs: 1 });
|
||||||
|
} catch {
|
||||||
|
orphans.delete(slot);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
try {
|
||||||
|
if (lock.lockFree || !(await orphanMembers([slot])).has(slot)) {
|
||||||
|
orphans.delete(slot);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
await fs.rm(slot, { recursive: true, force: true });
|
||||||
|
result.droppedMembers.push(slot);
|
||||||
|
} finally {
|
||||||
|
lock.release();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
slots = slots.filter((slot) => !orphans.has(slot));
|
slots = slots.filter((slot) => !orphans.has(slot));
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -50,6 +50,7 @@ const slotName = (p: string): string => {
|
||||||
const DISABLED_VALUES = new Set(['off', '0', 'false', 'no']);
|
const DISABLED_VALUES = new Set(['off', '0', 'false', 'no']);
|
||||||
const COMMIT_RE = /^[0-9a-f]{7,64}$/;
|
const COMMIT_RE = /^[0-9a-f]{7,64}$/;
|
||||||
const FEATURE_KEY_RE = /^[0-9a-f]{8,64}$/;
|
const FEATURE_KEY_RE = /^[0-9a-f]{8,64}$/;
|
||||||
|
const COMMIT_GRAPH_DIR_RE = /^[0-9a-f]{7,64}-[0-9a-f]{8,64}$/;
|
||||||
|
|
||||||
export interface SharedStoreLayout {
|
export interface SharedStoreLayout {
|
||||||
/** Store key: readable basename plus a hash of the canonical git common dir. */
|
/** Store key: readable basename plus a hash of the canonical git common dir. */
|
||||||
|
|
@ -267,8 +268,11 @@ export const resolveGraphPath = (storagePath: string): string => {
|
||||||
if (typeof recorded !== 'string' || !path.isAbsolute(recorded)) return own;
|
if (typeof recorded !== 'string' || !path.isAbsolute(recorded)) return own;
|
||||||
const graph = path.resolve(recorded);
|
const graph = path.resolve(recorded);
|
||||||
const commitDir = path.dirname(graph);
|
const commitDir = path.dirname(graph);
|
||||||
|
// Only a published `<commit>-<featureKey>` dir, never `.publish-*` staging.
|
||||||
const valid =
|
const valid =
|
||||||
path.basename(graph) === LBUG_DIRECTORY && isDirectChild(path.join(root, 'commits'), commitDir);
|
path.basename(graph) === LBUG_DIRECTORY &&
|
||||||
|
isDirectChild(path.join(root, 'commits'), commitDir) &&
|
||||||
|
COMMIT_GRAPH_DIR_RE.test(path.basename(commitDir));
|
||||||
return valid ? graph : own;
|
return valid ? graph : own;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -240,6 +240,30 @@ describe('reclaimSharedStore', () => {
|
||||||
expect(existsSync(outside)).toBe(true);
|
expect(existsSync(outside)).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('clean --gc keeps a slot whose index lock is held (analyze in progress)', async () => {
|
||||||
|
const slot = await member('busy-000000000000', { repoPath: '/nonexistent/checkout' });
|
||||||
|
const { acquireIndexLock } = await import('../../src/storage/index-lock.js');
|
||||||
|
const lock = await acquireIndexLock(slot);
|
||||||
|
try {
|
||||||
|
const result = await reclaimSharedStore(layout().root, { gc: true });
|
||||||
|
expect(result.droppedMembers).toEqual([]);
|
||||||
|
expect(existsSync(slot)).toBe(true);
|
||||||
|
} finally {
|
||||||
|
lock.release();
|
||||||
|
}
|
||||||
|
const after = await reclaimSharedStore(layout().root, { gc: true });
|
||||||
|
expect(after.droppedMembers).toEqual([slot]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('counts references correctly when GITNEXUS_HOME is relative', async () => {
|
||||||
|
const referenced = await commitGraph('ddddddd-4444444444444444');
|
||||||
|
await member('wt-000000000000', { graphPath: path.join(referenced, 'lbug') });
|
||||||
|
const relativeRoot = path.relative(process.cwd(), layout().root);
|
||||||
|
const result = await reclaimSharedStore(relativeRoot);
|
||||||
|
expect(result.removed).toEqual([]);
|
||||||
|
expect(existsSync(referenced)).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
it('reports a graph it cannot delete instead of failing', async () => {
|
it('reports a graph it cannot delete instead of failing', async () => {
|
||||||
const orphan = await commitGraph('ccccccc-3333333333333333');
|
const orphan = await commitGraph('ccccccc-3333333333333333');
|
||||||
await member('wt-000000000000', {});
|
await member('wt-000000000000', {});
|
||||||
|
|
|
||||||
|
|
@ -249,6 +249,10 @@ describe('resolveGraphPath', () => {
|
||||||
['outside GITNEXUS_HOME', () => '/etc/lbug'],
|
['outside GITNEXUS_HOME', () => '/etc/lbug'],
|
||||||
['a relative path', () => 'commits/abc1234-deadbeef/lbug'],
|
['a relative path', () => 'commits/abc1234-deadbeef/lbug'],
|
||||||
['a traversal', (l: SharedStoreLayout) => path.join(l.commitsDir, '..', '..', 'x', 'lbug')],
|
['a traversal', (l: SharedStoreLayout) => path.join(l.commitsDir, '..', '..', 'x', 'lbug')],
|
||||||
|
[
|
||||||
|
'in-progress publish staging',
|
||||||
|
(l: SharedStoreLayout) => path.join(l.commitsDir, '.publish-0f3c', 'lbug'),
|
||||||
|
],
|
||||||
[
|
[
|
||||||
'a non-lbug file',
|
'a non-lbug file',
|
||||||
(l: SharedStoreLayout) => path.join(l.commitsDir, 'abc1234-deadbeef', 'gitnexus.json'),
|
(l: SharedStoreLayout) => path.join(l.commitsDir, 'abc1234-deadbeef', 'gitnexus.json'),
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue