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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164
diff --git a/include/git2/checkout.h b/include/git2/checkout.h
index efafdc3..0e9d338 100644
--- a/include/git2/checkout.h
+++ b/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,
diff --git a/src/checkout.c b/src/checkout.c
index a292e3d..f7dd052 100644
--- a/src/checkout.c
+++ b/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] +
diff --git a/tests/checkout/tree.c b/tests/checkout/tree.c
index 66b01bc..ac48bfc 100644
--- a/tests/checkout/tree.c
+++ b/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;