Fix bug in gen_pktline() for deletes of missing remote refs * gen_pktline() in smart_protocol.c was skipping refspecs that deleted refs that were not advertised by the server. The new behavior is to send a delete command with an old-id of zero, which matches the behavior of the official git client. * Update test_network_push__delete() in reaction to above fix. * Obviate messy logic that handles missing push_spec rrefs by canonicalizing push_spec. After calculate_work(), loid, roid, and rref, are filled in with exactly what is sent to the server
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 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212
diff --git a/src/push.c b/src/push.c
index 1d63d57..efca743 100644
--- a/src/push.c
+++ b/src/push.c
@@ -101,24 +101,27 @@ static int parse_refspec(push_spec **spec, const char *str)
if (delim == NULL) {
s->lref = git__strdup(str);
check(s->lref);
- s->rref = NULL;
} else {
if (delim - str) {
s->lref = git__strndup(str, delim - str);
check(s->lref);
- } else
- s->lref = NULL;
+ }
if (strlen(delim + 1)) {
s->rref = git__strdup(delim + 1);
check(s->rref);
- } else
- s->rref = NULL;
+ }
}
if (!s->lref && !s->rref)
goto on_error;
+ /* If rref is ommitted, use the same ref name as lref */
+ if (!s->rref) {
+ s->rref = git__strdup(s->lref);
+ check(s->rref);
+ }
+
#undef check
*spec = s;
@@ -282,44 +285,24 @@ static int calculate_work(git_push *push)
push_spec *spec;
unsigned int i, j;
+ /* Update local and remote oids*/
+
git_vector_foreach(&push->specs, i, spec) {
if (spec->lref) {
+ /* This is a create or update. Local ref must exist. */
if (git_reference_name_to_id(
&spec->loid, push->repo, spec->lref) < 0) {
giterr_set(GIT_ENOTFOUND, "No such reference '%s'", spec->lref);
return -1;
}
+ }
- if (!spec->rref) {
- /*
- * No remote reference given; if we find a remote
- * reference with the same name we will update it,
- * otherwise a new reference will be created.
- */
- git_vector_foreach(&push->remote->refs, j, head) {
- if (!strcmp(spec->lref, head->name)) {
- /*
- * Update remote reference
- */
- git_oid_cpy(&spec->roid, &head->oid);
-
- break;
- }
- }
- } else {
- /*
- * Remote reference given; update the given
- * reference or create it.
- */
- git_vector_foreach(&push->remote->refs, j, head) {
- if (!strcmp(spec->rref, head->name)) {
- /*
- * Update remote reference
- */
- git_oid_cpy(&spec->roid, &head->oid);
-
- break;
- }
+ if (spec->rref) {
+ /* Remote ref may or may not (e.g. during create) already exist. */
+ git_vector_foreach(&push->remote->refs, j, head) {
+ if (!strcmp(spec->rref, head->name)) {
+ git_oid_cpy(&spec->roid, &head->oid);
+ break;
}
}
}
diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c
index a733159..34f0f84 100644
--- a/src/transports/smart_protocol.c
+++ b/src/transports/smart_protocol.c
@@ -499,61 +499,25 @@ on_error:
static int gen_pktline(git_buf *buf, git_push *push)
{
- git_remote_head *head;
push_spec *spec;
- unsigned int i, j, len;
- char hex[41]; hex[40] = '\0';
+ size_t i, len;
+ char old_id[41], new_id[41];
+
+ old_id[40] = '\0'; new_id[40] = '\0';
git_vector_foreach(&push->specs, i, spec) {
- len = 2*GIT_OID_HEXSZ + 7;
+ len = 2*GIT_OID_HEXSZ + 7 + strlen(spec->rref);
if (i == 0) {
- len +=1; /* '\0' */
+ ++len; /* '\0' */
if (push->report_status)
len += strlen(GIT_CAP_REPORT_STATUS);
}
- if (spec->lref) {
- len += spec->rref ? strlen(spec->rref) : strlen(spec->lref);
+ git_oid_fmt(old_id, &spec->roid);
+ git_oid_fmt(new_id, &spec->loid);
- if (git_oid_iszero(&spec->roid)) {
-
- /*
- * Create remote reference
- */
- git_oid_fmt(hex, &spec->loid);
- git_buf_printf(buf, "%04x%s %s %s", len,
- GIT_OID_HEX_ZERO, hex,
- spec->rref ? spec->rref : spec->lref);
-
- } else {
-
- /*
- * Update remote reference
- */
- git_oid_fmt(hex, &spec->roid);
- git_buf_printf(buf, "%04x%s ", len, hex);
-
- git_oid_fmt(hex, &spec->loid);
- git_buf_printf(buf, "%s %s", hex,
- spec->rref ? spec->rref : spec->lref);
- }
- } else {
- /*
- * Delete remote reference
- */
- git_vector_foreach(&push->remote->refs, j, head) {
- if (!strcmp(spec->rref, head->name)) {
- len += strlen(spec->rref);
-
- git_oid_fmt(hex, &head->oid);
- git_buf_printf(buf, "%04x%s %s %s", len,
- hex, GIT_OID_HEX_ZERO, head->name);
-
- break;
- }
- }
- }
+ git_buf_printf(buf, "%04x%s %s %s", len, old_id, new_id, spec->rref);
if (i == 0) {
git_buf_putc(buf, '\0');
@@ -563,6 +527,7 @@ static int gen_pktline(git_buf *buf, git_push *push)
git_buf_putc(buf, '\n');
}
+
git_buf_puts(buf, "0000");
return git_buf_oom(buf) ? -1 : 0;
}
diff --git a/tests-clar/network/push.c b/tests-clar/network/push.c
index 5f0a80c..ac9a8f5 100644
--- a/tests-clar/network/push.c
+++ b/tests-clar/network/push.c
@@ -453,6 +453,7 @@ void test_network_push__delete(void)
const char *specs_del_fake[] = { ":refs/heads/fake" };
/* Force has no effect for delete. */
const char *specs_del_fake_force[] = { "+:refs/heads/fake" };
+ push_status exp_stats_fake[] = { { "refs/heads/fake", NULL } };
const char *specs_delete[] = { ":refs/heads/tgt1" };
push_status exp_stats_delete[] = { { "refs/heads/tgt1", NULL } };
@@ -464,15 +465,18 @@ void test_network_push__delete(void)
exp_stats1, ARRAY_SIZE(exp_stats1),
exp_refs1, ARRAY_SIZE(exp_refs1), 0);
- /* Deleting a non-existent branch should fail before the request is sent to
- * the server because the client cannot find the old oid for the ref.
+ /* When deleting a non-existent branch, the git client sends zero for both
+ * the old and new commit id. This should succeed on the server with the
+ * same status report as if the branch were actually deleted. The server
+ * returns a warning on the side-band iff the side-band is supported.
+ * Since libgit2 doesn't support the side-band yet, there are no warnings.
*/
do_push(specs_del_fake, ARRAY_SIZE(specs_del_fake),
- NULL, 0,
- exp_refs1, ARRAY_SIZE(exp_refs1), -1);
+ exp_stats_fake, 1,
+ exp_refs1, ARRAY_SIZE(exp_refs1), 0);
do_push(specs_del_fake_force, ARRAY_SIZE(specs_del_fake_force),
- NULL, 0,
- exp_refs1, ARRAY_SIZE(exp_refs1), -1);
+ exp_stats_fake, 1,
+ exp_refs1, ARRAY_SIZE(exp_refs1), 0);
/* Delete one of the pushed branches. */
do_push(specs_delete, ARRAY_SIZE(specs_delete),