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

Remove icu_datagen legacy API #4962

Merged
merged 6 commits into from
May 30, 2024
Merged

Remove icu_datagen legacy API #4962

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ae1bc32
wave
robertbastian May 28, 2024
b673ef7
fmt
robertbastian May 29, 2024
5458fec
--fallback
robertbastian May 29, 2024
d9e4401
download
robertbastian May 29, 2024
2749f5f
doc
robertbastian May 29, 2024
fbd1f43
Merge branch 'main' into byelegacy
robertbastian May 30, 2024
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
2 changes: 1 addition & 1 deletion components/experimental/tests/transliterate/data/gen.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
cargo run -p icu_datagen --features experimental_components -- \
--keys "transliterator/rules@1" "normalizer/comp@1" "normalizer/decomp@1" "normalizer/nfd@1" "normalizer/nfdex@1" "normalizer/nfkd@1" "normalizer/nfkdex@1" "normalizer/uts46d@1" \
--locales full \
--fallback runtime-manual \
--runtime-fallback-location external \
--cldr-root $(dirname $0)/../../../../../provider/datagen/tests/data/cldr \
--format mod \
--out $(dirname $0)/baked \
Expand Down
2 changes: 1 addition & 1 deletion provider/datagen/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ postcard = { workspace = true }
simple_logger = { workspace = true }

