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

imports don't resolve symlinks absolutely, leading to unexpected behaviour #2109

Open
infinisil opened this issue Apr 25, 2018 · 16 comments
Open
Labels
feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc

Comments

@infinisil
Copy link
Member

Have a look at the following directory structure:

.
├── 1
│   ├── content.nix                       content: "In 1"
│   └── default.nix -> ../2/default.nix
└── 2
    ├── content.nix                       content: "In 2"
    └── default.nix                       content: import ./content.nix

Now this is what happens with evaluation:

$ nix-instantiate --eval 1
"In 1"
$ nix-instantiate --eval 2
"In 2"

However what I expect to happen is

$ nix-instantiate --eval 1
"In 2"
$ nix-instantiate --eval 2
"In 2"

because the symlink should have influenced all imports as well.

This can lead to unexpected behaviour. A good example of this is if I were to have a directory like this (with /path/to/nixpkgs being a nixpkgs checkout):

.
└── nixos -> /path/to/nixpkgs/nixos

Then trying to use this folder will fail:

$ nix-instantiate nixos/release-combined.nix -A tested
error: opening file '/home/infinisil/Test/nixsym/default.nix': No such file or directory
@dezgeg
Copy link
Contributor

dezgeg commented Apr 25, 2018

Another case to consider is the same directory layout as in the first example but with an explicit /default.nix suffix:

$ nix-instantiate --eval 1/default.nix
"In 2"
$ nix-instantiate --eval 2/default.nix 
"In 2"

i.e. the expected result. So I agree that there is a bug at least in the way that at minimum the result should be consistent.

@chreekat
Copy link
Contributor

Interesting. For me, the existing behavior is the expected behavior. Symlinks represent real files in real directories, and shouldn't serve as portals to elsewhere. I opened #2150 because nix-shell is tunneling through the portal, and that surprised me.

I guess now someone gets to figure out which of us has the "wrong" intuition. :)

@kirelagin
Copy link
Member

I actually came here to report the discrepancy that @dezgeg pointed out and before reading this issue I was on the same side as @chreekat.

But I have to admit that the nixos example is very convincing and it made me think about the standard practices of handling symlinks in terms of context. From my experiments if I do:

$ ln -s /bin .
$ ls bin/..

the output is the listing of the root directory, not of the current one, as I would expect, which means that my and @chreekat’s intuition is actually wrong and the standard practice is for symlinks to actually serve as portals 🤷‍♂️.

@stale
Copy link

stale bot commented Feb 23, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Feb 23, 2021
@ony
Copy link
Contributor

ony commented Mar 30, 2021

it looks even weirder with

.
├── 42.nix                            content: 42
├── dir
│   ├── ref.nix                       content: (import ../42.nix)
│   └── subdir -> .
└── ref.nix -> dir/ref.nix

Next works fine and returns 42

run nix eval '(import ./dir/ref.nix)'
run nix eval '(import ./ref.nix)'  # follows symlink into folder `dir` and subsequent `import ../42.nix` is relative to it

But when symlink is in the middle of path rather than terminal it computes it from path with symlink unresolved

run nix eval '(import ./dir/subdir/ref.nix)'
nix run nixpkgs-unstable.nixFlakes -c nix --experimental-features nix-command eval --impure --expr '(import ./dir/subdir/ref.nix)'

Causing error like:

error: while evaluating the file '/tmp/ws/example/dir/subdir/ref.nix':
getting status of '/tmp/ws/example/dir/42.nix': No such file or directory

Tried both 2.3.10 and 2.4pre20210317_8a5203d versions of Nyx.

P.S. Real use-case affected by this is when ~/.config/nixpkgs/overlays folder is a symlink to git repo folder with files that have relative imports to say ../lib.

@stale stale bot removed the stale label Mar 30, 2021
@ony
Copy link
Contributor

ony commented Mar 30, 2021

My best guess is this code for #resolveExprPath that reads only symlink at location pointed by path without considering intermediate paths.

Note that POSIX.1-2001 conformant systems have realpath that should do all resolves in a correct manner.

ony added a commit to ony/nix that referenced this issue Mar 31, 2021
ony added a commit to ony/nix that referenced this issue Mar 31, 2021
ony added a commit to ony/nix that referenced this issue Mar 31, 2021
@ony
Copy link
Contributor

ony commented Mar 31, 2021

