Merge pull request #2756 from libgit2/cmn/push-error-concerns Fold `git_push_unpack_ok()` into `git_push_finish()`
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
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a2c2191..cfb4f04 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -88,6 +88,9 @@ v0.21 + 1
* Rename git_remote_load() to git_remote_lookup() to bring it in line
with the rest of the lookup functions.
+* git_push_unpack_ok() has been removed and git_push_finish() now
+ returns an error if the unpacking failed.
+
* Introduce git_merge_bases() and the git_oidarray type to expose all
merge bases between two commits.
diff --git a/include/git2/push.h b/include/git2/push.h
index cbf1156..7bd1377 100644
--- a/include/git2/push.h
+++ b/include/git2/push.h
@@ -128,12 +128,14 @@ GIT_EXTERN(int) git_push_update_tips(
const char *reflog_message);
/**
- * Actually push all given refspecs
+ * Perform the push
*
- * Note: To check if the push was successful (i.e. all remote references
- * have been updated as requested), you need to call both
- * `git_push_unpack_ok` and `git_push_status_foreach`. The remote
- * repository might have refused to update some or all of the references.
+ * This function will return an error in case of a protocol error or
+ * the server being unable to unpack the data we sent.
+ *
+ * The return value does not reflect whether the server accepted or
+ * refused any reference updates. Use `git_push_status_foreach()` in
+ * order to find out which updates were accepted or rejected.
*
* @param push The push object
*
@@ -142,15 +144,6 @@ GIT_EXTERN(int) git_push_update_tips(
GIT_EXTERN(int) git_push_finish(git_push *push);
/**
- * Check if remote side successfully unpacked
- *
- * @param push The push object
- *
- * @return true if remote side successfully unpacked, false otherwise
- */
-GIT_EXTERN(int) git_push_unpack_ok(const git_push *push);
-
-/**
* Invoke callback `cb' on each status entry
*
* For each of the updated references, we receive a status report in the
diff --git a/src/push.c b/src/push.c
index 6671da4..fb51383 100644
--- a/src/push.c
+++ b/src/push.c
@@ -632,12 +632,12 @@ int git_push_finish(git_push *push)
(error = do_push(push)) < 0)
return error;
- return 0;
-}
+ if (!push->unpack_ok) {
+ error = -1;
+ giterr_set(GITERR_NET, "unpacking the sent packfile failed on the remote");
+ }
-int git_push_unpack_ok(const git_push *push)
-{
- return push->unpack_ok;
+ return error;
}
int git_push_status_foreach(git_push *push,
diff --git a/src/remote.c b/src/remote.c
index 531912c..dd9b178 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -2160,12 +2160,6 @@ int git_remote_push(git_remote *remote, git_strarray *refspecs, const git_push_o
if ((error = git_push_finish(push)) < 0)
goto cleanup;
- if (!git_push_unpack_ok(push)) {
- error = -1;
- giterr_set(GITERR_NET, "error in the remote while trying to unpack");
- goto cleanup;
- }
-
if (cbs->push_update_reference &&
(error = git_push_status_foreach(push, cbs->push_update_reference, cbs->payload)) < 0)
goto cleanup;
diff --git a/tests/network/remote/local.c b/tests/network/remote/local.c
index 170d67e..baeb25d 100644
--- a/tests/network/remote/local.c
+++ b/tests/network/remote/local.c
@@ -216,7 +216,6 @@ void test_network_remote_local__push_to_bare_remote(void)
cl_git_pass(git_push_new(&push, localremote));
cl_git_pass(git_push_add_refspec(push, "refs/heads/master"));
cl_git_pass(git_push_finish(push));
- cl_assert(git_push_unpack_ok(push));
/* Clean up */
git_push_free(push);
@@ -262,7 +261,6 @@ void test_network_remote_local__push_to_bare_remote_with_file_url(void)
cl_git_pass(git_push_new(&push, localremote));
cl_git_pass(git_push_add_refspec(push, "refs/heads/master"));
cl_git_pass(git_push_finish(push));
- cl_assert(git_push_unpack_ok(push));
/* Clean up */
git_push_free(push);
@@ -305,7 +303,6 @@ void test_network_remote_local__push_to_non_bare_remote(void)
cl_git_pass(git_push_new(&push, localremote));
cl_git_pass(git_push_add_refspec(push, "refs/heads/master"));
cl_git_fail_with(git_push_finish(push), GIT_EBAREREPO);
- cl_assert_equal_i(0, git_push_unpack_ok(push));
/* Clean up */
git_push_free(push);
@@ -452,7 +449,6 @@ void test_network_remote_local__update_tips_for_new_remote(void) {
cl_git_pass(git_push_new(&push, new_remote));
cl_git_pass(git_push_add_refspec(push, "refs/heads/master"));
cl_git_pass(git_push_finish(push));
- cl_assert(git_push_unpack_ok(push));
/* Update tips and make sure remote branch has been created */
cl_git_pass(git_push_update_tips(push, NULL, NULL));