[features]
default = ["bin", "use_wasm", "networking", "legacy_api", "rayon", "fs_exporter", "blob_exporter", "baked_exporter", "provider"]
default = ["bin", "use_wasm", "networking", "rayon", "fs_exporter", "blob_exporter", "baked_exporter", "provider"]
provider = [
"icu_calendar/datagen",
"icu_casemap/datagen",
Expand Down
4 changes: 0 additions & 4 deletions provider/datagen/README.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 0 additions & 31 deletions provider/datagen/src/baked_exporter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,6 @@ pub struct Options {
/// This is required when you are not using the `icu` crate, *and* you're building custom data providers;
/// data for `compiled_data` constructors uses `icu` names.
pub use_separate_crates: bool,
#[doc(hidden)] // deprecated, used by legacy testdata
pub insert_feature_gates: bool,
/// Whether to overwrite existing data. By default, errors if it is present.
pub overwrite: bool,
}
Expand All @@ -150,7 +148,6 @@ impl Default for Options {
fn default() -> Self {
Self {
pretty: false,
insert_feature_gates: false,
use_separate_crates: false,
overwrite: false,
}
Expand All @@ -163,7 +160,6 @@ pub struct BakedExporter {
// Input arguments
mod_directory: PathBuf,
pretty: bool,
insert_feature_gates: bool,
use_separate_crates: bool,
// Temporary storage for put_payload: key -> (bake -> {locale})
data: Mutex<HashMap<DataKey, BTreeMap<SyncTokenStream, BTreeSet<String>>>>,
Expand All @@ -178,7 +174,6 @@ impl std::fmt::Debug for BakedExporter {
f.debug_struct("BakedExporter")
.field("mod_directory", &self.mod_directory)
.field("pretty", &self.pretty)
.field("insert_feature_gates", &self.insert_feature_gates)
.field("use_separate_crates", &self.use_separate_crates)
// skip formatting intermediate data
.finish()
Expand All @@ -190,7 +185,6 @@ impl BakedExporter {
pub fn new(mod_directory: PathBuf, options: Options) -> Result<Self, DataError> {
let Options {
pretty,
insert_feature_gates,
use_separate_crates,
overwrite,
} = options;
Expand All @@ -208,7 +202,6 @@ impl BakedExporter {
mod_directory,
pretty,
use_separate_crates,
insert_feature_gates: insert_feature_gates && use_separate_crates,
data: Default::default(),
impl_data: Default::default(),
dependencies: Default::default(),
Expand Down Expand Up @@ -637,28 +630,6 @@ impl BakedExporter {

let data = move_out!(self.impl_data).into_inner().expect("poison");

let features = data
.iter()
.map(|(key, marker)| {
if !self.insert_feature_gates {
quote!()
} else if *key
== icu_datetime::provider::calendar::DateSkeletonPatternsV1Marker::KEY
// neo keys are also experimental
|| key.path().contains("datetime/symbols") || key.path().contains("datetime/patterns")
{
quote! { #[cfg(feature = "icu_datetime_experimental")] }
} else if *key == icu_plurals::provider::PluralRangesV1Marker::KEY {
quote! { #[cfg(feature = "icu_plurals_experimental")] }
} else if *key == icu_provider::hello_world::HelloWorldV1Marker::KEY {
quote!()
} else {
let feature = marker.split(" :: ").next().unwrap();
quote! { #[cfg(feature = #feature)] }
}
})
.collect::<Vec<_>>();

let markers = data
.values()
.map(|marker| marker.parse::<TokenStream>().unwrap())
Expand Down Expand Up @@ -747,7 +718,6 @@ impl BakedExporter {
($provider:ty) => {
make_provider!($provider);
#(
#features
#macro_idents ! ($provider);
)*
};
Expand All @@ -763,7 +733,6 @@ impl BakedExporter {
fn load_any(&self, key: icu_provider::DataKey, req: icu_provider::DataRequest) -> Result<icu_provider::AnyResponse, icu_provider::DataError> {
match key.hashed() {
#(
#features
h if h == <#markers as icu_provider::KeyedDataMarker>::KEY.hashed() =>
icu_provider::DataProvider::<#markers>::load(self, req).map(icu_provider::DataResponse::wrap_into_any_response),
)*
Expand Down
141 changes: 22 additions & 119 deletions provider/datagen/src/bin/icu4x-datagen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ struct Cli {
#[arg(help = "Requests verbose output")]
verbose: bool,

#[arg(long, value_enum, default_value_t = Format::DeprecatedDefault, hide_default_value = true)]
#[arg(long, value_enum)]
#[arg(
help = "Select the output format: a directory tree of files, a single blob, or a Rust module."
)]
Expand Down Expand Up @@ -132,34 +132,17 @@ struct Cli {
)]
include_collations: Vec<CollationTable>,

#[arg(long, hide = true)]
#[arg(help = "Deprecated, use --locales full or --locales modern")]
cldr_locale_subset: bool,

#[arg(long, short, num_args = 1..)]
#[arg(
help = "Include these resource keys in the output. Accepts multiple arguments.\n\
Set to 'all' for all keys, or 'none' for no keys."
)]
keys: Vec<String>,

#[arg(long, value_name = "KEY_FILE")]
#[arg(
help = "Path to text file with resource keys to include, one per line. Empty lines \
and lines starting with '#' are ignored.\n
Requires the `legacy_api` Cargo feature."
)]
#[cfg(feature = "legacy_api")]
key_file: Option<PathBuf>,

#[arg(long, value_name = "BINARY")]
#[arg(help = "Analyzes the binary and only includes keys that are used by the binary.")]
keys_for_bin: Option<PathBuf>,

#[arg(long, hide = true)]
#[arg(help = "Deprecated: alias for --keys all")]
all_keys: bool,

Comment on lines -159 to -162
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We've been piloting the new names for a while, but I still use --all-keys --all-locales because I find them easier to remember than --keys all --locales full. Please pull these two specific removals to their own PR

#[arg(long, short, num_args = 0..)]
#[cfg_attr(feature = "provider", arg(default_value = "recommended"))]
#[arg(
Expand All @@ -169,10 +152,6 @@ struct Cli {
)]
locales: Vec<String>,

#[arg(long, hide = true)]
#[arg(help = "Deprecated: alias for --locales full")]
all_locales: bool,

#[arg(long = "out", short, value_name = "PATH")]
#[arg(
help = "Path to output directory or file. Must be empty or non-existent, unless \
Expand All @@ -182,26 +161,12 @@ struct Cli {
)]
output: Option<PathBuf>,

#[arg(long, hide = true)]
#[arg(
help = "--format=mod only: insert feature gates for individual `icu_*` crates. Requires --use-separate-crates"
)]
insert_feature_gates: bool,

#[arg(long)]
#[arg(
help = "--format=mod only: use types from individual `icu_*` crates instead of the `icu` meta-crate."
)]
use_separate_crates: bool,

// TODO(#2856): Change the default to Auto in 2.0
#[arg(short, long, value_enum, default_value_t = Fallback::Hybrid)]
#[arg(
hide = true,
help = "Deprecated: use --deduplication, --runtime-fallback-location, or --without-fallback"
)]
fallback: Fallback,

