ardaku / whoami

Rust crate to get the current user and environment.
Apache License 2.0
195 stars 31 forks source link

Consider allowing newer versions of wasm dependencies #43

Closed djc closed 2 years ago

djc commented 2 years ago

In https://github.com/ardaku/whoami/pull/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.

AldaronLau commented 2 years ago

@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.

djc commented 2 years ago

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 commented 2 years ago

(You may have seen that I also submitted https://github.com/strawlab/iana-time-zone/pull/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 commented 2 years ago

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.

AldaronLau commented 2 years ago

Just released whoami version 1.2.3 which fixes this.

djc commented 2 years ago

Thanks!