#4678 fixes scenarios mentioned in this issue. But there is a problem with #canonPath in general since it require symlink resolution to produce correct result. In particular /abc/def/../default.nix without may get symlink /abc/def ignored, which will make a wrong path in case if link traverses some folders rather be just an "alias" for sibling folder.

@roberth
Copy link
Member

roberth commented Mar 31, 2021

I'm not surprised by the current behavior and I don't think the nixos symlink is a compelling example, because the same thing would happen if you were to copy nixos to somewhere. In my view, symlinks aren't "meant" to have any special meaning at the application level. The problems described here can be fixed trivially by using small files that consist of simple import expressions, which do have well-defined behavior in Nix.

./nixos.nix:

import /path/to/nixpkgs/nixos

If you need more than one file from the nixos tree, you can factor out the path into a separate file.
100% defined; no confusion about the meaning of symlinks.

@ony
Copy link
Contributor

ony commented Mar 31, 2021

@roberth, there is system behavior of symlinks and in POSIX systems they should be resolved for each path component incrementally from left to right. If I understand correctly Nix will not work on other systems anyway (at least C++ implementation).

Beside that behavior of Nix in terms of relative imports clearly indicates attempt to have same behavior, but before #4678 it had it inconsistent depending on where symlinks appears in path making only last element resolved and following relative imports with traversing will affect that.


More obvious bug of using #canonPath :

cat dir/subdir/../42.nix

returns 42

nix eval '(builtins.readFile ./dir/subdir/../42.nix)'

returns error: opening file '/tmp/ws/example/dir/42.nix': No such file or directory because somewhere we try to canonicalize paths in Nix without resolving symlinks

Unfortunately it is not fixed with #4678 because it is other places where call either #absPath or #canonPath


Work-around you suggest will work only for files. To emulate correct behavior of symlinked ~/.config/nixpkgs/overlays (one I hit) you'll have to re-generate those files whenever content of linked dir changes. I also considered use of overlays.nix, but overlays function is not exposed and I don't want to re-implement it.

@roberth
Copy link
Member

roberth commented Mar 31, 2021

Yeah, I'm not saying it's perfect. That's impossible with symlinks.
Just be aware that this is a breaking change. I'm ok with that because if someone chooses to use symlinks, they're inviting a bunch of complexity. You could even warn about symlinks as far as I'm concerned.

(OT: ~/.config/nixpkgs was a bad idea. It defies eval reproducibility. Some evaluation modes may deny it.)

@ony
Copy link
Contributor

ony commented Mar 31, 2021

Handling symlinks supposed to be transparent and that's why you don't usually have to write #canonPath yourself.
I can agree with warning in case if Nix cannot correctly handle this like in case we want to canonicalize path with traversals (..) that is not present on filesystem. But otherwise it should be fine.

My guess is that Nix wants to canonicalize paths that will appear in future in order to keep hashes insensitive to various forms of same path.
I'd suggest to treat such kinds of paths separately and ensure that "future paths" never use .. so they never rely on actual filesystem. Or simply never expand .. in paths at all.

Just checked os.path.realpath in Python 3 and it is lenient:

>>> os.path.realpath('dir/missing-subdir/../42.nix')
'/tmp/ws/example/dir/42.nix'
>>> os.path.realpath('dir/subdir/../42.nix')
'/tmp/ws/example/42.nix'

But I don't think Nix should follow that.

@stale
Copy link

stale bot commented Oct 2, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Oct 2, 2021
@infinisil
Copy link
Member Author

(I still think this should be considered)

@Ericson2314
Copy link
Member

It would be big breaking change, but one we should opt-into and then someday make the default.

@keithy
Copy link

keithy commented Mar 14, 2022

I just designed a configuration structure using symlinks and got bitten.

@stale stale bot removed the stale label Mar 14, 2022
@fricklerhandwerk fricklerhandwerk added language The Nix expression language; parser, interpreter, primops, evaluation, etc feature Feature request or proposal and removed improvement labels Oct 6, 2022
@jtojnar
Copy link
Contributor

jtojnar commented Nov 5, 2023

I noticed at least one case where this was fixed (regression in 2.16): #9298

Unfortunately, it did not fix the case from OP:

$ mkdir test test/{1,2}
ln -s ../2/default.nix test/1
echo '"In 1"' > test/1/content.nix
echo '"In 2"' > test/2/content.nix
echo 'import ./content.nix' > test/2/default.nix
$ nix-instantiate --eval test/1
"In 1"
$ nix-instantiate --eval test/2
"In 2"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc
Projects
None yet
Development

No branches or pull requests