code review 5556058: crypto/hmac: Deprecate hmac.NewMD5, hmac.NewSHA1 and hm...

crypto/hmac: Deprecate hmac.NewMD5, hmac.NewSHA1 and hmac.NewSHA256

Remove NewMD5, NewSHA1 and NewSHA256 in favor of using New and
explicitly importing the used hash-function. This way when using, for
example, HMAC with RIPEMD there's no md5, sha1 and sha256 linked in
through the hmac package.

A gofix rule is included, and applied to the standard library (3 files
altered).

This change is the result of a discussion at
http://codereview.appspot.com/5550043/ to pull the discussion about
deprecating these functions out of that issue.

[Luit, 2012-01-19 18:18:05]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com,
r@golang.org, rsc@golang.org),

I'd like you to review this change to
https://go.googlecode.com/hg/

[Luit, 2012-01-19 18:24:46]

To continue the discussion about this: I think it's better that there's one way
to do HMAC (supplying your own hash function) than having an arbitrary set of
default hash functions causing inclusion of unused code in the final binary. 

I think leaving these out makes for much cleaner and clearer code. As Go 1 is
getting closer, I can understand if this won't make it, but I think this is the
right thing to do.

This change doesn't add to the Go 1 release notes. Can someone else do that if
this change makes it through?

-Luit

[rsc, 2012-01-19 18:26:44]

+agl

It's hard for me to get excited about this one way or the other.
I am going to leave it up to Adam.

Russ

[Luit, 2012-01-19 19:03:12]

rsc: I don't fully understand why not adding sha512 was a good thing, but
removing sha256, sha1 and md5 from the dependencies is a bad thing. (I'm sorry
if that's a wrong representation of your point in this)

I think the benefit of taking out the dependencies outweighs the inconvenience
(if any) of the different syntax. The new gofix rule can fix the most common
(which probably is all) use of the removed functions.

-Luit

[agl2, 2012-01-19 19:03:58]

On Thu, Jan 19, 2012 at 2:03 PM,  <luitvd@gmail.com> wrote:
> I think the benefit of taking out the dependencies outweighs the
> inconvenience (if any) of the different syntax. The new gofix rule can
> fix the most common (which probably is all) use of the removed
> functions.

I'm happy with the change. Will take a detailed look and land later on today.


Cheers

AGL

[Luit, 2012-01-19 19:16:29]

On 2012/01/19 19:03:58, agl_google.com wrote:
> I'm happy with the change. Will take a detailed look and land later on today.

"later on today" might become tomorrow for me, if any change or input is needed.
I'm on Central European Time (Europe/Amsterdam time). 

Keep in mind that the gofix written is my first, and I don't have much
experience with language parsing, so any input on that part is welcome!

-Luit

[agl1, 2012-01-19 22:28:45]

*** Submitted as http://code.google.com/p/go/source/detail?r=7a1019785fd5 ***

crypto/hmac: Deprecate hmac.NewMD5, hmac.NewSHA1 and hmac.NewSHA256

Remove NewMD5, NewSHA1 and NewSHA256 in favor of using New and
explicitly importing the used hash-function. This way when using, for
example, HMAC with RIPEMD there's no md5, sha1 and sha256 linked in
through the hmac package.

A gofix rule is included, and applied to the standard library (3 files
altered).

This change is the result of a discussion at
http://codereview.appspot.com/5550043/ to pull the discussion about
deprecating these functions out of that issue.

R=golang-dev, agl
CC=golang-dev, r, rsc
http://codereview.appspot.com/5556058

Committer: Adam Langley <agl@golang.org>

[Luit, 2012-01-19 23:31:10]

agl: can you add to the release notes, to keep the commander happy? It's an API
change, right?

-Luit

[rsc, 2012-01-20 21:02:13]

The gofix seems to be buggy, but it may not
be the fault of the new module (it might be gofix
itself).  Filed golang.org/issue/2750.