Skip to content
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

sha1: switch name from sha-1 to sha1 #350

Merged
merged 6 commits into from
Jan 17, 2022
Merged

sha1: switch name from sha-1 to sha1 #350

merged 6 commits into from
Jan 17, 2022

Conversation

newpavlov
Copy link
Member

@newpavlov newpavlov commented Jan 15, 2022

@mitsuhiko has kindly transferred ownership of sha1 to us, so now we can publish our implementation under it instead of using sha-1.

For user convenience, we will skip v0.7-v0.9 and will release sha1 v0.10.0, so the version will be synchronized with sha-1. I plan to continue releasing v0.10.x patch versions for sha-1 in parallel with sha1 and deprecate it only after sha1 v0.11 will be released.

@mitsuhiko
Copy link

Ideally we can figure out a transition guide for folks currently using sha1. I would put that into the old repository to guide people to how to upgrade.

@mitsuhiko
Copy link

If we're adventurous it would probably be possible to make a sha1 release that uses the semver trick to pull in a newer version and shim out the old types. That way even if one were not to upgrade, you only end up with one SHA1 implementation in the dependency tree.

@newpavlov
Copy link
Member Author

Regarding update instructions, I think you can simply write that users should use the Digest trait (i.e. they would have to explicitly import it) and that instead of the Digest wrapper type we return bytes directly in the form of GenericArray. Half of methods in sha1 v0.6 have exactly the same name as in the Digest trait and the remaining can be mapped as:

  • from -> new_with_prefix
  • digest -> finalize
  • hexdigest can be replaced with base16ct::lower::encode_string(&hasher.finalize()) (or alternatively one could use the hex crate)

I don't think it's worth the trouble to use the semver trick or trying to implement the inherent methods.

@mitsuhiko
Copy link

mitsuhiko commented Jan 16, 2022

Let me be quite clear as the original crate author of the SHA1 crate: the change is absolutely worth it. I'm not okay with you just publishing a backwards incompatible change on the crate not considering existing users at all.

I will implement the semver trick. The change is massive for people who are currently using the crate as it needs to pull in many new dependencies. The original reason for the sha1 crate was that people find a minimal implementation of SHA1 that does not need to pull in the entire world. For instance the redis crate needed a SHA1 just to get the content addressable logic for scripts in.

Right now to get to the same level of support you don't just need to pull in the new rust crypto ecosystem and base16ct for the common case. That's a whole whopping 18 crates!

My suggestion is doing the following two things: I'm going to move the existing code from sha1 to sha1_small or similar and provide a semver trick to enable an automatic transition to the new sha1. Then people can have a sensible update experience and they can chose to switch to sha1_small if they are not okay with the new dependencies.

@newpavlov
Copy link
Member Author

I'm not okay with you just publishing a backwards incompatible change on the crate not considering existing users at all.

I think we had similar discussion previously and it looks like we have some sort of misunderstanding.

How exactly, in your opinion publishing sha1 v0.10 with the new code will negatively affect existing users of sha1 v0.6? In Rust's interpretation of semver those versions are incompatible with each other, so sha1 v0.10 will not be pulled by existing users without them manually editing their Cargo.toml (unless they use sha1 = "*", but I think we can safely disregard such cases). I do not intend to publish any patch v0.6.x versions, so the only inconvenience for existing sha1 users will be a warning that they use an "outdated" dependency version (e.g. if they use deps.rs or cargo outdated). If they don't like the number of pulled dependencies (which BTW is a bad metric in my opinion), they can simply stay on sha1 v0.6 indefinitely without migrating to sha1 v0.10.

Frankly, I don't see a point in using the semver trick in our case. Usually it's used in cases when types and values from a crate are used as part of public API in user crates. In the case of sha1 I think such cases are really rare.

@newpavlov
Copy link
Member Author

To be clear: While I do not see a point in it, I do not oppose in any way you publishing sha1 v0.6.1 with the semver trick on top of sha1_small. As I've stated, I do not plan to touch any versions below v0.10.

@mitsuhiko
Copy link

How exactly, in your opinion publishing sha1 v0.10 with the new code will negatively affect existing users of sha1 v0.6?

Because existing users signed up for one specific crate with one specific behavior. I can also start publishing SHA256 under sha1 0.7.0 which would be fine by semver but not the expectation I as a user have.

My proposal is the following that should make you and me happy:

  • I publish a 0.6.1 version of sha1 which depends on sha1-smol and re-exports stuff from there.
  • You add a note to your sha1 readme that the old crate is now available as sha1-smol for people who want to continue using it.

