[PATCH 0/2] remote: allow a list of remotes in remote.pushDefault

[PATCH 0/2] remote: allow a list of remotes in remote.pushDefault

[Harald Nordgren via GitGitGadget]

Let remote.pushDefault hold a space separated list of remote names and push
to the first one configured in the repository, so a single global setting
like fork origin works everywhere.

Harald Nordgren (2):
  remote: factor out lookup of a remote by name
  remote: allow a list of remotes in remote.pushDefault

 Documentation/config/remote.adoc |  7 +++++
 remote.c                         | 54 ++++++++++++++++++++++++--------
 t/t5516-fetch-push.sh            | 36 +++++++++++++++++++++
 3 files changed, 84 insertions(+), 13 deletions(-)


base-commit: 0f8e75abebff0877cae681a3d5ff31ac47f54220
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2423%2FHaraldNordgren%2Fremote-pushdefault-list-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2423/HaraldNordgren/remote-pushdefault-list-v1
Pull-Request: https://github.com/git/git/pull/2423
-- 
gitgitgadget

[PATCH 1/2] remote: factor out lookup of a remote by name

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

make_remote() looks up an existing remote in the hashmap before
creating a new one. Move that lookup into find_remote() so that code
which only wants to know whether a remote is configured can use it
without creating an entry.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 remote.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/remote.c b/remote.c
index fe62068463..6567ec91cc 100644
--- a/remote.c
+++ b/remote.c
@@ -128,23 +128,28 @@ static int remotes_hash_cmp(const void *cmp_data UNUSED,
 		return strcmp(a->name, b->name);
 }
 
+static struct remote *find_remote(struct remote_state *remote_state,
+				  const char *name, int len)
+{
+	struct remotes_hash_key lookup = { .str = name, .len = len };
+	struct hashmap_entry lookup_entry, *e;
+
+	hashmap_entry_init(&lookup_entry, memhash(name, len));
+	e = hashmap_get(&remote_state->remotes_hash, &lookup_entry, &lookup);
+	return e ? container_of(e, struct remote, ent) : NULL;
+}
+
 static struct remote *make_remote(struct remote_state *remote_state,
 				  const char *name, int len)
 {
 	struct remote *ret;
-	struct remotes_hash_key lookup;
-	struct hashmap_entry lookup_entry, *e;
 
 	if (!len)
 		len = strlen(name);
 
-	lookup.str = name;
-	lookup.len = len;
-	hashmap_entry_init(&lookup_entry, memhash(name, len));
-
-	e = hashmap_get(&remote_state->remotes_hash, &lookup_entry, &lookup);
-	if (e)
-		return container_of(e, struct remote, ent);
+	ret = find_remote(remote_state, name, len);
+	if (ret)
+		return ret;
 
 	CALLOC_ARRAY(ret, 1);
 	ret->prune = -1;  /* unspecified */
@@ -160,7 +165,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
 		   remote_state->remotes_alloc);
 	remote_state->remotes[remote_state->remotes_nr++] = ret;
 
-	hashmap_entry_init(&ret->ent, lookup_entry.hash);
+	hashmap_entry_init(&ret->ent, memhash(name, len));
 	if (hashmap_put_entry(&remote_state->remotes_hash, ret, ent))
 		BUG("hashmap_put overwrote entry after hashmap_get returned NULL");
 	return ret;
-- 
gitgitgadget

[PATCH 2/2] remote: allow a list of remotes in remote.pushDefault

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

Someone who pushes to a personal fork in some repositories and to
origin in others cannot set remote.pushDefault globally. Repositories
without the named remote fail to push, because git takes the missing
name for a URL.

Let remote.pushDefault hold a space separated list of remote names and
push to the first one that is configured in the repository. When none
of them is configured, fall back to branch.<name>.remote as if the
variable was not set. A single name keeps working as before, and a
value set in the repository still overrides one set globally.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/config/remote.adoc |  7 +++++++
 remote.c                         | 29 ++++++++++++++++++++++---
 t/t5516-fetch-push.sh            | 36 ++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 3a20d0f752..6a67c97fe8 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -2,6 +2,13 @@ remote.pushDefault::
 	The remote to push to by default.  Overrides
 	`branch.<name>.remote` for all branches, and is overridden by
 	`branch.<name>.pushRemote` for specific branches.
