Move the baked exporter into its own crate #5009
Conversation
robertbastian
changed the title
robertbastian
force-pushed
the
bkd
branch
7 times, most recently
from
c1de915 to
62f63f5
Compare
| #[cfg(feature = "export")] | ||
| pub mod export; | ||
|
|
||
| pub use icu_provider::prelude::*; |
There was a problem hiding this comment.
question: why do we need this?
also is this crate useful outside of the export mode? it doesn't seem to contain much otherwise, should it be called baked_exporter instead?
There was a problem hiding this comment.
I want to expose zerotrie through it in the next PR.
The prelude export I didn't end up using, but I might change that.
There was a problem hiding this comment.
Who would depend on this in non exporter contexts?
There was a problem hiding this comment.
Baked data users, like all our _data crates.
There was a problem hiding this comment.
I kinda would like those crates to have minimum deps, so they should depend on zerotrie directly and not a re-export from here.
Unless this crate exports something built on top of ZT then I guess that's fine.
But given that these are weird crates that get replaced in places like Google3, having zero deps is really good for them
cc @sffc for thoughts
There was a problem hiding this comment.
I think I prefer component compiled data depending directly on what it needs; also note that we can't re-export utils from here, at least not publicly, because they have a 0.x semver.
There was a problem hiding this comment.
Well we can. Either we keep the policy that baked data needs to be generated with the same minor version, then this is fine, or we commit to stability in 2.0, then this is fine.
| use icu_provider::prelude::*; | ||
|
|
||
| #[cfg(feature = "export")] | ||
| pub fn bake( |
There was a problem hiding this comment.
Thought: I'm happy that we can move more common runtime baked data code into this crate.
| #[cfg(feature = "export")] | ||
| pub mod export; | ||
|
|
||
| pub use icu_provider::prelude::*; |
| Note: the path in `ICU4X_DATA_DIR` is relative to `provider/baked/*/src/lib.rs` and it causes VSCode to build ICU4X with only the `und` locale. This reduces build times but also makes some tests fail; to run them normally, run `cargo test --all-features` on the command line. | ||
| Note: the path in `ICU4X_DATA_DIR` is relative to `provider/data/*/src/lib.rs` and it causes VSCode to build ICU4X with only the `und` locale. This reduces build times but also makes some tests fail; to run them normally, run `cargo test --all-features` on the command line. |
There was a problem hiding this comment.
Thought: This rename is going to cause annoying conflicts
There was a problem hiding this comment.
Ack. icu_provider_baked should have the provider/baked directory though, and I'd rather not sprinkle the data crates in there as well.
|
Please re-run CI against latest main before merging. |
#4721
The
icu_provider_datagencrate can be a runtime dependency for baked data, in order to expose dependencies that are required for databake operation (I'm thinking zerotrie and writeable).The former
icu_datagen::baked_exportermodule is this new crate'sexportermodule, behind anexportfeature, analogous toicu_provider_blobandicu_provider_fs.This is also a better home for the
test-baked-sourcetest, because inicu_datagen, if the baked source wouldn't build,make-testdataalso wouldn't, which is annoying (@younies).