@newpavlov
Copy link
Member Author

You add a note to your sha1 readme that the old crate is now available as sha1-smol for people who want to continue using it.

Done. Feel free to suggest an alternative wording if you don't like the current one.

@mitsuhiko
Copy link

mitsuhiko commented Jan 16, 2022

That works for me. I will publish the shim then once I ensured it works.

@mitsuhiko
Copy link

Done. The shim has been published. sha1 0.6.1 now depends on sha1-smol 1.0.0.

@newpavlov newpavlov merged commit 32029ac into master Jan 17, 2022
@newpavlov newpavlov deleted the sha1/migrate branch January 17, 2022 15:00
benesch added a commit to benesch/rust_mysql_common that referenced this pull request Aug 29, 2022
The RustCrypto project recently got control of the crate name `sha1`. This
commit switches over to the new name; the old `sha-1` name is now deprecated.

Details: RustCrypto/hashes#350
benesch added a commit to benesch/tungstenite-rs that referenced this pull request Aug 29, 2022
The RustCrypto project recently got control of the crate name `sha1`. This
commit switches over to the new name; the old `sha-1` name is now deprecated.

Details: RustCrypto/hashes#350
benesch added a commit to benesch/headers that referenced this pull request Aug 29, 2022
The RustCrypto project recently got control of the crate name `sha1`. This
commit switches over to the new name; the old `sha-1` name is now deprecated.

Details: RustCrypto/hashes#350
benesch added a commit to benesch/materialize that referenced this pull request Aug 29, 2022
The RustCrypto project recently got control of the crate name `sha1`.
This commit switches over to the new name; the old `sha-1` name is now
deprecated.

Details: RustCrypto/hashes#350
benesch added a commit to benesch/materialize that referenced this pull request Aug 29, 2022
The RustCrypto project recently got control of the crate name `sha1`.
This commit switches over to the new name; the old `sha-1` name is now
deprecated.

Details: RustCrypto/hashes#350
benesch added a commit to MaterializeInc/materialize that referenced this pull request Aug 29, 2022
The RustCrypto project recently got control of the crate name `sha1`.
This commit switches over to the new name; the old `sha-1` name is now
deprecated.

Details: RustCrypto/hashes#350
seanmonstar pushed a commit to hyperium/headers that referenced this pull request Aug 29, 2022
The RustCrypto project recently got control of the crate name `sha1`. This
commit switches over to the new name; the old `sha-1` name is now deprecated.

Details: RustCrypto/hashes#350
xmas7 pushed a commit to RubyOnWorld/rust_mysql_common that referenced this pull request Sep 6, 2022
The RustCrypto project recently got control of the crate name `sha1`. This
commit switches over to the new name; the old `sha-1` name is now deprecated.

Details: RustCrypto/hashes#350
a-miyashita pushed a commit to givery-technology/tungstenite-rs that referenced this pull request Jul 20, 2023
The RustCrypto project recently got control of the crate name `sha1`. This
commit switches over to the new name; the old `sha-1` name is now deprecated.

Details: RustCrypto/hashes#350
primeos-work added a commit to primeos-work/butido that referenced this pull request Jan 5, 2024
The `sha-1` crate doesn't get updated anymore and the `sha1` crate
should be used instead (the crate only got renamed [1]) [0]:
> This crate is deprecated! Use the sha1 crate instead.

Note: The `sha-1` crate is currently stuck at `v0.10.1` despite the
following claim to update it until v0.11:
> The SHA-1 implementation was previously published as sha-1, but
> migrated to sha1 since v0.10.0. sha-1 will continue to receive v0.10.x
> patch updates, but will be deprecated after sha1 v0.11 release.

[0]: https://crates.io/crates/sha-1
[1]: RustCrypto/hashes#350

Signed-off-by: Michael Weiss <[email protected]>
ammernico pushed a commit to ammernico/butido that referenced this pull request Apr 30, 2024
The `sha-1` crate doesn't get updated anymore and the `sha1` crate
should be used instead (the crate only got renamed [1]) [0]:
> This crate is deprecated! Use the sha1 crate instead.

Note: The `sha-1` crate is currently stuck at `v0.10.1` despite the
following claim to update it until v0.11:
> The SHA-1 implementation was previously published as sha-1, but
> migrated to sha1 since v0.10.0. sha-1 will continue to receive v0.10.x
> patch updates, but will be deprecated after sha1 v0.11 release.

[0]: https://crates.io/crates/sha-1
[1]: RustCrypto/hashes#350

Signed-off-by: Michael Weiss <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants