filterSource support #34
Conversation
|
Can you add some more motivation and a problem description to the commit messages? I don’t understand why we have to re-implement nixpkgs functions. |
|
better? |
|
The first commit is especially what I’m unsure about, it looks like it changes the watcher logic somewhat. |
|
Added a motivation to the first commit. |
The result of that may not change, but the content of the watched path may still change and there's really nothing preventing you from doing a |
|
The dependency will still be found when nix coerces /tmp/test 1 job
$ tree
.
├── foo
└── shell.nix
0 directories, 2 files
/tmp/test 1 job
$ cat shell.nix
with import <nixpkgs> {};
mkShell {
envvar = lib.concatMapStrings (name: (./. + ("/"+name))) (lib.attrNames (builtins.readDir ./.));
}
/tmp/test 1 job
$ nix-instantiate shell.nix -vv |& grep "copied source '/tmp"
copied source '/tmp/test/foo' -> '/nix/store/avva3pijyyj4bgrj1vcl3sj6hiadjxlj-foo'
copied source '/tmp/test/shell.nix' -> '/nix/store/mgn25m4drjdw2j9pay17jv4ln1bkd3k8-shell.nix' |
…readDir Currently if a nix file depends on `builtins.readDir path`, `path` is recursively watched. This is wasteful because if "path/foo/bar" is modified, `builtins.readDir path` does not change. We should reevaluate only if the list of direct children of `path` changed. This patch separates paths to be watched in paths to be watched recursively and patches to be watched non recursively. The latter is used for builtins.readDir.
nix does not log anything when evaluating builtins.filterSource, contrary to normal paths. We have to wrap builtins.filterSource with builtins.trace like builtins.readFile. The easy solution would be to trace "lorri read: $path", but then we also watch filtered out files. Reimplementing the full logic of filterSource in nix would be cumbersome, but we can find a treadeoff. This implementation is designed to ignore .git when using nix-gitignore: the top-level directory is watched as if it was readDir'ed, and only the children which are kept by filterSource (ie, not .git) are watched recursively. This way you can run git fetch in the filterSource'd directory and not reevaluate. I suppose nix-gitignore accounts for a large part of filterSource usage, as filterSource is kind of painful to use.
|
I fixed the merge conflict. |
There was a problem hiding this comment.
This looks amazing!
Once upon a time I didn’t have a good mental state of how readDir works, so we were watching over-zealously, but this should be just fine.
I can imagine the filterSource improvement to make a quite big difference on e.g. javascript source code, where node_modules can be easily 100k files, which will run out of fds immediately.
| # if all files were kept, let's log only ${path} | ||
| logTopLevel = builtins.trace "lorri read: '${toString path}'" result; | ||
| # otherwise log all kept files individually, and the listing of the toplevel dir | ||
| log1stLevel = builtins.trace "lorri readdir: '${toString path}'" (builtins.foldl' (acc: name: builtins.trace "lorri read: '${toString path}/${name}'" acc) result kept); |
There was a problem hiding this comment.
This is the tradeoff right? If a recursive directory is kept, it will be watched recursively.
Still, in combination with the previous commit a strict improvement!
There was a problem hiding this comment.
Ah, not quite. input sources were still watched 100%, so both commits are an improvement to the filtering logic. Awesome.
release.nix
Outdated
| { | ||
| version = 872; | ||
| changes = '' | ||
| Add support for builtins.filterSource |
There was a problem hiding this comment.
This is also missing the first commit, not watching readDir recursively by default is a huge win.
|
hum the failing test passes locally... |
|
Gosh our file watching tests are flaky |
Motivation: use nix-gitignore and not have to rebuild on each git fetch
I decided accurately respecting the filter in filterSource was too complex, so this implements an approximation.
This way, .git it ignored.
release.nix(seerelease.nixfor instructions)