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.

Sign up for GitHub

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

Jump to bottom

Increment MSRV to 1.81 and use core::error::Error #5964

Closed
hsivonen opened this issue Jan 9, 2025 · 1 comment · Fixed by #5973
Closed

Increment MSRV to 1.81 and use core::error::Error #5964

hsivonen opened this issue Jan 9, 2025 · 1 comment · Fixed by #5973
Assignees
@robertbastian
Labels
2.0-breaking Changes that are breaking API changes
Milestone
ICU4X 2.0 ⟨P1⟩

Comments

@hsivonen
Copy link
Member

hsivonen commented Jan 9, 2025

I will be pushing for 1.81 as soon as our policy allows (in six weeks), as that contains core::error and will let us get rid of most our std features. Might want to do that now already if we're trying to minimize the number of MSRV bumps after release.

Originally posted by @robertbastian in #5934 (comment)

core::error::Error available vs. not available has been messy for other crates (see e.g. servo/rust-url#992 ), so we really should get this done before 2.0 to avoid the mess for ICU4X 2.x series.
@hsivonen hsivonen added 2.0-breaking Changes that are breaking API changes discuss-priority Discuss at the next ICU4X meeting labels Jan 9, 2025
@Manishearth
Copy link
Member

  • @sffc: Yes we should. technically against policy, but will not be by the 2.0 release. major version is best time to do this, and we don't want to make breaking changes later. We'll just have to delay the release for six weeks
  • @robertbastian: We should not block on the six weeks
  • @Manishearth: agreement
  • @sffc: not sure if we will get 2.0 out before this but i'm fine even if we do
  • @robertbastian: Remove all the std features?
  • @Manishearth component crates should never have std features imo, we should use utils if we need them.
  • @hsivonen Moving core::error::Error away from being feature-dependent is a good change
  • @Manishearth: amending. component crates should never have std features except for hashmap and other pure-algorithm deps.
  • @robertbastian: should we keep std feature around just in case we might need it?
  • @sffc: no; we can add it back if we want to use hashmap or something.
  • @hsivonen We may also want to read system preferences, which would need std.
  • @Manishearth That would go into a util crate.
  • @Manishearth It would be nice to say, "we are no_std with alloc" with no caveats

Working Group Consensus:

  1. Bump MSRV to 1.81 in the 2.0 release
  2. OK to do it on the main branch now
  3. OK to release 2.0 even if Rust 1.85 isn't released yet
  4. Remove the std feature when it is not needed any more

Consensus: @Manishearth @sffc @robertbastian @hsivonen

We shall ask @zbraniecki to approve the above so that we can upgrade it to TC consensus (needed for point 3)

@hsivonen hsivonen removed the discuss-priority Discuss at the next ICU4X meeting label Jan 9, 2025
@Manishearth Manishearth added this to the ICU4X 2.0 ⟨P1⟩ milestone Jan 10, 2025
This was referenced Jan 13, 2025
Merged
Merged
robertbastian added a commit that referenced this issue Jan 13, 2025
@robertbastian
Use core::error (#5973)
263fdc2 
Fixes #5964 

The only remaining uses of `std` are:
* `DataError::Io` and `DataError::with_path`
* `with_path` can be replaced by `with_display_context(path.display())`,
but the `std::io::ErrorKind` is not in `core` (it could be, it's a
fieldless enum).
* `icu_provider::log` falling back to `println!` (negative `logging`
feature)
* `icu_capi` keeps its std feature, as that's required to build a static
library with panic=unwind. I'm not sure what the use case for this is,
but we use it as a baseline in the size benchmark.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@robertbastian robertbastian

Labels
2.0-breaking Changes that are breaking API changes
Projects
None yet
Milestone
ICU4X 2.0 ⟨P1⟩
Development

Successfully merging a pull request may close this issue.

Use core::error robertbastian/icu4x
3 participants
@hsivonen @Manishearth @robertbastian