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

Bump picky- crates #427

Merged
merged 2 commits into from
Sep 18, 2023
Merged

Bump picky- crates #427

merged 2 commits into from
Sep 18, 2023

Conversation

ionut-arm
Copy link
Member

A recent fix [1] in the picky-asn1-der crate highlights an issue that could lead to faulty results handed off by the abstractions in the tss-esapi crate. Specifically, mishandling of ASN.1 Integers could result in invalid encodings of various pieces of data such as elliptic curve points.

[1] https://github.com/Devolutions/picky-rs/pull/209/files

A recent fix [1] in the picky-asn1-der crate highlights an issue that
could lead to faulty results handed off by the abstractions in the
tss-esapi crate. Specifically, mishandling of ASN.1 Integers could
result in invalid encodings of various pieces of data such as elliptic
curve points.

[1] https://github.com/Devolutions/picky-rs/pull/209/files

Signed-off-by: Ionut Mihalcea <ionut.mihalcea@arm.com>
@ionut-arm
Copy link
Member Author

Unfortunately this won't work with the current MSRV of v7 (1.57.0). Would it make sense to bump that to 1.60.0, or do you reckon that's a breaking change?

@Superhepper
Copy link
Collaborator

For master I think it is a non issue to bump MSRV to 1.60

@ionut-arm
Copy link
Member Author

For master I think it is a non issue to bump MSRV to 1.60

True, though I'm hoping to bump it for the 7.x.y branch, as I'd need to release a new version for Parsec

@Superhepper
Copy link
Collaborator

That is slightly more complicated. Because that has the potential of breaking stuff if we only make a minor release bump. My suggestion, that I know for a lot of reason is not ideal is to do the following.

  1. Merge the stuff that you need into the Major Version 7 branch and make a new release of that branch.
  2. Create a Major Version 8 branch from the latest commit on the Major Version 7 branch. And then in that add a commit that updates the MSRV to the desired value and make a release of the Major Version 8 branch.

Ideally I would like for the the current master to be start of the Major Version 8 branch. But that might not be possible.

@ionut-arm
Copy link
Member Author

Mentioning the same thing as in the other PR: tentative recommendation for Rust APIs seems to be that minor bumps are ok for MSRV changes. So we should be good to make a v7 update just for the MSRV.

Also, it seems we should be more thorough in checking our MSRV, either with a lock file or with a compiler option to keep every dependency at minimum level.

Copy link
Collaborator

@wiktor-k wiktor-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, why it complains about rustix while I can't find it in the project: cargo tree --all-features --target=all | grep rustix :/

I'm OK with the change though 👍

Bumping MSRV to 1.60.0 as required for the picky-asn1 crate.

Also setting the MSRV for clippy and fixing some of the remaining
warnings. Some of the things clippy complains about for newer versions
of the compiler can be fixed/changed without losing support for older
compiler versions, but some features are incompatible. Since clippy is
now configured to understand the MSRV, there's also no need for some of
the `allow` attributes.

Signed-off-by: Ionut Mihalcea <ionut.mihalcea@arm.com>
@ionut-arm ionut-arm merged commit 285d5bc into parallaxsecond:7.x.y Sep 18, 2023
9 checks passed
@ionut-arm ionut-arm deleted the picky-bump-v7 branch September 18, 2023 14:05
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.

3 participants