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

Unpin unicode-normalization version #28

Closed

Conversation

trevyn
Copy link
Contributor

@trevyn trevyn commented Jul 27, 2022

I ran into an issue where the very old pinned version of unicode-normalization conflicted with another dependency:

error: failed to select a version for `unicode-normalization`.
    ... required by package `idna v0.2.3`
    ... which satisfies dependency `idna = "^0.2.3"` of package `cookie_store v0.16.0`
    ... which satisfies dependency `cookie_store = "^0.16"` of package `reqwest v0.11.11`
    ... which satisfies dependency `reqwest = "^0.11.11"` of package `turbo v0.1.0 (/Users/e/Documents/GitHub/turbo)`
versions that meet the requirements `^0.1.17` are: 0.1.21, 0.1.20, 0.1.19, 0.1.18, 0.1.17

all possible versions conflict with previously selected packages.

  previously selected package `unicode-normalization v0.1.9`
    ... which satisfies dependency `unicode-normalization = "=0.1.9"` of package `bip39 v1.0.0`
    ... which satisfies dependency `bip39 = "^1"` of package `turbo v0.1.0 (/Users/e/Documents/GitHub/turbo)`

failed to select a version for `unicode-normalization` which could resolve this conflict

Removing this pin resolved it, and I couldn't find a reason why it was pinned. cargo test passes.

Closes #29

@tcharding
Copy link
Member

Thanks for your contribution! The pinning is because the current MSRV of this library is currently 1.29. There is an open PR to bump it to 1.41.1 at which stage this PR will become valid. Most of the other crates in the rust-bitcoin stack have bumped the MSRV already - this one is lagging a bit :)

$ cargo +1.29 check                                                                                                                                      ✘ 101
error: unable to get packages from source

Caused by:
failed to parse manifest at `/home/tobin/.cargo/registry/src/gitpro.ttaallkk.top-1ecc6299db9ec823/unicode-normalization-0.1.21/Cargo.toml`

Caused by:
editions are unstable

Caused by:
feature `edition` is required

@tnull
Copy link

tnull commented Sep 6, 2022

Would be great to see some progress here, running into the same issue.

@tcharding
Copy link
Member

Gentle bump @stevenroose

@trevyn
Copy link
Contributor Author

trevyn commented Sep 12, 2022

A workaround for some use cases: If the no_std API surface of bip-39 is sufficient, you can set default-features = false and this won’t bring in unicode-normalization at all.

@dvc94ch

This comment was marked as abuse.

@tcharding
Copy link
Member

really sad to see that crate maintainers only care about rust-bitcoin crates and show no interest in anyone else.

Sorry if it comes across like that, that is certainly not how I feel. I may have narrow focus because I only work on rust-bitcoin crates but we do strive to make our lib useful for folk, that's the only reason to write a library, right? We are explicitly conservative and try to optimize for correctness - please do say if we fail at either of these things :)

pinning dependencies to avoid bumping msrv is extremely inconsiderate. running rustup update from time to time is a reasonable thing to do. require everyone else to work around build failures due to wrong dependency specifications is not.

We take dependencies very seriously, the ones we use and also how we are used by others - we are definitely not perfect, if you see ways we can improve please share them. Everyone is learning and evolving, and we are always trying to improve the library.

EDIT: I take my criticism back as it seems @tcharding is actually doing something about it since yesterday

I don't know what I did yesterday but I'm always doing something :)

@dvc94ch

This comment was marked as abuse.

@dvc94ch

This comment was marked as abuse.

@TheBlueMatt
Copy link
Member

Please be respectful when interacting with open source maintainers. Just because you have a desire to use the latest rustc doesn't mean that all downstream projects from the rust-bitcoin project do. Independently, it's true the rust-bitcoin projects don't have as many resources as they ideally would, this is very common for open-source and is no reason to fling insults.

@dvc94ch

This comment was marked as off-topic.

@dvc94ch

This comment was marked as off-topic.

@apoelstra
Copy link
Member

@dvc94ch your tone is very agressive. I appreciate you are frustrated but it is indeed insulting to show up on an open-source project and call the maintainers "inconsiderate" and that they "have no interest in anybody else".

@stevenroose we really should fix the MSRV and fix the pinning though to be consistent with the other crates in the org

@dvc94ch

This comment was marked as abuse.

@dvc94ch

This comment was marked as abuse.

@fernandolguevara
Copy link

so?

@apoelstra
Copy link
Member

It looks like this crate is maintained by @stevenroose who is currently unavailable. I'm not sure when he'll be back.

I don't think anybody else wants to take over for the reasons listed in the rust-bitcoin discussion topic. We might need to drop it from the org. Without input from Steven it's hard to make a decision here.

@stevenroose
Copy link
Collaborator

Sorry for my absence the last several months. I saw an MR to bump MSRV but it also contains a breaking API change, I'll work towards trying to bump MSRV without a breaking change so that we can remove the pinned dep.

Anyway, if this causes a problem downstream, that's a Cargo bug IMO because we don't expose our use of the unicode-normalization crate so your build should be able to include multiple versions and use this specific version only for the bip39 crate. Pinning an unexposed dependency shouldn't have any effect on dependent crates.

@trevyn
Copy link
Contributor Author

trevyn commented Feb 23, 2023

Apparently cargo will only allow multiple conflicting versions when those conflicts are semver-incompatible, per rust-lang/cargo#6584

@stevenroose
Copy link
Collaborator

Right, I might have bumped into that topic before. Something we could do as an alternative is pin an even older version of unicode-normalization if we find one that works for us. ... Ok I went to look and the crate is as 0.1.x since basically forever with a few 0.0.x versions from 8 years ago.

I mean, bumping it to a newer pinned version is not a breaking change, so we could just do that whenever needed and possible.

@stevenroose
Copy link
Collaborator

Closing this, I bumped up our unicode-normalization dependency. I recognize it's unfortunate that cargo doesn't support having multiple minor versions of a crate in a project's dependency tree, especially as many crates don't respect MSRV in their semver updates. We can't unpin unicode normalization, but it's bumped to the latest version right now.

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

Successfully merging this pull request may close these issues.

Old version of unicode-normalization
8 participants