Skip to content

refactor: remove redundant ICU version-detection mechanism #364

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

Open
clydegerber wants to merge 2 commits into
base: main
Choose a base branch
from
Open

refactor: remove redundant ICU version-detection mechanism #364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0758a56
refactor: remove redundant ICU version-detection mechanism
clydegerber May 18, 2026
5f91005
docs(contributing): clarify reason for version gating
clydegerber Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ jobs:
matrix:
icu_version: [74, 76, 77]
feature_set:
- "renaming,icu_version_in_env,icu_version_64_plus,icu_version_67_plus,icu_version_68_plus"
- "renaming,icu_version_64_plus,icu_version_67_plus,icu_version_68_plus,icu_config,use-bindgen"
- "renaming,icu_version_in_env"
- "renaming,icu_config,use-bindgen"
steps:
- uses: actions/checkout@v6
- name: 'Test ICU version ${{ matrix.icu_version }}'
Expand Down
41 changes: 41 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,47 @@ term health of the repository.
* Please keep PRs limited to a single topic of change. This makes the PR easier to
review, and easier to roll back, if that becomes necessary.

## Adding ICU-version-dependent code

The workspace currently targets a range of ICU versions across which the
wrapped APIs behave uniformly enough that no version-gated code is needed.
Version gating has historically been required when ICU's behavior changes
between versions in user-visible ways — for example, changes in locale
data or fixes to behavioral bugs that cause the same input to produce
different output across ICU versions. If a future change introduces this
kind of difference, follow the pattern below — it must be wired up in
**two** places that are kept in sync.

### 1. The Cargo feature

Declare a feature in each affected crate's `Cargo.toml`, named after the
boundary version (e.g. `icu_version_78_plus`). Propagate it to dependent
crates the same way `icu_version_in_env` is propagated.

### 2. Automatic activation via `build.rs`

Comment thread
clydegerber marked this conversation as resolved.
A crate's `build.rs` may emit `cargo:rustc-cfg=feature="icu_version_XX_plus"`
to activate the feature based on the ICU version detected at build time (via
`pkg-config`). This makes `#[cfg(feature = "icu_version_XX_plus")]`-gated
code light up automatically under the default feature set
(`use-bindgen`, `icu_config`, `renaming`) without requiring users to pass
`--features` manually.

**Any crate that contains `#[cfg(feature = "icu_version_XX_plus")]`-gated
code must have a `build.rs`** that emits the corresponding cfg. Without it,
the gated code is silently excluded when building with default features,
even if a matching ICU version is installed. Copy the build script from any
crate that has one (e.g. `rust_icu_uloc`) and add the matching
`[build-dependencies]` entry pointing at `rust_icu_release`.

### Explicit activation (when `icu_config` is inactive)

When `icu_config` is disabled (e.g. when using pre-generated static bindgen
files with `icu_version_in_env`), the `build.rs` does not emit cfgs, so
version features must be passed explicitly via
`--features=icu_version_XX_plus,...`. The CI `test-with-features` matrix
job is the right place to add coverage for that path.

## Community Guidelines

