From db99fc70b67b94ed2580e5a4bf7ab3c3333caaaa Mon Sep 17 00:00:00 2001 From: Gaurav <39389231+gsquared94@users.noreply.github.com> Date: Wed, 10 Sep 2025 12:48:07 -0700 Subject: [PATCH] fix: `gitignore` handling (#8177) --- .../core/src/services/fileDiscoveryService.ts | 22 +-- .../core/src/utils/geminiIgnoreParser.test.ts | 70 ++++++++ packages/core/src/utils/geminiIgnoreParser.ts | 81 +++++++++ .../core/src/utils/gitIgnoreParser.test.ts | 87 +++------- packages/core/src/utils/gitIgnoreParser.ts | 155 +++++++++--------- 5 files changed, 251 insertions(+), 164 deletions(-) create mode 100644 packages/core/src/utils/geminiIgnoreParser.test.ts create mode 100644 packages/core/src/utils/geminiIgnoreParser.ts diff --git a/packages/core/src/services/fileDiscoveryService.ts b/packages/core/src/services/fileDiscoveryService.ts index 1689d7ecce..4620362685 100644 --- a/packages/core/src/services/fileDiscoveryService.ts +++ b/packages/core/src/services/fileDiscoveryService.ts @@ -5,12 +5,12 @@ */ import type { GitIgnoreFilter } from '../utils/gitIgnoreParser.js'; +import type { GeminiIgnoreFilter } from '../utils/geminiIgnoreParser.js'; import { GitIgnoreParser } from '../utils/gitIgnoreParser.js'; +import { GeminiIgnoreParser } from '../utils/geminiIgnoreParser.js'; import { isGitRepository } from '../utils/gitUtils.js'; import * as path from 'node:path'; -const GEMINI_IGNORE_FILE_NAME = '.geminiignore'; - export interface FilterFilesOptions { respectGitIgnore?: boolean; respectGeminiIgnore?: boolean; @@ -24,27 +24,15 @@ export interface FilterReport { export class FileDiscoveryService { private gitIgnoreFilter: GitIgnoreFilter | null = null; - private geminiIgnoreFilter: GitIgnoreFilter | null = null; + private geminiIgnoreFilter: GeminiIgnoreFilter | null = null; private projectRoot: string; constructor(projectRoot: string) { this.projectRoot = path.resolve(projectRoot); if (isGitRepository(this.projectRoot)) { - const parser = new GitIgnoreParser(this.projectRoot); - try { - parser.loadGitRepoPatterns(); - } catch (_error) { - // ignore file not found - } - this.gitIgnoreFilter = parser; + this.gitIgnoreFilter = new GitIgnoreParser(this.projectRoot); } - const gParser = new GitIgnoreParser(this.projectRoot); - try { - gParser.loadPatterns(GEMINI_IGNORE_FILE_NAME); - } catch (_error) { - // ignore file not found - } - this.geminiIgnoreFilter = gParser; + this.geminiIgnoreFilter = new GeminiIgnoreParser(this.projectRoot); } /** diff --git a/packages/core/src/utils/geminiIgnoreParser.test.ts b/packages/core/src/utils/geminiIgnoreParser.test.ts new file mode 100644 index 0000000000..bf85cd8c69 --- /dev/null +++ b/packages/core/src/utils/geminiIgnoreParser.test.ts @@ -0,0 +1,70 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { GeminiIgnoreParser } from './geminiIgnoreParser.js'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import * as os from 'node:os'; + +describe('GeminiIgnoreParser', () => { + let projectRoot: string; + + async function createTestFile(filePath: string, content = '') { + const fullPath = path.join(projectRoot, filePath); + await fs.mkdir(path.dirname(fullPath), { recursive: true }); + await fs.writeFile(fullPath, content); + } + + beforeEach(async () => { + projectRoot = await fs.mkdtemp( + path.join(os.tmpdir(), 'geminiignore-test-'), + ); + }); + + afterEach(async () => { + await fs.rm(projectRoot, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + describe('when .geminiignore exists', () => { + beforeEach(async () => { + await createTestFile( + '.geminiignore', + 'ignored.txt\n# A comment\n/ignored_dir/\n', + ); + await createTestFile('ignored.txt', 'ignored'); + await createTestFile('not_ignored.txt', 'not ignored'); + await createTestFile( + path.join('ignored_dir', 'file.txt'), + 'in ignored dir', + ); + await createTestFile( + path.join('subdir', 'not_ignored.txt'), + 'not ignored', + ); + }); + + it('should ignore files specified in .geminiignore', () => { + const parser = new GeminiIgnoreParser(projectRoot); + expect(parser.getPatterns()).toEqual(['ignored.txt', '/ignored_dir/']); + expect(parser.isIgnored('ignored.txt')).toBe(true); + expect(parser.isIgnored('not_ignored.txt')).toBe(false); + expect(parser.isIgnored(path.join('ignored_dir', 'file.txt'))).toBe(true); + expect(parser.isIgnored(path.join('subdir', 'not_ignored.txt'))).toBe( + false, + ); + }); + }); + + describe('when .geminiignore does not exist', () => { + it('should not load any patterns and not ignore any files', () => { + const parser = new GeminiIgnoreParser(projectRoot); + expect(parser.getPatterns()).toEqual([]); + expect(parser.isIgnored('any_file.txt')).toBe(false); + }); + }); +}); diff --git a/packages/core/src/utils/geminiIgnoreParser.ts b/packages/core/src/utils/geminiIgnoreParser.ts new file mode 100644 index 0000000000..8518923de4 --- /dev/null +++ b/packages/core/src/utils/geminiIgnoreParser.ts @@ -0,0 +1,81 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import ignore from 'ignore'; + +export interface GeminiIgnoreFilter { + isIgnored(filePath: string): boolean; + getPatterns(): string[]; +} + +export class GeminiIgnoreParser implements GeminiIgnoreFilter { + private projectRoot: string; + private patterns: string[] = []; + private ig = ignore(); + + constructor(projectRoot: string) { + this.projectRoot = path.resolve(projectRoot); + this.loadPatterns(); + } + + private loadPatterns(): void { + const patternsFilePath = path.join(this.projectRoot, '.geminiignore'); + let content: string; + try { + content = fs.readFileSync(patternsFilePath, 'utf-8'); + } catch (_error) { + // ignore file not found + return; + } + + this.patterns = (content ?? '') + .split('\n') + .map((p) => p.trim()) + .filter((p) => p !== '' && !p.startsWith('#')); + + this.ig.add(this.patterns); + } + + isIgnored(filePath: string): boolean { + if (this.patterns.length === 0) { + return false; + } + + if (!filePath || typeof filePath !== 'string') { + return false; + } + + if ( + filePath.startsWith('\\') || + filePath === '/' || + filePath.includes('\0') + ) { + return false; + } + + const resolved = path.resolve(this.projectRoot, filePath); + const relativePath = path.relative(this.projectRoot, resolved); + + if (relativePath === '' || relativePath.startsWith('..')) { + return false; + } + + // Even in windows, Ignore expects forward slashes. + const normalizedPath = relativePath.replace(/\\/g, '/'); + + if (normalizedPath.startsWith('/') || normalizedPath === '') { + return false; + } + + return this.ig.ignores(normalizedPath); + } + + getPatterns(): string[] { + return this.patterns; + } +} diff --git a/packages/core/src/utils/gitIgnoreParser.test.ts b/packages/core/src/utils/gitIgnoreParser.test.ts index 25faf88920..1990332091 100644 --- a/packages/core/src/utils/gitIgnoreParser.test.ts +++ b/packages/core/src/utils/gitIgnoreParser.test.ts @@ -33,14 +33,16 @@ describe('GitIgnoreParser', () => { await fs.rm(projectRoot, { recursive: true, force: true }); }); - describe('initialization', () => { - it('should initialize without errors when no .gitignore exists', async () => { + describe('Basic ignore behaviors', () => { + beforeEach(async () => { await setupGitRepo(); - expect(() => parser.loadGitRepoPatterns()).not.toThrow(); }); - it('should load .gitignore patterns when file exists', async () => { - await setupGitRepo(); + it('should not ignore files when no .gitignore exists', async () => { + expect(parser.isIgnored('file.txt')).toBe(false); + }); + + it('should ignore files based on a root .gitignore', async () => { const gitignoreContent = ` # Comment node_modules/ @@ -50,52 +52,28 @@ node_modules/ `; await createTestFile('.gitignore', gitignoreContent); - parser.loadGitRepoPatterns(); - - expect(parser.getPatterns()).toEqual([ - '.git', - 'node_modules/', - '*.log', - '/dist', - '.env', - ]); expect(parser.isIgnored(path.join('node_modules', 'some-lib'))).toBe( true, ); expect(parser.isIgnored(path.join('src', 'app.log'))).toBe(true); expect(parser.isIgnored(path.join('dist', 'index.js'))).toBe(true); expect(parser.isIgnored('.env')).toBe(true); + expect(parser.isIgnored('src/index.js')).toBe(false); }); it('should handle git exclude file', async () => { - await setupGitRepo(); await createTestFile( path.join('.git', 'info', 'exclude'), 'temp/\n*.tmp', ); - parser.loadGitRepoPatterns(); - expect(parser.getPatterns()).toEqual(['.git', 'temp/', '*.tmp']); expect(parser.isIgnored(path.join('temp', 'file.txt'))).toBe(true); expect(parser.isIgnored(path.join('src', 'file.tmp'))).toBe(true); - }); - - it('should handle custom patterns file name', async () => { - // No .git directory for this test - await createTestFile('.geminiignore', 'temp/\n*.tmp'); - - parser.loadPatterns('.geminiignore'); - expect(parser.getPatterns()).toEqual(['temp/', '*.tmp']); - expect(parser.isIgnored(path.join('temp', 'file.txt'))).toBe(true); - expect(parser.isIgnored(path.join('src', 'file.tmp'))).toBe(true); - }); - - it('should initialize without errors when no .geminiignore exists', () => { - expect(() => parser.loadPatterns('.geminiignore')).not.toThrow(); + expect(parser.isIgnored('src/file.js')).toBe(false); }); }); - describe('isIgnored', () => { + describe('isIgnored path handling', () => { beforeEach(async () => { await setupGitRepo(); const gitignoreContent = ` @@ -107,7 +85,6 @@ src/*.tmp !src/important.tmp `; await createTestFile('.gitignore', gitignoreContent); - parser.loadGitRepoPatterns(); }); it('should always ignore .git directory', () => { @@ -205,8 +182,6 @@ src/*.tmp }); it('should handle nested .gitignore files correctly', async () => { - parser.loadGitRepoPatterns(); - // From root .gitignore expect(parser.isIgnored('root-ignored.txt')).toBe(true); expect(parser.isIgnored('a/root-ignored.txt')).toBe(true); @@ -230,34 +205,27 @@ src/*.tmp expect(parser.isIgnored('a/d/f/g')).toBe(true); expect(parser.isIgnored('a/f/g')).toBe(false); }); - - it('should correctly transform patterns from nested gitignore files', () => { - parser.loadGitRepoPatterns(); - const patterns = parser.getPatterns(); - - // From root .gitignore - expect(patterns).toContain('root-ignored.txt'); - - // From a/.gitignore - expect(patterns).toContain('/a/b'); // /b becomes /a/b - expect(patterns).toContain('/a/**/c'); // c becomes /a/**/c - - // From a/d/.gitignore - expect(patterns).toContain('/a/d/**/e.txt'); // e.txt becomes /a/d/**/e.txt - expect(patterns).toContain('/a/d/f/g'); // f/g becomes /a/d/f/g - }); }); describe('precedence rules', () => { - it('should prioritize root .gitignore over .git/info/exclude', async () => { + beforeEach(async () => { await setupGitRepo(); + }); + + it('should prioritize nested .gitignore over root .gitignore', async () => { + await createTestFile('.gitignore', '*.log'); + await createTestFile('a/b/.gitignore', '!special.log'); + + expect(parser.isIgnored('a/b/any.log')).toBe(true); + expect(parser.isIgnored('a/b/special.log')).toBe(false); + }); + + it('should prioritize .gitignore over .git/info/exclude', async () => { // Exclude all .log files await createTestFile(path.join('.git', 'info', 'exclude'), '*.log'); // But make an exception in the root .gitignore await createTestFile('.gitignore', '!important.log'); - parser.loadGitRepoPatterns(); - expect(parser.isIgnored('some.log')).toBe(true); expect(parser.isIgnored('important.log')).toBe(false); expect(parser.isIgnored(path.join('subdir', 'some.log'))).toBe(true); @@ -266,15 +234,4 @@ src/*.tmp ); }); }); - - describe('getIgnoredPatterns', () => { - it('should return the raw patterns added', async () => { - await setupGitRepo(); - const gitignoreContent = '*.log\n!important.log'; - await createTestFile('.gitignore', gitignoreContent); - - parser.loadGitRepoPatterns(); - expect(parser.getPatterns()).toEqual(['.git', '*.log', '!important.log']); - }); - }); }); diff --git a/packages/core/src/utils/gitIgnoreParser.ts b/packages/core/src/utils/gitIgnoreParser.ts index 4ac101978e..21d83651a5 100644 --- a/packages/core/src/utils/gitIgnoreParser.ts +++ b/packages/core/src/utils/gitIgnoreParser.ts @@ -6,93 +6,38 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; -import ignore, { type Ignore } from 'ignore'; -import { isGitRepository } from './gitUtils.js'; +import ignore from 'ignore'; export interface GitIgnoreFilter { isIgnored(filePath: string): boolean; - getPatterns(): string[]; } export class GitIgnoreParser implements GitIgnoreFilter { private projectRoot: string; - private ig: Ignore = ignore(); - private patterns: string[] = []; - private readonly maxScannedDirs = 200; + private cache: Map = new Map(); + private globalPatterns: string[] | undefined; constructor(projectRoot: string) { this.projectRoot = path.resolve(projectRoot); } - loadGitRepoPatterns(): void { - if (!isGitRepository(this.projectRoot)) return; - - // Always ignore .git directory regardless of .gitignore content - this.addPatterns(['.git']); - - this.loadPatterns(path.join('.git', 'info', 'exclude')); - this.findAndLoadGitignoreFiles(this.projectRoot); - } - - private findAndLoadGitignoreFiles(startDir: string): void { - const queue: string[] = [startDir]; - let scannedDirs = 0; - let queueHead = 0; - - while (queueHead < queue.length && scannedDirs < this.maxScannedDirs) { - const dir = queue[queueHead]; - queueHead++; - scannedDirs++; - - const relativeDir = path.relative(this.projectRoot, dir); - - // For sub-directories, check if they are ignored before proceeding. - // The root directory (relativeDir === '') should not be checked. - if (relativeDir && this.isIgnored(relativeDir)) { - continue; - } - - // Load patterns from .gitignore in the current directory - const gitignorePath = path.join(dir, '.gitignore'); - if (fs.existsSync(gitignorePath)) { - this.loadPatterns(path.relative(this.projectRoot, gitignorePath)); - } - - // Recurse into subdirectories - try { - const entries = fs.readdirSync(dir, { withFileTypes: true }); - for (const entry of entries) { - if (entry.name === '.git') { - continue; - } - if (entry.isDirectory()) { - queue.push(path.join(dir, entry.name)); - } - } - } catch (_error) { - // ignore readdir errors - } - } - } - - loadPatterns(patternsFileName: string): void { - const patternsFilePath = path.join(this.projectRoot, patternsFileName); + private loadPatternsForFile(patternsFilePath: string): string[] { let content: string; try { content = fs.readFileSync(patternsFilePath, 'utf-8'); } catch (_error) { - // ignore file not found - return; + return []; } - // .git/info/exclude file patterns are relative to project root and not file directory - const isExcludeFile = - patternsFileName.replace(/\\/g, '/') === '.git/info/exclude'; + const isExcludeFile = patternsFilePath.endsWith( + path.join('.git', 'info', 'exclude'), + ); + const relativeBaseDir = isExcludeFile ? '.' - : path.dirname(patternsFileName); + : path.dirname(path.relative(this.projectRoot, patternsFilePath)); - const patterns = (content ?? '') + return content .split('\n') .map((p) => p.trim()) .filter((p) => p !== '' && !p.startsWith('#')) @@ -150,12 +95,6 @@ export class GitIgnoreParser implements GitIgnoreFilter { return newPattern; }) .filter((p) => p !== ''); - this.addPatterns(patterns); - } - - private addPatterns(patterns: string[]) { - this.ig.add(patterns); - this.patterns.push(...patterns); } isIgnored(filePath: string): boolean { @@ -163,11 +102,8 @@ export class GitIgnoreParser implements GitIgnoreFilter { return false; } - if ( - filePath.startsWith('\\') || - filePath === '/' || - filePath.includes('\0') - ) { + const absoluteFilePath = path.resolve(this.projectRoot, filePath); + if (!absoluteFilePath.startsWith(this.projectRoot)) { return false; } @@ -186,13 +122,68 @@ export class GitIgnoreParser implements GitIgnoreFilter { return false; } - return this.ig.ignores(normalizedPath); + const ig = ignore(); + + // Always ignore .git directory + ig.add('.git'); + + // Load global patterns from .git/info/exclude on first call + if (this.globalPatterns === undefined) { + const excludeFile = path.join( + this.projectRoot, + '.git', + 'info', + 'exclude', + ); + this.globalPatterns = fs.existsSync(excludeFile) + ? this.loadPatternsForFile(excludeFile) + : []; + } + ig.add(this.globalPatterns); + + const pathParts = relativePath.split(path.sep); + + const dirsToVisit = [this.projectRoot]; + let currentAbsDir = this.projectRoot; + // Collect all directories in the path + for (let i = 0; i < pathParts.length - 1; i++) { + currentAbsDir = path.join(currentAbsDir, pathParts[i]); + dirsToVisit.push(currentAbsDir); + } + + for (const dir of dirsToVisit) { + const relativeDir = path.relative(this.projectRoot, dir); + if (relativeDir) { + const normalizedRelativeDir = relativeDir.replace(/\\/g, '/'); + if (ig.ignores(normalizedRelativeDir)) { + // This directory is ignored by an ancestor's .gitignore. + // According to git behavior, we don't need to process this + // directory's .gitignore, as nothing inside it can be + // un-ignored. + break; + } + } + + if (this.cache.has(dir)) { + const patterns = this.cache.get(dir); + if (patterns) { + ig.add(patterns); + } + } else { + const gitignorePath = path.join(dir, '.gitignore'); + if (fs.existsSync(gitignorePath)) { + const patterns = this.loadPatternsForFile(gitignorePath); + this.cache.set(dir, patterns); + ig.add(patterns); + } else { + this.cache.set(dir, []); // Cache miss + } + } + } + + return ig.ignores(normalizedPath); } catch (_error) { return false; } } - - getPatterns(): string[] { - return this.patterns; - } }