mirror of
https://github.com/firstsun-dev/git-files-sync.git
synced 2026-07-22 06:54:27 +00:00
fix: symlinked directories no longer break pull discovery
A folder that's actually an OS symlink (e.g. a shared skills folder linked in from elsewhere) is a single git blob on the remote, not a real tree. Local scanning code that walked every subfolder returned by adapter.list() didn't know this, so it recursed straight into whatever unrelated directory the link pointed at: - GitignoreManager.scanDir() picked up bogus nested .gitignore paths under the linked folder and tried to fetch them from the remote repo, spamming 404s on every refresh (this matches the console errors in the issue's screenshot). - SyncStatusView's hidden-file scan recursed the same way instead of registering the symlink itself as a trackable path, so the folder never resolved to anything pullable — it just showed up as permanently "remote only". Both now check readLocalSymlinkTarget() before recursing into a folder and treat it as a single link entry instead, matching how file symlinks are already handled by the existing push/pull machinery. Also fixes createLocalSymlink() to pass the correct `type` hint to Node's symlinkSync on Windows — omitting it defaults to 'file', which produces a broken link when the target is actually a directory (a no-op on macOS/Linux, but required on Windows). Closes #33
This commit is contained in:
parent
ef238cea59
commit
4c8896b6fa
6 changed files with 187 additions and 2 deletions
|
|
@ -2,6 +2,7 @@ import ignore, { Ignore } from 'ignore';
|
|||
import { App } from 'obsidian';
|
||||
import { GitServiceInterface } from '../services/git-service-interface';
|
||||
import { logger } from '../utils/logger';
|
||||
import { readLocalSymlinkTarget } from '../utils/symlink';
|
||||
|
||||
export class GitignoreManager {
|
||||
private readonly app: App;
|
||||
|
|
@ -95,6 +96,11 @@ export class GitignoreManager {
|
|||
}
|
||||
}
|
||||
for (const subFolder of listing.folders) {
|
||||
// A folder that's actually a symlink (e.g. a shared folder linked in from
|
||||
// elsewhere) is a single git blob on the remote, not a real tree — walking
|
||||
// into it would scan an unrelated directory structure and produce bogus
|
||||
// .gitignore lookups against paths that don't exist in this repo.
|
||||
if (readLocalSymlinkTarget(this.app, subFolder) !== null) continue;
|
||||
await this.scanDir(subFolder, out);
|
||||
}
|
||||
} catch { /* adapter.list may be unavailable in some environments */ }
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import { renderActionBar } from './components/ActionBar';
|
|||
import { renderFileItem, statusMeta, type FileItemCallbacks } from './components/FileListItem';
|
||||
import { ICONS } from './components/icons';
|
||||
import { isBinaryPath, contentsEqual } from '../utils/path';
|
||||
import { readLocalSymlinkTarget } from '../utils/symlink';
|
||||
|
||||
export const SYNC_STATUS_VIEW_TYPE = 'sync-status-view';
|
||||
|
||||
|
|
@ -370,6 +371,14 @@ export class SyncStatusView extends ItemView {
|
|||
}
|
||||
for (const folder of listing.folders) {
|
||||
if (folder === '.git' || folder.endsWith('/.git')) continue;
|
||||
// A symlinked folder is a single blob on the remote, not a real tree —
|
||||
// walking into it would scan whatever unrelated directory it points at.
|
||||
// Track the link itself (same as a hidden file) so push/pull can still
|
||||
// handle it via the existing symlink machinery, without recursing.
|
||||
if (readLocalSymlinkTarget(this.app, folder) !== null) {
|
||||
if (this.isHidden(folder)) result.push(folder);
|
||||
continue;
|
||||
}
|
||||
await this.recursiveScan(folder, result);
|
||||
}
|
||||
} catch { /* adapter may not support listing */ }
|
||||
|
|
|
|||
|
|
@ -14,16 +14,18 @@ import { logger } from './logger';
|
|||
// builtins (they don't exist in the mobile bundle).
|
||||
interface NodeFs {
|
||||
lstatSync(p: string): { isSymbolicLink(): boolean };
|
||||
statSync(p: string): { isDirectory(): boolean };
|
||||
readlinkSync(p: string): string;
|
||||
existsSync(p: string): boolean;
|
||||
mkdirSync(p: string, opts: { recursive: boolean }): void;
|
||||
rmSync(p: string, opts: { force: boolean }): void;
|
||||
symlinkSync(target: string, p: string): void;
|
||||
symlinkSync(target: string, p: string, type?: string): void;
|
||||
}
|
||||
|
||||
interface NodePath {
|
||||
join(...parts: string[]): string;
|
||||
dirname(p: string): string;
|
||||
isAbsolute(p: string): boolean;
|
||||
}
|
||||
|
||||
type NodeRequire = (id: string) => unknown;
|
||||
|
|
@ -88,7 +90,7 @@ export function createLocalSymlink(app: App, vaultPath: string, target: string):
|
|||
if (mods.fs.existsSync(abs) || isSymlink(mods.fs, abs)) {
|
||||
mods.fs.rmSync(abs, { force: true });
|
||||
}
|
||||
mods.fs.symlinkSync(target, abs);
|
||||
mods.fs.symlinkSync(target, abs, symlinkType(mods, abs, target));
|
||||
return true;
|
||||
} catch (e) {
|
||||
logger.warn(`Failed to create local symlink ${vaultPath} -> ${target}`, e);
|
||||
|
|
@ -103,3 +105,17 @@ function isSymlink(fs: NodeFs, abs: string): boolean {
|
|||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// Node's `type` hint to symlinkSync is a no-op on POSIX but required on
|
||||
// Windows: omitting it defaults to 'file', which produces a broken link
|
||||
// whenever the target is actually a directory (as with a symlinked folder).
|
||||
function symlinkType(mods: { fs: NodeFs; path: NodePath }, abs: string, target: string): 'file' | 'dir' {
|
||||
try {
|
||||
const resolvedTarget = mods.path.isAbsolute(target)
|
||||
? target
|
||||
: mods.path.join(mods.path.dirname(abs), target);
|
||||
return mods.fs.statSync(resolvedTarget).isDirectory() ? 'dir' : 'file';
|
||||
} catch {
|
||||
return 'file';
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,8 +3,10 @@ import { describe, it, expect, vi, beforeEach, Mocked } from 'vitest';
|
|||
import { GitignoreManager } from '../../src/logic/gitignore-manager';
|
||||
import { App, DataAdapter } from 'obsidian';
|
||||
import { GitServiceInterface } from '../../src/services/git-service-interface';
|
||||
import { readLocalSymlinkTarget } from '../../src/utils/symlink';
|
||||
|
||||
vi.mock('obsidian');
|
||||
vi.mock('../../src/utils/symlink', () => ({ readLocalSymlinkTarget: vi.fn() }));
|
||||
|
||||
describe('GitignoreManager', () => {
|
||||
let manager: GitignoreManager;
|
||||
|
|
@ -14,6 +16,7 @@ describe('GitignoreManager', () => {
|
|||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(readLocalSymlinkTarget).mockReturnValue(null);
|
||||
|
||||
const mockAdapter = {
|
||||
exists: vi.fn(),
|
||||
|
|
@ -272,5 +275,24 @@ describe('GitignoreManager', () => {
|
|||
expect(adapter.list).toHaveBeenCalledWith('');
|
||||
expect(adapter.list).toHaveBeenCalledWith('.claude');
|
||||
});
|
||||
|
||||
it('should not recurse into a folder that is actually a symlink', async () => {
|
||||
// "linked" is a directory symlink (e.g. a shared skills folder) pointing
|
||||
// outside the repo; walking into it would scan an unrelated tree and
|
||||
// produce bogus .gitignore lookups for paths that don't exist remotely.
|
||||
vi.mocked(adapter.list).mockImplementation((dir: string) => {
|
||||
if (dir === '' || dir === undefined) {
|
||||
return Promise.resolve({ files: ['.gitignore'], folders: ['linked'] });
|
||||
}
|
||||
return Promise.resolve({ files: ['linked/nested/.gitignore'], folders: ['linked/nested'] });
|
||||
});
|
||||
vi.mocked(readLocalSymlinkTarget).mockImplementation((_app, path) => path === 'linked' ? '/some/other/dir' : null);
|
||||
vi.mocked(adapter.read).mockResolvedValue('*.secret');
|
||||
|
||||
await manager.loadGitignores();
|
||||
|
||||
expect(adapter.list).toHaveBeenCalledWith('');
|
||||
expect(adapter.list).not.toHaveBeenCalledWith('linked');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -57,6 +57,10 @@ export const TFile = class {};
|
|||
export const requestUrl = vi.fn();
|
||||
export const setTooltip = vi.fn();
|
||||
export const setIcon = vi.fn();
|
||||
export const Platform = { isDesktopApp: true };
|
||||
export const FileSystemAdapter = class {
|
||||
getBasePath() { return '/mock/path'; }
|
||||
};
|
||||
|
||||
vi.mock('obsidian', () => ({
|
||||
Plugin,
|
||||
|
|
@ -71,4 +75,6 @@ vi.mock('obsidian', () => ({
|
|||
requestUrl,
|
||||
setTooltip,
|
||||
setIcon,
|
||||
Platform,
|
||||
FileSystemAdapter,
|
||||
}));
|
||||
|
|
|
|||
126
tests/utils/symlink.test.ts
Normal file
126
tests/utils/symlink.test.ts
Normal file
|
|
@ -0,0 +1,126 @@
|
|||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { App, FileSystemAdapter, Platform } from 'obsidian';
|
||||
import { canUseRealSymlinks, readLocalSymlinkTarget, createLocalSymlink } from '../../src/utils/symlink';
|
||||
|
||||
vi.mock('obsidian');
|
||||
|
||||
interface FakeFs {
|
||||
lstatSync: ReturnType<typeof vi.fn>;
|
||||
statSync: ReturnType<typeof vi.fn>;
|
||||
readlinkSync: ReturnType<typeof vi.fn>;
|
||||
existsSync: ReturnType<typeof vi.fn>;
|
||||
mkdirSync: ReturnType<typeof vi.fn>;
|
||||
rmSync: ReturnType<typeof vi.fn>;
|
||||
symlinkSync: ReturnType<typeof vi.fn>;
|
||||
}
|
||||
|
||||
describe('symlink utils', () => {
|
||||
let app: App;
|
||||
let fakeFs: FakeFs;
|
||||
const fakePath = {
|
||||
join: (...parts: string[]) => parts.join('/'),
|
||||
dirname: (p: string) => p.split('/').slice(0, -1).join('/'),
|
||||
isAbsolute: (p: string) => p.startsWith('/'),
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
Platform.isDesktopApp = true;
|
||||
|
||||
app = new App();
|
||||
app.vault.adapter = new FileSystemAdapter();
|
||||
|
||||
fakeFs = {
|
||||
lstatSync: vi.fn().mockReturnValue({ isSymbolicLink: () => false }),
|
||||
statSync: vi.fn().mockReturnValue({ isDirectory: () => false }),
|
||||
readlinkSync: vi.fn().mockReturnValue(''),
|
||||
existsSync: vi.fn().mockReturnValue(false),
|
||||
mkdirSync: vi.fn(),
|
||||
rmSync: vi.fn(),
|
||||
symlinkSync: vi.fn(),
|
||||
};
|
||||
|
||||
(window as unknown as { require: (id: string) => unknown }).require = (id: string) =>
|
||||
id === 'fs' ? fakeFs : fakePath;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
delete (window as unknown as { require?: unknown }).require;
|
||||
});
|
||||
|
||||
describe('canUseRealSymlinks', () => {
|
||||
it('is true on desktop with a FileSystemAdapter', () => {
|
||||
expect(canUseRealSymlinks(app)).toBe(true);
|
||||
});
|
||||
|
||||
it('is false when not a desktop app', () => {
|
||||
Platform.isDesktopApp = false;
|
||||
expect(canUseRealSymlinks(app)).toBe(false);
|
||||
});
|
||||
|
||||
it('is false when the adapter is not a FileSystemAdapter', () => {
|
||||
app.vault.adapter = {} as FileSystemAdapter;
|
||||
expect(canUseRealSymlinks(app)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('readLocalSymlinkTarget', () => {
|
||||
it('returns the raw target for a symlink', () => {
|
||||
fakeFs.lstatSync.mockReturnValue({ isSymbolicLink: () => true });
|
||||
fakeFs.readlinkSync.mockReturnValue('../target');
|
||||
|
||||
expect(readLocalSymlinkTarget(app, 'skills/blog-master')).toBe('../target');
|
||||
});
|
||||
|
||||
it('returns null for a non-symlink path', () => {
|
||||
fakeFs.lstatSync.mockReturnValue({ isSymbolicLink: () => false });
|
||||
expect(readLocalSymlinkTarget(app, 'notes/plain.md')).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null when real symlinks are unavailable', () => {
|
||||
Platform.isDesktopApp = false;
|
||||
expect(readLocalSymlinkTarget(app, 'skills/blog-master')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('createLocalSymlink', () => {
|
||||
it('creates a "dir" symlink when the target resolves to a directory', () => {
|
||||
fakeFs.statSync.mockReturnValue({ isDirectory: () => true });
|
||||
|
||||
const result = createLocalSymlink(app, 'skills/blog-master', '../blog/src');
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(fakeFs.symlinkSync).toHaveBeenCalledWith('../blog/src', expect.any(String), 'dir');
|
||||
});
|
||||
|
||||
it('creates a "file" symlink when the target resolves to a file', () => {
|
||||
fakeFs.statSync.mockReturnValue({ isDirectory: () => false });
|
||||
|
||||
createLocalSymlink(app, 'notes/link.md', '../shared/note.md');
|
||||
|
||||
expect(fakeFs.symlinkSync).toHaveBeenCalledWith('../shared/note.md', expect.any(String), 'file');
|
||||
});
|
||||
|
||||
it('falls back to "file" when the target cannot be stat-ed (dangling link)', () => {
|
||||
fakeFs.statSync.mockImplementation(() => { throw new Error('ENOENT'); });
|
||||
|
||||
createLocalSymlink(app, 'notes/link.md', '../missing.md');
|
||||
|
||||
expect(fakeFs.symlinkSync).toHaveBeenCalledWith('../missing.md', expect.any(String), 'file');
|
||||
});
|
||||
|
||||
it('removes an existing file or stale link before creating the new one', () => {
|
||||
fakeFs.existsSync.mockReturnValue(true);
|
||||
|
||||
createLocalSymlink(app, 'notes/link.md', '../shared/note.md');
|
||||
|
||||
expect(fakeFs.rmSync).toHaveBeenCalledWith(expect.any(String), { force: true });
|
||||
expect(fakeFs.symlinkSync).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns false when real symlinks are unavailable', () => {
|
||||
Platform.isDesktopApp = false;
|
||||
expect(createLocalSymlink(app, 'notes/link.md', '../shared/note.md')).toBe(false);
|
||||
expect(fakeFs.symlinkSync).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue