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

Update dependencies to build with cargo update -Z minimal-versions #299

Merged
merged 1 commit into from Feb 4, 2022

Conversation

KamilaBorowska
Copy link
Contributor

It also would be appreciated to release a new version of blockfish with an updated byteorder dependency because it's not a dev-dependency unlike other updated dependencies.

Not committing Cargo.lock is recommended, as libraries cannot really control what dependencies Cargo downloads, and as such it makes sense to test newest versions of dependencies on CI.

@tarcieri
Copy link
Member

tarcieri commented Feb 4, 2022

@xfix committing Cargo.lock keeps the build reproducible, and allows us to diagnose how changes in upstream dependencies cause breakages to our project.

This has occurred several times in the past, which is the reason why we check Cargo.lock in now. As an example, we had a user who just so happened to pick up a broken version of sha2 complain about breakages, and no one could reproduce his problem:

  • People who had an older Cargo.lock were fine because they cached an older version
  • People doing a clean checkout picked up the latest version which included a fix

When trying to debug an issue with several people involved like this, not having a Cargo.lock as a basepoint of truth can cause an awful lot of confusion.

There are pros/cons of either approach and it's a debatable topic. Thus far we have chosen to check in Cargo.lock.

@KamilaBorowska
Copy link
Contributor Author

KamilaBorowska commented Feb 4, 2022

Fair enough. Also, from what can I tell hex-literal 0.3.0 would require higher MSRV, but considering it's a dev-dependency it's not very important, so I will be removing that one from this pull request.

@KamilaBorowska
Copy link
Contributor Author

KamilaBorowska commented Feb 4, 2022

This won't allow cargo +nightly update -Z minimal-versions to work within repo itself, but it will work for crates that do depend on blowfish, so it's good enough for me. byteorder 1.0.0 dependency is not correct, it should be byteorder 1.1.0.

@tarcieri
Copy link
Member

tarcieri commented Feb 4, 2022

Note that we do have a tracking issue for testing with -Z minimal-versions: RustCrypto/traits#162

@newpavlov
Copy link
Member

Thank you!

@newpavlov newpavlov merged commit 81e0de5 into RustCrypto:master Feb 4, 2022
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.

None yet

3 participants