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

Regression in builtins.pathExists #8838

Closed
lucasew opened this issue Aug 16, 2023 · 6 comments · Fixed by #8869 or #9022
Closed

Regression in builtins.pathExists #8838

lucasew opened this issue Aug 16, 2023 · 6 comments · Fixed by #8869 or #9022
Labels

Comments

@lucasew
Copy link

lucasew commented Aug 16, 2023

Describe the bug

After Nix 2.13.3 something happened and broke builtins.pathExists behaviour.

Steps To Reproduce

  1. touch /tmp/test
  2. nix repl
  3. builtins.pathExists /tmp/test/.
    It will return true, should be false.

Expected behavior
It should return false.

nix-env --version output
nix-env (Nix) 2.17.0

Additional context
Nix 2.13.3 (tested on /nix/store/nd8dpdgpjkh3g3vvcjx9qdw7mjzz0l90-nix-2.13.3/bin/nix) doesn't have this issue.

This bug breaks impure overlays in nixpkgs. Relevant part of the code: https://github.com/NixOS/nixpkgs/blob/d28fdbe9742c680ec410bd70bf2962b90a374d36/pkgs/top-level/impure.nix#L40C1-L41C1

Priorities

Add 👍 to issues you find important.

@lucasew lucasew added the bug label Aug 16, 2023
@lucasew
Copy link
Author

lucasew commented Aug 16, 2023

image

lucasew added a commit to lucasew/nixcfg that referenced this issue Aug 16, 2023
Signed-off-by: lucasew <lucas59356@gmail.com>
@iFreilicht
Copy link
Contributor

From the linked commit I assume that this issue does not exist in 2.15.2?

@lucasew
Copy link
Author

lucasew commented Aug 17, 2023

From the linked commit I assume that this issue does not exist in 2.15.2?

Yes. 2.13.x was the version I was using before I bumped nixpkgs yesterday and I tried pinning to 2.15.2 that also doesn't have the issue.

@infinisil
Copy link
Member

Bisecting (with some good guessing) tracks this down to 94812cc from #8172

@infinisil
Copy link
Member

Note that this only changed behavior when passing strings to pathExists, so e.g. builtins.pathExists "/some/file/" now returns true when it returned false before. In comparison builtins.pathExists /some/file/ also returned true before.

This should definitely be fixed somehow.

roberth added a commit to hercules-ci/nix that referenced this issue Aug 25, 2023
edolstra added a commit that referenced this issue Sep 1, 2023
…sDir

Fix #8838, pathExists: isDir when ends with `/ `
github-actions bot pushed a commit that referenced this issue Sep 1, 2023
Fixes #8838

(cherry picked from commit 1e08e12)
github-actions bot pushed a commit that referenced this issue Sep 1, 2023
Fixes #8838

(cherry picked from commit 1e08e12)
edolstra added a commit that referenced this issue Sep 1, 2023
[Backport 2.17-maintenance] Fix #8838, pathExists: isDir when ends with `/ `
edolstra added a commit that referenced this issue Sep 1, 2023
[Backport 2.16-maintenance] Fix #8838, pathExists: isDir when ends with `/ `
@solson
Copy link
Member

solson commented Sep 1, 2023

This issue was closed, but it is IMO still broken after #8869 because it only checks for trailing /, and doesn't handle cases like /.. As a consequence, Nixpkgs overlays are still broken because of this line: https://github.com/NixOS/nixpkgs/blob/9ace7896297b51fee5ad34c1977bec1e6541c089/pkgs/top-level/impure.nix#L40

At this point it's probably better to focus on NixOS/nixpkgs#249165, but I thought it worth commenting here because Nix is the cause of the regression, not Nixpkgs.

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