Cargo.toml: pumped ring version to 0.17.5

[aggstam]

Clippy checks pass using pull/147.
Min supported rust version needs to be pumped to 1.61.0.
If its acceptable, I can edit the workflow files to the new min supported rust version.

[reubenmiller]

I can contribute to updating the MSRV to 1.61 in the workflows/docs (due to ring 0.17 release).

I've done the changes on my fork, but it probably makes sense to include them as part of this PR:

Changes showing the edit:
master...reubenmiller:x509-parser:update-ring

We're really interesting in updating the ring version as x509-parser is the last crate in our project thin-edge/thin-edge.io that still requires ring 0.16. We're really wanting to use ring 0.17 as it makes compiling for additional targets possible (such as some RISCV targets).

[reubenmiller]

@aggstam Would you be able to include this commit from my fork in this PR (acff5cd) - I don't mind about change of authorship (if that is easier for everyone).

Otherwise I can revive by PR #149 with both changes (bump in dependency and MSRV)

[aggstam]

Hey @reubenmiller, we are waiting on maintainers confirmation on how to proceed, in the meantime you can open a PR in my fork branch, when I push it will be included as part of this PR.

[reubenmiller]

Hey @reubenmiller, we are waiting on maintainers confirmation on how to proceed, in the meantime you can open a PR in my fork branch, when I push it will be included as part of this PR.

Though the approver (@cpu) did react positively to my suggestion so I’ll create a PR to your fork shortly.

[reubenmiller]

Hey @reubenmiller, we are waiting on maintainers confirmation on how to proceed, in the meantime you can open a PR in my fork branch, when I push it will be included as part of this PR.

Done. aggstam#1

[aggstam]

@reubenmiller Done.

[cpu]

Though the approver (@cpu) did react positively to my suggestion

Just to be clear my approval here is only informative :-) I have no special permissions compared to other folks.

There have been a handful of PRs open for a long time now. Perhaps if someone could get in touch with @chifflier they would be open for help with maintenance from the broader community?

[fspreiss]

Thanks for this PR, @aggstam and @reubenmiller! I'm working on a project that would also need this upgrade to a recent version of ring. Is there any update on this? @chifflier, would you be willing to accept this change?

[chifflier]

Hi all,
Updating the MSRV to a recent version can cause problems, since it breaks programs that are using these crate and have a long-term support policy (like Suricata) :/
Since the ring dependency is only used in a feature, it's even more annoying. If someone has a idea on how to express something like "MSRV is 1.xx unless you use feature f, then it becomes 1.yy", I'm very interested :)

In any case, I'm interested in updating all dependencies so I'll start reviewing and merging this PR as soon as possible.
Thanks again!

[aggstam]

Hey @chifflier ,

Thanks for clarifying the situation at hand! I feel you when dealing with LTS stuff...

Reading throught RFC2495, your proposal seems to be doable, therefore I can play around testing some config to do that, reflected in the crates tests, checking ring feature tests using the proper MSRV, while rest use the normal/LTS one.
Let me know if I (or any other contributor) should proceed with exploring this idea.

KR

[chifflier]

Reading throught RFC2495, your proposal seems to be doable, therefore I can play around testing some config to do that, reflected in the crates tests, checking ring feature tests using the proper MSRV, while rest use the normal/LTS one. Let me know if I (or any other contributor) should proceed with exploring this idea.

According to the links in this page, this PR seems merged, but that is a feature I have never used or seen used. I would definitely be interested in some tests.
I think a reasonable goal would be to have a different MSRV only for the ring feature.

[aggstam]

I think a reasonable goal would be to have a different MSRV only for the ring feature.

Yeah that should be the goal!
I will try to make something and report my finding here.

[aggstam]

Hey @chifflier and everyone,

So I did some testing based on RFC2495, with the tldr being that [target.'cfg(feature = "foo")'.package] is not supported in Cargo.toml, which makes sense, as the root crate MSRV is pretty much defined by the dependencies MSRVs.
In our case, it means that we can define MSRV = 1.57.0 for the crate, so anyone using it as a dependency without using verify feature is enforced by that. When verify feature is defined, ring will enforce its MSRV = 1.61.0.

We can only notify users about this enforced MSRV in docs.

Since we want to update dependencies, for new/future versions of the crate, we should define MSRV = 1.57.0 in Cargo.toml, and split check workflows to be feature specific. That will ensure that default features are always checked against LTS MSRV, while verify feature is checked against its ring enforced MSRV.

Happy to hear your thoughts!

Apendix - Testing methodology:

1. Created a new branch where we define crate MSRV and check if we can enforce a feature specific one (commit).
   Cargo nags with:
   warning: Found feature = ... in target.cfg(...).dependencies. This key is not supported for selecting dependencies and will not work as expected. Use the [features] section instead: https://doc.rust-lang.org/cargo/reference/features.html
2. Created a new dummy project whith dependency:
   x509-parser = {git = "https://github.com/aggstam/x509-parser", branch = "MSRV"}
3. Created a docker builder to check different rust versions:

