push: use the common refspec parser

There is one well-known and well-tested parser which we should use,
instead of implementing parsing a second time.

The common parser is also augmented to copy the LHS into the RHS if the
latter is empty.

The expressions test had to change a bit, as we now catch a bad RHS of a
refspec locally.

src/push.c

@@ -19,7 +19,7 @@ static int push_spec_rref_cmp(const void *a, const void *b)
 {
 	const push_spec *push_spec_a = a, *push_spec_b = b;
 
-	return strcmp(push_spec_a->rref, push_spec_b->rref);
+	return strcmp(push_spec_a->refspec.dst, push_spec_b->refspec.dst);
 }
 
 static int push_status_ref_cmp(const void *a, const void *b)
@@ -94,12 +94,7 @@ static void free_refspec(push_spec *spec)
 	if (spec == NULL)
 		return;
 
-	if (spec->lref)
+	git_refspec__free(&spec->refspec);
-		git__free(spec->lref);
-
-	if (spec->rref)
-		git__free(spec->rref);
-
 	git__free(spec);
 }
 
@@ -134,47 +129,25 @@ static int check_lref(git_push *push, char *ref)
 static int parse_refspec(git_push *push, push_spec **spec, const char *str)
 {
 	push_spec *s;
-	char *delim;
 
 	*spec = NULL;
 
 	s = git__calloc(1, sizeof(*s));
 	GITERR_CHECK_ALLOC(s);
 
-	if (str[0] == '+') {
+	if (git_refspec__parse(&s->refspec, str, false) < 0) {
-		s->force = true;
+		giterr_set(GITERR_INVALID, "invalid refspec %s", str);
-		str++;
-	}
-
-	delim = strchr(str, ':');
-	if (delim == NULL) {
-		s->lref = git__strdup(str);
-		if (!s->lref || check_lref(push, s->lref) < 0)
-			goto on_error;
-	} else {
-		if (delim - str) {
-			s->lref = git__strndup(str, delim - str);
-			if (!s->lref || check_lref(push, s->lref) < 0)
-				goto on_error;
-		}
-
-		if (strlen(delim + 1)) {
-			s->rref = git__strdup(delim + 1);
-			if (!s->rref || check_rref(s->rref) < 0)
-				goto on_error;
-		}
-	}
-
-	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);
-		if (!s->rref || check_rref(s->rref) < 0)
-			goto on_error;
 	}
 
+	if (s->refspec.src && s->refspec.src[0] != '\0' &&
+	    check_lref(push, s->refspec.src) < 0) {
+		goto on_error;
+	}
+
+	if (check_rref(s->refspec.dst) < 0)
+		goto on_error;
+
 	*spec = s;
 	return 0;
 
@@ -220,7 +193,7 @@ int git_push_update_tips(
 
 		/* Find matching  push ref spec */
 		git_vector_foreach(&push->specs, j, push_spec) {
-			if (!strcmp(push_spec->rref, status->ref))
+			if (!strcmp(push_spec->refspec.dst, status->ref))
 				break;
 		}
 
@@ -353,7 +326,7 @@ static int revwalk(git_vector *commits, git_push *push)
 		} else if (git_revwalk_push(rw, &spec->loid) < 0)
 			goto on_error;
 
