jsonnet icon indicating copy to clipboard operation
jsonnet copied to clipboard

implement std.sha256

Open mikedanese opened this issue 6 years ago • 3 comments

Adds a dep on openssl and absl. I'm not sure how problematic that is.

mikedanese avatar Feb 21 '19 06:02 mikedanese

Thanks, adding more standard hash functions is a good thing.

Unfortunately, going from 0 external dependencies to one (two with absl) is a pretty inconvenient. We need to support the users who don't use Bazel.

Are there any good header-only hash function libs? That would be much more convenient here. However, OpenSSL is going to be available on pretty much any system anyway and it provides us with all the crypto we may need. I'm not aware of any other reasonable option, so we probably should use it. But then we need to make it work with a regular Makefile, add dependency information to README and make the new requirement very clear during the release.

I don't think that absl::BytesToHexString is worth getting absl. It sounds like quite a pain to get working with Make and it's much less likely that the users have it already in their system.

I wonder what @sparkprime thinks about this.

sbarzowski avatar Feb 26 '19 01:02 sbarzowski

The only reason Mike did this is that someone internally wanted us to provide an alternative to MD5 in case anyone felt tempted to use MD5 in a context where crytographic hashing was needed. Personally I'm not really convinced it's worthwhile since this is a config language.

As for absl dependency, I'm sure we can write our own absl::BytesToHexString :)

There are already build problems on windows and mac and stuff like this will probably exacerbate them.

Maybe we should pull off the bandaid and start adding stuff to go that's not in C++?

sparkprime avatar Feb 26 '19 12:02 sparkprime

If we will go forward with parseYaml we will probably want to add shared library dependencies anyway, so adding openSSL should't change that much.

sbarzowski avatar Dec 05 '19 18:12 sbarzowski