# Usage(on repo root folder):
#   docker build . --pull --no-cache --shm-size=196m -t x509-parser:builder -f ./builder.Dockerfile
# If you want to keep building using the same builder, remove --no-cache option.

# Arguments configuration
ARG ALPINE_VER=edge
ARG RUST_VER=1.57
ARG RUST_TIME_VER=0.3.9

# Environment setup
FROM alpine:${ALPINE_VER} as builder

## Arguments import
ARG RUST_VER
ARG RUST_TIME_VER

## Install system dependencies
RUN apk update
RUN apk add gcc git musl-dev rustup

## Configure Rust
RUN rustup-init --default-toolchain none -y
ENV PATH "${PATH}:/root/.cargo/bin/"
RUN rustup default ${RUST_VER}

# Build binaries
FROM builder as rust_builder

WORKDIR /opt/x509-parser-builder
COPY . /opt/x509-parser-builder

RUN cargo update -p time --precise ${RUST_TIME_VER}
RUN cargo check

Building completes successfully with no issues.
5. Set rust verion to 1.56, we get error:
error: package x509-parser v0.15.1 (https://github.com/aggstam/x509-parser?branch=MSRV#efded960) cannot be built because it requires rustc 1.57.0 or newer, while the currently active rustc version is 1.56.1
6. Update dependency to use verify feature:
x509-parser = {git = "https://github.com/aggstam/x509-parser", branch = "MSRV", features = ["verify"]}
Build fails with:
error: package ring v0.17.7 cannot be built because it requires rustc 1.61.0 or newer, while the currently active rustc version is 1.60.0
7. Update builder rust version to 1.61.0 and project builds successfully using verify feature.

[cpu]

@aggstam Thanks for your experimentation. That's useful data for the discussion at hand.

@chifflier How firm is your requirement to keep the MSRV at 1.57? Looking around at the ecosystem of similar crates it feels like x509-parser is an outlier in keeping the MSRV so low. I suspect many projects using x509-parser already have to use a newer Rust edition based on having other dependencies they use with x509-parser. 1.57 is over two years old now and even Debian stable is packaging 1.63.

I think trying to have a separate MSRV for just the ring feature seems complex and gives mixed results. If the 1.57 MSRV is a firm requirement it might be better to create a stable release branch for x509-parser 0.15.x that maintains the MSRV and move forward with development of a 0.16.x that has an MSRV of 1.61+

[aggstam]

If the 1.57 MSRV is a firm requirement it might be better to create a stable release branch for x509-parser 0.15.x that maintains the MSRV and move forward with development of a 0.16.x that has an MSRV of 1.61+

++

[chifflier]

@chifflier How firm is your requirement to keep the MSRV at 1.57? Looking around at the ecosystem of similar crates it feels like x509-parser is an outlier in keeping the MSRV so low. I suspect many projects using x509-parser already have to use a newer Rust edition based on having other dependencies they use with x509-parser. 1.57 is over two years old now and even Debian stable is packaging 1.63.

Hi,

The MSRV policy aims to support versions from common distros, so 1.63 seems a good target.

I think trying to have a separate MSRV for just the ring feature seems complex and gives mixed results. If the 1.57 MSRV is a firm requirement it might be better to create a stable release branch for x509-parser 0.15.x that maintains the MSRV and move forward with development of a 0.16.x that has an MSRV of 1.61+

I agree on the complexity, and would like to avoid this as well. I just want to avoid supporting only the latest stable version.
Again, we can update the MSRV to 1.63

[aggstam]

Hey @chifflier ,

I've updated ring and time to be on latest versions and pumped MSRV to 1.63.0.
PR should be ready to get merged and we be done with it :D

[chifflier]

There is a new error during build with 1.63:

error: package `powerfmt v0.2.0` cannot be built because it requires rustc 1.67.0 or newer, while the currently active rustc version is 1.63.0

This may require a fix using cargo update -p xxx --precise <version>

[chifflier]

Update: an older time version seems to build with 1.63:

cargo update -p time --precise 0.3.20

[aggstam]

There is a new error during build with 1.63:

error: package `powerfmt v0.2.0` cannot be built because it requires rustc 1.67.0 or newer, while the currently active rustc version is 1.63.0

This may require a fix using cargo update -p xxx --precise <version>

yy reverting time to 0.3.19 which is the latest supporting our MSRV, forgot to check it using the local docker for 1.63.0 rust, my bad :D

[aggstam]

@chifflier time is good now, will sort rest issues now so everything is on proper versions

[aggstam]

@chifflier all done! PR is good to merge now.

[chifflier]

@aggstam Everything seems ok now. Merged, thanks!

[Rigidity]

Hello, sorry to necro an old thread, but thought I'd ask when the next release is planned for? rcgen updated to use ring 0.17 but unfortunately this dependency still uses 0.16 so it still fails to compile on ARM Windows.

[chifflier]

Hi,
I just need to upload a new version of asn1-rs (with a fix for a small problem), and then x509-parser. I expect to upload it a few days,

[chifflier]

x509-parser 0.16.0 has been tagged and released 🎉