This project follows [Google's Open Source Community
Expand Down
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,11 @@ macos-test:
&& PATH="$$ICU_PREFIX/bin:$$PATH" \
PKG_CONFIG_PATH="$$ICU_PREFIX/lib/pkgconfig:$$PKG_CONFIG_PATH" \
RUSTFLAGS="-L $$ICU_PREFIX/lib" \
cargo test --no-default-features --features=use-bindgen,icu_config,renaming,icu_version_64_plus,icu_version_67_plus,icu_version_68_plus \
cargo test \
&& PATH="$$ICU_PREFIX/bin:$$PATH" \
PKG_CONFIG_PATH="$$ICU_PREFIX/lib/pkgconfig:$$PKG_CONFIG_PATH" \
RUSTFLAGS="-L $$ICU_PREFIX/lib" \
cargo test --no-default-features --features=use-bindgen,icu_config,renaming,icu_version_64_plus,icu_version_67_plus,icu_version_68_plus,static
cargo test --features=static
.PHONY: macos-test

# Refreshes the static bindgen output (contents of ./rust_icu_sys/bindgen) based
Expand Down
76 changes: 0 additions & 76 deletions rust_icu/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,82 +116,6 @@ icu_version_in_env = [
"rust_icu_utext/icu_version_in_env",
"rust_icu_utrans/icu_version_in_env",
]
icu_version_64_plus = [
"rust_icu_common/icu_version_64_plus",
"rust_icu_sys/icu_version_64_plus",
"rust_icu_ubrk/icu_version_64_plus",
"rust_icu_ucal/icu_version_64_plus",
"rust_icu_ucol/icu_version_64_plus",
"rust_icu_ucsdet/icu_version_64_plus",
"rust_icu_udat/icu_version_64_plus",
"rust_icu_udata/icu_version_64_plus",
"rust_icu_uenum/icu_version_64_plus",
"rust_icu_ulistformatter/icu_version_64_plus",
"rust_icu_uloc/icu_version_64_plus",
"rust_icu_umsg/icu_version_64_plus",
"rust_icu_unorm2/icu_version_64_plus",
"rust_icu_ures/icu_version_64_plus",
"rust_icu_ustring/icu_version_64_plus",
"rust_icu_utext/icu_version_64_plus",
"rust_icu_utrans/icu_version_64_plus",
]
icu_version_67_plus = [
"rust_icu_common/icu_version_67_plus",
"rust_icu_sys/icu_version_67_plus",
"rust_icu_ubrk/icu_version_67_plus",
"rust_icu_ucal/icu_version_67_plus",
"rust_icu_ucol/icu_version_67_plus",
"rust_icu_ucsdet/icu_version_67_plus",
"rust_icu_udat/icu_version_67_plus",
"rust_icu_udata/icu_version_67_plus",
"rust_icu_uenum/icu_version_67_plus",
"rust_icu_ulistformatter/icu_version_67_plus",
"rust_icu_uloc/icu_version_67_plus",
"rust_icu_umsg/icu_version_67_plus",
"rust_icu_unorm2/icu_version_67_plus",
"rust_icu_ures/icu_version_67_plus",
"rust_icu_ustring/icu_version_67_plus",
"rust_icu_utext/icu_version_67_plus",
"rust_icu_utrans/icu_version_67_plus",
]
icu_version_68_plus = [
"rust_icu_common/icu_version_68_plus",
"rust_icu_sys/icu_version_68_plus",
"rust_icu_ubrk/icu_version_68_plus",
"rust_icu_ucal/icu_version_68_plus",
"rust_icu_ucol/icu_version_68_plus",
"rust_icu_ucsdet/icu_version_68_plus",
"rust_icu_udat/icu_version_68_plus",
"rust_icu_udata/icu_version_68_plus",
"rust_icu_uenum/icu_version_68_plus",
"rust_icu_ulistformatter/icu_version_68_plus",
"rust_icu_uloc/icu_version_68_plus",
"rust_icu_umsg/icu_version_68_plus",
"rust_icu_unorm2/icu_version_68_plus",
"rust_icu_ures/icu_version_68_plus",
"rust_icu_ustring/icu_version_68_plus",
"rust_icu_utext/icu_version_68_plus",
"rust_icu_utrans/icu_version_68_plus",
]
icu_version_69_max = [
"rust_icu_common/icu_version_69_max",
"rust_icu_sys/icu_version_69_max",
"rust_icu_ubrk/icu_version_69_max",
"rust_icu_ucal/icu_version_69_max",
"rust_icu_ucol/icu_version_69_max",
"rust_icu_ucsdet/icu_version_69_max",
"rust_icu_udat/icu_version_69_max",
"rust_icu_udata/icu_version_69_max",
"rust_icu_uenum/icu_version_69_max",
"rust_icu_ulistformatter/icu_version_69_max",
"rust_icu_uloc/icu_version_69_max",
"rust_icu_umsg/icu_version_69_max",
"rust_icu_unorm2/icu_version_69_max",
"rust_icu_ures/icu_version_69_max",
"rust_icu_ustring/icu_version_69_max",
"rust_icu_utext/icu_version_69_max",
"rust_icu_utrans/icu_version_69_max",
]
static = ["rust_icu_sys/static"]

[badges]
Expand Down
12 changes: 0 additions & 12 deletions rust_icu_common/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,6 @@ use-bindgen = ["rust_icu_sys/use-bindgen"]
renaming = ["rust_icu_sys/renaming"]
icu_config = ["rust_icu_sys/icu_config"]
icu_version_in_env = ["rust_icu_sys/icu_version_in_env"]
icu_version_64_plus = [
"rust_icu_sys/icu_version_64_plus",
]
icu_version_67_plus = [
"rust_icu_sys/icu_version_67_plus",
]
icu_version_68_plus = [
"rust_icu_sys/icu_version_68_plus",
]
icu_version_69_max = [
"rust_icu_sys/icu_version_69_max",
]

[badges]
maintenance = { status = "actively-developed" }
Expand Down
40 changes: 0 additions & 40 deletions rust_icu_ecma402/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,46 +74,6 @@ icu_version_in_env = [
"rust_icu_upluralrules/icu_version_in_env",
"rust_icu_ustring/icu_version_in_env",
]
icu_version_64_plus = [
"rust_icu_common/icu_version_64_plus",
"rust_icu_sys/icu_version_64_plus",
"rust_icu_ulistformatter/icu_version_64_plus",
"rust_icu_uloc/icu_version_64_plus",
"rust_icu_unum/icu_version_64_plus",
"rust_icu_unumberformatter/icu_version_64_plus",
"rust_icu_upluralrules/icu_version_64_plus",
"rust_icu_ustring/icu_version_64_plus",
]
icu_version_67_plus = [
"rust_icu_common/icu_version_67_plus",
"rust_icu_sys/icu_version_67_plus",
"rust_icu_ulistformatter/icu_version_67_plus",
"rust_icu_uloc/icu_version_67_plus",
"rust_icu_unum/icu_version_67_plus",
"rust_icu_unumberformatter/icu_version_67_plus",
"rust_icu_upluralrules/icu_version_67_plus",
"rust_icu_ustring/icu_version_67_plus",
]
icu_version_68_plus = [
"rust_icu_common/icu_version_68_plus",
"rust_icu_sys/icu_version_68_plus",
"rust_icu_ulistformatter/icu_version_68_plus",
"rust_icu_uloc/icu_version_68_plus",
"rust_icu_unum/icu_version_68_plus",
"rust_icu_unumberformatter/icu_version_68_plus",
"rust_icu_upluralrules/icu_version_68_plus",
"rust_icu_ustring/icu_version_68_plus",
]
icu_version_69_max = [
"rust_icu_common/icu_version_69_max",
"rust_icu_sys/icu_version_69_max",
"rust_icu_ulistformatter/icu_version_69_max",
"rust_icu_uloc/icu_version_69_max",
"rust_icu_unum/icu_version_69_max",
"rust_icu_unumberformatter/icu_version_69_max",
"rust_icu_upluralrules/icu_version_69_max",
"rust_icu_ustring/icu_version_69_max",
]

[badges]
maintenance = { status = "actively-developed" }
Expand Down
6 changes: 0 additions & 6 deletions rust_icu_ecma402/src/datetimeformat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,12 +285,6 @@ mod testing {
dates: vec![10000_f64],
expected_regex: vec!["1970."],
},
// In ICU 63 this gets reported as "GMT-08:00", likely the ICU data
// from then didn't contain the Serbian long spellout of the "uslax"
// time zone. Turning on from 67 onwards, since at the time this
// test was written that was the oldest version that did have the
// long spellout.
#[cfg(feature="icu_version_67_plus")]
TestCase {
locale: "sr",
opts: DateTimeFormatOptions{
Expand Down
30 changes: 5 additions & 25 deletions rust_icu_ecma402/src/listformat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ pub struct Format {
rep: ulfmt::UListFormatter,
}

// Full support for styled formatting is available since v67.
#[cfg(feature = "icu_version_67_plus")]
pub(crate) mod internal {
use ecma402_traits::listformat::options;
use rust_icu_sys as usys;
Expand Down Expand Up @@ -60,24 +58,13 @@ impl listformat::Format for Format {
/// Creates a new [Format], from a [ecma402_traits::Locale] and [listformat::Options].
fn try_new<L: ecma402_traits::Locale>(
l: L,
_opts: listformat::Options,
opts: listformat::Options,
) -> Result<Format, Self::Error> {
let locale = format!("{}", l);

#[cfg(feature = "icu_version_67_plus")]
{
let width = internal::to_icu_width(&_opts.style);
let in_type = internal::to_icu_type(&_opts.in_type);
let rep = ulfmt::UListFormatter::try_new_styled(&locale, in_type, width)?;
Ok(Format { rep })
}

// The non-v67 implementation is less featureful.
#[cfg(not(feature = "icu_version_67_plus"))]
{
let rep = ulfmt::UListFormatter::try_new(&locale)?;
Ok(Format { rep })
}
let width = internal::to_icu_width(&opts.style);
let in_type = internal::to_icu_type(&opts.in_type);
let rep = ulfmt::UListFormatter::try_new_styled(&locale, in_type, width)?;
Ok(Format { rep })
}

/// Formats the given string.
Expand Down Expand Up @@ -123,7 +110,6 @@ mod testing {
opts: listformat::Options::default(),
expected: "eenie, meenie, minie, and moe",
},
#[cfg(feature = "icu_version_67_plus")]
TestCase {
locale: "en-US",
array: vec!["eenie", "meenie", "minie", "moe"],
Expand All @@ -133,7 +119,6 @@ mod testing {
},
expected: "eenie, meenie, minie, & moe",
},
#[cfg(feature = "icu_version_67_plus")]
TestCase {
locale: "en-US",
array: vec!["eenie", "meenie", "minie", "moe"],
Expand All @@ -143,7 +128,6 @@ mod testing {
},
expected: "eenie, meenie, minie, or moe",
},
#[cfg(feature = "icu_version_67_plus")]
TestCase {
locale: "en-US",
array: vec!["eenie", "meenie", "minie", "moe"],
Expand All @@ -153,7 +137,6 @@ mod testing {
},
expected: "eenie, meenie, minie, moe",
},
#[cfg(feature = "icu_version_67_plus")]
TestCase {
locale: "en-US",
array: vec!["eenie", "meenie", "minie", "moe"],
Expand All @@ -172,7 +155,6 @@ mod testing {
},
expected: "eenie, meenie, minie, and moe",
},
#[cfg(feature = "icu_version_67_plus")]
TestCase {
locale: "en-US",
array: vec!["eenie", "meenie", "minie", "moe"],
Expand All @@ -193,7 +175,6 @@ mod testing {
},
expected: "Раја, Гаја и Влаја",
},
#[cfg(feature = "icu_version_67_plus")]
TestCase {
locale: "sr-RS",
array: vec!["Раја", "Гаја", "Влаја"],
Expand All @@ -203,7 +184,6 @@ mod testing {
},
expected: "Раја, Гаја или Влаја",
},
#[cfg(feature = "icu_version_67_plus")]
TestCase {
locale: "sr-RS",
array: vec!["Раја", "Гаја", "Влаја"],
Expand Down
5 changes: 1 addition & 4 deletions rust_icu_ecma402/src/numberformat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,7 @@ pub(crate) mod internal {
// 3 -> "integer-width/*000"
fn integer_digits(digits: usize) -> String {
let zeroes: String = std::iter::repeat("0").take(digits).collect();
#[cfg(feature = "icu_version_67_plus")]
return format!("integer-width/*{}", zeroes);
#[cfg(not(feature = "icu_version_67_plus"))]
return format!("integer-width/+{}", zeroes);
format!("integer-width/*{}", zeroes)
}

fn fraction_digits(min: usize, max: usize, min_sig: usize, max_sig: usize) -> String {
Expand Down
28 changes: 0 additions & 28 deletions rust_icu_intl/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,31 +56,3 @@ icu_version_in_env = [
"rust_icu_umsg/icu_version_in_env",
"rust_icu_ustring/icu_version_in_env",
]
icu_version_64_plus = [
"rust_icu_common/icu_version_64_plus",
"rust_icu_sys/icu_version_64_plus",
"rust_icu_uloc/icu_version_64_plus",
"rust_icu_umsg/icu_version_64_plus",
"rust_icu_ustring/icu_version_64_plus",
]
icu_version_67_plus = [
"rust_icu_common/icu_version_67_plus",
"rust_icu_sys/icu_version_67_plus",
"rust_icu_uloc/icu_version_67_plus",
"rust_icu_umsg/icu_version_67_plus",
"rust_icu_ustring/icu_version_67_plus",
]
icu_version_68_plus = [
"rust_icu_common/icu_version_68_plus",
"rust_icu_sys/icu_version_68_plus",
"rust_icu_uloc/icu_version_68_plus",
"rust_icu_umsg/icu_version_68_plus",
"rust_icu_ustring/icu_version_68_plus",
]
icu_version_69_max = [
"rust_icu_common/icu_version_69_max",
"rust_icu_sys/icu_version_69_max",
"rust_icu_uloc/icu_version_69_max",
"rust_icu_umsg/icu_version_69_max",
"rust_icu_ustring/icu_version_69_max",
]
8 changes: 0 additions & 8 deletions rust_icu_release/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,6 @@ icu_config = [
]
icu_version_in_env = [
]
icu_version_64_plus = [
]
icu_version_67_plus = [
]
icu_version_68_plus = [
]
icu_version_69_max = [
]

[badges]
maintenance = { status = "actively-developed" }
Expand Down
Loading
Loading