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

Fix feature alloc and pem in pkcs1 #1013

Merged
merged 1 commit into from
Apr 21, 2023
Merged

Fix feature alloc and pem in pkcs1 #1013

merged 1 commit into from
Apr 21, 2023

Conversation

NobodyXu
Copy link
Contributor

Avoid pulling in dep pkcs8 or zeroize just because feature alloc or pem is enabled.

@tarcieri
Copy link
Member

This is technically a breaking change, although probably an OK one.

You'll need to update the import gating accordingly, however, since the tests are now failing.

@tarcieri tarcieri closed this Apr 19, 2023
@tarcieri tarcieri reopened this Apr 19, 2023
@tarcieri
Copy link
Member

(whoops sorry, didn't mean to close it)

Avoid pulling in dep `pkcs8` or `zeroize` just because feature
`alloc`/`pem`/`std` is enabled.

Also removes unused dependency `zeeroize` and replace it with a feature.

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Copy link
Contributor Author

NobodyXu commented Apr 21, 2023

@tarcieri Thanks, I've fixed the error and I believe this PR does not include any breaking change, since

  • no feature is removed
  • no API change under cfg

@NobodyXu
Copy link
Contributor Author

Sorry that somehow I did not receive any notification for your reply.

@NobodyXu
Copy link
Contributor Author

I've also run cargo semver-checks check-release and it confirms that this PR does not include any breaking change.

@NobodyXu
Copy link
Contributor Author

@tarcieri All checks have passed, this PR is ready for another review.

@tarcieri
Copy link
Member

@NobodyXu the breaking change would be features being implicitly enabled before and not any longer. That's not going to be caught by a tool like cargo semver-checks.

But it's minor enough it's probably a non-issue.

@tarcieri tarcieri merged commit 5bd941f into RustCrypto:master Apr 21, 2023
@NobodyXu
Copy link
Contributor Author

@NobodyXu the breaking change would be features being implicitly enabled before and not any longer. That's not going to be caught by a tool like cargo semver-checks.

But it's minor enough it's probably a non-issue.

Yeah you are right, I kind of ignore that because I thought it was wrong to enable pkcs8 in feature alloc.
In fact that was one of the motivation for this PR since this bumps the msrv to 1.65.0 in sccache mozilla/sccache#1742

@tarcieri Can you make a new release for pkcs1?

@NobodyXu NobodyXu deleted the patch-1 branch April 21, 2023 12:47
@tarcieri tarcieri mentioned this pull request Apr 21, 2023
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