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

New builtin: pathType #3096

Closed
roberth opened this issue Sep 18, 2019 · 10 comments
Closed

New builtin: pathType #3096

roberth opened this issue Sep 18, 2019 · 10 comments
Labels
feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc stale

Comments

@roberth
Copy link
Member

roberth commented Sep 18, 2019

I'm proposing to add a new builtins.pathType that returns the node type, similar to the values returned by builtins.readDir.

Motivation

Avoid having to read a directory when all you need is a single path's node type.

Spec

Like builtins.pathExists, its argument is a path or string that may or may not exist on the filesystem.

Given argument p return value is

  • null when the path p does not point to an existing filesystem node
  • a value equal to (builtins.readDir (dirOf p))."$(baseNameOf p)", such as "directory", "regular", "symlink"

Advantages

  • Performance: no need to read the entire directory (may be big, like /nix/store)
  • Less strict: readDir (dirOf p) may not be allowed by dir permissions
  • Decreases the file watch scope for tools like lorri
@stale
Copy link

stale bot commented Feb 18, 2021

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

@stale stale bot added the stale label Feb 18, 2021
@stale
Copy link

stale bot commented Apr 28, 2022

I closed this issue due to inactivity. → More info

@stale stale bot closed this as completed Apr 28, 2022
@RuRo
Copy link

RuRo commented Jul 18, 2022

I would like to reopen this issue.

The current implementation has a significant drawback. In pure evaluation mode (flakes) it is AFAIK currently impossible to get the type for a top level store path. For example, pathType /nix/store/something fails with

error: access to absolute path '/nix/store' is forbidden in pure eval mode (use '--impure' to override)

since it tries to read the whole /nix/store which is obviously impure.

@roberth
Copy link
Member Author

roberth commented Jul 18, 2022

pathType on a store path is generally not something you'll want to do, because it blocks the evaluator, causing the evaluation heap to exist for longer than necessary (wasting memory not usable by builds) and creating a choke point for parallelism.

Most of the time, it's possible to check the type of the store path in the builder that uses that information.

For example, don't do

runCommand "foo" { } ''
  ${optionalString (pathType dep == "directory") ''
    echo dep is a directory
  ''}
''

but instead do

runCommand "foo" { } ''
  if [[ -d ${dep} ]]; then
    echo dep is a directory;
  fi
''

@RuRo
Copy link

RuRo commented Jul 18, 2022

Unfortunately, this doesn't work for me (and probably won't work for everybody). There are cases, where pathType is needed at expression evaluation time, not at build/run time, so it can't be offloaded to the build step. For example, running lib.filesystem.listFilesRecursive on some source tree and importing the .nix files.

Also, IMHO, it doesn't matter whether getting the pathType of store paths is a good practice or not. You should be able to get the type of whatever/path/foo without listing the contents of whatever/path.

I am currently working around this issue by adding an extra directory level to my flake, so I am not too invested in this issue, but I still think, that this ought to work.

@roberth
Copy link
Member Author

roberth commented Jul 18, 2022

Right, it used to be that you could just avoid putting your project sources in the store (except for derivation sources, by necessity). I hope #6530 makes that work a little better, but I don't assume it does. Reopening.

@roberth roberth reopened this Jul 18, 2022
@mkenigs
Copy link
Contributor

mkenigs commented Aug 5, 2022

+1 I ran into this as well

@stale stale bot removed the stale label Aug 5, 2022
@fricklerhandwerk fricklerhandwerk added the language The Nix expression language; parser, interpreter, primops, evaluation, etc label Dec 12, 2022
@thufschmitt thufschmitt moved this to 🏗 To discuss in Nix team Dec 16, 2022
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2022-12-16-nix-team-meeting-minutes-17/24120/1

@edolstra edolstra removed this from Nix team Jan 2, 2023
@stale stale bot added the stale label Jun 18, 2023
@thufschmitt
Copy link
Member

Fixed by #7447

@roberth
Copy link
Member Author

roberth commented Jul 31, 2023

@RuRo lib.pathType falls back to builtins.readDir if your Nix doesn't have the new primop.

Perhaps we should consider again

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 stale
Projects
None yet
Development

No branches or pull requests

7 participants