feat: Improve diff view when restoring to package manager state (fixes …

…#91)

crates/konfigkoll/src/apply.rs

@@ -39,6 +39,9 @@ pub(crate) fn create_applicator(
             inner_applicator,
             diff_command,
             pager_command,
+            files_backend,
+            interner,
+            package_maps,
         )),
         Paranoia::DryRun => Box::new(konfigkoll_core::apply::NoopApplicator::default()),
     };

crates/konfigkoll/src/main.rs

@@ -15,6 +15,7 @@ use konfigkoll_core::apply::apply_packages;
 use konfigkoll_core::diff::show_fs_instr_diff;
 use konfigkoll_core::state::DiffGoal;
 use konfigkoll_core::state::FsEntries;
+use konfigkoll_core::utils::pkg_backend_for_files;
 use konfigkoll_script::Phase;
 use konfigkoll_script::ScriptEngine;
 use konfigkoll_types::FsInstruction;
@@ -303,9 +304,17 @@ async fn main() -> eyre::Result<()> {
 
             let diff_cmd = script_engine.state().settings().diff();
             let pager_cmd = script_engine.state().settings().pager();
+            let pkg_file_backend = pkg_backend_for_files(&package_maps, &*backend_files)?;
             for change in fs_changes {
                 if change.path.starts_with(&path) {
-                    show_fs_instr_diff(&change, &diff_cmd, &pager_cmd)?;
+                    show_fs_instr_diff(
+                        &change,
+                        &diff_cmd,
+                        &pager_cmd,
+                        &interner,
+                        &*backend_files,
+                        &pkg_file_backend,
+                    )?;
                 }
             }
             // Let the OS clean these up, freeing in the program is slower

crates/konfigkoll_core/src/apply.rs

@@ -3,6 +3,8 @@
 use crate::confirm::Choices;
 use crate::confirm::MultiOptionConfirm;
 use crate::diff::show_fs_instr_diff;
+use crate::utils::original_file_contents;
+use crate::utils::pkg_backend_for_files;
 use crate::utils::IdKey;
 use crate::utils::NameToNumericResolveCache;
 use ahash::AHashMap;
@@ -18,7 +20,6 @@ use konfigkoll_types::PkgInstruction;
 use konfigkoll_types::PkgOp;
 use paketkoll_types::backend::Backend;
 use paketkoll_types::backend::Files;
-use paketkoll_types::backend::OriginalFileQuery;
 use paketkoll_types::backend::PackageBackendMap;
 use paketkoll_types::backend::PackageMap;
 use paketkoll_types::backend::PackageMapMap;
@@ -225,31 +226,10 @@ impl InProcessApplicator {
                 nix::unistd::chown(instr.path.as_std_path(), None, Some(gid))?;
             }
             FsOp::Restore => {
-                // Get package:
-                let owners = self
-                    .file_backend
-                    .owning_packages(&[instr.path.as_std_path()].into(), &self.interner)
-                    .wrap_err_with(|| format!("Failed to find owner for {}", instr.path))?;
-                let package = owners
-                    .get(instr.path.as_std_path())
-                    .ok_or_else(|| eyre::eyre!("Failed to find owner for {}", instr.path))?
-                    .ok_or_else(|| eyre::eyre!("No owner for {}", instr.path))?;
-                let package = package.as_str(&self.interner);
-                // Get original contents:
-                let queries = [OriginalFileQuery {
-                    package: package.into(),
-                    path: instr.path.as_str().into(),
-                }];
-                let original_contents =
-                    self.file_backend
-                        .original_files(&queries, pkg_map, &self.interner)?;
+                let contents =
+                    original_file_contents(&*self.file_backend, &self.interner, instr, pkg_map)?;
                 // Apply
-                for query in queries {
-                    let contents = original_contents
-                        .get(&query)
-                        .ok_or_else(|| eyre::eyre!("No original contents for {:?}", query))?;
-                    std::fs::write(&instr.path, contents)?;
-                }
+                std::fs::write(&instr.path, contents)?;
             }
             FsOp::Comment => {
                 tracing::warn!(
@@ -306,16 +286,7 @@ impl Applicator for InProcessApplicator {
     }
 
     fn apply_files(&mut self, instructions: &[FsInstruction]) -> eyre::Result<()> {
-        let pkg_map = self
-            .package_maps
-            .get(&self.file_backend.as_backend_enum())
-            .ok_or_else(|| {
-                eyre::eyre!(
-                    "No package map for file backend {:?}",
-                    self.file_backend.as_backend_enum()
-                )
-            })?
-            .clone();
+        let pkg_map = pkg_backend_for_files(&self.package_maps, &*self.file_backend)?;
         for instr in instructions {
             self.apply_single_file(instr, &pkg_map).wrap_err_with(|| {
                 format!("Failed to apply change for {}: {:?}", instr.path, instr.op)
@@ -389,10 +360,20 @@ pub struct InteractiveApplicator<Inner: std::fmt::Debug> {
     interactive_confirmer: MultiOptionConfirm<InteractivePromptChoices>,
     diff_command: Vec<String>,
     pager_command: Vec<String>,
+    file_backend: Arc<dyn Files>,
+    interner: Arc<Interner>,
+    package_maps: PackageMapMap,
 }
 
 impl<Inner: std::fmt::Debug> InteractiveApplicator<Inner> {
-    pub fn new(inner: Inner, diff_command: Vec<String>, pager_command: Vec<String>) -> Self {
+    pub fn new(
+        inner: Inner,
+        diff_command: Vec<String>,
+        pager_command: Vec<String>,
+        file_backend: &Arc<dyn Files>,
+        interner: &Arc<Interner>,
+        package_maps: &PackageMapMap,
+    ) -> Self {
         let mut prompt_builder = MultiOptionConfirm::builder();
         prompt_builder.prompt("Do you want to apply these changes?");
         let pkg_confirmer = prompt_builder.build();
@@ -411,6 +392,9 @@ impl<Inner: std::fmt::Debug> InteractiveApplicator<Inner> {
             interactive_confirmer,
             diff_command,
             pager_command,
+            file_backend: file_backend.clone(),
+            interner: interner.clone(),
+            package_maps: package_maps.clone(),
         }
     }
 }
@@ -465,8 +449,9 @@ impl<Inner: Applicator + std::fmt::Debug> Applicator for InteractiveApplicator<I
                 Ok(())
             }
             FsPromptChoices::Interactive => {
+                let pkg_map = pkg_backend_for_files(&self.package_maps, &*self.file_backend)?;
                 for instr in instructions {
-                    self.interactive_apply_single_file(instr)?;
+                    self.interactive_apply_single_file(instr, &pkg_map)?;
                 }
                 Ok(())
             }
@@ -495,7 +480,11 @@ fn show_pkg_diff(backend: Backend, install: &[&str], mark_explicit: &[&str], uni
 }
 
 impl<Inner: Applicator + std::fmt::Debug> InteractiveApplicator<Inner> {
-    fn interactive_apply_single_file(&mut self, instr: &FsInstruction) -> eyre::Result<()> {
+    fn interactive_apply_single_file(
+        &mut self,
+        instr: &FsInstruction,
+        pkg_map: &PackageMap,
+    ) -> eyre::Result<()> {
         println!(
             "Under consideration: {} with change {}",
             style(instr.path.as_str()).blue(),
@@ -520,6 +509,9 @@ impl<Inner: Applicator + std::fmt::Debug> InteractiveApplicator<Inner> {
                         instr,
                         self.diff_command.as_slice(),
                         self.pager_command.as_slice(),
+                        &self.interner,
+                        &*self.file_backend,
+                        pkg_map,
                     )?;
                 }
             };

crates/konfigkoll_core/src/diff.rs

@@ -2,13 +2,17 @@
 //!
 //! This module implements a generic algorithm similar to comm(1)
 
+use crate::utils::original_file_contents;
 use camino::Utf8Path;
 use camino::Utf8PathBuf;
 use console::style;
 use itertools::EitherOrBoth;
 use itertools::Itertools;
 use konfigkoll_types::FsInstruction;
 use konfigkoll_types::FsOp;
+use paketkoll_types::backend::Files;
+use paketkoll_types::backend::PackageMap;
+use paketkoll_types::intern::Interner;
 use paketkoll_utils::MODE_MASK;
 use std::iter::FusedIterator;
 use std::os::unix::fs::MetadataExt;
@@ -28,6 +32,9 @@ pub fn show_fs_instr_diff(
     instr: &FsInstruction,
     diff_command: &[String],
     pager_command: &[String],
+    interner: &Interner,
+    file_backend: &dyn Files,
+    pkg_map: &PackageMap,
 ) -> eyre::Result<()> {
     match &instr.op {
         FsOp::CreateFile(contents) => {
@@ -137,6 +144,9 @@ pub fn show_fs_instr_diff(
                 "{}: Would restore to original package manager state",
                 style(&instr.path).color256(202)
             );
+            let contents = original_file_contents(file_backend, interner, instr, pkg_map)?;
+            let contents = konfigkoll_types::FileContents::from_literal(contents.into());
+            show_file_diff(&instr.path, &contents, diff_command, pager_command)?;
         }
         FsOp::Comment => (),
     };

crates/konfigkoll_core/src/utils.rs

@@ -3,9 +3,17 @@
 use clru::CLruCache;
 use compact_str::CompactString;
 use eyre::eyre;
+use eyre::Context;
+use konfigkoll_types::FsInstruction;
+use paketkoll_types::backend::Files;
+use paketkoll_types::backend::OriginalFileQuery;
+use paketkoll_types::backend::PackageMap;
+use paketkoll_types::backend::PackageMapMap;
 use paketkoll_types::files::Gid;
 use paketkoll_types::files::Uid;
+use paketkoll_types::intern::Interner;
 use std::num::NonZeroUsize;
+use std::sync::Arc;
 
 /// UID/GID to name resolver / cache
 #[derive(Debug)]
@@ -105,3 +113,49 @@ where
     User(UserKey),
     Group(GroupKey),
 }
+
+/// Resolve the original contents of a file given a file backend and instruction
+pub(crate) fn original_file_contents(
+    file_backend: &dyn Files,
+    interner: &Interner,
+    instr: &FsInstruction,
+    pkg_map: &PackageMap,
+) -> Result<Vec<u8>, eyre::Error> {
+    // Get package owning the file
+    let owners = file_backend
+        .owning_packages(&[instr.path.as_std_path()].into(), interner)
+        .wrap_err_with(|| format!("Failed to find owner for {}", instr.path))?;
+    let package = owners
+        .get(instr.path.as_std_path())
+        .ok_or_else(|| eyre::eyre!("Failed to find owner for {}", instr.path))?
+        .ok_or_else(|| eyre::eyre!("No owner for {}", instr.path))?;
+    let package = package.as_str(interner);
+    // Create query
+    let queries = [OriginalFileQuery {
+        package: package.into(),
+        path: instr.path.as_str().into(),
+    }];
+    // Get file contents
+    let mut original_contents = file_backend.original_files(&queries, pkg_map, interner)?;
+    let original_contents = original_contents
+        .remove(&queries[0])
+        .ok_or_else(|| eyre::eyre!("No original contents for {:?}", queries[0]))?;
+    Ok(original_contents)
+}
+
+/// Helper to get the package map for the file backend
+pub fn pkg_backend_for_files(
+    package_maps: &PackageMapMap,
+    file_backend: &dyn Files,
+) -> Result<Arc<PackageMap>, eyre::Error> {
+    let pkg_map = package_maps
+        .get(&file_backend.as_backend_enum())
+        .ok_or_else(|| {
+            eyre::eyre!(
+                "No package map for file backend {:?}",
+                file_backend.as_backend_enum()
+            )
+        })?
+        .clone();
+    Ok(pkg_map)
+}

integration_tests/arch/basic_config/expected/2_output.txt

@@ -29,3 +29,28 @@ INFO konfigkoll: Waiting for file system scan results...
 INFO konfigkoll: Got file system scan results
 INFO konfigkoll: Computing diff
 /etc/passwd: Would restore to original package manager state
+--- /etc/passwd	2024-09-07 07:56:54.000000000 +0000
++++ /dev/stdin	2024-09-20 17:29:52.604937717 +0000
+@@ -1,22 +1 @@
+ root:x:0:0::/root:/usr/bin/bash
+-bin:x:1:1::/:/usr/bin/nologin
+-daemon:x:2:2::/:/usr/bin/nologin
+-mail:x:8:12::/var/spool/mail:/usr/bin/nologin
+-ftp:x:14:11::/srv/ftp:/usr/bin/nologin
+-http:x:33:33::/srv/http:/usr/bin/nologin
+-nobody:x:65534:65534:Kernel Overflow User:/:/usr/bin/nologin
+-dbus:x:81:81:System Message Bus:/:/usr/bin/nologin
+-systemd-coredump:x:980:980:systemd Core Dumper:/:/usr/bin/nologin
+-systemd-network:x:979:979:systemd Network Management:/:/usr/bin/nologin
+-systemd-oom:x:978:978:systemd Userspace OOM Killer:/:/usr/bin/nologin
+-systemd-journal-remote:x:977:977:systemd Journal Remote:/:/usr/bin/nologin
+-systemd-resolve:x:976:976:systemd Resolver:/:/usr/bin/nologin
+-systemd-timesync:x:975:975:systemd Time Synchronization:/:/usr/bin/nologin
+-tss:x:974:974:tss user for tpm2:/:/usr/bin/nologin
+-uuidd:x:68:68::/:/usr/bin/nologin
+-avahi:x:973:973:Avahi mDNS/DNS-SD daemon:/:/usr/bin/nologin
+-flatpak:x:972:972:Flatpak system helper:/:/usr/bin/nologin
+-geoclue:x:971:971:Geoinformation service:/var/lib/geoclue:/usr/bin/nologin
+-git:x:970:970:git daemon user:/:/usr/bin/git-shell
+-polkitd:x:102:102:User for polkitd:/:/usr/bin/nologin
+-rtkit:x:133:133:RealtimeKit:/proc:/usr/bin/nologin