craft-fonts-build: warn instead of cutting to kana when subset_to gets no catalogs, and when the manifest has no interface face; watch the manifest only after reading it

This commit is contained in:
aiongg committed 2026-10-23 05:40:04 +00:00
1 parent a30ff6bf2f
commit 1477ffff09
3 files changed
+39 -6

No files matched your search

+3 -2
View File
@@ -140,8 +140,9 @@ the five cuts take about 3 MB together.
every catalog, so a Japanese label in a Chinese catalog still draws. The cuts are written to
`OUT_DIR/craft-fonts-ui/` and embedded from there; the same inputs give the same bytes, for
reproducible releases. Cutting all five faces takes well under a second.
- Without `subset_to`, `for_ui()` leaves the interface faces out with a `cargo::warning`: a full
CJK face is never embedded for the interface by accident. With `CRAFT_FONTS_REQUIRED=1` that
- Without `subset_to`, or with an empty catalog list, `for_ui()` leaves the interface faces out
with a `cargo::warning`: a full CJK face is never embedded for the interface by accident. A
checkout whose manifest has no interface face is a `cargo::warning` too. With `CRAFT_FONTS_REQUIRED=1` that
makes `scripts(&["Hant"])` an error; with catalogs, an interface face satisfies it.
- An interface face with CFF outlines is an error (`Error::NotTrueType`), even when not required:
the subsetter cuts TrueType (`glyf`) faces correctly, but not CFF ones.
+21 -4
View File
@@ -81,6 +81,7 @@ impl Embed {
/// the manifest), cut to the characters of the catalogs given to `Embed::subset_to`. Other
/// CJK faces are dropped and the rest are kept. Without catalogs the interface faces are left
/// out too, with a `cargo::warning`, so a full CJK face is never embedded for the interface.
/// A manifest with no interface face is also a `cargo::warning` when that drops a CJK face.
/// Combines with [`Embed::scripts`] and [`Embed::filter`]; with `CRAFT_FONTS_REQUIRED=1`, an
/// interface face satisfies its script tags.
pub fn for_ui(mut self) -> Self {
@@ -89,7 +90,8 @@ impl Embed {
}
/// Cut the interface faces to the characters these translation catalogs use, and embed the
/// cut faces, written to `OUT_DIR`. Implies [`Embed::for_ui`].
/// cut faces, written to `OUT_DIR`. Implies [`Embed::for_ui`]. An empty list counts as no
/// catalogs: the interface faces are left out, with a `cargo::warning`.
///
/// The cut keeps the catalogs' characters from U+2E80 up (CJK and later blocks, without emoji,
/// variation selectors and tags), plus CJK punctuation (U+3000–303F), kana (U+3040–30FF,
@@ -201,8 +203,9 @@ impl Embed {
/// The wanted manifest entries with their canonical file paths.
fn select(&self, dir: &Path, cargo: &mut Vec<String>) -> Result<Vec<Face>, Error> {
let manifest = dir.join("fonts/manifest.txt");
cargo.push(format!("cargo::rerun-if-changed={}", manifest.display()));
let text = std::fs::read_to_string(&manifest).map_err(|source| Error::Read { path: manifest.clone(), source })?;
// Cargo treats a missing watched file as changed, which would rerun the build every time.
cargo.push(format!("cargo::rerun-if-changed={}", manifest.display()));
let entries = parse_manifest(&text).map_err(|error| Error::Parse { manifest: manifest.clone(), error })?;
if let Some(families) = &self.families {
let unknown: Vec<String> = families.iter().filter(|f| !entries.iter().any(|e| &e.family == *f)).cloned().collect();
@@ -216,8 +219,11 @@ impl Embed {
return Err(Error::UnknownFamilies { manifest, families: unknown, known });
}
}
let has_ui = entries.iter().any(ManifestEntry::is_ui);
let no_catalogs = self.catalogs.as_ref().is_none_or(Vec::is_empty);
let mut faces = Vec::new();
let mut uncut = Vec::new();
let mut dropped_cjk = false;
for entry in entries {
// Interface faces are the sources of the cuts: embedded whole only when named.
let named = match &self.families {
@@ -230,10 +236,12 @@ impl Embed {
let wanted = self.scripts.as_ref().is_none_or(|tags| tags.iter().any(|t| entry.has_script(t)));
let ui_dropped = self.ui && entry.is_cjk() && !entry.is_ui();
if !wanted || ui_dropped || self.filter.as_ref().is_some_and(|keep| !keep(&entry)) {
dropped_cjk |= wanted && ui_dropped;
continue;
}
let cut = self.ui && entry.is_ui();
if cut && self.catalogs.is_none() {
// An empty list would cut the faces to kana and punctuation, so it counts as none.
if cut && no_catalogs {
uncut.push(format!("{} {}", entry.family, entry.style));
continue;
}
@@ -245,12 +253,20 @@ impl Embed {
faces.push(Face { entry, path, index, cut });
}
if !uncut.is_empty() {
let why = if self.catalogs.is_some() { "Embed::subset_to was given no translation catalogs" } else { "no translation catalogs" };
cargo.push(format!(
"cargo::warning=craft-fonts: not embedding the interface faces {}: pass the translation catalogs to \
"cargo::warning=craft-fonts: not embedding the interface faces {}: {why}; pass the catalogs to \
Embed::subset_to (feature `subset`) to embed them cut to the catalogs' characters",
uncut.join(", ")
));
}
if dropped_cjk && !has_ui {
cargo.push(format!(
"cargo::warning=craft-fonts: not embedding any CJK face for the interface: {} has no interface face \
(role `ui`); use a craft-fonts checkout that has them",
manifest.display()
));
}
Ok(faces)
}
@@ -258,6 +274,7 @@ impl Embed {
#[cfg(feature = "subset")]
fn cut(&self, faces: &mut [Face], out_dir: &Path) -> Result<(), Error> {
let Some(catalogs) = &self.catalogs else { return Ok(()) };
// `select` leaves the faces to cut out when there are no catalogs.
if !faces.iter().any(|face| face.cut) {
return Ok(());
}
+15
View File
@@ -105,6 +105,8 @@ fn required_fails_without_a_usable_checkout() {
assert!(matches!(run(Embed::new(), Some(missing), Some("1")).result, Err(Error::Read { .. })));
let r = run(Embed::new(), Some(missing), None);
assert!(r.result.unwrap().is_empty());
// Watching a missing manifest would rerun the build script on every build.
assert!(!r.cargo.iter().any(|l| l.starts_with("cargo::rerun-if-changed")), "{:?}", r.cargo);
assert!(r.cargo.iter().any(|l| l.starts_with("cargo::warning=building without craft-fonts")), "{:?}", r.cargo);
let scratch = Scratch::new("missing-file");
@@ -358,6 +360,12 @@ fn for_ui_embeds_cjk_faces_only_as_cut_interface_faces() {
assert_eq!(w.len(), 1, "{w:#?}");
assert!(w[0].contains("not embedding the interface faces Noto Sans TC UI Regular, Noto Sans HK UI Regular"), "{}", w[0]);
assert!(w[0].contains("Embed::subset_to"), "{}", w[0]);
// A manifest without interface faces loses every CJK face, which is worth a warning.
let r = run(Embed::new().for_ui(), Some(&fixture()), None);
assert!(r.cargo.contains(&format!("cargo::rerun-if-changed={}", fixture().join("fonts/manifest.txt").display())), "{:?}", r.cargo);
let w = warnings(&r);
assert_eq!(w.len(), 1, "{w:#?}");
assert!(w[0].contains("not embedding any CJK face for the interface") && w[0].contains("role `ui`"), "{}", w[0]);
let r = run(Embed::new().for_ui().scripts(&["Hant", "Arab"]), Some(&dir), Some("1"));
let Err(Error::MissingScripts { scripts, .. }) = r.result else { panic!("{:?}", r.result) };
assert_eq!(scripts, ["Hant"]);
@@ -514,6 +522,13 @@ mod subset {
let r = run(Embed::new().subset_to([&tw, &ko]).check_text([scratch.0.join("zh-TW.tsv")]), Some(&dir), None);
assert_eq!(mapped(&std::fs::read(cut_file(&r, "Noto Sans TC UI")).unwrap()), [0x3001, 0x300C, 0x3042, 0x4E00, 0x4E01, 0xFF01]);
assert_eq!(warnings(&r).len(), 1, "{:#?}", warnings(&r));
// No catalogs (an app whose CRAFT_I18N_DIR is unset) leaves the faces out, not cut to kana.
let r = run(Embed::new().subset_to(Vec::<PathBuf>::new()), Some(&dir), None);
assert_eq!(r.result.as_ref().unwrap(), &["Noto Sans Arabic Regular"]);
let w = warnings(&r);
assert_eq!(w.len(), 1, "{w:#?}");
assert!(w[0].contains("Noto Sans TC UI Regular") && w[0].contains("given no translation catalogs"), "{}", w[0]);
}
#[test]