Merge pull request #723 from matthiasbeyer/libimagstorestdhook/git-ci-fixes

libimagstorestdhook: git ci fixes

libimagstore/src/store.rs

@@ -493,21 +493,10 @@ impl Store {
 
     /// Return the `FileLockEntry` and write to disk
     pub fn update<'a>(&'a self, mut entry: FileLockEntry<'a>) -> Result<()> {
-        if let Err(e) = self.execute_hooks_for_mut_file(self.pre_update_aspects.clone(), &mut entry) {
+        if let Err(e) = self._update(&mut entry) {
-            return Err(e)
-                .map_err_into(SEK::PreHookExecuteError)
-                .map_err_into(SEK::HookExecutionError)
-                .map_err_into(SEK::UpdateCallError);
-        }
-
-        if let Err(e) = self._update(&entry) {
             return Err(e).map_err_into(SEK::UpdateCallError);
         }
-
+        Ok(())
-        self.execute_hooks_for_mut_file(self.post_update_aspects.clone(), &mut entry)
-            .map_err_into(SEK::PostHookExecuteError)
-            .map_err_into(SEK::HookExecutionError)
-            .map_err_into(SEK::UpdateCallError)
     }
 
     /// Internal method to write to the filesystem store.
@@ -515,7 +504,14 @@ impl Store {
     /// # Assumptions
     /// This method assumes that entry is dropped _right after_ the call, hence
     /// it is not public.
-    fn _update<'a>(&'a self, entry: &FileLockEntry<'a>) -> Result<()> {
+    fn _update<'a>(&'a self, entry: &mut FileLockEntry<'a>) -> Result<()> {
+        if let Err(e) = self.execute_hooks_for_mut_file(self.pre_update_aspects.clone(), entry) {
+            return Err(e)
+                .map_err_into(SEK::PreHookExecuteError)
+                .map_err_into(SEK::HookExecutionError)
+                .map_err_into(SEK::UpdateCallError);
+        }
+
         let mut hsmap = match self.entries.write() {
             Err(_) => return Err(SE::new(SEK::LockPoisoned, None)),
             Ok(e) => e,
@@ -532,7 +528,11 @@ impl Store {
         try!(se.write_entry(&entry.entry));
         se.status = StoreEntryStatus::Present;
 
-        Ok(())
+
+        self.execute_hooks_for_mut_file(self.post_update_aspects.clone(), entry)
+            .map_err_into(SEK::PostHookExecuteError)
+            .map_err_into(SEK::HookExecutionError)
+            .map_err_into(SEK::UpdateCallError)
     }
 
     /// Retrieve a copy of a given entry, this cannot be used to mutate
@@ -865,7 +865,7 @@ impl<'a> DerefMut for FileLockEntry<'a> {
 impl<'a> Drop for FileLockEntry<'a> {
     /// This will silently ignore errors, use `Store::update` if you want to catch the errors
     fn drop(&mut self) {
-        let _ = self.store._update(self);
+        let _ = self.store._update(self).map_err(|e| trace_error(&e));
     }
 }
 

libimagstorestdhook/src/vcs/git/create.rs

@@ -89,56 +89,9 @@ impl StoreIdAccessor for CreateHook {
     /// After that, the UpdateHook will take care of committing the changes or new file.
     ///
     fn access(&self, id: &StoreId) -> HookResult<()> {
-        use vcs::git::action::StoreAction;
-        use vcs::git::config::commit_message;
-        use vcs::git::error::MapIntoHookError;
-        use vcs::git::util::fetch_index;
-
         debug!("[GIT CREATE HOOK]: {:?}", id);
-
+        debug!("[GIT CREATE HOOK]: Doing nothing as Store::create() is lazy and does not write to disk");
-        let path = try!(
+        Ok(())
-            id.clone()
-            .into_pathbuf()
-            .map_err_into(GHEK::StoreIdHandlingError)
-            .map_into_hook_error()
-        );
-
-        let action    = StoreAction::Create;
-        try!(self.runtime.ensure_cfg_branch_is_checked_out(&action));
-
-        let cfg       = try!(self.runtime.config_value_or_err(&action));
-        let repo      = try!(self.runtime.repository(&action));
-        let mut index = try!(fetch_index(repo, &action));
-
-        let file_status = try!(
-            repo
-                .status_file(&path)
-                .map_err_into(GHEK::RepositoryFileStatusError)
-                .map_into_hook_error()
-        );
-
-        let cb = &mut |path: &Path, _matched_spec: &[u8]| -> i32 {
-            if file_status.contains(STATUS_WT_MODIFIED) ||
-                file_status.contains(STATUS_WT_NEW) {
-
-                debug!("[GIT CREATE HOOK]: File is new or modified: {}", path.display());
-                0
-            } else {
-                debug!("[GIT CREATE HOOK]: Ignoring file: {}", path.display());
-                1
-            }
-        };
-
-        try!(
-            index.add_all(&[path], ADD_DEFAULT, Some(cb as &mut IndexMatchedPath))
-                .map_err_into(GHEK::RepositoryPathAddingError)
-                .map_into_hook_error()
-        );
-
-        index
-            .write()
-            .map_err_into(GHEK::RepositoryIndexWritingError)
-            .map_into_hook_error()
     }
 
 }

libimagstorestdhook/src/vcs/git/update.rs

@@ -1,4 +1,5 @@
 use std::path::PathBuf;
+use std::path::Path;
 use std::fmt::{Debug, Formatter, Error as FmtError};
 use std::result::Result as RResult;
 
@@ -88,6 +89,8 @@ impl StoreIdAccessor for UpdateHook {
         use vcs::git::config::commit_message;
         use vcs::git::error::MapIntoHookError;
         use vcs::git::util::fetch_index;
+        use git2::{Reference as GitReference, Repository, Error as Git2Error};
+        use git2::{ADD_DEFAULT, STATUS_WT_NEW, STATUS_WT_MODIFIED, IndexMatchedPath};
 
         debug!("[GIT UPDATE HOOK]: {:?}", id);
 
@@ -96,21 +99,50 @@ impl StoreIdAccessor for UpdateHook {
         let repo      = try!(self.runtime.repository(&action));
         let mut index = try!(fetch_index(repo, &action));
 
-        let tree_id = try!(
-            index.write_tree()
-                .map_err_into(GHEK::RepositoryIndexWritingError)
-                .map_into_hook_error()
-        );
-
         let signature = try!(
             repo.signature()
                 .map_err_into(GHEK::MkSignature)
+                .map_dbg_err_str("Failed to fetch signature")
                 .map_into_hook_error()
         );
 
         let head = try!(
             repo.head()
                 .map_err_into(GHEK::HeadFetchError)
+                .map_dbg_err_str("Failed to fetch HEAD")
+                .map_into_hook_error()
+        );
+
+        let file_status = try!(
+            repo
+                .status_file(id.local())
+                .map_dbg_err_str("Failed to fetch file status")
+                .map_dbg_err(|e| format!("\t->  {:?}", e))
+                .map_err_into(GHEK::RepositoryFileStatusError)
+                .map_into_hook_error()
+        );
+
+        let cb = &mut |path: &Path, _matched_spec: &[u8]| -> i32 {
+            if file_status.contains(STATUS_WT_NEW) || file_status.contains(STATUS_WT_MODIFIED) {
+                debug!("[GIT CREATE HOOK]: File is modified/new: {}", path.display());
+                0
+            } else {
+                debug!("[GIT CREATE HOOK]: Ignoring file: {}", path.display());
+                1
+            }
+        };
+
+        try!(
+            index.add_all(&[id.local()], ADD_DEFAULT, Some(cb as &mut IndexMatchedPath))
+                .map_err_into(GHEK::RepositoryPathAddingError)
+                .map_dbg_err_str("Failed to add to index")
+                .map_into_hook_error()
+        );
+
+        let tree_id = try!(
+            index.write_tree()
+                .map_err_into(GHEK::RepositoryIndexWritingError)
+                .map_dbg_err_str("Failed to write tree")
                 .map_into_hook_error()
         );
 
@@ -119,6 +151,7 @@ impl StoreIdAccessor for UpdateHook {
             let commit = try!(
                 repo.find_commit(head.target().unwrap())
                     .map_err_into(GHEK::RepositoryParentFetchingError)
+                    .map_dbg_err_str("Failed to find commit HEAD")
                     .map_into_hook_error()
             );
             parents.push(commit);
@@ -130,16 +163,25 @@ impl StoreIdAccessor for UpdateHook {
         let tree = try!(
             repo.find_tree(tree_id)
                 .map_err_into(GHEK::RepositoryParentFetchingError)
+                .map_dbg_err_str("Failed to find tree")
                 .map_into_hook_error()
         );
 
-        let message = try!(commit_message(cfg, StoreAction::Update));
+        let message = try!(commit_message(cfg, StoreAction::Update)
+                .map_dbg_err_str("Failed to get commit message"));
 
-        repo.commit(Some("HEAD"), &signature, &signature, &message, &tree, &parents)
+        try!(repo.commit(Some("HEAD"), &signature, &signature, &message, &tree, &parents)
+            .map_dbg_str("Committed")
+            .map_dbg_err_str("Failed to commit")
             .map_err_into(GHEK::RepositoryCommittingError)
             .map_into_hook_error()
+        );
+
+        index.write()
+            .map_err_into(GHEK::RepositoryIndexWritingError)
+            .map_dbg_err_str("Failed to write tree")
+            .map_into_hook_error()
             .map(|_| ())
-
     }
 
 }

tests/utils.sh

@@ -47,10 +47,16 @@ cat_entry() {
 }
 
 reset_store() {
+    rm -rf "${STORE}"/.git
     rm -r "${STORE}"
 }
 
 call_test() {
+    prepare_store_directory || {
+        err "Preparing store directory failed"
+        exit 1
+    }
+
     out "-- TESTING: '$1' --"
     $1
     result=$?
@@ -63,6 +69,27 @@ call_test() {
     success "-- SUCCESS: '$1' --"
 }
 
+__git() {
+    out "Calling git: $*"
+    git --work-tree=/tmp/store/ --git-dir=/tmp/store/.git $*
+}
+
+__git_commit() {
+    out "Calling git-commit: $*"
+    git --work-tree=/tmp/store/ --git-dir=/tmp/store/.git commit -m "$*"
+}
+
+prepare_store_directory() {
+    out "Preparing /tmp/store"
+    mkdir -p /tmp/store/                                &&\
+    touch /tmp/store/.gitkeep                           &&\
+    __git init                                          &&\
+    __git config --local user.email "imag@imag-pim.org" &&\
+    __git config --local user.name "Imag CI"            &&\
+    __git add .gitkeep                                  &&\
+    __git_commit 'Initial commit: .gitkeep'
+}
+
 invoke_tests() {
     out "Invoking tests."
     if [[ ! -z "$INVOKE_TEST" ]]; then