Add builtins.hashFile #2792
Add builtins.hashFile #2792
Conversation
| let | ||
| paths = [ ./this-file-is-definitely-not-there-7392097 "/and/neither/is/this/37293620" ]; | ||
| in | ||
| builtins.concatLists (map (hash: map (builtins.hashFile hash) paths) ["md5" "sha1" "sha256" "sha512"]) |
There was a problem hiding this comment.
Having more than one test here is probably pointless since evaluation will fail on the first one, if I'm not mistaken.
There was a problem hiding this comment.
You're right. But if you run make installcheck the test will fail i.e. the evaluation succeeds. But I think something else is wrong which makes the test fail.
For text files it is possible to do it like so: `builtins.hashString "sha256" (builtins.readFile /tmp/a)` but that doesn't work for binary files. With builtins.hashFile any kind of file can be conveniently hashed.
JohnAZoidberg
force-pushed
the
builtins-hash-file
branch
from
4f99b0e
to
3f192ac
Compare
|
I have no objection to this change, but just wanted to suggest that we might want to have changes to the nix language (including adding builtins) go through an RFC in the future, so that more eyes could view the changes and think of the overall consistency of the nix language than there are people watching the nix repo :) |
Yes, good idea. Didn't think about it for long because it's not a big change. @edolstra wait! Sorry, I should've probably called the PR WIP. The test that should fail, succeeds. At least on my system. Can you confirm? |
For text files it is possible to do it like so:
builtins.hashString "sha256" (builtins.readFile /tmp/a)but that doesn't work for binary files.
With builtins.hashFile any kind of file can be conveniently hashed.
Fixes: #1835
I added some tests and also tested it manually in the repl:
Also works on binary files:
I'm not sure why my test in
tests/lang/eval-fail-hashfile-missing.nixisn't working. It evaluates but it shouldn't. In the repl it doesn't evaluate.This is my first edit in the Nix source so please be kind if I forgot something ;)
By the way: I found it really weird that
EvalState::coerceToPathandEvalState::forceStringtake the same arguments but in a different order. Is that intentional or is that something which we should fix?