-
-
Notifications
You must be signed in to change notification settings - Fork 175
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
use multihash for cache filenames #584
use multihash for cache filenames #584
Conversation
My first reaction to this is "yes, hash agility is good" and "we should prefix with f too for multibase!"But thinking about it more, do we gain much here? The names could always have had the sha256 prefix for agility -- but they don't really need it because it's a cache. If a new hash were ever to be supported by Dhall new names would be created. The only way this could be a problem is if there were a new hash with a risk of generating the same name as sha256 for a different expression. Which seems very unlikely.
|
My goal is mainly a) consistency and b) implementation simplicity. If I have to encode things as multihash in the binary format, but as bare hash in a filename, that just feels arbitrary and special-casey. If we just use multihash everywhere, we don't need to have special cases. I agree that we're not super concerned about hash collisions or hash extensibility in the context of the cache, since cache files are by pretty much by definition ephemeral and disposable. I wasn't aware of multibase; i'd be happy to go with that. |
The multi-hash is supposed to begin with |
The multi-hash is supposed to begin with `\x12\x20`, not `1220`. It seems like the multi-hash standard was designed for a binary representation, not a textual representation like a path.
1220 is the correct rendering of the prefix in hex, though, just like we currently render the sha256 (which is a binary string also and not text) as hex for the path.
Specifying what base some text encoding of bytes is in is what multibase is for (hence my partly joking partly not suggestion to prefix with "f" above).
I'm fairly neutral on this change, partly because I'm not sure the standardised disk cache is that useful anyway. I can see how just base16 encoding the entire semantic hash + prefix could save work in some cases, though.
|
PR #549 adopted the use of multihash for binary encoding of semantic hashes; this commit extends that to the filenames of cache files on disk.
3a1eb61
to
1089dc9
Compare
Fixes dhall-lang#582 This introduces a new `exprToImport` utility that is used to cache the fully-resolved import
PR #549 adopted the use of multihash for binary encoding of semantic
hashes; this commit extends that to the filenames of cache files on
disk.