sync

Cargo.toml

@@ -41,6 +41,9 @@ anyhow = "1.0.33"
 unicode-width = "0.1"
 textwrap = "0.12"
 
+[target.'cfg(target_os = "linux")'.dependencies]
+which = "4.0.2"
+
 [target.'cfg(not(windows))'.dependencies]
 pprof = { version = "0.3", features = ["flamegraph"], optional = true }
 

asyncgit/src/error.rs

@@ -12,6 +12,9 @@ pub enum Error {
     #[error("git: remote url not found")]
     UnknownRemote,
 
+    #[error("git: work dir error")]
+    NoWorkDir,
+
     #[error("io error:{0}")]
     Io(#[from] std::io::Error),
 

asyncgit/src/lib.rs

@@ -7,6 +7,8 @@
 #![deny(clippy::unwrap_used)]
 #![deny(clippy::panic)]
 #![deny(clippy::perf)]
+//TODO: get this in someday since expect still leads us to crashes sometimes
+// #![deny(clippy::expect_used)]
 
 pub mod cached;
 mod commit_files;

asyncgit/src/sync/branch.rs

@@ -8,6 +8,8 @@ use git2::BranchType;
 use scopetime::scope_time;
 use utils::get_head_repo;
 
+use super::utils::bytes2string;
+
 /// returns the branch-name head is currently pointing to
 /// this might be expensive, see `cached::BranchName`
 pub(crate) fn get_branch_name(repo_path: &str) -> Result<String> {
@@ -41,6 +43,8 @@ pub struct BranchForDisplay {
     pub top_commit: CommitId,
     ///
     pub is_head: bool,
+    ///
+    pub has_upstream: bool,
 }
 
 /// Used to return only the nessessary information for displaying a branch
@@ -58,17 +62,14 @@ pub fn get_branches_to_display(
             let top_commit = branch.get().peel_to_commit()?;
 
             Ok(BranchForDisplay {
-                name: String::from_utf8(Vec::from(
-                    branch.name_bytes()?,
-                ))?,
-                reference: String::from_utf8(Vec::from(
-                    branch.get().name_bytes(),
-                ))?,
-                top_commit_message: String::from_utf8(Vec::from(
+                name: bytes2string(branch.name_bytes()?)?,
+                reference: bytes2string(branch.get().name_bytes())?,
+                top_commit_message: bytes2string(
                     top_commit.summary_bytes().unwrap_or_default(),
-                ))?,
+                )?,
                 top_commit: top_commit.id().into(),
                 is_head: branch.is_head(),
+                has_upstream: branch.upstream().is_ok(),
             })
         })
         .filter_map(Result::ok)
@@ -132,7 +133,9 @@ pub fn checkout_branch(
             git2::build::CheckoutBuilder::new().force(),
         )) {
             // This is safe beacuse cur_ref was just found
-            repo.set_head(cur_ref.name().expect("utf8 error"))?;
+            repo.set_head(
+                bytes2string(cur_ref.name_bytes())?.as_str(),
+            )?;
             return Err(Error::Git(e));
         }
         Ok(())
@@ -188,8 +191,7 @@ pub fn create_branch(repo_path: &str, name: &str) -> Result<()> {
 
     let branch = repo.branch(name, &head_commit, false)?;
     let branch_ref = branch.into_reference();
-    let branch_ref_name =
-        String::from_utf8(branch_ref.name_bytes().to_vec())?;
+    let branch_ref_name = bytes2string(branch_ref.name_bytes())?;
     repo.set_head(branch_ref_name.as_str())?;
 
     Ok(())

asyncgit/src/sync/commit_files.rs

@@ -1,5 +1,7 @@
 use super::{stash::is_stash_commit, utils::repo, CommitId};
-use crate::{error::Result, StatusItem, StatusItemType};
+use crate::{
+    error::Error, error::Result, StatusItem, StatusItemType,
+};
 use git2::{Diff, DiffDelta, DiffOptions, Repository};
 use scopetime::scope_time;
 
@@ -66,7 +68,10 @@ pub(crate) fn get_commit_diff(
     )?;
 
     if is_stash_commit(
-        repo.path().to_str().expect("repo path utf8 err"),
+        repo.path().to_str().map_or_else(
+            || Err(Error::Generic("repo path utf8 err".to_owned())),
+            Ok,
+        )?,
         &id,
     )? {
         if let Ok(untracked_commit) = commit.parent_id(2) {

asyncgit/src/sync/diff.rs

@@ -132,7 +132,7 @@ pub fn get_diff(
     scope_time!("get_diff");
 
     let repo = utils::repo(repo_path)?;
-    let work_dir = work_dir(&repo);
+    let work_dir = work_dir(&repo)?;
     let diff = get_diff_raw(&repo, &p, stage, false)?;
 
     raw_diff_to_file_diff(&diff, work_dir)
@@ -148,7 +148,7 @@ pub fn get_diff_commit(
     scope_time!("get_diff_commit");
 
     let repo = utils::repo(repo_path)?;
-    let work_dir = work_dir(&repo);
+    let work_dir = work_dir(&repo)?;
     let diff = get_commit_diff(&repo, id, Some(p))?;
 
     raw_diff_to_file_diff(&diff, work_dir)
@@ -219,40 +219,39 @@ fn raw_diff_to_file_diff<'a>(
         };
 
         let new_file_diff = if diff.deltas().len() == 1 {
-            let delta: DiffDelta = diff
-                .deltas()
-                .next()
-                .expect("it's safe to unwrap here because we check first that diff.deltas has a single element");
-
-            if delta.status() == Delta::Untracked {
-                let relative_path =
-                    delta.new_file().path().ok_or_else(|| {
-                        Error::Generic(
-                            "new file path is unspecified."
-                                .to_string(),
-                        )
-                    })?;
-
-                let newfile_path = work_dir.join(relative_path);
-
-                if let Some(newfile_content) =
-                    new_file_content(&newfile_path)
-                {
-                    let mut patch = Patch::from_buffers(
-                        &[],
-                        None,
-                        newfile_content.as_slice(),
-                        Some(&newfile_path),
-                        None,
-                    )?;
-
-                    patch
+            if let Some(delta) = diff.deltas().next() {
+                if delta.status() == Delta::Untracked {
+                    let relative_path =
+                        delta.new_file().path().ok_or_else(|| {
+                            Error::Generic(
+                                "new file path is unspecified."
+                                    .to_string(),
+                            )
+                        })?;
+
+                    let newfile_path = work_dir.join(relative_path);
+
+                    if let Some(newfile_content) =
+                        new_file_content(&newfile_path)
+                    {
+                        let mut patch = Patch::from_buffers(
+                            &[],
+                            None,
+                            newfile_content.as_slice(),
+                            Some(&newfile_path),
+                            None,
+                        )?;
+
+                        patch
                     .print(&mut |delta, hunk:Option<DiffHunk>, line: git2::DiffLine| {
                         put(delta,hunk,line);
                         true
                     })?;
 
-                    true
+                        true
+                    } else {
+                        false
+                    }
                 } else {
                     false
                 }
@@ -275,7 +274,10 @@ fn raw_diff_to_file_diff<'a>(
 
         if !current_lines.is_empty() {
             adder(
-                &current_hunk.expect("invalid hunk"),
+                &current_hunk.map_or_else(
+                    || Err(Error::Generic("invalid hunk".to_owned())),
+                    Ok,
+                )?,
                 &current_lines,
             );
         }
@@ -284,7 +286,8 @@ fn raw_diff_to_file_diff<'a>(
             res.borrow_mut().untracked = true;
         }
     }
-    let res = Rc::try_unwrap(res).expect("rc error");
+    let res = Rc::try_unwrap(res)
+        .map_err(|_| Error::Generic("rc unwrap error".to_owned()))?;
     Ok(res.into_inner())
 }
 

asyncgit/src/sync/hooks.rs

@@ -34,7 +34,7 @@ pub fn hooks_commit_msg(
             work_dir.as_str(),
             HOOK_COMMIT_MSG,
             &[HOOK_COMMIT_MSG_TEMP_FILE],
-        );
+        )?;
 
         // load possibly altered msg
         msg.clear();
@@ -54,9 +54,7 @@ pub fn hooks_pre_commit(repo_path: &str) -> Result<HookResult> {
     let work_dir = work_dir_as_string(repo_path)?;
 
     if hook_runable(work_dir.as_str(), HOOK_PRE_COMMIT) {
-        let res = run_hook(work_dir.as_str(), HOOK_PRE_COMMIT, &[]);
-
-        Ok(res)
+        Ok(run_hook(work_dir.as_str(), HOOK_PRE_COMMIT, &[])?)
     } else {
         Ok(HookResult::Ok)
     }
@@ -69,22 +67,21 @@ pub fn hooks_post_commit(repo_path: &str) -> Result<HookResult> {
     let work_dir_str = work_dir.as_str();
 
     if hook_runable(work_dir_str, HOOK_POST_COMMIT) {
-        Ok(run_hook(work_dir_str, HOOK_POST_COMMIT, &[]))
+        Ok(run_hook(work_dir_str, HOOK_POST_COMMIT, &[])?)
     } else {
         Ok(HookResult::Ok)
     }
 }
 
 fn work_dir_as_string(repo_path: &str) -> Result<String> {
     let repo = repo(repo_path)?;
-    work_dir(&repo)
-        .to_str()
-        .map(|s| s.to_string())
-        .ok_or_else(|| {
+    work_dir(&repo)?.to_str().map(|s| s.to_string()).ok_or_else(
+        || {
             Error::Generic(
                 "workdir contains invalid utf8".to_string(),
             )
-        })
+        },
+    )
 }
 
 fn hook_runable(path: &str, hook: &str) -> bool {
@@ -109,7 +106,7 @@ fn run_hook(
     path: &str,
     hook_script: &str,
     args: &[&str],
-) -> HookResult {
+) -> Result<HookResult> {
     let arg_str = format!("{} {}", hook_script, args.join(" "));
     let bash_args = vec!["-c".to_string(), arg_str];
 
@@ -123,18 +120,16 @@ fn run_hook(
             "DUMMY_ENV_TO_FIX_WINDOWS_CMD_RUNS",
             "FixPathHandlingOnWindows",
         )
-        .output();
-
-    let output = output.expect("general hook error");
+        .output()?;
 
     if output.status.success() {
-        HookResult::Ok
+        Ok(HookResult::Ok)
     } else {
         let err = String::from_utf8_lossy(&output.stderr);
         let out = String::from_utf8_lossy(&output.stdout);
         let formatted = format!("{}{}", out, err);
 
-        HookResult::NotOk(formatted)
+        Ok(HookResult::NotOk(formatted))
     }
 }
 

asyncgit/src/sync/hunks.rs

@@ -23,9 +23,12 @@ pub fn stage_hunk(
 
     let mut opt = ApplyOptions::new();
     opt.hunk_callback(|hunk| {
-        let header =
-            HunkHeader::from(hunk.expect("hunk unavailable"));
-        hash(&header) == hunk_hash
+        if let Some(hunk) = hunk {
+            let header = HunkHeader::from(hunk);
+            hash(&header) == hunk_hash
+        } else {
+            false
+        }
     });
 
     repo.apply(&diff, ApplyLocation::Index, Some(&mut opt))?;
@@ -105,9 +108,10 @@ pub fn unstage_hunk(
     let diff_count_positive = diff.deltas().len();
 
     let hunk_index = find_hunk_index(&diff, hunk_hash);
-    if hunk_index.is_none() {
-        return Err(Error::Generic("hunk not found".to_string()));
-    }
+    let hunk_index = hunk_index.map_or_else(
+        || Err(Error::Generic("hunk not found".to_string())),
+        Ok,
+    )?;
 
     let diff = get_diff_raw(&repo, &file_path, true, true)?;
 
@@ -124,8 +128,7 @@ pub fn unstage_hunk(
         let mut hunk_idx = 0;
         let mut opt = ApplyOptions::new();
         opt.hunk_callback(|_hunk| {
-            let res = if hunk_idx == hunk_index.expect("invalid hunk")
-            {
+            let res = if hunk_idx == hunk_index {
                 count += 1;
                 true
             } else {

asyncgit/src/sync/ignore.rs

@@ -18,7 +18,7 @@ pub fn add_to_ignore(
 
     let repo = repo(repo_path)?;
 
-    let ignore_file = work_dir(&repo).join(GITIGNORE);
+    let ignore_file = work_dir(&repo)?.join(GITIGNORE);
 
     let optional_newline = ignore_file.exists()
         && !file_ends_with_newline(&ignore_file)?;

asyncgit/src/sync/mod.rs

@@ -1,5 +1,8 @@
 //! sync git api
 
+//TODO: remove once we have this activated on the toplevel
+#![deny(clippy::expect_used)]
+
 mod branch;
 mod commit;
 mod commit_details;

asyncgit/src/sync/tags.rs

@@ -25,6 +25,7 @@ pub fn get_tags(repo_path: &str) -> Result<Tags> {
 
     repo.tag_foreach(|id, name| {
         if let Ok(name) =
+            // skip the `refs/tags/` part
             String::from_utf8(name[10..name.len()].into())
         {
             //NOTE: find_tag (git_tag_lookup) only works on annotated tags