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

Cannot search in symlink #256

Closed
phiresky opened this issue Nov 27, 2016 · 7 comments
Closed

Cannot search in symlink #256

phiresky opened this issue Nov 27, 2016 · 7 comments
Labels
bug A bug.

Comments

@phiresky
Copy link
Contributor

phiresky commented Nov 27, 2016

db is a symlink to a directory.

$ rg --version
ripgrep 0.3.1
$ rg test db
db: Is a directory (os error 21)
$ rg test db/
(works as expected)

I would expect it to traverse the symlink in the first example, or at least to show an error message that is less confusing ;)

@BurntSushi
Copy link
Owner

Oh interesting. I'm pretty sure this used to work. I wonder if it's a regression in 0.3. I'm on mobile, but can you try rg -j1 test db? Does that work?

@phiresky
Copy link
Contributor Author

phiresky commented Nov 27, 2016 via email

@jFransham
Copy link

jFransham commented Nov 28, 2016

I tried to have a look at this issue since it looked like a decent beginner bug - looks like running rg with --follow fixes this issue. It seems like this is intended behaviour, at least to some extent, but it's undeniably unintuitive that somelink and somelink/ act differently. Maybe a fix would be to traverse symbolic links for all input paths, but then respect --follow when walking the resultant paths, but that might not play nice with globbing (like, you might accidentally end up traversing symlinks and causing a loop, and have no way to disable that without manually excluding the path).

I'd just make it so that it displays a better message, something like somelink: is a symlinked directory - run again with --follow to traverse symbolic links. If you're up for this I'll implement it (I'd like to start contributing to this project, since I use it daily).

@BurntSushi
Copy link
Owner

Maybe a fix would be to traverse symbolic links for all input paths

This is indeed the intended behavior I think.

(like, you might accidentally end up traversing symlinks and causing a loop, and have no way to disable that without manually excluding the path)

The only way a loop can happen is if you use --follow, and when you do that, the underlying recursive iterator will detect loops. I don't think you can wind up in a loop if you follow only symlinks that have been explicitly given though?

(I'd like to start contributing to this project, since I use it daily)

Great! I haven't actually looked into this bug yet (just got back from vaca), but I do think we should just do the right thing here and follow symlinks if they've been explicitly given as a positional parameter. If I've missed some complication here that you see and I don't, then don't hesitate to push back! :-)

@BurntSushi BurntSushi added the bug A bug. label Nov 28, 2016
@BurntSushi BurntSushi assigned BurntSushi and unassigned BurntSushi Nov 28, 2016
@BurntSushi
Copy link
Owner

@jFransham Let me know if you want to take a crack at this!

@jFransham
Copy link

@BurntSushi Yeah, I'll get on it now. I was wondering whether you'd hang on opening if you had a recursive set of links (i.e. not a directory that contains a link to itself, but a link that transitively links to itself) but it looks like the Linux kernel just throws an exception. I don't know what the behaviour is on other operating systems (*BSD, Windows, OS X) but I'd guess that at best it's the same and at worst it's not considered to be a problem for the application developer. Either way, that can be tackled as a seperate bug if it ever becomes a problem.

I'll get back with a PR soon.

BurntSushi added a commit to BurntSushi/walkdir that referenced this issue Dec 6, 2016
When given a root like `foo` where `foo` is a symlink, it should always
be followed. This behavior is consistent with `foo/`, which will also be
followed.

See also: BurntSushi/ripgrep#256
@jdub
Copy link

jdub commented Dec 11, 2016

Thanks for fixing this interaction bug in such a user friendly manner! 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug.
Projects
None yet
Development

No branches or pull requests

4 participants