Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions crates/rstack-binding/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ impl GitIgnoreMatcher {
Self::default()
}

/// Releases compiled rules and matching state after directory traversal.
#[napi]
pub fn clear(&mut self) {
self.inner.clear();
}

/// Compiles or replaces rules rooted at a repository-relative POSIX directory.
#[napi]
pub fn add_source(&mut self, relative_root: String, patterns: String) -> napi::Result<bool> {
Expand Down
47 changes: 43 additions & 4 deletions crates/rstack-ignore/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![deny(clippy::all)]

use std::{borrow::Cow, collections::HashMap, path::Path, path::PathBuf};
use std::{borrow::Cow, collections::HashMap, path::Path, path::PathBuf, sync::Arc};

use ignore::{
gitignore::{Gitignore, GitignoreBuilder},
Expand Down Expand Up @@ -138,6 +138,7 @@ impl IgnoreMatcher {
/// A hierarchy of repository `.gitignore` files keyed by their root-relative directories.
#[derive(Default)]
pub struct GitIgnoreMatcher {
compiled_patterns: HashMap<Box<str>, Arc<Gitignore>>,
matchers: HashMap<Box<str>, GitIgnoreSourceMatcher>,
// Traversal checks every file's parent, so cache both ignored and included directories.
ignored_directories: HashMap<Box<str>, bool>,
Expand All @@ -149,6 +150,11 @@ impl GitIgnoreMatcher {
Self::default()
}

/// Releases compiled rules and matching state after directory traversal.
pub fn clear(&mut self) {
*self = Self::default();
}

/// Adds or replaces the patterns rooted at a POSIX, repository-relative directory.
///
/// Returns whether the hierarchy contains any effective rules after the update.
Expand All @@ -158,7 +164,15 @@ impl GitIgnoreMatcher {
patterns: &str,
) -> Result<bool, ignore::Error> {
let relative_root = normalize_relative_path(relative_root);
let matcher = compile_patterns(patterns)?;
// Compiled rules use relative paths, so identical contents can share them across roots.
let matcher = if let Some(matcher) = self.compiled_patterns.get(patterns) {
Arc::clone(matcher)
} else {
let matcher = Arc::new(compile_patterns(patterns)?);
self.compiled_patterns
.insert(patterns.into(), Arc::clone(&matcher));
matcher
};

self.invalidate_directory_cache(relative_root);
if matcher.is_empty() {
Expand Down Expand Up @@ -329,12 +343,12 @@ impl GitIgnoreMatcher {
/// One `.gitignore` source with the directory state needed to reproduce
/// `ignore.test(path)` without re-walking ancestors for every file.
struct GitIgnoreSourceMatcher {
matcher: Gitignore,
matcher: Arc<Gitignore>,
directory_states: HashMap<Box<str>, Option<bool>>,
}

impl GitIgnoreSourceMatcher {
fn new(matcher: Gitignore) -> Self {
fn new(matcher: Arc<Gitignore>) -> Self {
Self {
matcher,
directory_states: HashMap::new(),
Expand Down Expand Up @@ -550,6 +564,31 @@ mod tests {
use super::{GitIgnoreMatcher, IgnoreMatcher, IgnoreSource};
use std::path::Path;

#[test]
fn identical_gitignore_contents_keep_independent_roots() {
let mut matcher = GitIgnoreMatcher::new();
let patterns = "/dist/\n";
matcher.add_source("a", patterns).unwrap();
matcher.add_source("b", patterns).unwrap();
for root in ["a", "b"] {
assert!(matcher.is_ignored(&format!("{root}/dist/index.js"), false));
assert!(!matcher.is_ignored(&format!("{root}/nested/dist/index.js"), false));
}
matcher.add_source("a", "").unwrap();
assert!(!matcher.is_ignored("a/dist/index.js", false));
assert!(matcher.is_ignored("b/dist/index.js", false));
assert!(!matcher.is_ignored("dist/index.js", false));
}

#[test]
fn clearing_gitignore_removes_rules_and_directory_state() {
let mut matcher = GitIgnoreMatcher::new();
matcher.add_source("", "dist/\n").unwrap();
assert!(matcher.is_ignored("dist/index.js", false));
matcher.clear();
assert!(!matcher.is_ignored("dist/index.js", false));
}

#[test]
fn keeps_independent_ignore_sources_isolated() {
let mut matcher = IgnoreMatcher::new([
Expand Down
2 changes: 2 additions & 0 deletions packages/rstack/binding.d.cts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export declare const __napiBindingTarget: 'native' | 'wasm32-wasi' | 'wasm32-was
export declare class GitIgnoreMatcher {
/** Creates an empty matcher whose sources can be added during directory traversal. */
constructor()
/** Releases compiled rules and matching state after directory traversal. */
clear(): void
/** Compiles or replaces rules rooted at a repository-relative POSIX directory. */
addSource(relativeRoot: string, patterns: string): boolean
/** Returns whether one repository-relative POSIX path is ignored. */
Expand Down
116 changes: 71 additions & 45 deletions packages/rstack/src/fmt/discoverPaths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,25 @@ class GitIgnoreFiles {

static async create(cwd: string): Promise<GitIgnoreFiles> {
const matcher = new GitIgnoreFiles(await findGitRoot(cwd));
await matcher.loadThrough(cwd);
return matcher;
try {
await matcher.loadThrough(cwd);
return matcher;
} catch (error) {
await matcher.waitForLoads();
matcher.dispose();
throw error;
}
}

async waitForLoads(): Promise<void> {
// A failed operation may leave other ignore files still loading.
await Promise.allSettled(this.#loads.values());
}

dispose(): void {
this.#matcher?.clear();
this.#matcher = undefined;
this.#loads.clear();
}

async loadThrough(directoryPath: string): Promise<void> {
Expand Down Expand Up @@ -642,53 +659,62 @@ const discoverFmtPaths = async ({

if (traversalRoots.length) {
const gitIgnore = await GitIgnoreFiles.create(cwd);
const results = await Promise.all(
traversalRoots.map(async (rootPath) => {
const stats = await lstatSafe(rootPath);
if (!stats?.isDirectory()) {
return [];
}
const traversals = traversalRoots.map(async (rootPath) => {
const stats = await lstatSafe(rootPath);
if (!stats?.isDirectory()) {
return [];
}

await gitIgnore.loadThrough(rootPath);
if (
gitIgnore.isIgnored(rootPath, true) ||
isIgnored?.(rootPath, true) === true
) {
return [];
}
await gitIgnore.loadThrough(rootPath);
if (
gitIgnore.isIgnored(rootPath, true) ||
isIgnored?.(rootPath, true) === true
) {
return [];
}

const includesAll = directoryRoots.some((directoryPath) =>
isPathInside(directoryPath, rootPath),
);
const isIncluded = includesAll
? undefined
: (filePath: string): boolean => {
if (
directoryRoots.some((directoryPath) =>
isPathInside(directoryPath, filePath),
)
) {
return true;
}

const relativePath = toPosixPath(resolveRelativePath(filePath));
return globMatchers.some((matches) => matches(relativePath));
};

return discoverDirectoryFiles(
rootPath,
gitIgnore,
ignoredDirNames,
isIncluded,
isIgnored,
);
}),
);
const includesAll = directoryRoots.some((directoryPath) =>
isPathInside(directoryPath, rootPath),
);
const isIncluded = includesAll
? undefined
: (filePath: string): boolean => {
if (
directoryRoots.some((directoryPath) =>
isPathInside(directoryPath, filePath),
)
) {
return true;
}

const relativePath = toPosixPath(resolveRelativePath(filePath));
return globMatchers.some((matches) => matches(relativePath));
};

return discoverDirectoryFiles(
rootPath,
gitIgnore,
ignoredDirNames,
isIncluded,
isIgnored,
);
});

for (const files of results) {
for (const filePath of files) {
candidates.add(filePath);
try {
const results = await Promise.all(traversals);

for (const files of results) {
for (const filePath of files) {
candidates.add(filePath);
}
}
} catch (error) {
// A rejected traversal must not release rules still used by another root.
await Promise.allSettled(traversals);
await gitIgnore.waitForLoads();
throw error;
} finally {
gitIgnore.dispose();
}
}

Expand Down
68 changes: 68 additions & 0 deletions packages/rstack/tests/fmt/discoveryCleanup.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import path from 'node:path';
import { setImmediate } from 'node:timers/promises';
import { afterEach, expect, rs, test } from 'rstack/test';
import { discoverFmtPaths } from '../../src/fmt/discoverPaths.ts';
import { loadNativeBinding } from '../../src/native/index.ts';
import { withTempProject, writeProjectFile } from './helpers.ts';

const mocks = rs.hoisted(() => ({ readdir: rs.fn() }));
rs.mock('tiny-readdir', () => ({ default: mocks.readdir }));

afterEach(() => {
rs.restoreAllMocks();
mocks.readdir.mockReset();
});

test('waits for pending traversals before releasing rules after a failure', async () => {
await withTempProject(async (rootPath) => {
writeProjectFile(rootPath, '.gitignore', '*.log\n');
const firstFile = writeProjectFile(rootPath, 'first/index.ts');
const secondFile = writeProjectFile(rootPath, 'second/index.ts');
const pending = Promise.withResolvers<{ files: string[] }>();
const started = Promise.withResolvers<void>();
const error = new Error('Traversal failed');
const clear = rs.spyOn(
loadNativeBinding().GitIgnoreMatcher.prototype,
'clear',
);

mocks.readdir.mockImplementation((directory: string) => {
if (directory === path.dirname(firstFile)) {
return Promise.reject(error);
}
started.resolve();
return pending.promise;
});

const discovery = discoverFmtPaths({
cwd: rootPath,
patterns: ['first', 'second'],
});
const completion = expect(discovery).rejects.toBe(error);
try {
await started.promise;
// Let a sibling rejection propagate while this traversal is still pending.
await setImmediate();
expect(clear).not.toHaveBeenCalled();
} finally {
pending.resolve({ files: [secondFile] });
await completion;
}
expect(clear).toHaveBeenCalledTimes(1);
});
});

test('releases rules when initialization fails', async () => {
await withTempProject(async (rootPath) => {
writeProjectFile(rootPath, '.gitignore', '*.log\n');
const prototype = loadNativeBinding().GitIgnoreMatcher.prototype;
const error = new Error('Invalid ignore source');
rs.spyOn(prototype, 'addSource').mockImplementation(() => {
throw error;
});
const clear = rs.spyOn(prototype, 'clear');

await expect(discoverFmtPaths({ cwd: rootPath })).rejects.toBe(error);
expect(clear).toHaveBeenCalledTimes(1);
});
});
Loading