-		if (!spec->force) {
+		if (!spec->refspec.force) {
 			git_oid base;
 
 			if (git_oid_iszero(&spec->roid))
@@ -571,22 +544,20 @@ static int calculate_work(git_push *push)
 	/* Update local and remote oids*/
 
 	git_vector_foreach(&push->specs, i, spec) {
-		if (spec->lref) {
+		if (spec->refspec.src && spec->refspec.src[0]!= '\0') {
 			/* This is a create or update.  Local ref must exist. */
 			if (git_reference_name_to_id(
-					&spec->loid, push->repo, spec->lref) < 0) {
+					&spec->loid, push->repo, spec->refspec.src) < 0) {
-				giterr_set(GITERR_REFERENCE, "No such reference '%s'", spec->lref);
+				giterr_set(GITERR_REFERENCE, "No such reference '%s'", spec->refspec.src);
 				return -1;
 			}
 		}
 
-		if (spec->rref) {
+		/* Remote ref may or may not (e.g. during create) already exist. */
-			/* Remote ref may or may not (e.g. during create) already exist. */
+		git_vector_foreach(&push->remote->refs, j, head) {
-			git_vector_foreach(&push->remote->refs, j, head) {
+			if (!strcmp(spec->refspec.dst, head->name)) {
-				if (!strcmp(spec->rref, head->name)) {
+				git_oid_cpy(&spec->roid, &head->oid);
-					git_oid_cpy(&spec->roid, &head->oid);
+				break;
-					break;
-				}
 			}
 		}
 	}

src/push.h

@@ -8,15 +8,13 @@
 #define INCLUDE_push_h__
 
 #include "git2.h"
+#include "refspec.h"
 
 typedef struct push_spec {
-	char *lref;
+	struct git_refspec refspec;
-	char *rref;
 
 	git_oid loid;
 	git_oid roid;
-
-	bool force;
 } push_spec;
 
 typedef struct push_status {

src/refspec.c

@@ -119,6 +119,12 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch)
 			if (!git_reference__is_valid_name(refspec->dst, flags))
 				goto invalid;
 		}
+
+		/* if the RHS is empty, then it's a copy of the LHS */
+		if (!refspec->dst) {
+			refspec->dst = git__strdup(refspec->src);
+			GITERR_CHECK_ALLOC(refspec->dst);
+		}
 	}
 
 	refspec->string = git__strdup(input);

src/transports/local.c

@@ -405,7 +405,7 @@ static int local_push(
 	git_vector_foreach(&push->specs, j, spec) {
 		push_status *status;
 		const git_error *last;
-		char *ref = spec->rref ? spec->rref : spec->lref;
+		char *ref = spec->refspec.dst;
 
 		status = git__calloc(sizeof(push_status), 1);
 		if (!status)
@@ -417,7 +417,7 @@ static int local_push(
 			goto on_error;
 		}
 
-		error = local_push_update_remote_ref(remote_repo, spec->lref, spec->rref,
+		error = local_push_update_remote_ref(remote_repo, spec->refspec.src, spec->refspec.dst,
 			&spec->loid, &spec->roid);
 
 		switch (error) {

src/transports/smart_protocol.c

@@ -645,7 +645,7 @@ static int gen_pktline(git_buf *buf, git_push *push)
 	old_id[GIT_OID_HEXSZ] = '\0'; new_id[GIT_OID_HEXSZ] = '\0';
 
 	git_vector_foreach(&push->specs, i, spec) {
-		len = 2*GIT_OID_HEXSZ + 7 + strlen(spec->rref);
+		len = 2*GIT_OID_HEXSZ + 7 + strlen(spec->refspec.dst);
 
 		if (i == 0) {
 			++len; /* '\0' */
@@ -657,7 +657,7 @@ static int gen_pktline(git_buf *buf, git_push *push)
 		git_oid_fmt(old_id, &spec->roid);
 		git_oid_fmt(new_id, &spec->loid);
 
-		git_buf_printf(buf, "%04"PRIxZ"%s %s %s", len, old_id, new_id, spec->rref);
+		git_buf_printf(buf, "%04"PRIxZ"%s %s %s", len, old_id, new_id, spec->refspec.dst);
 
 		if (i == 0) {
 			git_buf_putc(buf, '\0');
@@ -816,7 +816,7 @@ static int add_ref_from_push_spec(git_vector *refs, push_spec *push_spec)
 
 	added->type = GIT_PKT_REF;
 	git_oid_cpy(&added->head.oid, &push_spec->loid);
-	added->head.name = git__strdup(push_spec->rref);
+	added->head.name = git__strdup(push_spec->refspec.dst);
 
 	if (!added->head.name ||
 		git_vector_insert(refs, added) < 0) {
@@ -855,7 +855,7 @@ static int update_refs_from_report(
 
 		/* For each push spec we sent to the server, we should have
 		 * gotten back a status packet in the push report which matches */
-		if (strcmp(push_spec->rref, push_status->ref)) {
+		if (strcmp(push_spec->refspec.dst, push_status->ref)) {
 			giterr_set(GITERR_NET, "report-status: protocol error");
 			return -1;
 		}
@@ -872,7 +872,7 @@ static int update_refs_from_report(
 		push_status = git_vector_get(push_report, i);
 		ref = git_vector_get(refs, j);
 
-		cmp = strcmp(push_spec->rref, ref->head.name);
+		cmp = strcmp(push_spec->refspec.dst, ref->head.name);
 
 		/* Iterate appropriately */
 		if (cmp <= 0) i++;
@@ -985,7 +985,7 @@ int git_smart__push(git_transport *transport, git_push *push)
 	 * cases except when we only send delete commands
 	 */
 	git_vector_foreach(&push->specs, i, spec) {
-		if (spec->lref) {
+		if (spec->refspec.src && spec->refspec.src[0] != '\0') {
 			need_pack = 1;
 			break;
 		}

tests/network/remote/local.c

@@ -212,7 +212,7 @@ void test_network_remote_local__push_to_bare_remote(void)
 
 	/* Try to push */
 	cl_git_pass(git_push_new(&push, localremote));
-	cl_git_pass(git_push_add_refspec(push, "refs/heads/master:"));
+	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));
 
@@ -258,7 +258,7 @@ void test_network_remote_local__push_to_bare_remote_with_file_url(void)
 
 	/* Try to push */
 	cl_git_pass(git_push_new(&push, localremote));
-	cl_git_pass(git_push_add_refspec(push, "refs/heads/master:"));
+	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));
 
@@ -301,7 +301,7 @@ void test_network_remote_local__push_to_non_bare_remote(void)
 
 	/* Try to push */
 	cl_git_pass(git_push_new(&push, localremote));
-	cl_git_pass(git_push_add_refspec(push, "refs/heads/master:"));
+	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));
 

tests/online/push.c

@@ -631,11 +631,11 @@ void test_online_push__multi(void)
 
 void test_online_push__implicit_tgt(void)
 {
-	const char *specs1[] = { "refs/heads/b1:" };
+	const char *specs1[] = { "refs/heads/b1" };
 	push_status exp_stats1[] = { { "refs/heads/b1", 1 } };
 	expected_ref exp_refs1[] = { { "refs/heads/b1", &_oid_b1 } };
 
-	const char *specs2[] = { "refs/heads/b2:" };
+	const char *specs2[] = { "refs/heads/b2" };
 	push_status exp_stats2[] = { { "refs/heads/b2", 1 } };
 	expected_ref exp_refs2[] = {
 	{ "refs/heads/b1", &_oid_b1 },
@@ -838,22 +838,19 @@ void test_online_push__bad_refspecs(void)
 
 void test_online_push__expressions(void)
 {
+	git_push *push;
+
 	/* TODO: Expressions in refspecs doesn't actually work yet */
 	const char *specs_left_expr[] = { "refs/heads/b2~1:refs/heads/b2" };
 
-	/* expect not NULL to indicate failure (core git replies "funny refname",
+	cl_git_pass(git_push_new(&push, _remote));
-	 * other servers may be less pithy. */
+	cl_git_fail(git_push_add_refspec(push, "refs/heads/b2:refs/heads/b2~1"));
-	const char *specs_right_expr[] = { "refs/heads/b2:refs/heads/b2~1" };
+	git_push_free(push);
-	push_status exp_stats_right_expr[] = { { "refs/heads/b2~1", 0 } };
 
 	/* TODO: Find a more precise way of checking errors than a exit code of -1. */
 	do_push(specs_left_expr, ARRAY_SIZE(specs_left_expr),
 		NULL, 0,
 		NULL, 0, -1, 0, 0);
-
-	do_push(specs_right_expr, ARRAY_SIZE(specs_right_expr),
-		exp_stats_right_expr, ARRAY_SIZE(exp_stats_right_expr),
-		NULL, 0, 0, 1, 1);
 }
 
 void test_online_push__notes(void)