Fix checkout notify callback docs and tests

The checkout notify callback behavior on non-zero return values
was not being tested.  This adds tests, fixes a bug with positive
values, and clarifies the documentation to make it clear that the
checkout can be canceled via this mechanism.

include/git2/checkout.h

@@ -174,7 +174,12 @@ typedef enum {
  * - GIT_CHECKOUT_NOTIFY_IGNORED notifies about ignored files.
  *
  * Returning a non-zero value from this callback will cancel the checkout.
- * Notification callbacks are made prior to modifying any files on disk.
+ * The non-zero return value will be propagated back and returned by the
+ * git_checkout_... call.
+ *
+ * Notification callbacks are made prior to modifying any files on disk,
+ * so canceling on any notification will still happen prior to any files
+ * being modified.
  */
 typedef enum {
 	GIT_CHECKOUT_NOTIFY_NONE      = 0,
@@ -252,9 +257,9 @@ typedef struct git_checkout_opts {
  *
  * @param repo repository to check out (must be non-bare)
  * @param opts specifies checkout options (may be NULL)
- * @return 0 on success, GIT_EUNBORNBRANCH when HEAD points to a non existing
- * branch, GIT_ERROR otherwise (use giterr_last for information
- * about the error)
+ * @return 0 on success, GIT_EUNBORNBRANCH if HEAD points to a non
+ *         existing branch, non-zero value returned by `notify_cb`, or
+ *         other error code < 0 (use giterr_last for error details)
  */
 GIT_EXTERN(int) git_checkout_head(
 	git_repository *repo,
@@ -266,8 +271,8 @@ GIT_EXTERN(int) git_checkout_head(
  * @param repo repository into which to check out (must be non-bare)
  * @param index index to be checked out (or NULL to use repository index)
  * @param opts specifies checkout options (may be NULL)
- * @return 0 on success, GIT_ERROR otherwise (use giterr_last for information
- * about the error)
+ * @return 0 on success, non-zero return value from `notify_cb`, or error
+ *         code < 0 (use giterr_last for error details)
  */
 GIT_EXTERN(int) git_checkout_index(
 	git_repository *repo,
@@ -282,8 +287,8 @@ GIT_EXTERN(int) git_checkout_index(
  * @param treeish a commit, tag or tree which content will be used to update
  * the working directory (or NULL to use HEAD)
  * @param opts specifies checkout options (may be NULL)
- * @return 0 on success, GIT_ERROR otherwise (use giterr_last for information
- * about the error)
+ * @return 0 on success, non-zero return value from `notify_cb`, or error
+ *         code < 0 (use giterr_last for error details)
  */
 GIT_EXTERN(int) git_checkout_tree(
 	git_repository *repo,

src/checkout.c

@@ -993,7 +993,7 @@ static int checkout_get_actions(
 
 	git_vector_foreach(deltas, i, delta) {
 		error = checkout_action(&act, data, delta, workdir, &wditem, &pathspec);
-		if (error)
+		if (error != 0)
 			goto fail;
 
 		actions[i] = act;
@@ -1957,7 +1957,7 @@ int git_checkout_iterator(
 	 * actions to be taken, plus look for conflicts and send notifications,
 	 * then loop through conflicts.
 	 */
-	if ((error = checkout_get_actions(&actions, &counts, &data, workdir)) < 0)
+	if ((error = checkout_get_actions(&actions, &counts, &data, workdir)) != 0)
 		goto cleanup;
 
 	data.total_steps = counts[CHECKOUT_ACTION__REMOVE] +

tests/checkout/tree.c

@@ -486,6 +486,84 @@ void test_checkout_tree__donot_update_deleted_file_by_default(void)
 	git_index_free(index);
 }
 
+struct checkout_cancel_at {
+	const char *filename;
+	int error;
+	int count;
+};
+
+static int checkout_cancel_cb(
+	git_checkout_notify_t why,
+	const char *path,
+	const git_diff_file *b,
+	const git_diff_file *t,
+	const git_diff_file *w,
+	void *payload)
+{
+	struct checkout_cancel_at *ca = payload;
+
+	GIT_UNUSED(why); GIT_UNUSED(b); GIT_UNUSED(t); GIT_UNUSED(w);
+
+	ca->count++;
+
+	if (!strcmp(path, ca->filename))
+		return ca->error;
+
+	return 0;
+}
+
+void test_checkout_tree__can_cancel_checkout_from_notify(void)
+{
+	struct checkout_cancel_at ca;
+	git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT;
+	git_oid oid;
+	git_object *obj = NULL;
+
+	assert_on_branch(g_repo, "master");
+
+	cl_git_pass(git_reference_name_to_id(&oid, g_repo, "refs/heads/dir"));
+	cl_git_pass(git_object_lookup(&obj, g_repo, &oid, GIT_OBJ_ANY));
+
+	ca.filename = "new.txt";
+	ca.error = -5555;
+	ca.count = 0;
+
+	opts.notify_flags = GIT_CHECKOUT_NOTIFY_UPDATED;
+	opts.notify_cb = checkout_cancel_cb;
+	opts.notify_payload = &ca;
+	opts.checkout_strategy = GIT_CHECKOUT_FORCE;
+
+	cl_assert(!git_path_exists("testrepo/new.txt"));
+
+	cl_git_fail_with(git_checkout_tree(g_repo, obj, &opts), -5555);
+
+	cl_assert(!git_path_exists("testrepo/new.txt"));
+
+	/* on case-insensitive FS = a/b.txt, branch_file.txt, new.txt */
+	/* on case-sensitive FS   = README, then above */
+
+	if (cl_repo_get_bool(g_repo, "core.ignorecase"))
+		cl_assert_equal_i(3, ca.count);
+	else
+		cl_assert_equal_i(4, ca.count);
+
+	/* and again with a different stopping point and return code */
+	ca.filename = "README";
+	ca.error = 123;
+	ca.count = 0;
+
+	cl_git_fail_with(git_checkout_tree(g_repo, obj, &opts), 123);
+
+	cl_assert(!git_path_exists("testrepo/new.txt"));
+
+	if (cl_repo_get_bool(g_repo, "core.ignorecase"))
+		cl_assert_equal_i(4, ca.count);
+	else
+		cl_assert_equal_i(1, ca.count);
+
+	git_object_free(obj);
+}
+
 void test_checkout_tree__can_checkout_with_last_workdir_item_missing(void)
 {
 	git_index *index = NULL;