diff --git a/crates/fonts-build/README.md b/crates/fonts-build/README.md index d5b1c4e2..62c71ced 100644 --- a/crates/fonts-build/README.md +++ b/crates/fonts-build/README.md @@ -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. diff --git a/crates/fonts-build/src/embed.rs b/crates/fonts-build/src/embed.rs index d8decac1..69fc5657 100644 --- a/crates/fonts-build/src/embed.rs +++ b/crates/fonts-build/src/embed.rs @@ -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) -> Result, 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 = 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(()); } diff --git a/crates/fonts-build/src/tests.rs b/crates/fonts-build/src/tests.rs index 03a04413..532353d9 100644 --- a/crates/fonts-build/src/tests.rs +++ b/crates/fonts-build/src/tests.rs @@ -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::::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]