Added crypto_auth_hmacsha512256 wrappers, new wrapper tests, and made sumo wrapper tests easier to run

[bschick]

Wanted to compare crypto_auth_hmacsha512256 and keyed crypto_generichash from javascript. So did the following:

• Added wrappers for.crypto_auth_hmacsha512256*
• Added wrapper tests for crypto_auth_hmacsha512256 and crypto_generichash
• Made is easier to test normal vs sumo packages (without errors)

[jedisct1]

Thank you!

But for unrelated changes, distinct PRs would have been better!

Thanks!

[jedisct1]

But wait...

Now, in the regular packages, there's a testsumo target that doesn't work, and in sumo packages, the test target doesn't test everything.

Looks like a regression.

[bschick]

Hmmm, not sure what you are seeing (or what is different on my system). When I do the following it all works as expected:

> git clone https://github.com/bschick/libsodium.js.git
> cd libsodium.js/
> make 
> npm run test
> npm run testsumo

It all works as expected and tests pass.

Before my changes, two make test targets failed because the crypto_pwhash is only in the sumo package. The new testsumo target now includes tests of crypto_pwhash and the newly added crypto_auth_hmacsha512256.

Or are you talking about the package-libsodium-wrappers.json and package-libsodium-wrappers-sumo.json files? I didn't pay attention to those. Is the 'script' section even needed in those?

[jedisct1]

I was referring to the actual packages, that will be distributed on npm.

[jedisct1]

Is the 'script' section even needed in those?

Yes.

That's actually my release script:

#! /bin/sh

for f in \
  package-libsodium.json \
  package-libsodium-sumo.json \
  package-libsodium-wrappers-sumo.json \
  package-libsodium-wrappers.json \
; do
  rm -f package.json
  cp "$f" package.json
  npm publish || yarn publish
done
rm -f package.json
cp package-libsodium-wrappers.json package.json

Maybe there's an easier way to publish these different packages. I don't know.

[bschick]

Got it, apologies for missing those. I made this update, but it looks like you already make these changes upstream :) https://github.com/jedisct1/libsodium.js/compare/master...bschick:libsodium.js:master

I see you also removed testsumo from the base pacakge.json. I'd propose leaving that in so you don't have to run the release and overwrite package.json to test both versions of wrappers. It did work as expected, but you have to run it with npm run testsumo since testsumo is not one of the special npm targets like "build", "start", "test" that allow you to drop "run".

Glad you also updated to unix CR in all files. That was a bit annoying

[bschick]

Oh, but then the final cp package-libsodium-wrappers.json package.json in the release script would leave a diff. Could save and replace the original package.json

[jedisct1]

package.json is not really meant to be used. It's just there for convenience in order to install the dependencies.

But it can be. copy of package-libsodium-wrappers-sumo.json instead of package-libsodium-wrappers.json if you prefer.

[bschick]

Sure. Certainly cleaner as you did it, although a bit less obvious. Thanks for taking the pull request and the library overall.