From b8fcea9b4c7ef1e0f3152fbda86503357a91c9d9 Mon Sep 17 00:00:00 2001 From: Torstein Tauno Svendsen Date: Sun, 6 Sep 2026 23:39:58 +0200 Subject: [PATCH] An extractor that resolves nowhere says where it looked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by typing the shipped example's own command line, which was also wrong. A reading directory is listed only if it EXISTS, so on a host where none does the list was empty and the failure read "nor a document in" with nothing after it — no directory named, no hint where the file should go. It now names all three, and says which one the timberfs package owns. The list of what a populated directory holds was the DOCUMENT names, where this lookup takes the FILE stem. Where they differ, printing the document name printed a word that does not resolve. It prints the stem, and says what the document inside is called. Written down in the man page and in tools/README.md too: a provisioning's APPLY names the document, `--extractor` names the file, and keeping them the same is what everything shipped does. The example said `tally --try NAME`; `--try` is a flag and the name goes to `--extractor`. `--etc` claimed `requires = "provision"` and never enforced it — `--try` and `--check` resolve names against it too, which is how a site extractor is tried before anything is provisioned. --- packaging/timberfs-tally.conf.example | 3 +- packaging/timberfs.1 | 7 +++ src/main.rs | 11 ++--- src/tally.rs | 63 ++++++++++++++++++++++++++- tools/README.md | 4 +- 5 files changed, 77 insertions(+), 11 deletions(-) diff --git a/packaging/timberfs-tally.conf.example b/packaging/timberfs-tally.conf.example index ba15aef..362222a 100644 --- a/packaging/timberfs-tally.conf.example +++ b/packaging/timberfs-tally.conf.example @@ -69,7 +69,8 @@ APPLY=timberfs-apache-combined timberfs-volume # Read a log through a document and print the tally it produces, writing no # store at all: # -# timberfs tally --try timberfs-apache-combined < /var/log/apache2/access.log +# timberfs tally --try --extractor timberfs-apache-combined \ +# < /var/log/apache2/access.log # # That is also how a document of your own is developed: a metric that # claimed lines and matched none says so, which a store quietly holding no diff --git a/packaging/timberfs.1 b/packaging/timberfs.1 index d6e747a..b95ceff 100644 --- a/packaging/timberfs.1 +++ b/packaging/timberfs.1 @@ -1435,6 +1435,13 @@ later shadowing earlier \(em so a document can be tried without root, and shadow a shipped one while it is. .RS .PP +⚠ The NAME here is the FILE's, +.IB name .json \fR, +where a provisioning's +.B APPLY +names the DOCUMENT. Keeping the two the same is what everything shipped +does and what a fork of one should. +.PP ⚠ That last one is a READER's. A .B \-\-provision never looks in a home directory: it runs as a service, creating stores diff --git a/src/main.rs b/src/main.rs index 1c931dc..2fe3a20 100644 --- a/src/main.rs +++ b/src/main.rs @@ -689,13 +689,10 @@ enum Command { /// the follower whose command this is #[arg(long, value_name = "SET", conflicts_with_all = ["extractors", "fold", "try_it", "provision"])] run: Option, - /// Where the provisioning and the site's extractors live - #[arg( - long, - value_name = "DIR", - default_value = "/etc/timberfs", - requires = "provision" - )] + /// Where the provisioning and the site's extractors live. Also + /// read by --try and --check, which resolve an extractor NAME + /// against it + #[arg(long, value_name = "DIR", default_value = "/etc/timberfs")] etc: PathBuf, /// Forests to resolve the selection against; default every /// configured one diff --git a/src/tally.rs b/src/tally.rs index c1eb8cd..d70935c 100644 --- a/src/tally.rs +++ b/src/tally.rs @@ -1433,10 +1433,38 @@ pub fn resolve_extractors(args: &[PathBuf], etc: &Path) -> anyhow::Result out.push(p), None => { + // ⚠ A directory is listed only if it EXISTS, so on a host + // where none does the list is empty — and a resolution + // failure naming nowhere tells the reader nothing about + // where to put the file. + if dirs.is_empty() { + bail!( + "no extractor {name:?} — it is not a path that exists, and there \ + is no extractor directory to search: none of \ + {PACKAGED_EXTRACTORS} (the timberfs package), {} or \ + ~/.config/timberfs/{EXTRACTOR_DIR} exists", + etc.join(EXTRACTOR_DIR).display(), + ); + } + // ⚠ What is listed is the FILE STEM, because that is + // what this lookup takes — a provisioning's APPLY names + // the DOCUMENT instead, and the two can differ on a + // site's own file. Naming the document here would print + // a word that does not resolve. let known = load_extractors(&dirs) .map(|docs| { docs.iter() - .map(|(_, d)| d.name.clone()) + .map(|(p, d)| { + let stem = p + .file_stem() + .map(|s| s.to_string_lossy().to_string()) + .unwrap_or_default(); + if stem == d.name { + stem + } else { + format!("{stem} (the document {:?})", d.name) + } + }) .collect::>() .join(", ") }) @@ -1449,7 +1477,7 @@ pub fn resolve_extractors(args: &[PathBuf], etc: &Path) -> anyhow::Result>() .join(", "), if known.is_empty() { - String::new() + ", which hold none".to_string() } else { format!(", which hold {known}") } @@ -3345,6 +3373,37 @@ mod tests { std::fs::remove_dir_all(&site).ok(); } + #[test] + fn a_name_that_resolves_nowhere_says_where_it_looked() { + // A directory is listed only if it EXISTS, so on a host with + // none the list is empty — and the failure then named nowhere at + // all, which tells a reader nothing about where to put the file. + let empty = tempdir(); + let err = resolve_extractors(&[PathBuf::from("nope")], &empty) + .unwrap_err() + .to_string(); + assert!(err.contains(PACKAGED_EXTRACTORS), "{err}"); + assert!(err.contains(EXTRACTOR_DIR), "{err}"); + + let site = empty.join(EXTRACTOR_DIR); + std::fs::create_dir_all(&site).unwrap(); + std::fs::write( + site.join("x.json"), + doc(r#"{"name":"m","measure":[{"count":true}]}"#), + ) + .unwrap(); + let err = resolve_extractors(&[PathBuf::from("nope")], &empty) + .unwrap_err() + .to_string(); + assert!(err.contains(&site.display().to_string()), "{err}"); + // The FILE STEM, because that is what this lookup takes. The + // document is named `t`, and printing that would print a word + // that does not resolve. + assert!(err.contains("which hold x "), "{err}"); + assert!(err.contains(r#"the document "t""#), "{err}"); + std::fs::remove_dir_all(&empty).ok(); + } + /// Every extractor this repository SHIPS, run against a fixture and /// compared to a committed answer. /// diff --git a/tools/README.md b/tools/README.md index 1174380..6541e6c 100644 --- a/tools/README.md +++ b/tools/README.md @@ -701,7 +701,9 @@ timberfs query my-app-tally | timbergraph -m requests --using my-app A name is looked up in `/usr/lib/timberfs/tally.extractors.d`, then `/etc/…`, then `~/.config/…`, later shadowing earlier — so a document can be -written and tried without root, and can shadow a shipped one while it is. +written and tried without root, and can shadow a shipped one while it is. ⚠ The +name here is the **file's**, `.json`; a provisioning's `APPLY` names the +**document**. Keep the two the same, as everything shipped does. ⚠ That last directory is a **reader's**. `timberfs tally --provision` never looks in a home: it runs as a service, creating stores and registering