diff --git a/fontc_crater/README.md b/fontc_crater/README.md index b4066772..d172130a 100644 --- a/fontc_crater/README.md +++ b/fontc_crater/README.md @@ -7,13 +7,16 @@ fonts. ```sh -$ cargo run --release -p=fontc_crater -- compile FONT_CACHE --fonts-repo GOOGLE/FONTS -o results.json -``` +# By default font repositories and results will be written to ~/.fontc_crater_cache -or, to run ttx_diff (comparing with fontmake) +# Just see if we can compile with fontc +$ cargo run --release -p=fontc_crater -- compile -```sh -$ cargo run --release -p=fontc_crater -- diff FONT_CACHE --fonts-repo GOOGLE/FONTS -o results.json +# To take it for a test-spin do a limited # of fonts +$ cargo run --release -p=fontc_crater -- compile --limit 8 + +# Build with fontmake and fontc and compare the results with ttx_diff +$ cargo run --release -p=fontc_crater -- diff ``` This is a binary for executing font compilation (and possibly other tasks) in @@ -39,7 +42,7 @@ You can generate a report from the saved json by passing it back to `fontc_crater`: ```sh -$ cargo run -p fontc_crater -- report results.json +$ cargo run -p fontc_crater -- report ``` ## CI @@ -48,6 +51,17 @@ This binary is also run in CI. In that case, the execution is managed by a script in the [`fontc_crater` repo][crater-repo] and results are posted to [github pages][crater-results]. +To run in CI mode locally to play with the html output: + +```shell +# clone git@github.com:googlefonts/fontc_crater.git somewhere, we'll assume at ../fontc_crater +# CI currently has it's own format for the repo list, use the file from ^ + +$ cargo run --release -p=fontc_crater -- ci ../fontc_crater/gf-repos-2024-08-12.json -o ../fontc_crater/results/ --html_only + +# Review ../fontc_crater/results/index.html (NOT ../fontc_crater/index.html) +``` + [google-fonts-sources]: https://github.com/googlefonts/google-fonts-sources [google/fonts]: https://github.com/google/fonts [rust-lang/crater]: https://github.com/rust-lang/crater diff --git a/fontc_crater/src/args.rs b/fontc_crater/src/args.rs index ad5fd211..837990d5 100644 --- a/fontc_crater/src/args.rs +++ b/fontc_crater/src/args.rs @@ -21,18 +21,19 @@ pub(super) enum Commands { #[derive(Debug, PartialEq, clap::Args)] pub(super) struct RunArgs { - /// Directory to store font sources. + /// Directory to store font sources and the google/fonts repo. /// /// Reusing this directory saves us having to clone all the repos on each run. /// /// This directory is also used to write cached results during repo discovery. - pub(super) font_cache: PathBuf, + #[arg(short, long, default_value = "~/.fontc_crater_cache")] + pub(super) cache_dir: PathBuf, /// Optional path to write out results (as json) #[arg(short = 'o', long = "out")] pub(super) out_path: Option, /// for debugging, execute only a given number of fonts #[arg(long)] - pub(super) n_fonts: Option, + pub(super) limit: Option, } #[derive(Debug, PartialEq, clap::Args)] diff --git a/fontc_crater/src/ci.rs b/fontc_crater/src/ci.rs index 74150f52..c509c424 100644 --- a/fontc_crater/src/ci.rs +++ b/fontc_crater/src/ci.rs @@ -94,6 +94,7 @@ fn run_crater_and_save_results(args: &CiArgs) -> Result<(), Error> { } let inputs: Vec = super::try_read_json(&args.to_run)?; + let out_file = result_path_for_current_date(); let out_path = args.out_dir.join(&out_file); // for now we are going to be cloning each repo freshly diff --git a/fontc_crater/src/ci/html.rs b/fontc_crater/src/ci/html.rs index 31ef0059..29b86250 100644 --- a/fontc_crater/src/ci/html.rs +++ b/fontc_crater/src/ci/html.rs @@ -11,7 +11,7 @@ use crate::{ error::Error, ttx_diff_runner::{CompilerFailure, DiffError, DiffOutput, DiffValue}, }; -use maud::{html, Markup}; +use maud::{html, Markup, PreEscaped}; use super::{DiffResults, RunSummary}; @@ -71,6 +71,22 @@ fn make_html( _ => html!(), }; + let script = PreEscaped( + " + + ", + ); + let raw_html = html! { (maud::DOCTYPE) html { @@ -78,6 +94,7 @@ fn make_html( title { "fontc_crater results" } style { (css) } meta charset="utf-8"; + (script) } body { h1 { "fontc_crater" } @@ -159,8 +176,8 @@ fn make_table_body(runs: &[RunSummary]) -> Markup { html! { td.fontc_err { a href = "#fontc-failures" { (run.stats.fontc_failed) " " (fontc_err_diff) } } td.fontmake_err { a href = "#fontmake-failures" { (run.stats.fontmake_failed) " " (fontmake_err_diff) } } - td.both_err { a href = "both-failures" { (run.stats.both_failed) " " (both_err_diff) } } - td.other_err { a href = "other-failures" { (run.stats.other_failure) " " (other_err_diff) } } + td.both_err { a href = "#both-failures" { (run.stats.both_failed) " " (both_err_diff) } } + td.other_err { a href = "#other-failures" { (run.stats.other_failure) " " (other_err_diff) } } } } else { html! { @@ -230,6 +247,18 @@ fn make_delta_decoration + Display + Defa } } +fn make_repro_cmd(repo_url: &str, repo_path: &Path) -> String { + // the first component of the path is the repo, drop that + let mut repo_path = repo_path.components(); + repo_path.next(); + let repo_path = repo_path.as_path(); + + format!( + "copyText('python resources/scripts/ttx_diff.py {repo_url}#{}');", + repo_path.display() + ) +} + fn make_detailed_report( current: &DiffResults, prev: &DiffResults, @@ -282,6 +311,8 @@ fn make_diff_report( continue; } + let repo_url = get_repo_url(path); + let repro_cmd = make_repro_cmd(repo_url, path); let decoration = make_delta_decoration(*ratio, prev_ratio, More::IsBetter); let details = format_diff_report_details(diff_details, prev_details); // avoid .9995 printing as 100% @@ -291,7 +322,10 @@ fn make_diff_report( details { summary { span.font_path { - a href = ({ get_repo_url(path) }) { (path.display()) } + a href = (repo_url) { (path.display()) } + " (" + a href = "#" onclick = (repro_cmd) { "copy repro" } + ")" } span.diff_result { (ratio_fmt) " " (decoration) } } diff --git a/fontc_crater/src/main.rs b/fontc_crater/src/main.rs index 91a991fb..1455a261 100644 --- a/fontc_crater/src/main.rs +++ b/fontc_crater/src/main.rs @@ -2,7 +2,8 @@ use std::{ collections::{BTreeMap, BTreeSet}, - path::{Path, PathBuf}, + ffi::OsStr, + path::{self, Path, PathBuf}, sync::atomic::{AtomicUsize, Ordering}, time::Instant, }; @@ -10,6 +11,7 @@ use std::{ use clap::Parser; use fontc::JobTimer; use google_fonts_sources::{LoadRepoError, RepoInfo}; +use log::warn; use rayon::{prelude::*, ThreadPoolBuilder}; mod args; @@ -21,7 +23,7 @@ mod ttx_diff_runner; use serde::{de::DeserializeOwned, Serialize}; use sources::RepoList; -use args::{Args, Commands, ReportArgs}; +use args::{Args, Commands, ReportArgs, RunArgs}; use error::Error; use ttx_diff_runner::{DiffError, DiffOutput}; @@ -34,33 +36,55 @@ fn main() { } fn run(args: &Args) -> Result<(), Error> { - let run_args = match &args.command { - Commands::Compile(args) => args, - Commands::Diff(args) => args, - Commands::Report(args) => return generate_report(args), - Commands::Ci(args) => return ci::run_ci(args), + match &args.command { + Commands::Compile(args) => compile_and_maybe_diff(args, false), + Commands::Diff(args) => compile_and_maybe_diff(args, true), + Commands::Report(args) => generate_report(args), + Commands::Ci(args) => ci::run_ci(args), + } +} + +fn resolve_home(path: &Path) -> PathBuf { + let Some(home_dir) = std::env::home_dir() else { + warn!("No known home directory, ~ will not be resolved"); + return path.to_path_buf(); }; + let home = path::Component::Normal(OsStr::new("~")); + let mut result = PathBuf::new(); + for c in path.components() { + if c == home { + result.push(home_dir.clone()); + } else { + result.push(c); + } + } + result +} - if !run_args.font_cache.exists() { - try_create_dir(&run_args.font_cache)?; +fn compile_and_maybe_diff(args: &RunArgs, diff: bool) -> Result<(), Error> { + let cache_dir = resolve_home(&args.cache_dir); + if !cache_dir.exists() { + try_create_dir(&cache_dir)?; } - let sources = RepoList::get_or_create(&run_args.font_cache)?; + eprintln!("Operating in {cache_dir:?}"); + let sources = RepoList::get_or_create(&cache_dir)?; - let pruned = run_args.n_fonts.map(|n| prune_sources(&sources.sources, n)); + let pruned = args.limit.map(|n| prune_sources(&sources.sources, n)); let inputs = pruned.as_ref().unwrap_or(&sources.sources); - match args.command { - Commands::Compile { .. } => run_all(inputs, &run_args.font_cache, compile_one) - .and_then(|r| print_or_write_results(r, run_args.out_path.as_deref()))?, - Commands::Diff { .. } => { - ttx_diff_runner::assert_can_run_script(); - run_all(inputs, &run_args.font_cache, ttx_diff_runner::run_ttx_diff) - .and_then(|r| print_or_write_results(r, run_args.out_path.as_deref()))? - } - Commands::Report { .. } => unreachable!("handled above"), - Commands::Ci(_) => todo!(), - }; - sources.save(&run_args.font_cache)?; + eprintln!("Pruned {} to {}", sources.sources.len(), inputs.len()); + + // Courtesy of differing result types we have to be duplicative here :( + if diff { + ttx_diff_runner::assert_can_run_script(); + run_all(inputs, &cache_dir, ttx_diff_runner::run_ttx_diff) + .and_then(|r| print_or_write_results(r, args.out_path.as_deref()))?; + } else { + run_all(inputs, &cache_dir, compile_one) + .and_then(|r| print_or_write_results(r, args.out_path.as_deref()))?; + } + + sources.save(&cache_dir)?; Ok(()) } @@ -114,6 +138,7 @@ fn deserialize_compile_json(json_str: &str, path: &Path) -> Result(sources: &[T], n_items: usize) -> Vec { if n_items == 0 || sources.is_empty() { diff --git a/resources/scripts/ttx_diff.py b/resources/scripts/ttx_diff.py index b92d6259..453f1764 100755 --- a/resources/scripts/ttx_diff.py +++ b/resources/scripts/ttx_diff.py @@ -42,6 +42,7 @@ import shutil import subprocess import sys +from urllib.parse import urlparse from cdifflib import CSequenceMatcher as SequenceMatcher from typing import MutableSequence from glyphsLib import GSFont @@ -70,7 +71,7 @@ def maybe_print(*objects): flags.DEFINE_enum( "compare", - "both", + "default", ["both", _COMPARE_DEFAULTS, _COMPARE_GFTOOLS], "Compare results with default flags, with the flags gftools uses, or both. Default both. Note that as of 5/21/2023 defaults still sets flags for fontmake to match fontc behavior.", ) @@ -588,13 +589,34 @@ def report_errors_and_exit_if_there_were_any(errors: dict): print_json({"error": errors}) sys.exit(2) + +def resolve_source(source: str) -> Path: + if source.startswith("git@") or source.startswith("https://"): + source_url = urlparse(source) + repo_path = source_url.fragment + last_path_segment = source_url.path.split("/")[-1] + local_repo = (Path(__file__).parent / ".." / ".." / "font_repos" / last_path_segment).resolve() + if not local_repo.parent.is_dir(): + local_repo.parent.mkdir() + if not local_repo.is_dir(): + cmd = ("git", "clone", source_url._replace(fragment="").geturl()) + print("Running", " ".join(cmd), "in", local_repo.parent) + subprocess.run(cmd, cwd=local_repo.parent, check=True) + else: + print(f"Reusing existing {local_repo}") + source = local_repo / repo_path + else: + source = Path(source) + if not source.exists(): + sys.exit(f"No such source: {source}") + return source + + def main(argv): if len(argv) != 2: sys.exit("Only one argument, a source file, is expected") - source = Path(argv[1]) - if not source.exists(): - sys.exit(f"No such source: {source}") + source = resolve_source(argv[1]) root = Path(".").resolve() if root.name != "fontc": @@ -618,7 +640,7 @@ def main(argv): "JSON output does not support multiple comparisons (try --compare default|gftools)") comparisons = (_COMPARE_DEFAULTS, _COMPARE_GFTOOLS) - no_diffs = True + diffs = False for compare in comparisons: build_dir = (out_dir / compare) build_dir.mkdir(parents=True, exist_ok=True) @@ -650,7 +672,7 @@ def main(argv): maybe_print("output is identical") continue - no_diffs = False + diffs = True if not FLAGS.json: print_output(build_dir, output) @@ -658,10 +680,7 @@ def main(argv): output = jsonify_output(output) print_json(output) - if no_diffs: - sys.exit(0) - else: - sys.exit(2) + sys.exit(diffs * 2) # 0 or 2 if __name__ == "__main__":