Re: push.default: current vs upstream

[Jeff King <peff@peff.net>]

On Fri, Mar 30, 2012 at 04:07:12PM -0700, Junio C Hamano wrote:

> There were two issues I raised in that message.  It turns out that we
> already have the code for the first one.  The second one should look
> something like this:
> 
> -- >8 --
> Subject: push: detect nonsense "upstream" check more carefully

Thanks, I think this is an improvement. I absolutely think the existing
behavior was a bug, but I do wonder if any users were depending on this
"feature".

Squashable tests are below.

> +	if (strcmp(branch->remote_name, remote->name)) {
> +		struct remote *branch_dest = remote_get(branch->remote_name);
> +		const char **branch_dest_url, **dest_url;
> +
> +		if (!push_url_of_remote(remote, &dest_url) ||
> +		    !push_url_of_remote(branch_dest, &branch_dest_url) ||
> +		    strcmp(dest_url[0], branch_dest_url[0]))
> +			die(_("You are pushing to remote '%s', which is not the "
> +			      "upstream of your\ncurrent branch '%s'.\n"),
> +			    remote->name, branch->name);
> +	}

Hmm. So this will actually detect "git push $URL" when $URL matches the
remote's configured URL. I feel like this distinction has come up
before, and we decided not to equate the two. But now I can't remember
where (maybe it when fetching via URL versus via remote?).

What should happen if there are multiple push URLs configured? Your code
will match iff it is the first one. I would think it should either
require all to match, or it should proceed if any of the URLs match.
I think the latter makes more sense, though personally I would simply
have compared the remote names.

-Peff

---
Here are the tests.

diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
new file mode 100755
index 0000000..c334c51
--- /dev/null
+++ b/t/t5528-push-default.sh
@@ -0,0 +1,54 @@
+#!/bin/sh
+
+test_description='check various push.default settings'
+. ./test-lib.sh
+
+test_expect_success 'setup bare remotes' '
+	git init --bare repo1 &&
+	git remote add parent1 repo1 &&
+	git init --bare repo2 &&
+	git remote add parent2 repo2 &&
+	test_commit one &&
+	git push parent1 HEAD &&
+	git push parent2 HEAD
+'
+
+test_expect_success '"upstream" pushes to configured upstream' '
+	git checkout master &&
+	test_config branch.master.remote parent1 &&
+	test_config branch.master.merge refs/heads/foo &&
+	test_config push.default upstream &&
+	test_commit two &&
+	git push &&
+	echo two >expect &&
+	git --git-dir=repo1 log -1 --format=%s foo >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '"upstream" does not push on unconfigured remote' '
+	git checkout master &&
+	test_unconfig branch.master.remote &&
+	test_config push.default upstream &&
+	test_commit three &&
+	test_must_fail git push
+'
+
+test_expect_success '"upstream" does not push on unconfigured branch' '
+	git checkout master &&
+	test_config branch.master.remote parent1 &&
+	test_unconfig branch.master.merge &&
+	test_config push.default upstream
+	test_commit four &&
+	test_must_fail git push
+'
+
+test_expect_success '"upstream" does not push when remotes do not match' '
+	git checkout master &&
+	test_config branch.master.remote parent1 &&
+	test_config branch.master.merge refs/heads/foo &&
+	test_config push.default upstream &&
+	test_commit five &&
+	test_must_fail git push parent2
+'
+
+test_done