++
+The value may be a space separated list of remote names, in which case
+the first one that is configured in the repository is used. If none of
+them is configured, `branch.<name>.remote` is used as if
+`remote.pushDefault` was not set. This allows setting a list such as
+`fork origin` globally and have repositories without a `fork` remote
+push to `origin`.
 
 remote.<name>.url::
 	The URL of a remote repository.  See linkgit:git-fetch[1] or
diff --git a/remote.c b/remote.c
index 6567ec91cc..3cad0673b9 100644
--- a/remote.c
+++ b/remote.c
@@ -700,6 +700,25 @@ const char *remote_for_branch(struct branch *branch, int *explicit)
 					 explicit);
 }
 
+static const char *pushdefault_candidate(struct remote_state *remote_state)
+{
+	const char *p = remote_state->pushremote_name;
+
+	if (!strchr(p, ' '))
+		return p;
+
+	while (*p) {
+		size_t len = strcspn(p, " ");
+		struct remote *remote = find_remote(remote_state, p, len);
+
+		if (remote && valid_remote(remote))
+			return remote->name;
+		p += len;
+		p += strspn(p, " ");
+	}
+	return NULL;
+}
+
 static const char *
 remotes_pushremote_for_branch(struct remote_state *remote_state,
 			      struct branch *branch, int *explicit)
@@ -710,9 +729,13 @@ remotes_pushremote_for_branch(struct remote_state *remote_state,
 		return branch->pushremote_name;
 	}
 	if (remote_state->pushremote_name) {
-		if (explicit)
-			*explicit = 1;
-		return remote_state->pushremote_name;
+		const char *name = pushdefault_candidate(remote_state);
+
+		if (name) {
+			if (explicit)
+				*explicit = 1;
+			return name;
+		}
 	}
 	return remotes_remote_for_branch(remote_state, branch, explicit);
 }
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b982b209bf..7a0cdafd65 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -591,6 +591,42 @@ test_expect_success 'push with remote.pushdefault' '
 	check_push_result down_repo $the_commit heads/main
 '
 
+test_expect_success 'push with remote.pushdefault list picks first existing remote' '
+	mk_test up_repo heads/main &&
+	mk_test down_repo heads/main &&
+	test_config remote.up.url up_repo &&
+	test_config remote.down.url down_repo &&
+	test_config branch.main.remote up &&
+	test_config remote.pushdefault "missing down up" &&
+	test_config push.default matching &&
+	git push &&
+	check_push_result up_repo $the_first_commit heads/main &&
+	check_push_result down_repo $the_commit heads/main
+'
+
+test_expect_success 'push with remote.pushdefault list of missing remotes' '
+	mk_test up_repo heads/main &&
+	test_config remote.up.url up_repo &&
+	test_config branch.main.remote up &&
+	test_config remote.pushdefault "missing also-missing" &&
+	test_config push.default matching &&
+	git push &&
+	check_push_result up_repo $the_commit heads/main
+'
+
+test_expect_success 'repository remote.pushdefault overrides global list' '
+	mk_test up_repo heads/main &&
+	mk_test down_repo heads/main &&
+	test_config remote.up.url up_repo &&
+	test_config remote.down.url down_repo &&
+	test_config_global remote.pushdefault "down up" &&
+	test_config remote.pushdefault up &&
+	test_config push.default matching &&
+	git push &&
+	check_push_result up_repo $the_commit heads/main &&
+	check_push_result down_repo $the_first_commit heads/main
+'
+
 test_expect_success 'push with config remote.*.pushurl' '
 	mk_test testrepo heads/main &&
 	git checkout main &&
-- 
gitgitgadget