@infinisil made the change :)
Thakns for the review

Note that:

This is useful if you want to easily iterate or concatStringsSep the list offiles all found within a directory. (i.e. when constructing Java's CLASSPATH)

sounds likely to create accidental import from derivation.

Note that:

This is useful if you want to easily iterate or concatStringsSep the list offiles all found within a directory. (i.e. when constructing Java's CLASSPATH)

sounds likely to create accidental import from derivation.

Can you elaborate ? Is it because the returned lists are PATH and not string ?

Ah that sounds like what @roberth was commenting at in another PR of mine at #100660 (comment)

I had the impression that if you had the outPath interpolated then it must have built. I guess Nix is smart enough to discern between simply referencing the /nix/store entry and reading it.

Thank you for the clarification.

I really don't like the listFilesRecursive, which is not very grammatically correct. Maybe listTree or something like that?

It's also not clear that the API is very good. E.g. it takes a single dir argument, but maybe we'd want to add other arguments in the future (like a filter argument to stop descent into certain directories), which the current interface precludes. So we're sure to end up with a gazillion variants of this function.

BTW we're currently adding lib functions in a fairly ad hoc way, with no naming or API conventions. Maybe we can take some inspiration from Rust. (They probably also don't add standard library functions after one day of review. Everything we add here is technical debt that we need to support forever.)

For filtering we can make it accept a cleanSourceWith result in addition to paths.
I had that in mind when reviewing it, but I dismissed it because I couldn't think of a good place to document unimplemented features. I guess I could have suggested an inline comment.

I'm not sure if filtering is needed, because the expensive part of builtins.path that requires filtering is the store importing. This function however only lists files (like find), which isn't very expensive, even for TB big directories.

And the result can still be filtered with just the list filter

No, it is because you can’t know what is in the result of a build before you build it, but the evaluation is a distinct phase before any building happens.
…
On Tue, Oct 20, 2020, at 7:28 PM, Farid Zakaria wrote: > Note that: >> This is useful if you want to easily iterate or concatStringsSep the list offiles all found within a directory. (i.e. when constructing Java's CLASSPATH) > sounds likely to create accidental import from derivation. Can you elaborate ? Is it because the returned lists are PATH and not string ? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#101096 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAASXLBCFURKPDJXLFJ2FWTSLYMIHANCNFSM4SWYGOPQ.

For anyone curious about @grahamc feedback I wrote a little writeup on it https://fzakaria.com/2020/10/20/nix-parallelism-import-from-derivation.html

@edolstra I had the same intention as @infinisil that the filtering can always happen afterwards on the full list.
Happy to augment it though.
Funny you mentioned readTree since that was my first draft of the function name before renaming it to recursive with feedback on this PR to match other parts of nixpkgs (mapAttrsRecursive); so even when trying to align names we fail ;)

Happy to make any changes we land on.