Merge branch 'jk/detect-push-typo-early'

Catch "git push $there no-such-branch" early.

* jk/detect-push-typo-early:
  push: detect local refspec errors early
  match_explicit_lhs: allow a "verify only" mode
  match_explicit: hoist refspec lhs checks into their own function

Author: gitster

remote.c

@@ -1036,11 +1036,13 @@ int count_refspec_match(const char *pattern,
 		}
 	}
 	if (!matched) {
-		*matched_ref = matched_weak;
+		if (matched_ref)
+			*matched_ref = matched_weak;
 		return weak_match;
 	}
 	else {
-		*matched_ref = matched;
+		if (matched_ref)
+			*matched_ref = matched;
 		return match;
 	}
 }
@@ -1060,18 +1062,25 @@ static struct ref *alloc_delete_ref(void)
 	return ref;
 }
 
-static struct ref *try_explicit_object_name(const char *name)
+static int try_explicit_object_name(const char *name,
+				    struct ref **match)
 {
 	unsigned char sha1[20];
-	struct ref *ref;
 
-	if (!*name)
-		return alloc_delete_ref();
+	if (!*name) {
+		if (match)
+			*match = alloc_delete_ref();
+		return 0;
+	}
+
 	if (get_sha1(name, sha1))
-		return NULL;
-	ref = alloc_ref(name);
-	hashcpy(ref->new_sha1, sha1);
-	return ref;
+		return -1;
+
+	if (match) {
+		*match = alloc_ref(name);
+		hashcpy((*match)->new_sha1, sha1);
+	}
+	return 0;
 }
 
 static struct ref *make_linked_ref(const char *name, struct ref ***tail)
@@ -1101,12 +1110,37 @@ static char *guess_ref(const char *name, struct ref *peer)
 	return strbuf_detach(&buf, NULL);
 }
 
