Resolve symlinks when symlink points to file in same directory #689
Conversation
|
Almost exactly the same bug is described in rbenv#868 as well. |
|
Could you update the changes as exactly same as rbenv#868 to avoid conflict? |
|
I'm not sure that rbenv#868 actually works properly, since it won't actually break out of the loop. |
|
Actually, I take that back. I can update it so it looks the same as rbenv#868, except that rbenv#868 was never merged into rbenv, either, so there's not actually a conflict. |
I encountered a bug in `abs_dirname` when using pyenv. I had a file
structure that looked like this:
/dept/is/dev/mdippery/opt/bin/asciidoc -> ../Cellar/asciidoc/bin/asciidoc
/dept/is/dev/mdippery/opt/Cellar/asciidoc/bin/asciidoc -> asciidoc.py
/dept/is/dev/mdippery/opt/Cellar/asciidoc/bin/asciidoc.py
pyenv's `abs_dirname` was encountering an error when it tried to resolve
this system of symlinks. When `abs_dirname
/dept/is/dev/mdippery/opt/bin/asciidoc` called, the following steps
would happen:
1. `$path` was set to `/dept/is/dev/mdippery/opt/bin/asciidoc`.
2. Because `[ -n $path ]` was true, pyenv would enter the for loop on
Line 42 of `~/.pyenv/libexec/pyenv`.
3. `$parent` would be set to `/dept/is/dev/mdippery/opt/bin`.
4. pyenv would `cd` to `$parent`.
5. `resolve_link asciidoc` would be called, and return
`../Cellar/asciidoc/bin/asciidoc`, which would be set to `$path`.
6. The for loop would start again, since `[ -n $path ]` is still true.
7. `$parent` would be set to `../Cellar/asciidoc/bin`.
8. pyenv would `cd` to `$parent`.
9. `resolve_link asciidoc` would be called, and return `asciidoc.py`,
which would be set to `$path`.
10. The for loop would start again, since `[ -n $path ]` is still true.
11. _Here's the bug:_ `$parent` would be set to `asciidoc.py`, since
that is what `${path%/*}` returns when `$path` is simply
`asciidoc.py`, with no parent information.
12. pyenv would naïvely assume that is a directory, and attempt to
`cd` to it. _This causes an error to be thrown._
The simplest solution seems to be to check whether the "basename" of a
path is the same as its "parent" (i.e., there is a symlink that points
to another file in the same directory), and break from the loop if that
is the case.
|
I have this wrong output also when running |
|
Yeah, came across my old PR (rbenv/rbenv#868) for this again. |
| cd "${path%/*}" | ||
| local parent="${path%/*}" | ||
| if [ "$path" = "$parent" ]; then break; fi | ||
| cd "$parent" |
There was a problem hiding this comment.
Breaking is wrong here: the local file might be a symlink itself that needs to get resolved.
Without this patch you would see the following error for a symlink like
"/usr/bin/asciidoc -> asciidoc.py":
> …/libexec/{py,rb}env: line 43: cd: asciidoc.py: Not a directory
Ref: sstephenson/bats#224
Ref: pyenv/pyenv#689
Closes: rbenv#868
|
A PR with a test is ready in rbenv itself now: rbenv/rbenv#868 (comment). |
|
btw: a workaround, which also improves performance is to run |
|
|
|
Yeah, I have to pick that up again. |
|
If I understood the rbenv discussion, mislav didn't think this was a relevant use case. On the pyenv side at least, I should point out that the bug shows up in the last few releases of ansible (i.e. note that in the bin directory, most executables are symlinks to |
|
Closing in favor of #1216. |
btw: that does not seem to be the case anymore, right? It happens for pyenv because pyenv uses abs_dirname for the file argument, which rbenv does not. |
I encountered a bug in
abs_dirnamewhen using pyenv. I had a file structure that looked like this:pyenv's
abs_dirnamewas encountering an error when it tried to resolve this system of symlinks. Whenabs_dirname /dept/is/dev/mdippery/opt/bin/asciidoccalled, the following steps would happen:$pathwas set to/dept/is/dev/mdippery/opt/bin/asciidoc.[ -n $path ]was true, pyenv would enter the for loop on Line 42 of~/.pyenv/libexec/pyenv.$parentwould be set to/dept/is/dev/mdippery/opt/bin.cdto$parent.resolve_link asciidocwould be called, and return../Cellar/asciidoc/bin/asciidoc, which would be set to$path.[ -n $path ]is still true.$parentwould be set to../Cellar/asciidoc/bin.cdto$parent.resolve_link asciidocwould be called, and returnasciidoc.py, which would be set to$path.[ -n $path ]is still true.$parentwould be set toasciidoc.py, since that is what${path%/*}returns when$pathis simplyasciidoc.py, with no parent information.cdto it. This causes an error to be thrown.The simplest solution seems to be to check whether the "basename" of a path is the same as its "parent" (i.e., there is a symlink that points to another file in the same directory), and break from the loop if that is the case.
This fixes a bug that is referenced by #580, #588, and #607. A fix was also supplied in #502, but was never merged. (This fix is a bit different and may work better.)