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

Consider allowing newer versions of wasm dependencies #43

Closed
djc opened this issue Sep 12, 2022 · 6 comments · Fixed by #44
Closed

Consider allowing newer versions of wasm dependencies #43

djc opened this issue Sep 12, 2022 · 6 comments · Fixed by #44
Labels
bug dependencies Pull requests that update a dependency file help wanted

Comments

@djc
Copy link

djc commented Sep 12, 2022

In #38 you constrained the allowed versions for wasm-bindgen:

 [target.'cfg(target_arch = "wasm32")'.dependencies.wasm-bindgen]
-version = "0.2"
+version = ">= 0.2, <= 0.2.78"

However, this means that all transitive downstream dependencies are no longer allowed to depend on newer wasm-bindgen, which seems like a bad idea. If you want to test support for older MSRVs, it's probably worth downgrading your wasm-bindgen versions explicitly in the CI job instead of constraining the upper allowed version.

For example, I ran into this because I'd like to use chrono 0.4.22 (the latest version). I happen to have a transitive dependency on whoami through sqlx-core, which I wasn't aware of before today. Therefore, Cargo silently downgraded my chrono dependency as I pulled in, since chrono depends on iana-time-zone, which recently started depending on wasm-bindgen and (reasonably) requires the current version at the time (0.2.81).

This is an issue in particular for wasm-bindgen, because it is set up to be treated as a native library, which means only one version is allowed to be linked. Here's the error I get when I try to force usage of chrono 0.4.22:

error: failed to select a version for `wasm-bindgen`.
    ... required by package `whoami v1.2.2`
    ... which satisfies dependency `whoami = "^1.2.2"` of package `test-rs v0.1.0 (/Users/djc/src/test-rs)`
versions that meet the requirements `>=0.2, <=0.2.78` are: 0.2.78, 0.2.77, 0.2.76, 0.2.75, 0.2.74, 0.2.73, 0.2.72, 0.2.71, 0.2.70, 0.2.69, 0.2.68, 0.2.67, 0.2.66, 0.2.65, 0.2.64, 0.2.63, 0.2.62, 0.2.61, 0.2.60, 0.2.59, 0.2.58, 0.2.57, 0.2.56, 0.2.55, 0.2.54, 0.2.53, 0.2.52, 0.2.51, 0.2.50, 0.2.49, 0.2.48, 0.2.47, 0.2.46, 0.2.45, 0.2.44, 0.2.43, 0.2.42, 0.2.41, 0.2.40, 0.2.39, 0.2.38, 0.2.37, 0.2.36, 0.2.35, 0.2.34, 0.2.33, 0.2.32, 0.2.31, 0.2.30, 0.2.29, 0.2.28, 0.2.27, 0.2.26, 0.2.25, 0.2.24, 0.2.23, 0.2.22, 0.2.21, 0.2.20, 0.2.19, 0.2.18, 0.2.17, 0.2.16, 0.2.15, 0.2.14, 0.2.13, 0.2.12, 0.2.11, 0.2.10, 0.2.9, 0.2.8, 0.2.7, 0.2.6, 0.2.5, 0.2.4, 0.2.3, 0.2.2, 0.2.1, 0.2.0

the package `wasm-bindgen` links to the native library `wasm_bindgen`, but it conflicts with a previous package which links to `wasm_bindgen` as well:
package `wasm-bindgen-shared v0.2.81`
    ... which satisfies dependency `wasm-bindgen-shared = "=0.2.81"` of package `wasm-bindgen-backend v0.2.81`
    ... which satisfies dependency `wasm-bindgen-backend = "=0.2.81"` of package `wasm-bindgen-macro-support v0.2.81`
    ... which satisfies dependency `wasm-bindgen-macro-support = "=0.2.81"` of package `wasm-bindgen-macro v0.2.81`
    ... which satisfies dependency `wasm-bindgen-macro = "=0.2.81"` of package `wasm-bindgen v0.2.81`
    ... which satisfies dependency `wasm-bindgen = "^0.2.81"` of package `iana-time-zone v0.1.45`
    ... which satisfies dependency `iana-time-zone = "^0.1.44"` of package `chrono v0.4.22`
    ... which satisfies dependency `chrono = "^0.4.22"` of package `test-rs v0.1.0 (/Users/djc/src/test-rs)`
Only one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary. Try to adjust your dependencies so that only one package uses the links ='wasm-bindgen' value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.

all possible versions conflict with previously selected packages.

  previously selected package `wasm-bindgen v0.2.81`
    ... which satisfies dependency `wasm-bindgen = "^0.2.81"` of package `iana-time-zone v0.1.45`
    ... which satisfies dependency `iana-time-zone = "^0.1.44"` of package `chrono v0.4.22`
    ... which satisfies dependency `chrono = "^0.4.22"` of package `test-rs v0.1.0 (/Users/djc/src/test-rs)`

failed to select a version for `wasm-bindgen` which could resolve this conflict

So I would like to request you reconsider how you support older MSRVs.

djc added a commit to djc/iana-time-zone that referenced this issue Sep 12, 2022
WebAssembly support was added in strawlab#38 targeting the current versions
versions of wasm-bindgen and js-sys at the time, which is
reasonable. Unfortunately the whoami crate, in trying to support
older Rust versions, has reduced the upper version of wasm-bindgen
that it supports, see ardaku/whoami#43.

This PR thus reduces the minimum version required for wasm-bindgen.
I used `cargo +nightly update -Z minimal-versions` with
`cargo build --target wasm32-unknown-unknown` to find the earliest
version that compiles. We could theoretically set up checks in
CI to verify this on an ongoing basis, but this might be overkill
for now.
@AldaronLau
Copy link
Member

@djc Thanks, I can see it will be important to revert this regression, although I'm currently not sure how to still test the MSRV of WASM in CI once reverted. I can probably figure something out, but if you have any ideas, it might save me some time.

@AldaronLau AldaronLau added bug help wanted dependencies Pull requests that update a dependency file labels Sep 12, 2022
@djc
Copy link
Author

djc commented Sep 12, 2022

Thanks for the quick response! The opentelemetry project has a similar issue, see here:

https://github.com/open-telemetry/opentelemetry-rust/pull/866/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR73

@djc
Copy link
Author

djc commented Sep 12, 2022

(You may have seen that I also submitted strawlab/iana-time-zone#58 to reduce its dependency on wasm-bindgen, but I still think it would be good to come up with a better solution here.)

@AldaronLau
Copy link
Member

Thanks for the quick response! The opentelemetry project has a similar issue, see here:

https://github.com/open-telemetry/opentelemetry-rust/pull/866/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR73

Thanks, I think something like this will work fine! I'll try to get to releasing a new patch version later today.

Kijewski pushed a commit to strawlab/iana-time-zone that referenced this issue Sep 12, 2022
WebAssembly support was added in #38 targeting the current versions
versions of wasm-bindgen and js-sys at the time, which is
reasonable. Unfortunately the whoami crate, in trying to support
older Rust versions, has reduced the upper version of wasm-bindgen
that it supports, see ardaku/whoami#43.

This PR thus reduces the minimum version required for wasm-bindgen.
I used `cargo +nightly update -Z minimal-versions` with
`cargo build --target wasm32-unknown-unknown` to find the earliest
version that compiles. We could theoretically set up checks in
CI to verify this on an ongoing basis, but this might be overkill
for now.
@AldaronLau
Copy link
Member

Just released whoami version 1.2.3 which fixes this.

@djc
Copy link
Author

djc commented Sep 13, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug dependencies Pull requests that update a dependency file help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants