From 95ff12b91348d4c2a0f7856a4e8fb280b7c11adb Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Tue, 9 Mar 2021 19:01:35 +0000
Subject: [PATCH] rust: Add and use fn-error-context

Same motivation as https://github.com/coreos/bootupd/pull/163

Effectively what we're doing here is creating a human-readable subset
of the stack trace. This is nicer than having the calling functions
add with_context() because it's more verbose (gets duplicative at
each call site), easy to forget, etc.
---
 Cargo.lock              | 12 ++++++++++++
 Cargo.toml              |  1 +
 rust/src/cxxrsutil.rs   |  7 +++++--
 rust/src/dirdiff.rs     |  6 ++++--
 rust/src/history.rs     |  6 ++++--
 rust/src/live.rs        |  3 +--
 rust/src/ostree_diff.rs |  2 ++
 7 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 9781e206..f5e80a52 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -324,6 +324,17 @@ dependencies = [
  "thiserror",
 ]
 
+[[package]]
+name = "fn-error-context"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ac0b92c51c8b8183a0c101a7b2ea5fac11d2f1e01057f91e390284c75d76ef44"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "foreign-types"
 version = "0.5.0"
@@ -1203,6 +1214,7 @@ dependencies = [
  "curl",
  "cxx",
  "envsubst",
+ "fn-error-context",
  "gio",
  "gio-sys",
  "glib",
diff --git a/Cargo.toml b/Cargo.toml
index 7e786365..3ffb83b4 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -32,6 +32,7 @@ clap = "2.33.3"
 curl = "0.4.34"
 cxx = "1.0.32"
 envsubst = "0.2.0"
+fn-error-context = "0.1.1"
 gio = "0.9.1"
 gio-sys = "0.10.1"
 gobject-sys = "0.10.0"
diff --git a/rust/src/cxxrsutil.rs b/rust/src/cxxrsutil.rs
index fb9fa984..63bfc9fd 100644
--- a/rust/src/cxxrsutil.rs
+++ b/rust/src/cxxrsutil.rs
@@ -161,18 +161,21 @@ mod err {
     #[cfg(test)]
     mod tests {
         use super::*;
+        use fn_error_context::context;
+
         #[test]
         fn throwchain() {
             use anyhow::Context;
             fn outer() -> CxxResult<()> {
+                #[context("inner")]
                 fn inner() -> anyhow::Result<()> {
-                    anyhow::bail!("inner")
+                    anyhow::bail!("oops")
                 }
                 Ok(inner().context("Failed in outer")?)
             }
             assert_eq!(
                 format!("{}", outer().err().unwrap()),
-                "Failed in outer: inner"
+                "Failed in outer: inner: oops"
             )
         }
     }
diff --git a/rust/src/dirdiff.rs b/rust/src/dirdiff.rs
index a026c87a..7e686bdb 100644
--- a/rust/src/dirdiff.rs
+++ b/rust/src/dirdiff.rs
@@ -6,7 +6,8 @@
 
 //! Compute difference between two filesystem trees.
 
-use anyhow::{Context, Result};
+use anyhow::Result;
+use fn_error_context::context;
 use openat_ext::OpenatDirExt;
 use serde_derive::{Deserialize, Serialize};
 use std::borrow::Cow;
@@ -204,11 +205,12 @@ fn diff_recurse(
 }
 
 /// Given two directories, compute the diff between them.
+#[context("Computing filesystem diff")]
 pub(crate) fn diff(src: &openat::Dir, dest: &openat::Dir) -> Result<Diff> {
     let mut diff = Diff {
         ..Default::default()
     };
-    diff_recurse(None, src, dest, &mut diff).context("Failed to compute fsdiff")?;
+    diff_recurse(None, src, dest, &mut diff)?;
     Ok(diff)
 }
 
diff --git a/rust/src/history.rs b/rust/src/history.rs
index a8c39585..1e73b62c 100644
--- a/rust/src/history.rs
+++ b/rust/src/history.rs
@@ -35,7 +35,8 @@
 // (FIXME: Convert to Apache-2.0 OR MIT for consistency?)
 use crate::cxxrsutil::*;
 use crate::ffi::HistoryEntry;
-use anyhow::{anyhow, Context, Result};
+use anyhow::{anyhow, Result};
+use fn_error_context::context;
 use openat::{self, Dir, SimpleType};
 use std::collections::VecDeque;
 use std::fs;
@@ -176,6 +177,7 @@ fn history_get_oldest_deployment_msg_timestamp() -> Result<Option<u64>> {
 /// Gets the oldest deployment message in the journal, and nuke all the GVariant data files
 /// that correspond to deployments older than that one. Essentially, this binds pruning to
 /// journal pruning.
+#[context("Failed to prune history")]
 fn history_prune_inner() -> Result<()> {
     if !Path::new(RPMOSTREE_HISTORY_DIR).exists() {
         return Ok(());
@@ -211,7 +213,7 @@ fn history_prune_inner() -> Result<()> {
 }
 
 pub(crate) fn history_prune() -> CxxResult<()> {
-    Ok(history_prune_inner().context("Failed to prune history")?)
+    Ok(history_prune_inner()?)
 }
 
 pub(crate) fn history_ctx_new() -> CxxResult<Box<HistoryCtx>> {
diff --git a/rust/src/live.rs b/rust/src/live.rs
index f7b74c02..2ad3751b 100644
--- a/rust/src/live.rs
+++ b/rust/src/live.rs
@@ -439,8 +439,7 @@ pub(crate) fn transaction_apply_live(
         .filter(|s| !s.is_empty())
         .unwrap_or(booted_commit);
     // Compute the filesystem-level diff
-    let diff = crate::ostree_diff::diff(repo, source_commit, &target_commit, Some("/usr"))
-        .context("Failed computing diff")?;
+    let diff = crate::ostree_diff::diff(repo, source_commit, &target_commit, Some("/usr"))?;
     // And then the package-level diff
     let pkgdiff = {
         cxx::let_cxx_string!(from = source_commit);
diff --git a/rust/src/ostree_diff.rs b/rust/src/ostree_diff.rs
index cc83807a..6b1b22f7 100644
--- a/rust/src/ostree_diff.rs
+++ b/rust/src/ostree_diff.rs
@@ -7,6 +7,7 @@
  */
 
 use anyhow::{Context, Result};
+use fn_error_context::context;
 use gio::prelude::*;
 use ostree::RepoFileExt;
 use serde_derive::{Deserialize, Serialize};
@@ -149,6 +150,7 @@ fn diff_recurse(
 }
 
 /// Given two ostree commits, compute the diff between them.
+#[context("Computing ostree diff")]
 pub(crate) fn diff<P: AsRef<str>>(
     repo: &ostree::Repo,
     from: &str,