NixOS / nix

Nix, the purely functional package manager
https://nixos.org/
GNU Lesser General Public License v2.1
12.32k stars 1.49k forks source link

nix flakes: flakes in subdirectories can access files in parent directories (breaking the implied seal of flake.nix) #4414

Open colemickens opened 3 years ago

colemickens commented 3 years ago

Describe the bug

This came up while thinking about #4413. I was looking at these commands:

❯ ref="$(git ls-remote git@github.com:colemickens/redact master|cut -d'  ' -f1)"
❯ nix run "git+ssh://git@github.com/colemickens/redact?rev=$ref&dir=examples/test01" --no-write-lock-file
Hello, world!

And wondered if this meant that nix could be smart enough to only download a small subset of git objects as the VFSForGit progresses, and it could opt to only clone that specific subdirectory. This prompted me to think of edge cases and consider what that might break -- it would break any places that a flake in a subdirectory reached into a parent or sibling directory.

And it seems that this is allowed, based on a test:

❯ cd redact
❯ tree     
.
└── examples
    ├── test01
    │   ├── flake.lock
    │   └── flake.nix
    └── test.txt

❯ cat test.txt
contents_of_testtxt

❯ nix run "git+ssh://git@github.com/colemickens/redact?rev=$ref&dir=examples/test01" --no-write-lock-file
contents_of_testtxt

❯ cd examples/test01; nix run . --no-write-lock-file
contents_of_testtxt

To me this seems a bit surprising. When I see a flake.nix I expect that it seals everything next to, and beneath it. This behavior violates that assumption.

Steps To Reproduce

  1. make a git repo
  2. make two subdirs
  3. place a valid flake.nix in each subdir
  4. place a file outside those subdirs
  5. use builtins.readFile somehow in one of the flakes to reach "up" and read the file

Note that it works.

Expected behavior

Somehow, nix is able to isolate the directory containing flake.nix to maintain the implied seal of flake.nix

nix-env --version output

Additional context

Alternatively, I could be entirely off the mark and this might intentionally be allowed, but it strikes me as sort of a pretty big mis-match of my expectations when I see a flake.nix.

cole-h commented 3 years ago

I always figured it was limited to the git repository, not necessarily the directory holding the flake.nix/flake.lock.

colemickens commented 3 years ago

@cole-h I'm sort of split-brained on this. After filing and sleeping on it, this functionality is potentially useful to me.

I think the thing is when this is taken together with #4413. Consider Docker. Docker allows you to place the Dockerfile somewhere else in the repo, but the "build context" directory is still where the docker build command is actually invoked.

So, imagine this:

❯ tree     
.
└── examples
    ├── test01
    │   └── Dockerfile
    ├── data
    │   ├── image.png

I would issue docker build -f ./test01/Dockerfile to use the repo as the build context, and to use a specific path from the build context. On the other hand, nix forces us to be IN the subdirectory and then is able to reach out of that dir.

I also do acknowledge that Nix doesn't have to work exactly like Docker and that maybe this doesn't matter that much.

Kha commented 3 years ago

Having access to the whole repo could potentially be useful in combination with #3978. Rust repositories for example often have a root crate depending on crates in subdirectories, which again may depend on other subdirectory crates not necessarily contained within their own folder.

maisiliym commented 3 years ago

Breaking up inputs is a big step to achive substantially higher energy-efficiency. See https://github.com/NixOS/nix/issues/4217#issuecomment-723834593

As @kha implied, a lot of repos out there are incorrectly structured, with crates accessing files in parent directories, because git has become the de-facto smallest unit of data, which is absurd since it enforces absolutely no logic on content. Treesitter does this.

Nix is perfectly positionned to attack this widespread problem right at the spine, if it could break up any directory(repo) into components, or compose directories from components (git-submodules problem), without deep-cloning data.

Here's a mock-up API: https://github.com/maisiliym/aidiyz/blob/9ccac4a716343724a453e14b631521eed1200ff9/mkSubModulesSrc.nix

edolstra commented 3 years ago

This is actually intended behaviour. A flake in a subdirectory is allowed to access other directories in the same input tree. (For instance, I have NixOS configurations in subdirectory flakes that access modules like ../common/stuff.nix.)

This means that the flake reference /path?dir=foo is not equivalent to /path/foo since the latter doesn't have access to /path.

edolstra commented 3 years ago

Maybe we want an alternative attribute to dir that really extracts a subtree (e.g. /path?subtree=foo).

nixos-discourse commented 3 years ago

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/smoothing-the-flakes-learning-curve/11667/2

stale[bot] commented 3 years ago

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

rehno-lindeque commented 2 years ago

Maybe we want an alternative attribute to dir that really extracts a subtree (e.g. /path?subtree=foo).

I think subtree is very close to sparseCheckout recently added to fetchgit / fetchFromGithub: See also https://github.com/NixOS/nix/issues/5811.

marksisson commented 1 year ago

It seems something like /path?subtree=foo may still needed. I am working with a flake repo that has sub-flakes, and the sub-flakes are referenced by the top-level flake using /path/foo syntax. But if someone directly references the sub-flake from outside the repo with /path?dir=foo they are surprised that the results are different (mainly around how self is evaluated).

lolbinarycat commented 6 months ago

note that subdirectories can simply specify the parent directory as a flake input (or even a non-flake input) to break this isolation anyways. basically just take self.url and strip the dir argument, then import that (unsure if builtins.getFlake is available to flakes usually, if not, i'm sure there's a workaround)