Skip to content

Fix gitignore ancestor detection for git worktrees#1424

Merged
webpro merged 5 commits into
webpro-nl:mainfrom
altendky:detect-git-worktree-roots
Jan 2, 2026
Merged

Fix gitignore ancestor detection for git worktrees#1424
webpro merged 5 commits into
webpro-nl:mainfrom
altendky:detect-git-worktree-roots

Conversation

@altendky
Copy link
Copy Markdown
Contributor

Git worktrees use a .git file (containing a pointer to the actual git
directory) instead of a .git directory. The gitignore ancestor traversal
was only checking for directories, causing it to incorrectly continue
searching beyond the worktree root into the parent repository.

This adds an isGitRoot() helper that checks for both .git directory
and .git file, fixing the boundary detection for worktrees.

Git worktrees use a `.git` file (containing a pointer to the actual git
directory) instead of a `.git` directory. The gitignore ancestor traversal
was only checking for directories, causing it to incorrectly continue
searching beyond the worktree root into the parent repository.

This adds an `isGitRoot()` helper that checks for both `.git` directory
and `.git` file, fixing the boundary detection for worktrees.
@altendky
Copy link
Copy Markdown
Contributor Author

following up on my spam in #990

@altendky altendky marked this pull request as ready for review December 28, 2025 03:03
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Jan 1, 2026

Open in StackBlitz

npm i https://pkg.pr.new/knip@1424

commit: 7998413

git doesn't allow .git files to be committed so it has to be created
@altendky
Copy link
Copy Markdown
Contributor Author

altendky commented Jan 1, 2026

i fixed some of the basic test failures. i forgot to make sure git included the .git file for the tests, which it did not. i'm not sure about some other failures that had occurred.

Copy link
Copy Markdown
Member

@webpro webpro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Hopefully we can get the test issues fixed.


// Create the .git file for worktree tests (git won't track files named .git)
const worktreeRoot = resolve('fixtures/glob-worktree/root');
copyFileSync(join(worktreeRoot, 'dot-git'), join(worktreeRoot, '.git'));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably good to clean up as well, also see

await fs.rename(join(cwd, '.git'), join(cwd, '_git'));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7998413 made it not-global as well

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! What I meant is to clean up after the test ran, but it does not seem to matter, so I guess we're good here.

Organize git worktree tests into a describe block and move the .git file
setup from module-level into test.before() for better test isolation.
@webpro webpro merged commit 67a93c6 into webpro-nl:main Jan 2, 2026
29 checks passed
@webpro
Copy link
Copy Markdown
Member

webpro commented Jan 2, 2026

Thanks Kyle! Way to go 😉

@webpro
Copy link
Copy Markdown
Member

webpro commented Jan 2, 2026

🚀 This pull request is included in v5.79.0. See Release 5.79.0 for release notes.

Using Knip in a commercial project? Please consider becoming a sponsor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants