Skip to content
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.

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

New .git Handling has no opt out #1457

Closed
raphael-ratepay opened this issue Dec 21, 2023 · 16 comments · Fixed by #1540
Closed

New .git Handling has no opt out #1457

raphael-ratepay opened this issue Dec 21, 2023 · 16 comments · Fixed by #1540

Comments

@raphael-ratepay
Copy link

I have the following code:

fd --type d --base-directory ~/Source -H '\.git' | sed 's|/.git||'

which lists all folders that contain a .git folder (basically a project search). This worked fine in 8.x.x but breaks in 9.x.x due to the breaking change of the .git handling.

Sadly there is no option to opt out for this. because --no-ignore-vcs does bring back all ignored files (so .terraform, .terragrunt-cache, .node-modules, ...) which I do not want. I just want to include ".git" but not everything .gitignore

How can this be achieved? Could you bring back the old behaviour with some cli flag?

@sharkdp
Copy link
Owner

sharkdp commented Dec 21, 2023

@skoriop FYI

@tmccombs
Copy link
Collaborator

@raphael-ratepay in the example you gave I don't think adding --no-ignore-vcs would work, unless you had a .git directory inside a directory that was itself gitignored. Although, it would still search ignored files like node_modules which could hurt performance.

One possible way we could address is this add a --ignore-pattern option that lets you add ignore patterns inline (like the inverse of the --glob option in ripgrep), and then you could do fd --ignore-pattern '!.git' ... to add it back.

p.s. what you currently have would also find something like a dir/.github directory and change it to dirhub you probably want a $ at the end of your pattern.

BerkeleyTrue added a commit to BerkeleyTrue/dotfiles that referenced this issue Jan 1, 2024
Use an overlay to pre 9 of fd until the below is fixed

sharkdp/fd#1457
@jlucktay
Copy link

jlucktay commented Jan 2, 2024

I was just bitten by this as well. Would like to be able to discover the paths of git repositories, whilst still honouring their respective .gitignore files, please. 🙏

@Console32
Copy link

Any update on this, I could provide an MR for --ignore-pattern if that is desired?

@tmccombs
Copy link
Collaborator

tmccombs commented Jan 8, 2024

Actually fd already has an option like --ignore-pattern: --exclude.

Unfortunately, using --exclude !.git doesn't work, due to how overrides are implemented in the ignore crate (since it takes a list of "include" rules, we prefix the input with "!" to get an exclude rule).

I tried implementing an --include option that negates a previous exclude or ignore rule, but ran into two problems:

  1. The excluded !.git rule is higher priority than the included .git priority. I can sort of hack around this by checking if ".git" was given, and if so, don't add "!.git"
  2. If I add a non-negated pattern to the overrides, then it restricts results to only directories, and files that match that pattern.

I'm not sure what the best path forward is with the current API of the ignore crate.

See also BurntSushi/ripgrep#2705.

@raphael-ratepay
Copy link
Author

None of the options sounds Great, could a flag like --legacy-git-handling be implement that is checked here (this clutters configuration, and is not ideal either)

@tavianator
Copy link
Collaborator

I also tried for a bit to make something like -E '!.git' work but I don't think it's possible. I think the two options are:

  • Add an option to restore the old behaviour, something like --no-ignore-vcs-dirs
  • Revert the breaking change, and just tell people to add -E .git if they need the 9.0.0 behaviour

I also thought of this compromise, which might work for the people in this thread but is semantically kinda weird:

  • Make fd -H include .git itself, but nothing under it

By the way, this line is also wrong because .git may be a regular file, not a directory, for things like submodules and worktrees.

fd/src/walk.rs

Line 338 in 0dc3342

builder.add("!.git/").expect("Invalid exclude pattern");

@raphael-ratepay
Copy link
Author

Make fd -H include .git itself, but nothing under it.

That actually sounds like a good option, that does not break anybody else workflow?

@mehalter
Copy link

@raphael-ratepay you can achieve this with:

fd -HI '^.git$' --type d --base-directory ~/Source | sed 's|/.git||'

@raphael-ratepay
Copy link
Author

@mehalter that does not work as expected.

Using -I includes all node_modules, .terraform, .... folders as well - so it returns all hidden dependencies. Which is not the case in version 8.7 if used without -I

@mehalter
Copy link

Oh yeah that's true @raphael-ratepay ! My bad!

@raphael-ratepay
Copy link
Author

Any updates / Ideas? I can propose a PR if I know in which direction :D

@sharkdp
Copy link
Owner

sharkdp commented Feb 7, 2024

I agree we should fix this rather sooner than later, especially if we want to revert the breaking change.

I think the two options are:
* Add an option to restore the old behaviour, something like --no-ignore-vcs-dirs
* Revert the breaking change, and just tell people to add -E .git if they need the 9.0.0 behaviour

A third option might be to make the -E '!.git' thing work, that several people have tried. It doesn't work out-of-the box, but we could probably special-case it? In the sense that we would not add the .git pattern to the ignore builder in the first place.

I'm personally okay with all options, but if we can make -E '!.git' work, I'd favor that over introducing yet another flag, I think.

Given that this new behavior has a risk for confusion (.git is a hidden folder and it is not gitignored by default), I might be slightly in favor of reverting the breaking change. Other thoughts?

@tmccombs
Copy link
Collaborator

tmccombs commented Feb 7, 2024

I'm also fine with any of those options.

@raphael-ratepay
Copy link
Author

I expect that most of the users are in favour of the new behaviour, so I think rollback is not the best option. If nobody obliges I would add --no-ignore-vcs-dirs as PR.

@tmccombs
Copy link
Collaborator

I'm torn on this.

Reasons I think automatically ignoring .git is good:

  • It is consistent with the behavior of git commands. For example, git ls-files --cached --others --exclude-standard doesn't inlcude .git. In fact, even without --exclude-standard it doesn't include .git.
  • The only use cases I've seen brought up are specifically looking for .git folders, which is somewhat niche.
  • This is something that has been requested by a few people: Include .git/ folder in the "gitignored" filtering #1387

Reasons it is bad:

Maybe we should revert, but add some documentation about the workaround of adding .git to your ignore file?

tmccombs added a commit to tmccombs/fd that referenced this issue Apr 28, 2024
tmccombs added a commit to tmccombs/fd that referenced this issue Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants