change!: remove Option<impl Progress> in favor of impl Progress (#279)

Author: Byron

git-pack/src/bundle/mod.rs

@@ -20,7 +20,7 @@ pub mod verify {
             /// The packs traversal outcome
             pub pack_traverse_outcome: crate::index::traverse::Outcome,
             /// The provided progress instance.
-            pub progress: Option<P>,
+            pub progress: P,
         }
     }
 
@@ -35,7 +35,7 @@ pub mod verify {
             traversal: crate::index::traverse::Algorithm,
             make_pack_lookup_cache: impl Fn() -> C + Send + Clone,
             thread_limit: Option<usize>,
-            progress: Option<P>,
+            progress: P,
             should_interrupt: &AtomicBool,
         ) -> Result<integrity::Outcome<P>, crate::index::traverse::Error<crate::index::verify::integrity::Error>>
         where

git-pack/src/index/traverse/mod.rs

@@ -1,9 +1,6 @@
 use std::sync::atomic::AtomicBool;
 
-use git_features::{
-    parallel,
-    progress::{self, Progress},
-};
+use git_features::{parallel, progress::Progress};
 
 use crate::index;
 
@@ -67,7 +64,7 @@ impl index::File {
     pub fn traverse<P, C, Processor, E>(
         &self,
         pack: &crate::data::File,
-        progress: Option<P>,
+        progress: P,
         new_processor: impl Fn() -> Processor + Send + Clone,
         new_cache: impl Fn() -> C + Send + Clone,
         Options {
@@ -76,7 +73,7 @@ impl index::File {
             check,
             should_interrupt,
         }: Options<'_>,
-    ) -> Result<(git_hash::ObjectId, Outcome, Option<P>), Error<E>>
+    ) -> Result<(git_hash::ObjectId, Outcome, P), Error<E>>
     where
         P: Progress,
         C: crate::cache::DecodeEntry,
@@ -85,10 +82,9 @@ impl index::File {
             git_object::Kind,
             &[u8],
             &index::Entry,
-            &mut progress::DoOrDiscard<<<P as Progress>::SubProgress as Progress>::SubProgress>,
+            &mut <<P as Progress>::SubProgress as Progress>::SubProgress,
         ) -> Result<(), E>,
     {
-        let progress = progress::DoOrDiscard::from(progress);
         match algorithm {
             Algorithm::Lookup => self.traverse_with_lookup(
                 new_processor,
@@ -105,7 +101,6 @@ impl index::File {
                 self.traverse_with_index(check, thread_limit, new_processor, progress, pack, should_interrupt)
             }
         }
-        .map(|(a, b, p)| (a, b, p.into_inner()))
     }
 
     fn possibly_verify<E>(

git-pack/src/index/verify.rs

@@ -1,6 +1,6 @@
 use std::sync::atomic::AtomicBool;
 
-use git_features::progress::{self, Progress};
+use git_features::progress::Progress;
 use git_object::{bstr::ByteSlice, WriteTo};
 
 use crate::index;
@@ -35,7 +35,7 @@ pub mod integrity {
         /// The packs traversal outcome, if one was provided
         pub pack_traverse_outcome: Option<crate::index::traverse::Outcome>,
         /// The provided progress instance.
-        pub progress: Option<P>,
+        pub progress: P,
     }
 }
 
@@ -129,15 +129,14 @@ impl index::File {
         &self,
         pack: Option<PackContext<'_, C, F>>,
         thread_limit: Option<usize>,
-        progress: Option<P>,
+        mut progress: P,
         should_interrupt: &AtomicBool,
     ) -> Result<integrity::Outcome<P>, index::traverse::Error<crate::index::verify::integrity::Error>>
     where
         P: Progress,
         C: crate::cache::DecodeEntry,
         F: Fn() -> C + Send + Clone,
     {
-        let mut root = progress::DoOrDiscard::from(progress);
         match pack {
             Some(PackContext {
                 data: pack,
@@ -147,7 +146,7 @@ impl index::File {
             }) => self
                 .traverse(
                     pack,
-                    root.into_inner(),
+                    progress,
                     || {
                         let mut encode_buf = Vec::with_capacity(2048);
                         move |kind, data, index_entry, progress| {
@@ -162,18 +161,18 @@ impl index::File {
                         should_interrupt,
                     },
                 )
-                .map(|(id, outcome, root)| integrity::Outcome {
+                .map(|(id, outcome, progress)| integrity::Outcome {
                     actual_index_checksum: id,
                     pack_traverse_outcome: Some(outcome),
-                    progress: root,
+                    progress,
                 }),
             None => self
-                .verify_checksum(root.add_child("Sha1 of index"), should_interrupt)
+                .verify_checksum(progress.add_child("Sha1 of index"), should_interrupt)
                 .map_err(Into::into)
                 .map(|id| integrity::Outcome {
                     actual_index_checksum: id,
                     pack_traverse_outcome: None,
-                    progress: root.into_inner(),
+                    progress,
                 }),
         }
     }

git-pack/src/multi_index/verify.rs

@@ -23,7 +23,7 @@ pub mod integrity {
         /// The for each entry in [`index_names()`][super::File::index_names()] provide the corresponding pack traversal outcome.
         pub pack_traverse_outcomes: Vec<crate::index::traverse::Outcome>,
         /// The provided progress instance.
-        pub progress: Option<P>,
+        pub progress: P,
     }
 }
 
@@ -61,7 +61,7 @@ impl File {
         traversal: crate::index::traverse::Algorithm,
         make_pack_lookup_cache: impl Fn() -> C + Send + Clone,
         thread_limit: Option<usize>,
-        mut progress: Option<P>,
+        mut progress: P,
         should_interrupt: &AtomicBool,
     ) -> Result<integrity::Outcome<P>, crate::index::traverse::Error<integrity::Error>>
     where
@@ -70,24 +70,21 @@ impl File {
     {
         let parent = self.path.parent().expect("must be in a directory");
 
-        let mut progress = git_features::progress::DoOrDiscard::from(progress);
         let actual_index_checksum = self
             .verify_checksum(
                 progress.add_child(format!("checksum of '{}'", self.path.display())),
                 should_interrupt,
             )
             .map_err(integrity::Error::from)
             .map_err(crate::index::traverse::Error::Processor)?;
-        let mut progress = progress.into_inner();
 
         let mut pack_traverse_outcomes = Vec::new();
         for index_file_name in &self.index_names {
             let bundle = crate::Bundle::at(parent.join(index_file_name), self.object_hash)
                 .map_err(integrity::Error::from)
                 .map_err(crate::index::traverse::Error::Processor)?;
-            if let Some(progress) = progress.as_mut() {
-                progress.set_name(index_file_name.display().to_string());
-            }
+
+            progress.set_name(index_file_name.display().to_string());
             let crate::bundle::verify::integrity::Outcome {
                 actual_index_checksum: _,
                 pack_traverse_outcome,

git-pack/tests/pack/data/output/count_and_entries.rs

@@ -392,7 +392,7 @@ fn write_and_verify(
         pack::index::traverse::Algorithm::Lookup,
         || pack::cache::Never,
         None,
-        progress::Discard.into(),
+        progress::Discard,
         &should_interrupt,
     )?;
 

git-pack/tests/pack/index.rs

@@ -308,7 +308,7 @@ mod file {
                                 make_cache_fn: || cache::Never
                             }),
                             None,
-                            progress::Discard.into(),
+                            progress::Discard,
                             &AtomicBool::new(false)
                         )
                         .map(|o| (o.actual_index_checksum, o.pack_traverse_outcome))?,
@@ -408,7 +408,7 @@ mod file {
                 idx.verify_integrity(
                     None::<git_pack::index::verify::PackContext<'_, _, fn() -> cache::Never>>,
                     None,
-                    progress::Discard.into(),
+                    progress::Discard,
                     &AtomicBool::new(false)
                 )
                 .map(|o| (o.actual_index_checksum, o.pack_traverse_outcome))?,

git-pack/tests/pack/multi_index.rs

@@ -76,7 +76,7 @@ mod verify {
                 git_pack::index::traverse::Algorithm::DeltaTreeLookup,
                 || git_pack::cache::Never,
                 None,
-                Some(progress::Discard),
+                progress::Discard,
                 &AtomicBool::new(false),
             )
             .unwrap();

gitoxide-core/src/pack/explode.rs

@@ -11,7 +11,7 @@ use git_repository::{
     hash::ObjectId,
     objs, odb,
     odb::{loose, pack, Write},
-    progress, Progress,
+    Progress,
 };
 use quick_error::quick_error;
 
@@ -151,7 +151,7 @@ pub fn pack_or_pack_index(
     pack_path: impl AsRef<Path>,
     object_path: Option<impl AsRef<Path>>,
     check: SafetyCheck,
-    progress: Option<impl Progress>,
+    progress: impl Progress,
     Context {
         thread_limit,
         delete_pack,
@@ -191,7 +191,7 @@ pub fn pack_or_pack_index(
                 pack::index::traverse::Algorithm::DeltaTreeLookup
             }
         });
-    let mut progress = bundle
+    let (_, _, mut progress) = bundle
         .index
         .traverse(
             &bundle.pack,
@@ -239,7 +239,6 @@ pub fn pack_or_pack_index(
                 should_interrupt: &should_interrupt,
             },
         )
-        .map(|(_, _, c)| progress::DoOrDiscard::from(c))
         .with_context(|| "Failed to explode the entire pack - some loose objects may have been created nonetheless")?;
 
     let (index_path, data_path) = (bundle.index.path().to_owned(), bundle.pack.path().to_owned());

gitoxide-core/src/pack/verify.rs

@@ -8,7 +8,7 @@ use git_repository::{
     hash::ObjectId,
     odb,
     odb::{pack, pack::index},
-    progress, Progress,
+    Progress,
 };
 pub use index::verify::Mode;
 
@@ -103,7 +103,7 @@ impl<const SIZE: usize> pack::cache::DecodeEntry for EitherCache<SIZE> {
 
 pub fn pack_or_pack_index<W1, W2>(
     path: impl AsRef<Path>,
-    progress: Option<impl Progress>,
+    mut progress: impl Progress,
     Context {
         mut out,
         mut err,
@@ -129,11 +129,8 @@ where
     let res = match ext {
         "pack" => {
             let pack = odb::pack::data::File::at(path, object_hash).with_context(|| "Could not open pack file")?;
-            pack.verify_checksum(
-                progress::DoOrDiscard::from(progress).add_child("Sha1 of pack"),
-                should_interrupt,
-            )
-            .map(|id| (id, None))?
+            pack.verify_checksum(progress.add_child("Sha1 of pack"), should_interrupt)
+                .map(|id| (id, None))?
         }
         "idx" => {
             let idx =

src/plumbing/main.rs

@@ -114,7 +114,6 @@ pub fn main() -> Result<()> {
                             core::pack::create::ObjectExpansion::None
                         }),
                     };
-                    let progress = git_features::progress::DoOrDiscard::from(progress);
                     core::pack::create(repository, tips, input, output_directory, progress, context)
                 },
             )
@@ -233,7 +232,7 @@ pub fn main() -> Result<()> {
                 core::pack::index::from_pack(
                     input,
                     directory,
-                    git_features::progress::DoOrDiscard::from(progress),
+                    progress,
                     core::pack::index::Context {
                         thread_limit,
                         iteration_mode,

src/shared.rs

@@ -49,6 +49,7 @@ pub mod pretty {
     use std::io::{stderr, stdout};
 
     use anyhow::Result;
+    use git_repository::progress;
 
     use crate::shared::ProgressRange;
 
@@ -59,26 +60,34 @@ pub mod pretty {
         #[cfg_attr(not(feature = "prodash-render-tui"), allow(unused_variables))] progress_keep_open: bool,
         #[cfg_attr(not(feature = "prodash-render-line"), allow(unused_variables))] range: impl Into<Option<ProgressRange>>,
         #[cfg(not(any(feature = "prodash-render-line", feature = "prodash-render-tui")))] run: impl FnOnce(
-            Option<prodash::progress::Log>,
+            progress::DoOrDiscard<prodash::progress::Log>,
             &mut dyn std::io::Write,
             &mut dyn std::io::Write,
         )
             -> Result<T>,
-        #[cfg(any(feature = "prodash-render-line", feature = "prodash-render-tui"))] run: impl FnOnce(Option<prodash::tree::Item>, &mut dyn std::io::Write, &mut dyn std::io::Write) -> Result<T>
+        #[cfg(any(feature = "prodash-render-line", feature = "prodash-render-tui"))] run: impl FnOnce(
+                progress::DoOrDiscard<prodash::tree::Item>,
+                &mut dyn std::io::Write,
+                &mut dyn std::io::Write,
+            ) -> Result<T>
             + Send
             + std::panic::UnwindSafe
             + 'static,
     ) -> Result<T> {
         crate::shared::init_env_logger(false);
 
         match (verbose, progress) {
-            (false, false) => run(None, &mut stdout(), &mut stderr()),
+            (false, false) => run(progress::DoOrDiscard::from(None), &mut stdout(), &mut stderr()),
             (true, false) => {
                 let progress = crate::shared::progress_tree();
                 let sub_progress = progress.add_child(name);
                 #[cfg(not(feature = "prodash-render-line"))]
                 {
-                    run(Some(sub_progress), &mut stdout(), &mut stderr())
+                    run(
+                        progress::DoOrDiscard::from(Some(sub_progress)),
+                        &mut stdout(),
+                        &mut stderr(),
+                    )
                 }
                 #[cfg(feature = "prodash-render-line")]
                 {
@@ -105,7 +114,7 @@ pub mod pretty {
                     let join_handle = std::thread::spawn(move || {
                         let mut out = Vec::<u8>::new();
                         let res = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
-                            run(Some(sub_progress), &mut out, &mut stderr())
+                            run(progress::DoOrDiscard::from(Some(sub_progress)), &mut out, &mut stderr())
                         }));
                         match res {
                             Ok(res) => tx.send(Event::ComputationDone(res, out)).ok(),
@@ -174,7 +183,7 @@ pub mod pretty {
                     // We might have something interesting to show, which would be hidden by the alternate screen if there is a progress TUI
                     // We know that the printing happens at the end, so this is fine.
                     let mut out = Vec::new();
-                    let res = run(Some(sub_progress), &mut out, &mut stderr());
+                    let res = run(progress::DoOrDiscard::from(Some(sub_progress)), &mut out, &mut stderr());
                     tx.send(Event::ComputationDone(res, out)).ok();
                 });
                 loop {