fix: add build.rs to crates with version-gated code lacking auto-detection#363
Open
clydegerber wants to merge 1 commit into
Open
fix: add build.rs to crates with version-gated code lacking auto-detection#363clydegerber wants to merge 1 commit into
clydegerber wants to merge 1 commit into
Conversation
…ction rust_icu_ulistformatter, rust_icu_unumberformatter, and rust_icu_ecma402 contained #[cfg(feature = "icu_version_XX_plus")]-gated code but had no build.rs. Without a build.rs calling rust_icu_release::run(), the installed ICU version is never queried for these crates and the version cfg flags are never emitted, causing version-gated code and tests to be silently excluded when building with default features, even if a modern ICU version is installed. Add a build.rs (following the pattern established in rust_icu_uloc) and the corresponding [build-dependencies] to each affected crate. With all version-gated crates now having a build.rs for auto-detection, the macos-test Makefile target no longer needs to pass explicit version feature flags; simplify it to use default features for the dynamic-link pass and --features=static for the static-link pass. The Makefile is taken from the fix/macos-native-bindgen branch which also updates the brew prefix detection and adds PKG_CONFIG_PATH and RUSTFLAGS for the Homebrew ICU installation. Document the two-mechanism version detection design in CONTRIBUTING.md so future contributors know to add a build.rs when adding version-gated code. This commit was created by an automated coding assistant, with human supervision.
Member
|
I'm not sure what to think about the proliferation of Flip side, most Thoughts? |
Contributor
Author
|
Removing the obsolete feature flags is a good call. I'll work it up. |
Contributor
Author
|
I created a new PR (364) for this approach. I can close whichever one we don't want to go with. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
build.rstorust_icu_ecma402,rust_icu_ulistformatter, andrust_icu_unumberformatter. These crates have#[cfg(feature = "icu_version_XX_plus")]-gated code but lacked their ownbuild.rs, so they relied on downstream feature propagation rather than ICU auto-detection frompkg-config.The new
build.rsfiles follow the same pattern asrust_icu_uloc/build.rs, callingrust_icu_release::run()to emitcargo:rustc-cfgflags based on the installed ICU version.Also:
CONTRIBUTING.mddocumenting how version-gated code is wired up.macos-testMakefile target, since the affected crates now self-configure.Test plan
cargo test -p rust_icu_ulistformatterruns the previously-skipped version-gated testThis commit was created by an automated coding assistant, with human supervision.