Skip to content

Commit

Permalink
feat: Improve diff view when restoring to package manager state (fixes …
Browse files Browse the repository at this point in the history 
…#91)
  • Loading branch information
@VorpalBlade
VorpalBlade committed Sep 20, 2024
1 parent 8eceb9e commit 39af860
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 39 deletions.
3 changes: 3 additions & 0 deletions crates/konfigkoll/src/apply.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
};
Expand Down
11 changes: 10 additions & 1 deletion crates/konfigkoll/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
68 changes: 30 additions & 38 deletions crates/konfigkoll_core/src/apply.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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();
Expand All @@ -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(),
}
}
}
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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(),
Expand All @@ -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,
)?;
}
};
Expand Down
10 changes: 10 additions & 0 deletions crates/konfigkoll_core/src/diff.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) => {
Expand Down Expand Up @@ -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 => (),
};
Expand Down
54 changes: 54 additions & 0 deletions crates/konfigkoll_core/src/utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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)
}

0 comments on commit 39af860

Please sign in to comment.