Add Automatic Gitignore Support

[technicalpickles]

Summary

Adds automatic .gitignore support to pks, respecting gitignored files during analysis. Enabled by default with option to disable for packwerk-identical behavior.

Motivation

Previously, pks analyzed all files regardless of .gitignore status, including build artifacts (node_modules/), temporary files (scratch/), and logs. This meant:

• Unnecessary analysis of ignored files
• Manual exclusion patterns needed
• Behavior inconsistent with git-aware tools

What Changed

For users with .gitignore files:

• Gitignored files are now automatically skipped
• Performance improvement from pruning ignored directories early
• No manual exclusion configuration needed

Configuration:

# packwerk.yml - disable to match packwerk behavior
respect_gitignore: false  # default: true

Supports:

• Local .gitignore files
• Global gitignore (~/.gitignore_global or core.excludesFile)
• .git/info/exclude
• All standard patterns (wildcards, negation, etc.)

Implementation

• Uses ignore crate (same as ripgrep) for battle-tested performance
• Prunes ignored directories during traversal (no stat calls)
• Gracefully handles missing/malformed gitignore files
• Works correctly in non-git directories

Testing

• 10 integration tests covering core scenarios
• 7 unit tests for helper functions
• Test fixtures validate common patterns and edge cases
• All existing tests pass (no regressions)

Troubleshooting

Files unexpectedly ignored?

# Check what's matching
git check-ignore -v path/to/file.rb

# Temporarily disable
echo "respect_gitignore: false" >> packwerk.yml

See ADVANCED_USAGE.md for detailed documentation.

Backward Compatibility

✅ Existing projects work without changes
✅ Can be disabled if needed
✅ Non-git directories continue working
⚠️ Behavior change: gitignored files now skipped (generally desirable)

---

Ready for review! This brings pks behavior in line with git-aware tooling while providing an escape hatch for compatibility.

[Copilot]

Pull Request Overview

This PR adds automatic .gitignore support to pks, making it respect gitignored files during analysis by default. Users can disable this behavior to maintain packwerk-identical functionality.

• Automatically excludes gitignored files from violation checking and file listing
• Adds respect_gitignore configuration option (defaults to true)
• Supports local, global, and git-specific gitignore files with full pattern support

Reviewed Changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/gitignore_test.rs	Comprehensive integration tests covering gitignore functionality scenarios
tests/fixtures/app_with_gitignore_disabled/*	Test fixture demonstrating disabled gitignore behavior
tests/fixtures/app_with_gitignore/*	Test fixture with gitignore patterns for testing
src/packs/walk_directory.rs	Core gitignore implementation with matcher building and directory walking integration
src/packs/raw_configuration.rs	Configuration schema extension for respect_gitignore option
src/packs.rs	Module visibility change to expose walk_directory functions
README.md	Documentation updates describing gitignore support
Cargo.toml	Added ignore and tempfile dependencies
ADVANCED_USAGE.md	Detailed gitignore configuration and troubleshooting guide

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[perryqh]

Performance looks good!

[perryqh]

Ah! The whole purpose of expand_tilde is to support core.excludesFile

[technicalpickles]

Yes! Added comments explaining that to the function

[perryqh]

Given the check below:

  if gitignore.matched(&relative_path, false).is_ignore() {
      continue;
  }

should we even attempt check is_ignore on line 260 if is_dir == false?

[technicalpickles]

Good catch! Implemented in 8387c3b. The first gitignore check now only runs when is_dir == true since:

• Setting read_children_path = None on files is a no-op (files have no children)
• Files are already checked in the main loop (line ~304)
• Eliminates redundant matched() calls

[Copilot]

Pull Request Overview

Copilot reviewed 40 out of 41 changed files in this pull request and generated 9 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The expand_tilde function only checks the HOME environment variable, which works on Unix-like systems (Linux, macOS) but not on Windows. On Windows, the home directory is typically stored in the USERPROFILE environment variable, or as a combination of HOMEDRIVE and HOMEPATH.

Consider using a cross-platform solution:

pub fn expand_tilde(path: &str) -> PathBuf {
    if let Some(stripped) = path.strip_prefix("~/") {
        // Try HOME first (Unix/Linux/macOS)
        if let Some(home) = std::env::var_os("HOME") {
            return PathBuf::from(home).join(stripped);
        }
        // Fall back to USERPROFILE (Windows)
        if let Some(home) = std::env::var_os("USERPROFILE") {
            return PathBuf::from(home).join(stripped);
        }
    }
    PathBuf::from(path)
}

Alternatively, consider using the home or dirs crate for proper cross-platform home directory detection.

Suggested change

	if let Some(stripped) = path.strip_prefix("~/") {
	if let Some(home) = std::env::var_os("HOME") {
	return PathBuf::from(home).join(stripped);
	}
	if let Some(stripped) = path.strip_prefix("~/") {
	// Try HOME first (Unix/Linux/macOS)
	if let Some(home) = std::env::var_os("HOME") {
	return PathBuf::from(home).join(stripped);
	}
	// Fall back to USERPROFILE (Windows)
	if let Some(home) = std::env::var_os("USERPROFILE") {
	return PathBuf::from(home).join(stripped);
	}
	// Try HOMEDRIVE + HOMEPATH (Windows legacy)
	let homedrive = std::env::var_os("HOMEDRIVE");
	let homepath = std::env::var_os("HOMEPATH");
	if let (Some(hd), Some(hp)) = (homedrive, homepath) {
	let mut home = PathBuf::from(hd);
	home.push(hp);
	return home.join(stripped);
	}

[Copilot]

The cleanup code at lines 357-373 won't execute if any assertion fails before it (e.g., at line 332-335 or 351-355). This could leave the global git config in a modified state, potentially affecting other tests or the developer's environment.

Consider using a guard or defer-like pattern to ensure cleanup always happens:

// After line 320, add a cleanup guard
struct GitConfigGuard {
    original: Option<String>,
    test_file: PathBuf,
    global_gitignore: PathBuf,
}

impl Drop for GitConfigGuard {
    fn drop(&mut self) {
        // Restore git config
        if let Some(ref orig) = self.original {
            if !orig.is_empty() {
                StdCommand::new("git")
                    .args(["config", "--global", "core.excludesFile", orig])
                    .status()
                    .ok();
            }
        } else {
            StdCommand::new("git")
                .args(["config", "--global", "--unset", "core.excludesFile"])
                .status()
                .ok();
        }
        // Remove test files
        fs::remove_file(&self.test_file).ok();
        fs::remove_file(&self.global_gitignore).ok();
    }
}

let _guard = GitConfigGuard {
    original: original_excludes_file,
    test_file: test_file.clone(),
    global_gitignore: global_gitignore.clone(),
};

This ensures cleanup happens even if the test panics or assertions fail.

[Copilot]

The test references ignored_folder/violating.rb in the app_with_gitignore fixture (lines 13, 19), but this file is not included in the PR changes. The file appears to exist only in the app_with_gitignore_disabled fixture.

Either:

1. The ignored_folder/violating.rb file needs to be added to the app_with_gitignore fixture directory, or
2. The test expectations need to be adjusted if the file is intentionally missing (but this seems unlikely given the test's purpose)

[Copilot]

This troubleshooting advice incorrectly states that include patterns can override gitignore patterns. As noted in a previous comment on lines 64-76, the implementation actually checks gitignore first (with a continue statement), so gitignored files are skipped before include patterns are evaluated.

This suggestion should be removed or corrected to indicate that negation patterns in .gitignore (step 4) are the proper way to un-ignore specific files, not include patterns in packwerk.yml.

[Copilot]

This statement is incorrect. Based on the implementation, include patterns do NOT override gitignore patterns. The gitignore check happens before include pattern matching, so gitignored files will never reach the include pattern check.

The comment at line 189 "This will include ALL .rb files, even if gitignored" is misleading - gitignored .rb files will still be skipped regardless of this include pattern.

This section should be corrected to accurately reflect the actual behavior.

Suggested change

	- "**/*.rb" # This will include ALL .rb files, even if gitignored
	- "**/*.rb" # This will include all non-gitignored .rb files; gitignored files will still be excluded

