Skip to content

Unpin unicode-normalization version - #28

Closed
trevyn wants to merge 1 commit into
rust-bitcoin:masterfrom
trevyn:unpin-unicode-normalization
Closed

Unpin unicode-normalization version#28
trevyn wants to merge 1 commit into
rust-bitcoin:masterfrom
trevyn:unpin-unicode-normalization

Conversation

@trevyn

@trevyn trevyn commented Jul 27, 2022

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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/github.com-1ecc6299db9ec823/unicode-normalization-0.1.21/Cargo.toml`

Caused by:
editions are unstable

Caused by:
feature `edition` is required

@tnull

tnull commented Sep 6, 2022

Copy link
Copy Markdown
Collaborator

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

@tcharding

Copy link
Copy Markdown
Member

Gentle bump @stevenroose

@trevyn

trevyn commented Sep 12, 2022

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

so?

@apoelstra

Copy link
Copy Markdown
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
Copy Markdown
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

trevyn commented Feb 23, 2023

Copy link
Copy Markdown
Contributor Author

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

@stevenroose

Copy link
Copy Markdown
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
Copy Markdown
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.

@dpc

dpc commented May 12, 2025

Copy link
Copy Markdown

Hello! :D

> cargo update -p unicode-normalization
    Updating crates.io index
error: failed to select a version for `unicode-normalization`.
    ... required by package `precis-core v0.1.11`
    ... which satisfies dependency `precis-core = "^0.1.11"` of package `stun-rs v0.1.11`
    ... which satisfies dependency `stun-rs = "^0.1.11"` of package `iroh v0.35.0`
    ... which satisfies dependency `iroh-next = "^0.35.0"` of package `fedimint-api-client v0.8.0-alpha (/home/dpc/lab/fedimint/fedimint/fedimint-api-client)`
    ... which satisfies path dependency `fedimint-api-client` (locked to 0.8.0-alpha) of package `fedimint-cli v0.8.0-alpha (/home/dpc/lab/fedimint/fedimint/fedimint-cli)`
versions that meet the requirements `^0.1.24` are: 0.1.24

all possible versions conflict with previously selected packages.

  previously selected package `unicode-normalization v0.1.22`
    ... which satisfies dependency `unicode-normalization = "=0.1.22"` of package `bip39 v2.1.0`
    ... which satisfies dependency `bip39 = "^2.1.0"` of package `fedimint-bip39 v0.8.0-alpha (/home/dpc/lab/fedimint/fedimint/fedimint-bip39)`
    ... which satisfies path dependency `fedimint-bip39` (locked to 0.8.0-alpha) of package `fedimint-cli v0.8.0-alpha (/home/dpc/lab/fedimint/fedimint/fedimint-cli)`

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

I can't update stuffs in my Cargo.toml, which lead me here to rehash an old issue. :D

I don't understand why is this crate pinned in the first place. If some downstream projects need some MSRV, shouldn't they pin it downstream? 🤷

Also, wouldn't be possible to have unicode as whole behind a feature flag (not just behind std?) My full name has non-ascii characters, but for bip39 applications I'm fine with just English. :D

Let me see if avoiding std would help... (Edit: Nope, it will not).

Edit: Oh, another thought. Could just fork unicode-normalization and publish is as unicode-normalization-rust-bitcoin and that probably would avoid any issues.

dpc added a commit to fedimint/rust-bip39 that referenced this pull request May 12, 2025
To unblock dual-stack Iroh testing.

See rust-bitcoin#28
elsirion pushed a commit to fedimint/rust-bip39 that referenced this pull request May 19, 2025
To unblock dual-stack Iroh testing.

See rust-bitcoin#28
@stevenroose

Copy link
Copy Markdown
Collaborator

The unicode-normalization crate was just known to violate semver when bumping their msrv, so we had to pin it to guarantee semver ourselves. I can see if I can maybe make a range dependency.

But a unicode normalization feature is also something I wouldn't be opposed to.

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

9 participants