#[arg(long)]
#[arg(
help = "disables locale fallback, instead exporting exactly the locales specified in --locales. \
Expand Down Expand Up @@ -244,7 +209,6 @@ enum Format {
Blob,
Blob2,
Mod,
DeprecatedDefault,
}

#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum, Debug)]
Expand Down Expand Up @@ -332,10 +296,15 @@ fn main() -> eyre::Result<()> {
.unwrap()
}

enum PreprocessedLocales {
LanguageIdentifiers(Vec<LanguageIdentifier>),
Full,
}

#[allow(unused_mut)]
let mut preprocessed_locales = if cli.locales.as_slice() == ["none"] {
Some(PreprocessedLocales::LanguageIdentifiers(vec![]))
} else if cli.locales.as_slice() == ["full"] || cli.all_locales {
} else if cli.locales.as_slice() == ["full"] {
Some(PreprocessedLocales::Full)
} else {
if cli.locales.as_slice() == ["all"] {
Expand Down Expand Up @@ -453,18 +422,10 @@ fn main() -> eyre::Result<()> {

let mut driver = DatagenDriver::new();

driver = driver.with_keys(if cli.all_keys {
icu_datagen::all_keys()
} else if !cli.keys.is_empty() {
driver = driver.with_keys(if !cli.keys.is_empty() {
match cli.keys.as_slice() {
[x] if x == "none" => Default::default(),
[x] if x == "all" => icu_datagen::all_keys(),
[x] if x == "experimental-all" => {
log::warn!("--keys=experimental-all is deprecated, using --keys=all.");
log::warn!("--keys=all behavior is dependent on activated Cargo features, so");
log::warn!("building with experimental features includes experimental keys");
icu_datagen::all_keys()
}
keys => keys
.iter()
.map(|k| icu_datagen::key(k).ok_or(eyre::eyre!(k.to_string())))
Expand All @@ -473,27 +434,10 @@ fn main() -> eyre::Result<()> {
} else if let Some(bin_path) = &cli.keys_for_bin {
icu_datagen::keys_from_bin(bin_path)?.into_iter().collect()
} else {
#[cfg(feature = "legacy_api")]
if let Some(key_file_path) = &cli.key_file {
log::warn!("The --key-file argument is deprecated.");
#[allow(deprecated)]
icu_datagen::keys_from_file(key_file_path)
.with_context(|| key_file_path.to_string_lossy().into_owned())?
.into_iter()
.collect()
} else {
eyre::bail!("--keys or --keys-for-bin are required.")
}
#[cfg(not(feature = "legacy_api"))]
eyre::bail!("--keys or --keys-for-bin are required.")
});

enum PreprocessedLocales {
LanguageIdentifiers(Vec<LanguageIdentifier>),
Full,
}

if cli.without_fallback || matches!(cli.fallback, Fallback::Preresolved) {
if cli.without_fallback {
driver = driver.with_locales_no_fallback(
match preprocessed_locales {
Some(PreprocessedLocales::Full) => {
Expand Down Expand Up @@ -521,52 +465,22 @@ fn main() -> eyre::Result<()> {
.map(|family_str| family_str.parse().wrap_err(family_str))
.collect::<eyre::Result<Vec<_>>>()?,
};
let mut options: FallbackOptions = Default::default();
options.deduplication_strategy =
match (cli.deduplication, cli.fallback, cli.without_fallback) {
(None, _, true) => None,
(Some(_), _, true) => {
eyre::bail!("cannot combine --without-fallback and --deduplication")
}
(Some(x), _, false) => match x {
Deduplication::Maximal => Some(icu_datagen::DeduplicationStrategy::Maximal),
Deduplication::RetainBaseLanguages => {
Some(icu_datagen::DeduplicationStrategy::RetainBaseLanguages)
}
Deduplication::None => Some(icu_datagen::DeduplicationStrategy::None),
},
(None, fallback_mode, false) => match fallback_mode {
Fallback::Auto => None,
Fallback::Hybrid => Some(icu_datagen::DeduplicationStrategy::None),
Fallback::Runtime => Some(icu_datagen::DeduplicationStrategy::Maximal),
Fallback::RuntimeManual => Some(icu_datagen::DeduplicationStrategy::Maximal),
Fallback::Preresolved => None,
},
};
options.runtime_fallback_location = match (
cli.runtime_fallback_location,
cli.fallback,
cli.without_fallback,
) {
(None, _, true) => None,
(Some(_), _, true) => {
eyre::bail!("cannot combine --without-fallback and --runtime-fallback-location")
}
(Some(RuntimeFallbackLocation::Internal), _, false) => {
Some(icu_datagen::RuntimeFallbackLocation::Internal)
}
(Some(RuntimeFallbackLocation::External), _, false) => {
Some(icu_datagen::RuntimeFallbackLocation::External)
let mut options = FallbackOptions::default();
options.deduplication_strategy = match cli.deduplication {
Some(Deduplication::Maximal) => Some(icu_datagen::DeduplicationStrategy::Maximal),
Some(Deduplication::RetainBaseLanguages) => {
Some(icu_datagen::DeduplicationStrategy::RetainBaseLanguages)
}
(None, Fallback::Auto, false) => None,
(None, Fallback::Hybrid, false) => Some(icu_datagen::RuntimeFallbackLocation::External),
(None, Fallback::Runtime, false) => {
Some(Deduplication::None) | None => Some(icu_datagen::DeduplicationStrategy::None),
};
options.runtime_fallback_location = match cli.runtime_fallback_location {
Some(RuntimeFallbackLocation::Internal) => {
Some(icu_datagen::RuntimeFallbackLocation::Internal)
}
(None, Fallback::RuntimeManual, false) => {
Some(RuntimeFallbackLocation::External) => {
Some(icu_datagen::RuntimeFallbackLocation::External)
}
(None, Fallback::Preresolved, false) => None,
None => Some(icu_datagen::RuntimeFallbackLocation::External),
};
driver = driver.with_locales_and_fallback(locale_families, options);
}
Expand All @@ -593,19 +507,13 @@ fn main() -> eyre::Result<()> {
driver.with_segmenter_models(cli.segmenter_models.clone())
};

if cli.format == Format::DeprecatedDefault {
log::warn!(
"Defaulting to --format=dir. This will become a required parameter in the future."
);
}

match cli.format {
#[cfg(not(feature = "fs_exporter"))]
Format::Dir | Format::DeprecatedDefault => {
Format::Dir => {
eyre::bail!("Exporting to an FsProvider requires the `fs_exporter` Cargo feature")
}
#[cfg(feature = "fs_exporter")]
Format::Dir | Format::DeprecatedDefault => driver.export(&provider, {
Format::Dir => driver.export(&provider, {
use icu_provider_fs::export::*;

FilesystemExporter::try_new(
Expand All @@ -621,10 +529,6 @@ fn main() -> eyre::Result<()> {
if cli.overwrite {
options.overwrite = OverwriteOption::RemoveAndReplace
}
#[allow(deprecated)] // obviously
{
options.fingerprint = cli.fingerprint;
}
options
},
)?
Expand Down Expand Up @@ -663,7 +567,6 @@ fn main() -> eyre::Result<()> {
{
let mut options = icu_datagen::baked_exporter::Options::default();
options.pretty = cli.pretty;
options.insert_feature_gates = cli.insert_feature_gates;
options.use_separate_crates = cli.use_separate_crates;
options.overwrite = cli.overwrite;
options
Expand Down
Loading
Loading