Add setting to warn about copying/hashing large paths #10661
Conversation
github-actions
bot
added
the
store
edolstra
force-pushed
the
large-path-warning
branch
from
80b3fa4
to
5ab2897
Compare
edolstra
force-pushed
the
large-path-warning
branch
from
5ab2897
to
2fbf0cf
Compare
|
Generally, rather than a new option, could this have been a higher-verbosity-level output with a greppable message pattern? |
|
@fricklerhandwerk Yeah it could be done that way, but the main motivation is to get warnings when you aren't aware yet that you're doing something wrong (like having an attribute |
|
Yes I was wondering indeed why the default is maxint, because (at least in the past?) there has been a warning when the path is > 256 MB. Would make sense to leave it at that and simply make that configurable. |
| total += n; | ||
| return n; | ||
| } | ||
| }; |
There was a problem hiding this comment.
thoughts, optional
Previously we've used such a source or sink that warned as soon as the threshold was passed. I believe it counted the wrong thing, but that doesn't seem to be the case with its caller in this PR.
The benefit of warning as we go is that we can catch mistakes before the user has to wait for their store to fill up.
(For example, the backport action will instruct users to create nixpkgs/.worktree, which blows up ${nixpkgs} massively if not fetched via git, and it takes a long time to copy and produces no diagnostic feedback during that time)
Possible scheme and defaults:
- Add the first 1GB silently
- Print when passing 1GB
- Print size when larger than 256 MB (nixpkgs seems to be around 170)
Example log (brief)
copying very large path /home/user/nixpkgs to the store. Is this what you intended? See `<url>#conf-large-path-warning-limit`
copied /home/user/nixpkgs to the store (2.3 GiB)
There was a problem hiding this comment.
Another helpful feature is to have an enforced limit, which will also let us print a backtrace or launch the debugger.
There was a problem hiding this comment.
The benefit of warning as we go is that we can catch mistakes before the user has to wait for their store to fill up.
Nowadays this is already somewhat handled by the progress bar (which will show "copying to the store" for a long time if it's a large path). It could be improved by showing the amount of data copied, like a download.
This is useful for diagnosing whether an evaluation is copying large paths to the store. Example: $ nix build .#packages.x86_64-linux.default --large-path-warning-threshold 1000000 warning: copied large path '/home/eelco/Dev/nix-master/' to the store (6271792 bytes) warning: copied large path '«github:NixOS/nixpkgs/b550fe4b4776908ac2a861124307045f8e717c8e?narHash=sha256-7kkJQd4rZ%2BvFrzWu8sTRtta5D1kBG0LSRYAfhtmMlSo%3D»/' to the store (155263768 bytes) warning: copied large path '«github:libgit2/libgit2/45fd9ed7ae1a9b74b957ef4f337bc3c8b3df01b5?narHash=sha256-oX4Z3S9WtJlwvj0uH9HlYcWv%2Bx1hqp8mhXl7HsLu2f0%3D»/' to the store (22175416 bytes) warning: copied large path '/nix/store/z985088mcd6w23qwdlirsinnyzayagki-source' to the store (5885872 bytes)
Also always include the unit (i.e. "MiB" instead of "M").
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
edolstra
force-pushed
the
large-path-warning
branch
from
5f66462
to
5534682
Compare
| Setting<uint64_t> largePathWarningThreshold{ | ||
| this, | ||
| std::numeric_limits<uint64_t>::max(), | ||
| "large-path-warning-threshold", |
There was a problem hiding this comment.
Potential to improve discovery a bit by alphabetical grouping. Puts it next to warn-dirty.
| "large-path-warning-threshold", | |
| "warn-large-path-threshold", |
|
Oh, don't forget the release note. |
Motivation
This is useful for diagnosing whether an evaluation is copying large paths to the store. Example:
The default is to not warn about large paths.
Fixes #4034.
Context
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.