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

wasm-bindgen is broken and untested at MSRV (1.57) #3918

Open
micolous opened this issue Apr 8, 2024 · 1 comment
Open

wasm-bindgen is broken and untested at MSRV (1.57) #3918

micolous opened this issue Apr 8, 2024 · 1 comment
Labels

Comments

@micolous
Copy link

micolous commented Apr 8, 2024

webauthn-rs is a downstream user of wasm-bindgen, and we run tests in CI at our MSRV (currently 1.70) and on Rust stable.

We've recently started getting build failures at MSRV due to this library's dependency on bumpalo:

% cargo tree -i bumpalo
bumpalo v3.15.4
└── wasm-bindgen-backend v0.2.92
    └── wasm-bindgen-macro-support v0.2.92
        └── wasm-bindgen-macro v0.2.92 (proc-macro)
            └── wasm-bindgen v0.2.92

bumpalo 3.15.1 and later have an MSRV of 1.73 – and this was noticed and bumped in wasm-bindgen's CI configuration. However, wasm-bindgen has a stated MSRV of 1.57.

Going back in history, bumpalo 3.14.0 has an MSRV of 1.60, but there's no version of bumpalo which says it supports wasm-bindgen's current stated MSRV (1.57) – probably because this metadata was never collected.

wasm-bindgen-backend depends on bumpalo 3.0.0, so there's a theoretical solution, but Cargo doesn't consider the MSRV when selecting a dependency version.

As webauthn-rs is a collection of libraries and binaries (so we don't always control Cargo.lock), and bumpalo is a transitive dependency for us, we're very limited as to what we can do to mitigate the issue, aside from telling users on Rust < 1.73 and building for WASM targets to run cargo update -p bumpalo --precise 3.14.0 first.

Digging deeper

The version of Rust used in wasm-bindgen's CI config has been bumped several times to "fix build failures", without updating the MSRV metadata:

Because these changes were all made to work around build and/or test failures in CI on those lower Rust versions, they've become de facto MSRV changes. This means the MSRV metadata has been unverified (and quite probably false) for a bit over a year (since #3293).

In fairness to those involved, this was an incident waiting to happen – there is no documented MSRV policy, and none of the CI configuration makes reference to MSRVs.

Proposed solution

Assuming a new MSRV target of 1.60, I propose:

  • Update wasm-bindgen packages' MSRV metadata to 1.60.

    I'd be happy with wasm-bindgen's MSRV going as high as 1.70 – that's at least what I'm confident we've actually tested, and shouldn't impact our users.

  • Test wasm-bindgen in CI on the new MSRV and the current stable version of Rust.

    Make it clear in the CI config that changing the tested Rust versions is a de facto MSRV change.

  • Pin bumpalo to >=3.14.0,<3.15.0 (which has an MSRV of 1.60), as well as any other dependencies which have higher MSRV requirements – at least until Cargo can solve this itself and the new resolver is available to the MSRV.

  • Publish SemVer-appropriate version bumps for all packages with the corrected MSRV.

Going forward, this should make wasm-bindgens published MSRV metadata accurate, put guard rails around the CI config, and give downstream users a way to manage their own MSRV bumps.

@daxpedda
Copy link
Collaborator

This is definitely an issue, the MSRV has to absolutely be tested in CI, I'm happy to look at a PR!
In the meantime I will make a small PR to at least fix the current MSRV at 1.57.

The whole dependency MSRVs are changing is supposed to be addressed by Cargo with RFC 3537, which was already stabilized and is going to be out in v1.79 (obviously not really solving our problem here. So we don't plan to address this here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants