digest: update NewDigestFromHex signature to use Algorithm

[cyphar]

For consistency reasons, make NewDigestFromHex also take an Algorithm as the alg argument.

Signed-off-by: Aleksa Sarai asarai@suse.de

[stevvooe]

Is this backwards compatible? Seems like this would break existing code that uses this function.

Looking at the uses of this function across docker, using the algorithm type here makes a lot of sense.

It might actually make sense to hang a method off the algorithm to hit this use case and deprecate NewDigestFromHex for that.

[cyphar]

So Algorithm.FromHex, as well as presumably the other .From___s?

[stevvooe]

@cyphar Do you want to submit a PR for Algorithm.FromHex?

[cyphar]

Currently Algorithm already implements FromString, FromBytes and FromReader -- all of which return the digest of the argument as opposed to create a Digest with the same value of the arguments. So I'm not sure implementing FromHex will make sense, we should probably name it something different.

[stevvooe]

Currently Algorithm already implements FromString, FromBytes and FromReader -- all of which return the digest of the argument as opposed to create a Digest with the same value of the arguments. So I'm not sure implementing FromHex will make sense, we should probably name it something different.

I'm not sure this is a huge concern. These are all just constructor methods. The fact that these are have similar structure is just a coincidence.

[dmcgowan]

I think it is a valid concern that today From_ means generate a new digest from the input, while an implementation of FromHex just uses the input as the digest. It would be safer to just to name it with a different preposition, such as WithHex.

Is this backwards compatible? Seems like this would break existing code that uses this function

While this could still have the same naming ambiguity, NewDigestFromHex was intended to recreate digests based on the separated strings (such as a file on disk stored as ./sha256/abcdef....). Most use cases which are using this function by casting the algorithm to string should update to use the new function on algorithm.

[stevvooe]

While this could still have the same naming ambiguity, NewDigestFromHex was intended to recreate digests based on the separated strings (such as a file on disk stored as ./sha256/abcdef....). Most use cases which are using this function by casting the algorithm to string should update to use the new function on algorithm.

Yes, but this will break existing code.

I like to be able to run gofmt -r 'digest.NewDigestFromHex(a.String(), b) -> a.FromHex(b), eliding the cast. NewDigestFromHex should be deprecated in favor of this method.

[stevvooe]

33 covers this by adding NewDigestFromEncoded. We can add an Algorithm.FromXXX method later, if that makes sense.

[cyphar]

:+1: Sure, works for me.