From aad638f3a1e4d98296c2ec9c4ed08f217a652c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 7 Nov 2014 15:00:11 +0100 Subject: [PATCH] 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 | 73 ++++++++++----------------------- src/push.h | 6 +-- src/refspec.c | 6 +++ src/transports/local.c | 4 +- src/transports/smart_protocol.c | 12 +++--- tests/network/remote/local.c | 6 +-- tests/online/push.c | 17 ++++---- 7 files changed, 48 insertions(+), 76 deletions(-) diff --git a/src/push.c b/src/push.c index be5ec1c0e..88cd4cbaa 100644 --- a/src/push.c +++ b/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__free(spec->lref); - - if (spec->rref) - git__free(spec->rref); - + git_refspec__free(&spec->refspec); 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] == '+') { - s->force = true; - 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) + if (git_refspec__parse(&s->refspec, str, false) < 0) { + giterr_set(GITERR_INVALID, "invalid refspec %s", str); 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) { - giterr_set(GITERR_REFERENCE, "No such reference '%s'", spec->lref); + &spec->loid, push->repo, spec->refspec.src) < 0) { + 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. */ - git_vector_foreach(&push->remote->refs, j, head) { - if (!strcmp(spec->rref, head->name)) { - git_oid_cpy(&spec->roid, &head->oid); - break; - } + /* Remote ref may or may not (e.g. during create) already exist. */ + git_vector_foreach(&push->remote->refs, j, head) { + if (!strcmp(spec->refspec.dst, head->name)) { + git_oid_cpy(&spec->roid, &head->oid); + break; } } } diff --git a/src/push.h b/src/push.h index 6c8bf7229..68fa868dd 100644 --- a/src/push.h +++ b/src/push.h @@ -8,15 +8,13 @@ #define INCLUDE_push_h__ #include "git2.h" +#include "refspec.h" typedef struct push_spec { - char *lref; - char *rref; + struct git_refspec refspec; git_oid loid; git_oid roid; - - bool force; } push_spec; typedef struct push_status { diff --git a/src/refspec.c b/src/refspec.c index 9f0df35a7..a56c44cc0 100644 --- a/src/refspec.c +++ b/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); diff --git a/src/transports/local.c b/src/transports/local.c index f859f0b70..05302a13f 100644 --- a/src/transports/local.c +++ b/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) { diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c index 7c20382dc..e110da07e 100644 --- a/src/transports/smart_protocol.c +++ b/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; } diff --git a/tests/network/remote/local.c b/tests/network/remote/local.c index 7e575b7b9..8d6f90c95 100644 --- a/tests/network/remote/local.c +++ b/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)); diff --git a/tests/online/push.c b/tests/online/push.c index b09c7ad1f..1d4f9bc60 100644 --- a/tests/online/push.c +++ b/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", - * other servers may be less pithy. */ - const char *specs_right_expr[] = { "refs/heads/b2:refs/heads/b2~1" }; - push_status exp_stats_right_expr[] = { { "refs/heads/b2~1", 0 } }; + cl_git_pass(git_push_new(&push, _remote)); + cl_git_fail(git_push_add_refspec(push, "refs/heads/b2:refs/heads/b2~1")); + git_push_free(push); /* 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)