git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 1/1] remote.c: fix handling of %(push:remoteref)

From
DRDamien Robert <damien.olivier.robert@gmail.com>
Date
Mar 12, 2020, 16:45 UTC
Message-ID
<20200312164558.2388589-1-damien.olivier.robert+git@gmail.com>
In-Reply-To
<20200303161223.1870298-3-damien.olivier.robert+git@gmail.com>

Looking at the value of %(push:remoteref) only handles the case when an explicit push refspec is passed. But it does not handle the fallback cases of looking at the configuration value of `push.default`.

In particular, doing something like
    git config push.default current
    git for-each-ref --format='%(push)'
    git for-each-ref --format='%(push:remoteref)'

prints a useful tracking ref for the first for-each-ref, but an empty string for the second.

Since the intention of %(push:remoteref), from 9700fae5ee (for-each-ref: let upstream/push report the remote ref name) is to get exactly which branch `git push` will push to, even in the fallback cases, fix this.

To get the meaning of %(push:remoteref), `ref-filter.c` calls `remote_ref_for_branch`. We simply add a new static helper function, `branch_get_push_remoteref` that follows the logic of `branch_get_push_1`, and call it from `remote_ref_for_branch`.

We also update t/6300-for-each-ref.sh to handle all `push.default` strategies. This involves testing `push.default=simple` twice, once where there is a matching upstream branch and once when there is none.

Signed-off-by: Damien Robert <damien.olivier.robert+git@gmail.com>
---
I ended up following most of Junio's suggestion, except having a
    default: BUG(...)
and returning NULL at the end of the case.

I prefer to return explicitly in each case statement rather than use break to fallback at the end of the case.

I said I would also update branch_get_push1 to be as similar as possible to branch_get_push_remoteref, but because of the error handling of the latter, it would makes the syntax a bit weird, so I did not touch it.

I am still a bit annoyed that I cannot call branch_get_push_remoteref from branch_get_push1 because of the PUSH_DEFAULT_UPSTREAM case, but this can wait and we will need to work with the code duplication meanwhile.

 remote.c                | 94 +++++++++++++++++++++++++++++++----------
 t/t6300-for-each-ref.sh | 29 ++++++++++++-
 2 files changed, 100 insertions(+), 23 deletions(-)
diff --git a/remote.c b/remote.c
index c43196ec06..352ea240cd 100644
--- a/remote.c
+++ b/remote.c
@@ -516,28 +516,6 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit)
 	return remote_for_branch(branch, explicit);
 }
 
