Skip to content

fix: Properly handle directories that are nested within excluded directories #21373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
ChayimFriedman2 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: Properly handle directories that are nested within excluded directories #21373

ChayimFriedman2 wants to merge 2 commits into from

Conversation

@ChayimFriedman2
Copy link
Contributor

@ChayimFriedman2 ChayimFriedman2 commented Dec 31, 2025

That is /excluded is excluded, and /excluded/project is the project.

@ChayimFriedman2
Properly handle directories that are nested within excluded directories
f06e64d 
That is `/excluded` is excluded, and `/excluded/project` is the project.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 31, 2025
@ChayimFriedman2
Do not put target in excluded directories
93d4c4e 
It's a common practice to put `include!()`d files there.

Previously it was just ignored, but now we respect the exclude which causes trouble.
@ChayimFriedman2
Copy link
Contributor Author

ChayimFriedman2 commented Jan 1, 2026

Fixed the bug; see the second commit.

ShoyuVanilla
ShoyuVanilla approved these changes Jan 2, 2026
View reviewed changes
Copy link
Member

@ShoyuVanilla ShoyuVanilla left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I agree that not excluding ./target/ by default is the correct choice, as stated in the commit message. Excluding it would be even quite incorrect, since it is hard-coded rather than respecting the cargo config.

That said, I'm a bit concerned that this could overwhelm our vfs or project discovery in cases where large projects are cloned inside the target directory 😅

@Veykril
Copy link
Member

Veykril commented Jan 3, 2026

I am actually surprised we had target excluded to be honest 🤔

That said, I'm a bit concerned that this could overwhelm our vfs or project discovery in cases where large projects are cloned inside the target directory 😅

Yea that might cause issues to be fair. Can we exclude target but re-include the relevant subdirectories? (that is target/<profile>/build) To slight reduce this risk

@ChayimFriedman2
Copy link
Contributor Author

ChayimFriedman2 commented Jan 3, 2026

To be clear; target was already not excluded; the bug in the code prevented it from being.

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

Reviewers

@lnicola lnicola lnicola approved these changes

@ShoyuVanilla ShoyuVanilla ShoyuVanilla approved these changes

Assignees

No one assigned

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ChayimFriedman2 @Veykril @lnicola @ShoyuVanilla @rustbot