+static int match_explicit_lhs(struct ref *src,
+			      struct refspec *rs,
+			      struct ref **match,
+			      int *allocated_match)
+{
+	switch (count_refspec_match(rs->src, src, match)) {
+	case 1:
+		if (allocated_match)
+			*allocated_match = 0;
+		return 0;
+	case 0:
+		/* The source could be in the get_sha1() format
+		 * not a reference name.  :refs/other is a
+		 * way to delete 'other' ref at the remote end.
+		 */
+		if (try_explicit_object_name(rs->src, match) < 0)
+			return error("src refspec %s does not match any.", rs->src);
+		if (allocated_match)
+			*allocated_match = 1;
+		return 0;
+	default:
+		return error("src refspec %s matches more than one.", rs->src);
+	}
+}
+
 static int match_explicit(struct ref *src, struct ref *dst,
 			  struct ref ***dst_tail,
 			  struct refspec *rs)
 {
 	struct ref *matched_src, *matched_dst;
-	int copy_src;
+	int allocated_src;
 
 	const char *dst_value = rs->dst;
 	char *dst_guess;
@@ -1115,23 +1149,8 @@ static int match_explicit(struct ref *src, struct ref *dst,
 		return 0;
 
 	matched_src = matched_dst = NULL;
-	switch (count_refspec_match(rs->src, src, &matched_src)) {
-	case 1:
-		copy_src = 1;
-		break;
-	case 0:
-		/* The source could be in the get_sha1() format
-		 * not a reference name.  :refs/other is a
-		 * way to delete 'other' ref at the remote end.
-		 */
-		matched_src = try_explicit_object_name(rs->src);
-		if (!matched_src)
-			return error("src refspec %s does not match any.", rs->src);
-		copy_src = 0;
-		break;
-	default:
-		return error("src refspec %s matches more than one.", rs->src);
-	}
+	if (match_explicit_lhs(src, rs, &matched_src, &allocated_src) < 0)
+		return -1;
 
 	if (!dst_value) {
 		unsigned char sha1[20];
@@ -1176,7 +1195,9 @@ static int match_explicit(struct ref *src, struct ref *dst,
 		return error("dst ref %s receives from more than one src.",
 		      matched_dst->name);
 	else {
-		matched_dst->peer_ref = copy_src ? copy_ref(matched_src) : matched_src;
+		matched_dst->peer_ref = allocated_src ?
+					matched_src :
+					copy_ref(matched_src);
 		matched_dst->force = rs->force;
 	}
 	return 0;
@@ -1357,6 +1378,31 @@ static void prepare_ref_index(struct string_list *ref_index, struct ref *ref)
 	sort_string_list(ref_index);
 }
 
+/*
+ * Given only the set of local refs, sanity-check the set of push
+ * refspecs. We can't catch all errors that match_push_refs would,
+ * but we can catch some errors early before even talking to the
+ * remote side.
+ */
+int check_push_refs(struct ref *src, int nr_refspec, const char **refspec_names)
+{
+	struct refspec *refspec = parse_push_refspec(nr_refspec, refspec_names);
+	int ret = 0;
+	int i;
+
+	for (i = 0; i < nr_refspec; i++) {
+		struct refspec *rs = refspec + i;
+
+		if (rs->pattern || rs->matching)
+			continue;
+
+		ret |= match_explicit_lhs(src, rs, NULL, NULL);
+	}
+
+	free_refspec(nr_refspec, refspec);
+	return ret;
+}
+
 /*
  * Given the set of refs the local repository has, the set of refs the
  * remote repository has, and the refspec used for push, determine

remote.h

@@ -166,6 +166,7 @@ extern int query_refspecs(struct refspec *specs, int nr, struct refspec *query);
 char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
 		     const char *name);
 
+int check_push_refs(struct ref *src, int nr_refspec, const char **refspec);
 int match_push_refs(struct ref *src, struct ref **dst,
 		    int nr_refspec, const char **refspec, int all);
 void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,

t/t5529-push-errors.sh

@@ -0,0 +1,48 @@
+#!/bin/sh
+
+test_description='detect some push errors early (before contacting remote)'
+. ./test-lib.sh
+
+test_expect_success 'setup commits' '
+	test_commit one
+'
+
+test_expect_success 'setup remote' '
+	git init --bare remote.git &&
+	git remote add origin remote.git
+'
+
+test_expect_success 'setup fake receive-pack' '
+	FAKE_RP_ROOT=$(pwd) &&
+	export FAKE_RP_ROOT &&
+	write_script fake-rp <<-\EOF &&
+	echo yes >"$FAKE_RP_ROOT"/rp-ran
+	exit 1
+	EOF
+	git config remote.origin.receivepack "\"\$FAKE_RP_ROOT/fake-rp\""
+'
+
+test_expect_success 'detect missing branches early' '
+	echo no >rp-ran &&
+	echo no >expect &&
+	test_must_fail git push origin missing &&
+	test_cmp expect rp-ran
+'
+
+test_expect_success 'detect missing sha1 expressions early' '
+	echo no >rp-ran &&
+	echo no >expect &&
+	test_must_fail git push origin master~2:master &&
+	test_cmp expect rp-ran
+'
+
+test_expect_success 'detect ambiguous refs early' '
+	git branch foo &&
+	git tag foo &&
+	echo no >rp-ran &&
+	echo no >expect &&
+	test_must_fail git push origin foo &&
+	test_cmp expect rp-ran
+'
+
+test_done

transport.c

@@ -1132,8 +1132,7 @@ int transport_push(struct transport *transport,
 
 		return transport->push(transport, refspec_nr, refspec, flags);
 	} else if (transport->push_refs) {
-		struct ref *remote_refs =
-			transport->get_refs_list(transport, 1);
+		struct ref *remote_refs;
 		struct ref *local_refs = get_local_heads();
 		int match_flags = MATCH_REFS_NONE;
 		int verbose = (transport->verbose > 0);
@@ -1142,6 +1141,11 @@ int transport_push(struct transport *transport,
 		int pretend = flags & TRANSPORT_PUSH_DRY_RUN;
 		int push_ret, ret, err;
 
+		if (check_push_refs(local_refs, refspec_nr, refspec) < 0)
+			return -1;
+
+		remote_refs = transport->get_refs_list(transport, 1);
+
 		if (flags & TRANSPORT_PUSH_ALL)
 			match_flags |= MATCH_REFS_ALL;
 		if (flags & TRANSPORT_PUSH_MIRROR)