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

Remove failure crate #564

Merged
merged 12 commits into from
Mar 13, 2024
Merged

Remove failure crate #564

merged 12 commits into from
Mar 13, 2024

Conversation

nshyrei
Copy link
Contributor

@nshyrei nshyrei commented Mar 7, 2024

Changes all usages of failure crate (https://crates.io/crates/failure) across the project into thiserror (https://crates.io/crates/thiserror) and anyhow (https://crates.io/crates/anyhow). This change fixes vulnerability introduced by the failure crate (https://rustsec.org/advisories/RUSTSEC-2020-0036)

@nshyrei nshyrei self-assigned this Mar 7, 2024
Copy link
Collaborator

@Taowyoo Taowyoo left a comment

Choose a reason for hiding this comment

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

The changes so far LGTM.

Need to bump version number

But since this upgrade changes the function signatures, this will be a breaking change of all crates that are updated. Which means you need to increase the minor version number of all crates you updated.

Concerns about changes and crate version before this breaking change

Before we merge this PR, I have to get clear about out release strategy about crates under rust-sgx.
If we want to have a version without this change but contains all changes before this PR and after last publish.
We need to publish new version of crates with new patch version if they only contain some version compatible changes.

Otherwise, we could just bump the minor versions in this PR.

@Taowyoo
Copy link
Collaborator

Taowyoo commented Mar 7, 2024

The changes so far LGTM.

But since this upgrade change the function signatures, this will be a breaking change of all crates that are updated. Which means you need to increase the minor version number of all crates you updated.

ALSO Before we merge this PR, I have to get clear about out release strategy about crates under rust-sgx. If we want to have a version without this change but contains all changes before this PR and after last publish. We need to publish new version of crates with new patch version if they only contains some version compatible changes.

Otherwise, we could just bump the minor versions in this PR

What's you idea about this ^^
@raoulstrackx @ssavvides

@nshyrei nshyrei requested a review from Taowyoo March 8, 2024 13:19
@nshyrei nshyrei requested a review from Taowyoo March 10, 2024 20:10
@nshyrei
Copy link
Contributor Author

nshyrei commented Mar 10, 2024

@Taowyoo Please review last 2 commits

Copy link
Collaborator

@Taowyoo Taowyoo left a comment

Choose a reason for hiding this comment

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

When using a new MINOR version, you should start with 0 for MINOR.
Note:
You could use this tool https://github.com/crate-ci/cargo-release to bump version in a more reliable way with command cargo release version minor under root of each crate.

intel-sgx/aesm-client/Cargo.toml Outdated Show resolved Hide resolved
intel-sgx/dcap-provider/Cargo.toml Outdated Show resolved Hide resolved
intel-sgx/dcap-ql/Cargo.toml Outdated Show resolved Hide resolved
intel-sgx/dcap-ql/Cargo.toml Outdated Show resolved Hide resolved
intel-sgx/dcap-retrieve-pckid/Cargo.toml Outdated Show resolved Hide resolved
intel-sgx/fortanix-sgx-tools/Cargo.toml Outdated Show resolved Hide resolved
intel-sgx/report-test/Cargo.toml Outdated Show resolved Hide resolved
intel-sgx/sgxs-loaders/Cargo.toml Outdated Show resolved Hide resolved
intel-sgx/sgxs-tools/Cargo.toml Outdated Show resolved Hide resolved
intel-sgx/sgxs/Cargo.toml Outdated Show resolved Hide resolved
@nshyrei nshyrei requested a review from Taowyoo March 12, 2024 13:28
@Taowyoo
Copy link
Collaborator

Taowyoo commented Mar 12, 2024

Please fix the ci, some versions in Cargo.toml are not correct.
@nshyrei

@nshyrei
Copy link
Contributor Author

nshyrei commented Mar 12, 2024

@Taowyoo Fixed, please review

Copy link
Collaborator

@Taowyoo Taowyoo left a comment

Choose a reason for hiding this comment

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

Good job! Thank you for working on this big upgrade!

@nshyrei nshyrei added this pull request to the merge queue Mar 13, 2024
Merged via the queue into master with commit e3f87a0 Mar 13, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants