Minor fixes

angrr/Cargo.toml

@@ -30,3 +30,4 @@ serde_default = "*"
 serde_json = "*"
 serde_regex = "*"
 ignore = "*"
+subprocess = "*"

angrr/src/command.rs

@@ -1,4 +1,5 @@
 use clap::{Parser, Subcommand, ValueEnum};
+use humantime::Duration;
 
 use core::fmt;
 use std::{ffi::OsString, path::PathBuf};
@@ -105,6 +106,10 @@ pub struct RunOptions {
     #[arg(long)]
     pub null_output_delimiter: bool,
 
+    /// Timeout for external filters
+    #[arg(long, default_value = "1000ms")]
+    pub filter_timeout: Duration,
+
     /// Do not remove file.
     #[arg(long)]
     pub dry_run: bool,

angrr/src/config.rs

@@ -232,7 +232,7 @@ pub struct ProfileConfig {
     /// Each rule retains `n` generations every `bucket-window` duration for `bucket-amount` buckets.
     /// `n` defaults to 1.
     #[serde(default)]
-    pub keep_n_per_bucket: Vec<KeepNPerBucket>,
+    pub keep_n_per_bucket: Vec<KeepNPerBucketConfig>,
 }
 
 const fn default_periodic_rule_n() -> usize {
@@ -242,15 +242,11 @@ const fn default_periodic_rule_n() -> usize {
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case")]
 #[serde(deny_unknown_fields)]
-pub struct KeepNPerBucket {
+pub struct KeepNPerBucketConfig {
     #[serde(default = "default_periodic_rule_n")]
     pub n: usize,
-
     #[serde(with = "humantime_serde")]
-    #[serde(default)]
     pub bucket_window: Duration,
-
-    #[serde(default)]
     pub bucket_amount: u32,
 }
 
@@ -266,12 +262,17 @@ impl ProfileConfig {
             );
         }
         for path in &self.profile_paths {
-            if !(path.starts_with("~") || path.is_absolute()) {
+            if !(path.starts_with("~/") || path.is_absolute()) {
                 anyhow::bail!(
-                    "invalid profile policy {name}: profile path \"{path:?}\" must be absolute or start with `~`",
+                    "invalid profile policy {name}: profile path \"{path:?}\" must be absolute or start with `~/`",
                 );
             }
         }
+        for cfg in &self.keep_n_per_bucket {
+            if cfg.bucket_window.is_zero() {
+                anyhow::bail!("invalid keep-n-per-bucket: bucket window is zero");
+            }
+        }
         Ok(())
     }
 }
@@ -434,7 +435,7 @@ mod tests {
     foo = true
     ",
         );
-        shouldnt_parse::<KeepNPerBucket>("foo = 1");
+        shouldnt_parse::<KeepNPerBucketConfig>("foo = 1");
         shouldnt_parse::<CommonPolicyConfig>("foo = true");
         shouldnt_parse::<TouchConfig>("foo = true");
     }

angrr/src/filter.rs

@@ -1,11 +1,11 @@
-use std::{
-    io::Write,
-    path::PathBuf,
-    process::{Command, Stdio},
-};
+use std::path::PathBuf;
 
 use anyhow::Context;
+use log::Level;
 use serde::{Deserialize, Serialize};
+use subprocess::Exec;
+
+use crate::command::RunOptions;
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case")]
@@ -22,52 +22,40 @@ pub struct Input {
 }
 
 impl Filter {
-    pub fn run(&self, input: &Input) -> anyhow::Result<bool> {
+    pub fn run(&self, options: &RunOptions, input: &Input) -> anyhow::Result<bool> {
         let json = serde_json::to_string(input)
             .with_context(|| format!("failed to serialize input for filter: {input:?}"))?;
         log::trace!(
             "starting filter program {:?} with arguments: {:?}",
             self.program,
             self.arguments
         );
-        let mut child = Command::new(&self.program)
+        let job = Exec::cmd(&self.program)
             .args(&self.arguments)
-            .stdin(Stdio::piped())
-            .stdout(Stdio::piped())
-            .stderr(Stdio::piped())
-            .spawn()
+            .stdin(json.into_bytes())
+            .start()
             .with_context(|| {
                 format!(
                     "failed to invoke external filter {:?} with arguments: {:?}",
                     self.program, self.arguments
                 )
             })?;
-        {
-            let mut stdin = child
-                .stdin
-                .take()
-                .with_context(|| "failed to take stdin of external filter")?;
-            stdin
-                .write_all(json.as_bytes())
-                .context("can not write to stdin of external filter")?;
-            // stdin drop and closed here
-        }
-        let output = child
-            .wait_with_output()
-            .context("failed to wait on external filter")?;
-        // log stdout and stderr for debugging
-        if !output.stdout.is_empty() {
-            log::debug!(
-                "external filter stdout: {}",
-                String::from_utf8_lossy(&output.stdout)
-            );
+        let capture = job.capture_timeout(options.filter_timeout.into())?;
+        if !capture.stdout.is_empty() {
+            log::debug!("external filter stdout: {}", capture.stdout_str());
         }
-        if !output.stderr.is_empty() {
-            log::warn!(
-                "external filter stderr: {}",
-                String::from_utf8_lossy(&output.stderr)
-            );
+        if !capture.stderr.is_empty() {
+            log::warn!("external filter stderr: {}", capture.stderr_str());
         }
-        Ok(output.status.success())
+        log::log!(
+            if capture.exit_status.success() {
+                Level::Trace
+            } else {
+                Level::Debug
+            },
+            "external filter exit with status: {}",
+            capture.exit_status
+        );
+        Ok(capture.exit_status.success())
     }
 }

angrr/src/policy/profile.rs

@@ -2,7 +2,7 @@ use std::collections::BTreeSet;
 use std::{fs, path::PathBuf};
 
 use crate::{
-    config::KeepNPerBucket, config::ProfileConfig, profile::Generation, profile::Profile,
+    config::KeepNPerBucketConfig, config::ProfileConfig, profile::Generation, profile::Profile,
     utils::format_duration_short,
 };
 use anyhow::Context;
@@ -104,24 +104,23 @@ impl ProfilePolicy {
         };
         // Keep track of what was processed and skip them.
         let mut processed: BTreeSet<usize> = BTreeSet::new();
-        for &KeepNPerBucket {
+        for &KeepNPerBucketConfig {
             n,
             bucket_window,
             bucket_amount,
         } in &self.config.keep_n_per_bucket
         {
             for i in 0..bucket_amount {
                 let mut processed_curr: BTreeSet<usize> = BTreeSet::new();
-                sorted_generations.iter().filter(|(_, generation)| {
+                sorted_generations.iter().filter(|(gen_index, generation)| {
                         let within_window = bucket_window * i <= generation.root.age
                             && generation.root.age < bucket_window * (i + 1);
-                        let not_processed = !processed.contains(&generation.number);
+                        let not_processed = !processed.contains(gen_index);
                         within_window && not_processed
                     })
                     .take(n)
                     .for_each(|(gen_index, generation)| {
-                        processed_curr.insert(generation.number);
-                        keep_generation[*gen_index] = true;
+                        processed_curr.insert(*gen_index);
                         log::debug!(
                             "[{}] keep generation {} by keep_n_per_bucket, namely {} generation each bucket spanning {} for {} buckets",
                             self.name,
@@ -130,6 +129,7 @@ impl ProfilePolicy {
                             format_duration_short(bucket_window),
                             bucket_amount,
                         );
+                        keep_generation[*gen_index] = true;
                 });
 
                 processed.extend(processed_curr);

angrr/src/policy/temporary.rs

@@ -3,6 +3,7 @@ use std::os::unix::ffi::OsStrExt;
 use std::{fs, os::unix::fs::MetadataExt, path::Path};
 use uzers::{get_user_by_uid, os::unix::UserExt};
 
+use crate::command::RunOptions;
 use crate::gc_root::GcRoot;
 use crate::{config::TemporaryRootConfig, filter};
 
@@ -19,7 +20,7 @@ impl TemporaryRootPolicy {
 }
 
 impl TemporaryRootPolicy {
-    pub fn monitored(&self, root: &GcRoot) -> anyhow::Result<bool> {
+    pub fn monitored(&self, options: &RunOptions, root: &GcRoot) -> anyhow::Result<bool> {
         if self.ignored_by_prefix(&root.path)
             || self.ignored_by_prefix_in_home(&root.path, &root.path_metadata)
         {
@@ -50,14 +51,18 @@ impl TemporaryRootPolicy {
                 path: root.path.clone(),
                 gc_root: root.link_path.clone(),
             };
-            let not_ignored = filter
-                .run(&input)
-                .with_context(|| format!("failed to run filter on input: {input:?}"))?;
+            let not_ignored = filter.run(options, &input).with_context(|| {
+                format!(
+                    "failed to run filter {:?} on input: {input:?}",
+                    filter.program
+                )
+            })?;
             if !not_ignored {
                 log::debug!(
-                    "[{}] ignore {:?}, filtered out by external filter",
+                    "[{}] ignore {:?}, filtered out by external filter {:?}",
                     self.name,
                     root.path,
+                    filter.program
                 );
                 return Ok(false);
             }

angrr/src/run.rs

@@ -145,7 +145,7 @@ impl RunContext {
         for gc_root in gc_roots {
             let mut matched = None;
             for (name, policy) in policies {
-                if policy.monitored(gc_root)? {
+                if policy.monitored(&self.options, gc_root)? {
                     matched = Some((name, policy));
                     break;
                 }
@@ -379,7 +379,7 @@ impl RunContext {
         P: AsRef<Path>,
     {
         let path = path.as_ref();
-        if path.starts_with("~") {
+        if path.starts_with("~/") {
             if self.owned_only {
                 let home = utils::current_user_home()?;
                 Ok(vec![home.join(path.strip_prefix("~").unwrap())])

angrr/src/statistics.rs

@@ -9,7 +9,6 @@ pub struct Statistics {
     pub traversed: Counter,
     pub monitored: Counter,
     pub expired: Counter,
-    pub invalid: Counter,
     pub removed: Counter,
 }
 
@@ -19,7 +18,6 @@ impl Statistics {
         let monitored = self.monitored.done();
         let expired = self.expired.done();
         let removed = self.removed.done();
-        let invalid = self.invalid.done();
         let kept = traversed - removed;
         let num_style = |n| term.style().bold().apply_to(n);
         [
@@ -31,7 +29,6 @@ impl Statistics {
                 num_style(removed),
                 dry_run_indicator(term, removed != 0 && dry_run)
             ),
-            format!("invalid:   {}", num_style(invalid)),
             format!("kept:      {}", num_style(kept)),
         ]
         .join("\n")

angrr/src/utils.rs

@@ -15,23 +15,35 @@ pub fn validate_store_path<P1: AsRef<Path>, P2: AsRef<Path>>(
     store: P1,
     target: P2,
 ) -> Option<PathBuf> {
-    let store = store.as_ref();
     let target = target.as_ref();
-    match fs::canonicalize(target) {
-        Ok(path) => {
-            if path.starts_with(store) {
-                Some(path)
-            } else {
-                None
-            }
-        }
+    match canonicalize_to_store(store, target) {
+        Ok(path) => Some(path),
         Err(e) => {
             log::warn!("failed to canonicalize {target:?} for validation: {e}");
             None
         }
     }
 }
 
+pub fn canonicalize_to_store<P1: AsRef<Path>, P2: AsRef<Path>>(
+    store: P1,
+    path: P2,
+) -> anyhow::Result<PathBuf> {
+    let store = store.as_ref();
+    let path = path.as_ref();
+    if path.starts_with(store) {
+        Ok(PathBuf::from(path))
+    } else {
+        let metadata = fs::symlink_metadata(path)?;
+        if metadata.is_symlink() {
+            let next = fs::read_link(path)?;
+            canonicalize_to_store(store, next)
+        } else {
+            anyhow::bail!("canonicalized to a non-store path")
+        }
+    }
+}
+
 pub fn discover_users(roots: &[Arc<GcRoot>]) -> anyhow::Result<Vec<uzers::User>> {
     let uids: BTreeSet<_> = roots.iter().map(|root| root.path_metadata.uid()).collect();
     let mut users = Vec::new();