[Copilot]

The comment "highest priority - override gitignore" is incorrect. Based on the implementation, gitignore patterns are checked before include patterns, so gitignored files cannot be overridden by include patterns.

This comment should be corrected to:

# Include patterns (what file extensions to analyze)

Suggested change

	# Include patterns (highest priority - override gitignore)
	# Include patterns (what file extensions to analyze)

[Copilot]

[nitpick] The comment references "line ~304" for where files are checked, but this is brittle as line numbers can change. Consider using a more descriptive reference, such as:

// Check gitignore for directories only (optimization: prune ignored directory trees early)
// Files are checked separately in the main file processing loop below

Suggested change

	// Files are checked separately in the main loop below (see line ~304)
	// Files are checked separately in the main file processing loop below

[Copilot]

There's unnecessary double wrapping with Arc. The gitignore_matcher is already Option<Arc<Gitignore>>, and then it's wrapped again with Arc::new(gitignore_matcher) at line 200, resulting in Arc<Option<Arc<Gitignore>>>.

Consider simplifying to:

// Build gitignore matcher if enabled
let gitignore_matcher = if raw.respect_gitignore {
    match build_gitignore_matcher(&absolute_root) {
        Ok(matcher) => Some(matcher),
        Err(e) => {
            debug!("Failed to build gitignore matcher: {}. Continuing without gitignore support.", e);
            None
        }
    }
} else {
    None
};

let gitignore_ref = Arc::new(gitignore_matcher);
let gitignore_ref_for_loop = gitignore_ref.clone();

This way, the type becomes Arc<Option<Gitignore>> instead of Arc<Option<Arc<Gitignore>>>, eliminating unnecessary indirection and reference counting overhead.

[Copilot]

The documentation claims that "Include patterns" have highest priority and can override gitignore patterns (line 68-74), but this is incorrect based on the implementation.

In the code (src/packs/walk_directory.rs lines 305-310), gitignored files are skipped with a continue statement before the include patterns are checked (line 328). This means gitignored files cannot be overridden by include patterns.

The actual precedence order is:

1. Gitignore patterns - Files/directories in .gitignore are skipped first (if respect_gitignore: true)
2. Include patterns - Files must match include: patterns to be considered
3. Exclude patterns - Files matching exclude: are filtered out
4. Default exclusions - Applied during directory traversal

The documentation should be corrected to reflect this actual behavior, or the implementation should be changed to allow include patterns to override gitignore (though the current behavior seems more reasonable for most use cases).