Keep the files many PRs add to conflict-free (#1564)

Message catalogs sorted one message per line (cargo xtask lists [--fix], checked in cargo xtask ci and the Lists workflow); the language list one per line, English first then by id; THIRD-PARTY-LICENSES.txt and docs/features.md rewritten on main by the Generated files workflow instead of in PRs.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

xtask/src/licences.rs

@@ -2,8 +2,10 @@
 //! into the shipped programs (solidcraft, solidcraft-cli, solidcraft-web), read from
 //! `cargo metadata` and the crates' own licence files. The programs embed it (Help ▸ About ▸
 //! Licences, `solidcraft-cli licences`), so a release download needs no licence files beside it.
-//! `--check` (part of `cargo xtask ci`) fails when the committed file is out of date, e.g. after a
-//! dependency changed.
+//! `--check` (part of `cargo xtask ci`) generates it without writing it: it fails when the
+//! dependency graph can't be read, and only reports a file that is out of date, since pull
+//! requests leave it to the Generated files workflow (`.github/workflows/generated.yml`), which
+//! rewrites it on main after each merge.
 
 use std::collections::{BTreeMap, BTreeSet, VecDeque};
 use std::path::Path;
@@ -21,7 +23,10 @@ pub fn run(root: &Path, check: bool) -> Result<(), String> {
     if check {
         let old = std::fs::read_to_string(&path).unwrap_or_default();
         if old.replace("\r\n", "\n") != text {
-            return Err(format!("{FILE} is out of date (dependencies changed): run `cargo xtask licences` and commit it"));
+            // Not an error: pull requests don't edit the file (two that did would always conflict);
+            // the Generated files workflow rewrites it on main after the merge.
+            println!("licences: {FILE} will change when this lands (the Generated files workflow rewrites it on main)");
+            return Ok(());
         }
         println!("licences: {FILE} is up to date");
         return Ok(());

xtask/src/lists.rs

@@ -0,0 +1,68 @@
+//! `cargo xtask lists [--fix]`: files that many pull requests add to are kept in a canonical,
+//! sorted form, so two changes that each add an item meet only when their items are neighbours
+//! (CONTRIBUTING.md, "Lists many changes add to"). Checked by `cargo xtask ci` and the Lists
+//! workflow; `--fix` rewrites them.
+//!
+//! - The message catalogs (`assets/locales/*.json`): one message per line, sorted by id, no
+//!   duplicate ids, as `serde_json` pretty-prints them.
+
+use std::path::Path;
+
+use serde_json::{Map, Value};
+
+const LOCALES: &str = "assets/locales";
+
+pub fn run(root: &Path, fix: bool) -> Result<(), String> {
+    let dir = root.join(LOCALES);
+    let entries = std::fs::read_dir(&dir).map_err(|e| format!("{}: {e}", dir.display()))?;
+    let mut files: Vec<_> = entries.filter_map(Result::ok).map(|e| e.path()).filter(|p| p.extension().is_some_and(|x| x == "json")).collect();
+    files.sort();
+    let mut stale = Vec::new();
+    for path in files {
+        let text = std::fs::read_to_string(&path).map_err(|e| format!("{}: {e}", path.display()))?;
+        let canonical = canonical(&text).map_err(|e| format!("{}: {e}", path.display()))?;
+        if canonical == text.replace("\r\n", "\n") {
+            continue;
+        }
+        if fix {
+            std::fs::write(&path, &canonical).map_err(|e| format!("{}: {e}", path.display()))?;
+            println!("lists: sorted {}", path.display());
+        } else {
+            stale.push(path.strip_prefix(root).unwrap_or(&path).display().to_string());
+        }
+    }
+    if !stale.is_empty() {
+        return Err(format!(
+            "not sorted one message per line (or an id appears twice): {}\nrun `cargo xtask lists --fix` and commit the result",
+            stale.join(", ")
+        ));
+    }
+    println!("lists: message catalogs are sorted");
+    Ok(())
+}
+
+/// A catalog as it should be written: its messages sorted by id, one per line. A repeated id
+/// keeps its last message, the one the app reads.
+pub fn canonical(text: &str) -> Result<String, String> {
+    let map: Map<String, Value> = match serde_json::from_str(text).map_err(|e| e.to_string())? {
+        Value::Object(map) => map,
+        _ => return Err("a catalog must be a JSON object".into()),
+    };
+    if let Some((id, _)) = map.iter().find(|(_, v)| !v.is_string()) {
+        return Err(format!("message `{id}` is not a string"));
+    }
+    Ok(serde_json::to_string_pretty(&Value::Object(map)).map_err(|e| e.to_string())? + "\n")
+}
+
+#[cfg(test)]
+mod tests {
+    use super::canonical;
+
+    #[test]
+    fn catalogs_are_sorted_one_per_line_and_a_repeated_id_keeps_its_last_message() {
+        let text = "{\n  \"b\": \"B\",\n  \"a\": \"A\",\n  \"b\": \"B2\"\n}";
+        assert_eq!(canonical(text).unwrap(), "{\n  \"a\": \"A\",\n  \"b\": \"B2\"\n}\n");
+        assert!(canonical("[]").is_err());
+        assert!(canonical("{\"a\": 1}").is_err());
+    }
+}

xtask/src/main.rs

@@ -10,6 +10,7 @@ mod features;
 mod i18n;
 mod layers;
 mod licences;
+mod lists;
 mod test;
 mod web_offline;
 mod web_offline_test;
@@ -46,7 +47,10 @@ commands:
                   into target/book
   licences [--check]
                   write THIRD-PARTY-LICENSES.txt (the licences of every shipped crate, embedded
-                  in the programs); --check fails when it is out of date
+                  in the programs); --check only reports when it is out of date: pull requests
+                  leave it to the Generated files workflow on main
+  lists [--fix]   check that files many changes add to are sorted one item per line (the
+                  message catalogs in assets/locales); --fix sorts them
   test [CARGO ARGS] [-- TEST ARGS]
                   build the tests with the ci profile, then run every test binary at once
                   (and the doc tests alongside)
@@ -57,7 +61,7 @@ commands:
   web-offline-test [--site DIR]
                   check in headless Chrome that the web app works offline and updates to a new
                   build (a stand-in app, or the web build in DIR such as dist/web)
-  ci              fmt --check, assets, licences --check, layers, clippy -D warnings, then the tests
+  ci              fmt --check, assets, licences --check, lists, layers, clippy -D warnings, then the tests
                   with the wasm check alongside, then web-offline-test when Chrome is installed
                   (stops at the first failing step)
 ";
@@ -78,6 +82,7 @@ fn main() -> ExitCode {
         }
         Some("book") => book::run(&root(), !rest.contains(&"--no-build")),
         Some("licences") => licences::run(&root(), rest.contains(&"--check")),
+        Some("lists") => lists::run(&root(), rest.contains(&"--fix")),
         Some("test") => {
             let (cargo_args, test_args) = rest.iter().position(|a| *a == "--").map_or((&rest[..], &[][..]), |i| (&rest[..i], &rest[i + 1..]));
             test::run(cargo_args, test_args, vec![test::doc_tests(cargo_args, test_args)])
@@ -358,6 +363,7 @@ fn cmd_ci() -> Result<(), String> {
     run(c, "cargo fmt --check")?;
     assets::run(&root())?;
     licences::run(&root(), true)?;
+    lists::run(&root(), false)?;
     cmd_layers()?;
     let mut c = cargo();
     c.args(["clippy", "--workspace", "--all-targets", "--profile", "ci", "--", "-D", "warnings"]);