Merge pull request #1941 from libgit2/rb/preserve-iterator-error

Preserve error messages during file system iterator cleanup

include/git2/errors.h

@@ -8,6 +8,7 @@
 #define INCLUDE_git_errors_h__
 
 #include "common.h"
+#include "buffer.h"
 
 /**
  * @file git2/errors.h
@@ -45,6 +46,7 @@ typedef struct {
 
 /** Error classes */
 typedef enum {
+	GITERR_NONE = 0,
 	GITERR_NOMEMORY,
 	GITERR_OS,
 	GITERR_INVALID,
@@ -85,6 +87,18 @@ GIT_EXTERN(const git_error *) giterr_last(void);
 GIT_EXTERN(void) giterr_clear(void);
 
 /**
+ * Get the last error data and clear it.
+ *
+ * This copies the last error into the given `git_error` struct
+ * and returns 0 if the copy was successful, leaving the error 
+ * cleared as if `giterr_clear` had been called.
+ *
+ * If there was no existing error in the library, -1 will be returned
+ * and the contents of `cpy` will be left unmodified.
+ */
+GIT_EXTERN(int) giterr_detach(git_error *cpy);
+
+/**
  * Set the error message string for this thread.
  *
  * This function is public so that custom ODB backends and the like can

src/errors.c

@@ -112,6 +112,24 @@ void giterr_clear(void)
 #endif
 }
 
+int giterr_detach(git_error *cpy)
+{
+	git_error *error = GIT_GLOBAL->last_error;
+
+	assert(cpy);
+
+	if (!error)
+		return -1;
+
+	cpy->message = error->message;
+	cpy->klass = error->klass;
+
+	error->message = NULL;
+	giterr_clear();
+
+	return 0;
+}
+
 const git_error *giterr_last(void)
 {
 	return GIT_GLOBAL->last_error;

src/iterator.c

@@ -991,8 +991,21 @@ static int fs_iterator__expand_dir(fs_iterator *fi)
 		fi->base.start, fi->base.end, &ff->entries);
 
 	if (error < 0) {
+		git_error last_error = {0};
+
+		giterr_detach(&last_error);
+
+		/* these callbacks may clear the error message */
 		fs_iterator__free_frame(ff);
 		fs_iterator__advance_over(NULL, (git_iterator *)fi);
+		/* next time return value we skipped to */
+		fi->base.flags &= ~GIT_ITERATOR_FIRST_ACCESS;
+
+		if (last_error.message) {
+			giterr_set_str(last_error.klass, last_error.message);
+			free(last_error.message);
+		}
+
 		return error;
 	}
 

tests-clar/repo/iterator.c

@@ -926,3 +926,37 @@ void test_repo_iterator__fs2(void)
 	expect_iterator_items(i, 12, expect_base, 12, expect_base);
 	git_iterator_free(i);
 }
+
+void test_repo_iterator__fs_preserves_error(void)
+{
+	git_iterator *i;
+	const git_index_entry *e;
+
+	if (!cl_is_chmod_supported())
+		return;
+
+	g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+	cl_must_pass(p_mkdir("empty_standard_repo/r", 0777));
+	cl_git_mkfile("empty_standard_repo/r/a", "hello");
+	cl_must_pass(p_mkdir("empty_standard_repo/r/b", 0777));
+	cl_git_mkfile("empty_standard_repo/r/b/problem", "not me");
+	cl_must_pass(p_chmod("empty_standard_repo/r/b", 0000));
+	cl_must_pass(p_mkdir("empty_standard_repo/r/c", 0777));
+	cl_git_mkfile("empty_standard_repo/r/d", "final");
+
+	cl_git_pass(git_iterator_for_filesystem(
+		&i, "empty_standard_repo/r", 0, NULL, NULL));
+
+	cl_git_pass(git_iterator_advance(&e, i)); /* a */
+	cl_git_fail(git_iterator_advance(&e, i)); /* b */
+	cl_assert(giterr_last());
+	cl_assert(giterr_last()->message != NULL);
+	/* skip 'c/' empty directory */
+	cl_git_pass(git_iterator_advance(&e, i)); /* d */
+	cl_assert_equal_i(GIT_ITEROVER, git_iterator_advance(&e, i));
+
+	cl_must_pass(p_chmod("empty_standard_repo/r/b", 0777));
+
+	git_iterator_free(i);
+}