-const char *remote_ref_for_branch(struct branch *branch, int for_push)
-{
-	if (branch) {
-		if (!for_push) {
-			if (branch->merge_nr) {
-				return branch->merge_name[0];
-			}
-		} else {
-			const char *dst, *remote_name =
-				pushremote_for_branch(branch, NULL);
-			struct remote *remote = remote_get(remote_name);
-
-			if (remote && remote->push.nr &&
-			    (dst = apply_refspecs(&remote->push,
-						  branch->refname))) {
-				return dst;
-			}
-		}
-	}
-	return NULL;
-}
-
 static struct remote *remote_get_1(const char *name,
 				   const char *(*get_default)(struct branch *, int *))
 {
@@ -1656,6 +1634,64 @@ static const char *tracking_for_push_dest(struct remote *remote,
 	return ret;
 }
 
+/**
+ * Return the local name of the remote tracking branch, as in
+ * %(push:remoteref), that corresponds to the ref we would push to given a
+ * bare `git push` while `branch` is checked out.
+ * See also branch_get_push_1 below.
+ */
+static const char *branch_get_push_remoteref(struct branch *branch)
+{
+	struct remote *remote;
+
+	remote = remote_get(pushremote_for_branch(branch, NULL));
+	if (!remote)
+		return NULL;
+
+	if (remote->push.nr) {
+		return apply_refspecs(&remote->push, branch->refname);
+	}
+
+	if (remote->mirror)
+		return branch->refname;
+
+	switch (push_default) {
+	case PUSH_DEFAULT_NOTHING:
+		return NULL;
+
+	case PUSH_DEFAULT_MATCHING:
+	case PUSH_DEFAULT_CURRENT:
+		return branch->refname;
+
+	case PUSH_DEFAULT_UPSTREAM:
+		if (branch && branch->merge && branch->merge[0] &&
+		    branch->merge[0]->dst)
+			return branch->merge[0]->src;
+		else
+			return NULL;
+
+	case PUSH_DEFAULT_UNSPECIFIED:
+	case PUSH_DEFAULT_SIMPLE:
+		{
+			const char *up, *cur;
+
+			up = branch_get_upstream(branch, NULL);
+			cur = tracking_for_push_dest(remote, branch->refname, NULL);
+			if (up && cur && !strcmp(cur, up))
+				return branch->refname;
+			else
+				return NULL;
+
+		}
+	}
+	BUG("unhandled push situation");
+}
+
+/**
+ * Return the tracking branch, as in %(push), that corresponds to the ref we
+ * would push to given a bare `git push` while `branch` is checked out.
+ * See also branch_get_push_remoteref above.
+ */
 static const char *branch_get_push_1(struct branch *branch, struct strbuf *err)
 {
 	struct remote *remote;
@@ -1735,6 +1771,20 @@ static int ignore_symref_update(const char *refname)
 	return (flag & REF_ISSYMREF);
 }
 
+const char *remote_ref_for_branch(struct branch *branch, int for_push)
+{
+	if (branch) {
+		if (!for_push) {
+			if (branch->merge_nr) {
+				return branch->merge_name[0];
+			}
+		} else {
+			return branch_get_push_remoteref(branch);
+		}
+	}
+	return NULL;
+}
+
 /*
  * Create and return a list of (struct ref) consisting of copies of
  * each remote_ref that matches refspec.  refspec must be a pattern.
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 9c910ce746..60e21834fd 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -874,7 +874,34 @@ test_expect_success ':remotename and :remoteref' '
 		actual="$(git for-each-ref \
 			--format="%(push:remotename),%(push:remoteref)" \
 			refs/heads/push-simple)" &&
-		test from, = "$actual"
+		test from, = "$actual" &&
+		git config branch.push-simple.remote from &&
+		git config branch.push-simple.merge refs/heads/master &&
+		actual="$(git for-each-ref \
+			--format="%(push:remotename),%(push:remoteref)" \
+			refs/heads/push-simple)" &&
+		test from, = "$actual" &&
+		actual="$(git -c push.default=upstream for-each-ref \
+			--format="%(push:remotename),%(push:remoteref)" \
+			refs/heads/push-simple)" &&
+		test from,refs/heads/master = "$actual" &&
+		actual="$(git -c push.default=current for-each-ref \
+			--format="%(push:remotename),%(push:remoteref)" \
+			refs/heads/push-simple)" &&
+		test from,refs/heads/push-simple = "$actual" &&
+		actual="$(git -c push.default=matching for-each-ref \
+			--format="%(push:remotename),%(push:remoteref)" \
+			refs/heads/push-simple)" &&
+		test from,refs/heads/push-simple = "$actual" &&
+		actual="$(git -c push.default=nothing for-each-ref \
+			--format="%(push:remotename),%(push:remoteref)" \
+			refs/heads/push-simple)" &&
+		test from, = "$actual" &&
+		git config branch.push-simple.merge refs/heads/push-simple &&
+		actual="$(git for-each-ref \
+			--format="%(push:remotename),%(push:remoteref)" \
+			refs/heads/push-simple)" &&
+		test from,refs/heads/push-simple = "$actual"
 	)
 '
 

-- 
Patched on top of v2.26.0-rc1-6-ga56d361f66 (git version 2.25.1)
Previous: Junio C HamanoNext: Damien Robert
Message 16 of 32 in “remote.c: fix handling of push:remote_ref”
  1. 1/1 remote.c: fix handling of push:remote_refDamien Robert, Feb 28, 2020
  2. Jeff KingFeb 28, 2020
  3. Damien RobertMar 1, 2020
  4. Jeff KingMar 2, 2020
  5. 0/2 Damien Robert, Mar 3, 2020
  6. 1/2 remote: drop "explicit" parameter from remote_ref_for_branch()Damien Robert, Mar 3, 2020
  7. Junio C HamanoMar 3, 2020
  8. Jeff KingMar 3, 2020
  9. Junio C HamanoMar 3, 2020
  10. 2/2 remote.c: fix handling of %(push:remoteref)Damien Robert, Mar 3, 2020
  11. Damien RobertMar 3, 2020
  12. Junio C HamanoMar 3, 2020
  13. Junio C HamanoMar 3, 2020
  14. Damien RobertMar 3, 2020
  15. Junio C HamanoMar 3, 2020
  16. 1/1 remote.c: fix handling of %(push:remoteref)Damien Robert, Mar 12, 2020
  17. Damien RobertMar 25, 2020
  18. Junio C HamanoMar 27, 2020
  19. Damien RobertMar 28, 2020
  20. Jeff KingMar 28, 2020
  21. Jeff KingMar 28, 2020
  22. Damien RobertApr 16, 2020
  23. Damien RobertApr 6, 2020
  24. Jeff KingApr 6, 2020
  25. 1/1 remote.c: fix handling of %(push:remoteref)Damien Robert, Apr 16, 2020
  26. Damien RobertApr 16, 2020
  27. Junio C HamanoSep 3, 2020
  28. Damien RobertSep 11, 2020
  29. Junio C HamanoSep 14, 2020
  30. Damien RobertMar 3, 2020
  31. Jeff KingMar 2, 2020
  32. Damien RobertMar 3, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.