[RFC PATCH 0/1] making --set-upstream have default arguments

[RFC PATCH 0/1] making --set-upstream have default arguments

[Abhradeep Chakraborty]

To track a upstream branch from a local branch we need to pass
<repository> and <refspec> to --set-upstream (in case of git push)
or to --set-upstream-to (in case of git branch). In most cases,
users track the upstream branch with the same name as the local
branch they are currently on. For example, users most of the time
do 'git push <repository> <current_branch_refspec>'.

So, it would be great if 'git push -u' by default do this. This
patch series address this. The patches of this patch-set set
some default values for <repository> and <refspec> if they are
not given. It first tries to get the value of <repository> from
'branch.<current_branch>.remote'. If not then it will set the
value of <repository> as 'origin'. <refspec>'s value would be
the short name of the current branch.

The first patch implements it for push command. However, before
moving to the 'git branch' part, it would be great to have
discussions about the proposed changes in this patch and whether
the current changes are the best way to address it or not.

Abhradeep Chakraborty (1):
  push: make '-u' have default arguments

 Documentation/git-push.txt |  6 +++++
 builtin/push.c             | 48 ++++++++++++++++++++++++++++----------
 t/t5523-push-upstream.sh   | 11 +++++++++
 3 files changed, 53 insertions(+), 12 deletions(-)

-- 
2.17.1

[RFC PATCH 1/1] push: make '-u' have default arguments

[Abhradeep Chakraborty]

For now, -u in 'push' command requires two arguments (<repository>
and <refspec>) to successfully track upstream branch. In most cases,
users want to set an upstream branch for the local branch they are
currently on and the short names of these two branches are same in
most of the cases. There are plenty of configurations to set default
branches for push but again users can't run argumentless pull, rebase
etc. So it will be good to have '-u' having default arguments.

This commit gives ability to '-u' to have default arguments. 'git push
-u' runs normally if <repository> and <refspec> are given. But
if those are not given then it tries to get the value of <repository>
from 'branch.<current_branch>.remote'. If not found, it sets 'origin'
as the value of <repository>. <refspec> would be the current branch's
short name.

However 'git push -u --all' work normally as before.

Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
---
 Documentation/git-push.txt |  6 +++++
 builtin/push.c             | 48 ++++++++++++++++++++++++++++----------
 t/t5523-push-upstream.sh   | 11 +++++++++
 3 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 2f25aa3a29..e1a8b41818 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -375,6 +375,12 @@ Specifying `--no-force-if-includes` disables this behavior.
 	upstream (tracking) reference, used by argument-less
 	linkgit:git-pull[1] and other commands. For more information,
 	see `branch.<name>.merge` in linkgit:git-config[1].
++
+If you use -u without any arguments (i.e. no <repository> and <refspec>),
+it will first try to get the <repository> from current branch's remote
+configuration (i.e. from `branch.<name>.remote`). If not found, it will set
+`origin` as the value of <repository> and <refspec> will be the current
+branch's refspec.
 
 --[no-]thin::
 	These options are passed to linkgit:git-send-pack[1]. A thin transfer
diff --git a/builtin/push.c b/builtin/push.c
index 4b026ce6c6..2e417a06ad 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -527,6 +527,25 @@ static int git_push_config(const char *k, const char *v, void *cb)
 	return git_default_config(k, v, NULL);
 }
 
+static struct remote *pushremote_get_remote(const char *repo)
+{
+	struct remote *remote = pushremote_get(repo);
+	if (!remote) {
+		if (repo)
+			die(_("bad repository '%s'"), repo);
+		die(_("No configured push destination.\n"
+		    "Either specify the URL from the command-line or configure a remote repository using\n"
+		    "\n"
+		    "    git remote add <name> <url>\n"
+		    "\n"
+		    "and then push using the remote name\n"
+		    "\n"
+		    "    git push <name>\n"));
+	}
+
+	return remote;
+}
+
 int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int flags = 0;
@@ -537,6 +556,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	struct string_list push_options_cmdline = STRING_LIST_INIT_DUP;
 	struct string_list *push_options;
 	const struct string_list_item *item;
+	struct remote *default_remote = NULL;
 	struct remote *remote;
 
 	struct option options[] = {
@@ -603,23 +623,27 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	if (tags)
 		refspec_append(&rs, "refs/tags/*");
 
+	if ((argc == 0) && (flags & TRANSPORT_PUSH_SET_UPSTREAM) && !(flags & TRANSPORT_PUSH_ALL)) {
+		struct branch *branch = branch_get(NULL);
+		if (branch) {
+			argc += 2;
+			default_remote = pushremote_get_remote(repo);
+			argv[0] = default_remote->name;
+			argv[1] = branch->name;
+		}
+	}
+
 	if (argc > 0) {
 		repo = argv[0];
 		set_refspecs(argv + 1, argc - 1, repo);
 	}
 
-	remote = pushremote_get(repo);
-	if (!remote) {
-		if (repo)
-			die(_("bad repository '%s'"), repo);
-		die(_("No configured push destination.\n"
-		    "Either specify the URL from the command-line or configure a remote repository using\n"
-		    "\n"
-		    "    git remote add <name> <url>\n"
-		    "\n"
-		    "and then push using the remote name\n"
-		    "\n"
-		    "    git push <name>\n"));
+	if (default_remote) {
+		remote = default_remote;
+		default_remote = NULL;
+	}
+	else {
+		remote = pushremote_get_remote(repo);
 	}
 
 	if (remote->mirror)
diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
index fdb4292056..69970b6263 100755
--- a/t/t5523-push-upstream.sh
+++ b/t/t5523-push-upstream.sh
@@ -60,6 +60,17 @@ test_expect_success 'push -u :topic_2' '
 	check_config topic_2 upstream refs/heads/other2
 '
 
+test_expect_success 'push -u' '
+	git push -u &&
+	check_config main upstream refs/heads/main
+'
+
+test_expect_success 'push -u --dry-run' '
+	git push -u upstream main:other &&
+	git push -u --dry-run &&
+	check_config main upstream refs/heads/other
+'
+
 test_expect_success 'push -u --all' '
 	git branch all1 &&
 	git branch all2 &&
-- 
2.17.1

Re: [RFC PATCH 1/1] push: make '-u' have default arguments

[Junio C Hamano]

Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> writes:

> For now, -u in 'push' command requires two arguments (<repository>

Drop "For now"; we start our log message by explaining the current
system without the proposed change in the present tense, so it is
unneeded.  Spelling out the option name "--set-upstream" in full, or
at least "-u" in quotes, would make it more readable.

> and <refspec>) to successfully track upstream branch. In most cases,
> users want to set an upstream branch for the local branch they are
> currently on and the short names of these two branches are same in
> most of the cases.
>
> There are plenty of configurations to set default
> branches for push but again users can't run argumentless pull, rebase
> etc. So it will be good to have '-u' having default arguments.

Don't judge what's "most" common without a survey.  A casual "Often"
is acceptable.

Taking all together, something like

    "git push -u" (set-upstream) requires where to push to and what
    to push.  Often people push only the current branch to update
    the branch of the same name at the 'origin' repository.  For
    them, it would be convenient if "git push -u" without repository
    or refspec defaulted to push to the branch of the same name at
    the remote repository that is used by default.

> This commit gives ability to '-u' to have default arguments. 'git push

cf. Documentation/SubmittingPatches[[imperative-mood]]

> -u' runs normally if <repository> and <refspec> are given. But
> if those are not given then it tries to get the value of <repository>
> from 'branch.<current_branch>.remote'. If not found, it sets 'origin'
> as the value of <repository>. <refspec> would be the current branch's
> short name.

Do not invent an undefined word "short name".  The name of the
'main' branch is 'main', and it is not a short name.  When people
encounter multi-level names, like ac/push-u-default, use of an
undefined word "short name" will mislead readers that you meant
the leaf level, 'push-u-default', but I do not think that is what
you meant (this is not the only instance of "short name" in this
submission; all need to be fixed).

> However 'git push -u --all' work normally as before.

Is this even necessary?  --all is to push all branches to the
default repository, so clearly it is outside the "we need default
because the user did not tell us what to push to where" case.

Taking the above together, perhaps something along this line,

    Teach "git push -u" not to require repository and refspec.  When
    the user did not give what repository to push to, or which
    branch(es) to push, behave as if the default remote repository
    and the name of the current branch are given.  Note that use of
    "--all" option, together with "-u", behaves as before, since the
    user is telling us to push all the branches to the default
    remote repository and there is no need for this new behaviour to
    kick in.

perhaps?

One thing that bothers me is that unlike your assumption, not
everybody uses push.default set to simple or upstream.  I am not
convinced that the "git push -u" that defaults to do the 'current'
push with TRANSPORT_PUSH_SET_UPSTREAM for them is an improvement
for them.  If the new feature does not kick in for them, that should
be explained in the proposed log message when you sell the patch to
reviewers and documented for the users.

> @@ -375,6 +375,12 @@ Specifying `--no-force-if-includes` disables this behavior.
>  	upstream (tracking) reference, used by argument-less
>  	linkgit:git-pull[1] and other commands. For more information,
>  	see `branch.<name>.merge` in linkgit:git-config[1].
> ++
> +If you use -u without any arguments (i.e. no <repository> and <refspec>),

Be careful to quote `-u` and things like that by studyng the text
around what you are changing.

> +it will first try to get the <repository> from current branch's remote
> +configuration (i.e. from `branch.<name>.remote`). If not found, it will set
> +`origin` as the value of <repository> and <refspec> will be the current
> +branch's refspec.

This makes it sound as if the push will only affect the current
branch even for folks who use the matching push.  As I said, I do
not know if that is desirable.

> diff --git a/builtin/push.c b/builtin/push.c
> index 4b026ce6c6..2e417a06ad 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -527,6 +527,25 @@ static int git_push_config(const char *k, const char *v, void *cb)
>  	return git_default_config(k, v, NULL);
>  }
>  
> +static struct remote *pushremote_get_remote(const char *repo)
> +{
> +	struct remote *remote = pushremote_get(repo);
> +	if (!remote) {
> +		if (repo)
> +			die(_("bad repository '%s'"), repo);
> +		die(_("No configured push destination.\n"
> +		    "Either specify the URL from the command-line or configure a remote repository using\n"
> +		    "\n"
> +		    "    git remote add <name> <url>\n"
> +		    "\n"
> +		    "and then push using the remote name\n"
> +		    "\n"
> +		    "    git push <name>\n"));
> +	}
> +
> +	return remote;
> +}
> +
>  int cmd_push(int argc, const char **argv, const char *prefix)
>  {
>  	int flags = 0;
> @@ -537,6 +556,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>  	struct string_list push_options_cmdline = STRING_LIST_INIT_DUP;
>  	struct string_list *push_options;
>  	const struct string_list_item *item;
> +	struct remote *default_remote = NULL;
>  	struct remote *remote;
>  
>  	struct option options[] = {
> @@ -603,23 +623,27 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>  	if (tags)
>  		refspec_append(&rs, "refs/tags/*");
>  
> +	if ((argc == 0) && (flags & TRANSPORT_PUSH_SET_UPSTREAM) && !(flags & TRANSPORT_PUSH_ALL)) {
> +		struct branch *branch = branch_get(NULL);
> +		if (branch) {
> +			argc += 2;
> +			default_remote = pushremote_get_remote(repo);
> +			argv[0] = default_remote->name;
> +			argv[1] = branch->name;

This does look like it breaks unless the user is a novice without
custom configuration.  For example, if the current branch has a
configuration to integrate with a branch at the default remote of a
different name already, this (1) clobbers the tip of a wrong branch
by pushing to it, and (2) overrites the upstream configuration.  If
the user uses push.default set to 'current' or 'simple', this would
be OK, but for all other users, I doubt this would be an improvement.


> diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
> index fdb4292056..69970b6263 100755
> --- a/t/t5523-push-upstream.sh
> +++ b/t/t5523-push-upstream.sh
> @@ -60,6 +60,17 @@ test_expect_success 'push -u :topic_2' '
>  	check_config topic_2 upstream refs/heads/other2
>  '
>  
> +test_expect_success 'push -u' '

We may want to future-proof by checking the current tracking info
(or lack of it) before doing "git push -u" here?  You cannot control
what other developers would do in the future to tests before this
one.

> +	git push -u &&
> +	check_config main upstream refs/heads/main
> +'

And we make sure "-u" without the repository or branch works
in the basic case, which is a good "positive" test.

> +test_expect_success 'push -u --dry-run' '
> +	git push -u upstream main:other &&
> +	git push -u --dry-run &&
> +	check_config main upstream refs/heads/other
> +'

This verifies that under '--dry-run' the upstream configuration does
not get changed.  It is a good "negative" test to have, but there
probably are a lot more "negative" tests to ensure that the new
feature does not kick in in cases where it should not.  Various
settings of push.default is probably a good place to start and with
or without existing upstream info already set up.

Thanks for working on this topic.  I suspect that the implementation
and design covers too broadly to hurt some users while helping
others, and needs tightening up to fix that, but I think the users
appreciate the part that helps some users ;-)

Re: [RFC PATCH 1/1] push: make '-u' have default arguments

[Abhradeep Chakraborty]

Junio C Hamano <gitster@pobox.com> wrote:

> Taking all together, something like
>
>    "git push -u" (set-upstream) requires where to push to and what
>    to push.  Often people push only the current branch to update
>    the branch of the same name at the 'origin' repository.  For
>    them, it would be convenient if "git push -u" without repository
>    or refspec defaulted to push to the branch of the same name at
>    the remote repository that is used by default.

Thanks for the guidance. Improving the cover-letter and commit message
now. :)

> Do not invent an undefined word "short name".  The name of the
> 'main' branch is 'main', and it is not a short name.  When people
> encounter multi-level names, like ac/push-u-default, use of an
> undefined word "short name" will mislead readers that you meant
> the leaf level, 'push-u-default', but I do not think that is what
> you meant (this is not the only instance of "short name" in this
> submission; all need to be fixed).

Sorry for that, I was referring 'branch->name' as 'short name' (and
'branch->refname' as the 'long name' :| ). Will fix it.

> One thing that bothers me is that unlike your assumption, not
> everybody uses push.default set to simple or upstream.  I am not
> convinced that the "git push -u" that defaults to do the 'current'
> push with TRANSPORT_PUSH_SET_UPSTREAM for them is an improvement
> for them.

May be you're right. It may not be an improvement for all. But I
think they also would be happy seeing this 'default' case of 
'set-upstream'.

> If the new feature does not kick in for them, that should
> be explained in the proposed log message when you sell the patch to
> reviewers and documented for the users.

Ok.

> This makes it sound as if the push will only affect the current
> branch even for folks who use the matching push.  As I said, I do
> not know if that is desirable.

Yeah, this only affects the current branch. In that case they may
not use 'git push -u'. As you said previously, this would not be
an improvement in this case ( not a deterioration also).

> This does look like it breaks unless the user is a novice without
> custom configuration.  For example, if the current branch has a
> configuration to integrate with a branch at the default remote of a
> different name already, this (1) clobbers the tip of a wrong branch
> by pushing to it, and (2) overrites the upstream configuration.  If
> the user uses push.default set to 'current' or 'simple', this would
> be OK, but for all other users, I doubt this would be an improvement.

I don't think so. When an user use '--set-upstream' in push command,
it means he/she want to set the upstream to a different branch (if
the specified <repository> and <refspec> are not same as the current
one) rather than the current upstream branch (if exists). So, I think
it would be safe for '--set-upstream' to have default values. Isn't it?
If you are fearing for those two points you specified, should we add
a safety permission before proceeding? e.g. like this -
 
     this would change the upstream branch "%s" to "%s" for this local branch
     would you like to continue? [y]es [n]no

> We may want to future-proof by checking the current tracking info
> (or lack of it) before doing "git push -u" here?  You cannot control
> what other developers would do in the future to tests before this
> one.

Oh yeah, you're right. Will surely add it.

> Various settings of push.default is probably a good place to start and
> with or without existing upstream info already set up.

Thanks for the suggestion, I will add more tests to address these things.

> Thanks for working on this topic.  I suspect that the implementation
> and design covers too broadly to hurt some users while helping
> others, and needs tightening up to fix that, but I think the users
> appreciate the part that helps some users ;-)

Thanks.

Re: [RFC PATCH 1/1] push: make '-u' have default arguments

[Junio C Hamano]

Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> writes:

>> One thing that bothers me is that unlike your assumption, not
>> everybody uses push.default set to simple or upstream.  I am not
>> convinced that the "git push -u" that defaults to do the 'current'
>> push with TRANSPORT_PUSH_SET_UPSTREAM for them is an improvement
>> for them.
>
> May be you're right. It may not be an improvement for all. But I
> think they also would be happy seeing this 'default' case of 
> 'set-upstream'.

Not at all.  

The argument for the "good" case is for "simple" or "current" users,
they want their "git push" without repo and branch arguments to
push their current branch to the branch with the same name at the
default remote repository, and if we arrange "git push -u" to do the
sameand set up "branch.$current.merge", they will find it convenient.

The same reasoning applies for other users who do *not* want their
"git push" without repo and branch arguments to push as if they are
doing the push.default=current push.  If we make "git push -u" push
the current (and only the current) branch to the branch of the same
name at the default remote repository by overwriting argv[] like the
patch under discussion does, we would be giving these users a
convenient way to do what they do not want to do.  Besides, I think
with the current code

    $ git -c push.default=matching push -u

already does the right thing by pushing the matching branches and
sets up upstream for the branches that get pushed without the patch
in question.  With the patch, because it blindly mucks with argv[]
to force pushing only the current branch to the default remote, the
established expectation by existing users is broken.

That of course is not an improvement but actively hurts them.  We
shouldn't be making it easier for our users to hurt themselves.

So, no.  The patch in its current form is totally unacceptable.

Shouldn't the rule be something like "if 'git push $args' (where
$args may be nothing, or any options other than '-u') pushes a
branch (or a set of branches) to a repository, 'git push -u $args'
(with the same $args) should set the branch.*.{remote,merge} for the
branch(es) to the same repository" for the introduction of default
to be truly an improvement?  Or is it too strict and makes the rule
not to trigger even for the intended audience?

Re: [RFC PATCH 1/1] push: make '-u' have default arguments

[Abhradeep Chakraborty]

Junio C Hamano <gitster@pobox.com> wrote:

> That of course is not an improvement but actively hurts them.  We
> shouldn't be making it easier for our users to hurt themselves.

Hmm. In the scenario you mentioned, the proposed change is clearly
breaking. Thanks for notifying.

> Shouldn't the rule be something like "if 'git push $args' (where
> $args may be nothing, or any options other than '-u') pushes a
> branch (or a set of branches) to a repository, 'git push -u $args'
> (with the same $args) should set the branch.*.{remote,merge} for the
> branch(es) to the same repository" for the introduction of default
> to be truly an improvement?  Or is it too strict and makes the rule
> not to trigger even for the intended audience?

Sounds good to me. But what if 'push.default' set to 'nothing'? Do the
proposed default arguments (I am saying about only the default arguments;
not the changes in code) are right fit for that case?

Re: [RFC PATCH 0/1] making --set-upstream have default arguments

[Philip Oakley]

On 02/12/2021 14:43, Abhradeep Chakraborty wrote:
> To track a upstream branch from a local branch we need to pass
> <repository> and <refspec> to --set-upstream (in case of git push)
> or to --set-upstream-to (in case of git branch). In most cases,
> users track the upstream branch with the same name as the local
> branch they are currently on. For example, users most of the time
> do 'git push <repository> <current_branch_refspec>'.
>
> So, it would be great if 'git push -u' by default do this. This
> patch series address this. The patches of this patch-set set
> some default values for <repository> and <refspec> if they are
> not given. It first tries to get the value of <repository> from
> 'branch.<current_branch>.remote'. If not then it will set the
> value of <repository> as 'origin'. <refspec>'s value would be
> the short name of the current branch.

Can we protect the expectations of a user with a `pushDefault` setting?
If the user has one set, then the upstream won't be where they push in a
triangular repo workflow.

Philip
>
> The first patch implements it for push command. However, before
> moving to the 'git branch' part, it would be great to have
> discussions about the proposed changes in this patch and whether
> the current changes are the best way to address it or not.
>
> Abhradeep Chakraborty (1):
>   push: make '-u' have default arguments
>
>  Documentation/git-push.txt |  6 +++++
>  builtin/push.c             | 48 ++++++++++++++++++++++++++++----------
>  t/t5523-push-upstream.sh   | 11 +++++++++
>  3 files changed, 53 insertions(+), 12 deletions(-)
>

Re: [RFC PATCH 0/1] making --set-upstream have default arguments

[Abhradeep Chakraborty]

Philip Oakley wrote:

> Can we protect the expectations of a user with a `pushDefault` setting?

Are you talking about 'push.default'? If so, then I think, the proposed
change would not affect the working of 'push.default' (if the idea is
implemented in the right way). I am adding tests to be sure about it. 

> If the user has one set, then the upstream won't be where they push in a
> triangular repo workflow.

Pardon me, I am unable to understand what you are trying to say. Could you
please explain a little bit?

Thanks.

Re: [RFC PATCH 0/1] making --set-upstream have default arguments

[Philip Oakley]

On 03/12/2021 16:03, Abhradeep Chakraborty wrote:
> Philip Oakley wrote:
>
>> Can we protect the expectations of a user with a `pushDefault` setting?
> Are you talking about 'push.default'? If so, then I think, the proposed
> change would not affect the working of 'push.default' (if the idea is
> implemented in the right way). I am adding tests to be sure about it. 
>
>> If the user has one set, then the upstream won't be where they push in a
>> triangular repo workflow.
> Pardon me, I am unable to understand what you are trying to say. Could you
> please explain a little bit?
>
> Thanks.
>
In my scenario I am tracking various upstream repositories, none of
which I have push permission for. This means I have set up a
`remote.pushDefault` [1] to the remote "my", which is mapped to my
GitHub repo where I can publish work (i.e. push). 

So when I push, I am pushing to "my" remote, but when rebasing, the
upstream is not that destination, and in a collaboration environment,
may not even be the place I first forked from (e.g. the distinction
between 'git.git' [git], 'git-for-windows.git' [gfw], and Junio's repo
[gitster], all with the same root). I can then either send PRs (if
acceptable) or send patches (cover letter link to my publish repo).

In the case where a user has set their remote.pushDefault, then it's not
clear that there should be a default at all, though I maybe
misunderstanding the approach here.

Philip

[1]
https://git-scm.com/docs/git-config#Documentation/git-config.txt-remotepushDefault

Re: [RFC PATCH 0/1] making --set-upstream have default arguments

[Abhradeep Chakraborty]

Philip Oakley wrote:

> Can we protect the expectations of a user with a `pushDefault` setting? 
> If the user has one set, then the upstream won't be where they push in a
> triangular repo workflow.

Ohh, sorry! now I understand. You are talking about 'remote.pushDefault'.
I didn't think about it. Checking if the proposed change is affecting it.
Thanks for pointing out!

[PATCH v2 0/1] making --set-upstream have default arguments

[Abhradeep Chakraborty]

The changes regarding 'git push -u' in the previous patch version
were hurting the intention of using 'push.default'. This patch version
fixes that.

Argumentless 'git push -u' sets the default remote as '<repository>'.
'<refspec>' depends on the 'push.default' configuration. For
'push.default'=matching, it pushes refs to those branches that it
should and sets them as the upstream of their respective local
branches. For all other values of 'push.default', it uses the
current branch for the refspec.

'<repository>' value depends on 'branch.*.remote' and 'remote.pushDefault'
(if 'branch.*.remote' not found). If none of them are set then it
defaults to 'origin'.

Abhradeep Chakraborty (1):
  push: make '-u' have default arguments

 Documentation/git-push.txt |  10 ++++
 builtin/push.c             |  11 +++-
 t/t5523-push-upstream.sh   | 114 +++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH v2 1/1] push: make '-u' have default arguments

[Abhradeep Chakraborty]

"git push -u" (set-upstream) requires where to push to and what
to push.  Often people push only the current branch to update
the branch of the same name at the 'origin' repository.  For
them, it would be convenient if "git push -u" without repository
or refspec, defaulted to push and set upstream to the branch as
configured by the "push.default" setting, of the remote repository
that is used by default.

Teach "git push -u" not to require repository and refspec.  When
the user do not give what repository to push to, or which
branch(es) to push, behave as if the default remote repository
and a refspec (depending on the "push.default" configuration)
are given.

If "push.default"=matching, push all the branches matched on both
remote and local side and set those remote branches as the upstream
of their respective local matched branches. Otherwise, set the
refspec to the refspec for current branch.

Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
---
 Documentation/git-push.txt |  10 ++++
 builtin/push.c             |  11 +++-
 t/t5523-push-upstream.sh   | 114 +++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 2f25aa3a29..048c087e23 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -375,6 +375,16 @@ Specifying `--no-force-if-includes` disables this behavior.
 	upstream (tracking) reference, used by argument-less
 	linkgit:git-pull[1] and other commands. For more information,
 	see `branch.<name>.merge` in linkgit:git-config[1].
++
+`-u` can also work with zero arguments( i.e. no `<repository>` and
+`<refspec>` are given). In that case, it tries to get the `<repository>`
+value from `branch.*.remote` configuration. If not found, it defaults to
+`origin`. If `remote.pushDefault` is set then it uses that instead. The
+value of `<refspec>` depends on the current `push.default` configuration.
+If `push.default` is set to `matching`, all remote branches to which
+local branches pushed, will be set as upstream of respective local
+branches. For all other values of `push.default`, current branch's
+`<refspec>` will be used as the `<refspec>`.
 
 --[no-]thin::
 	These options are passed to linkgit:git-send-pack[1]. A thin transfer
diff --git a/builtin/push.c b/builtin/push.c
index 4b026ce6c6..8bc206c9d8 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -202,11 +202,12 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na
 	return branch->merge[0]->src;
 }
 
-static void setup_default_push_refspecs(struct remote *remote)
+static void setup_default_push_refspecs(struct remote *remote, int flags)
 {
 	struct branch *branch;
 	const char *dst;
 	int same_remote;
+	int is_default_u = (flags & TRANSPORT_PUSH_SET_UPSTREAM);
 
 	switch (push_default) {
 	case PUSH_DEFAULT_MATCHING:
@@ -214,6 +215,8 @@ static void setup_default_push_refspecs(struct remote *remote)
 		return;
 
 	case PUSH_DEFAULT_NOTHING:
+		if (is_default_u)
+			break;
 		die(_("You didn't specify any refspecs to push, and "
 		    "push.default is \"nothing\"."));
 		return;
@@ -234,11 +237,15 @@ static void setup_default_push_refspecs(struct remote *remote)
 	case PUSH_DEFAULT_SIMPLE:
 		if (!same_remote)
 			break;
+		if (is_default_u)
+			break;
 		if (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))
 			die_push_simple(branch, remote);
 		break;
 
 	case PUSH_DEFAULT_UPSTREAM:
+		if (is_default_u)
+			break;
 		if (!same_remote)
 			die(_("You are pushing to remote '%s', which is not the upstream of\n"
 			      "your current branch '%s', without telling me what to push\n"
@@ -401,7 +408,7 @@ static int do_push(int flags,
 		if (remote->push.nr) {
 			push_refspec = &remote->push;
 		} else if (!(flags & TRANSPORT_PUSH_MIRROR))
-			setup_default_push_refspecs(remote);
+			setup_default_push_refspecs(remote, flags);
 	}
 	errs = 0;
 	url_nr = push_url_of_remote(remote, &url);
diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
index fdb4292056..fbcdf303ef 100755
--- a/t/t5523-push-upstream.sh
+++ b/t/t5523-push-upstream.sh
@@ -60,6 +60,75 @@ test_expect_success 'push -u :topic_2' '
 	check_config topic_2 upstream refs/heads/other2
 '
 
+default_u_setup() {
+	git checkout main
+	remote=$(git config --get branch.main.remote)
+	if [ ! -z "$remote" ]; then
+		git branch --unset-upstream
+	fi
+	git config push.default $1
+	git config remote.pushDefault upstream
+}
+
+test_expect_success 'push -u with push.default=simple' '
+	default_u_setup simple &&
+	git push -u &&
+	check_config main upstream refs/heads/main &&
+	git push -u upstream main:other &&
+	git push -u &&
+	check_config main upstream refs/heads/main
+'
+
+test_expect_success 'push -u with push.default=current' '
+	default_u_setup current &&
+	git push -u &&
+	check_config main upstream refs/heads/main &&
+	git push -u upstream main:other &&
+	git push -u &&
+	check_config main upstream refs/heads/main
+'
+
+test_expect_success 'push -u with push.default=upstream' '
+	default_u_setup upstream &&
+	git push -u &&
+	check_config main upstream refs/heads/main &&
+	git push -u upstream main:other &&
+	git push -u &&
+	check_config main upstream refs/heads/main
+'
+
+check_empty_config() {
+	test_expect_code 1 git config "branch.$1.remote"
+	test_expect_code 1 git config "branch.$1.merge"
+}
+
+test_expect_success 'push -u with push.default=matching' '
+	default_u_setup matching &&
+	git branch test_u &&
+	git branch test_u2 &&
+	git push upstream main:test_u2 &&
+	git push -u &&
+	check_config main upstream refs/heads/main &&
+	check_config test_u2 upstream refs/heads/test_u2 &&
+	check_empty_config test_u
+'
+
+test_expect_success 'push -u with push.default=nothing' '
+	default_u_setup nothing &&
+	git push -u &&
+	check_config main upstream refs/heads/main &&
+	git push -u upstream main:other &&
+	git push -u &&
+	check_config main upstream refs/heads/main
+'
+
+test_expect_success 'push -u --dry-run' '
+	git checkout main &&
+	git push -u upstream main:other &&
+	git push -u --dry-run &&
+	check_config main upstream refs/heads/other
+'
+
 test_expect_success 'push -u --all' '
 	git branch all1 &&
 	git branch all2 &&
@@ -81,6 +150,13 @@ test_expect_success TTY 'progress messages go to tty' '
 	test_i18ngrep "Writing objects" err
 '
 
+test_expect_success TTY 'progress messages go to tty with default -u' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push -u >out 2>err &&
+	test_i18ngrep "Writing objects" err
+'
+
 test_expect_success 'progress messages do not go to non-tty' '
 	ensure_fresh_upstream &&
 
@@ -89,6 +165,14 @@ test_expect_success 'progress messages do not go to non-tty' '
 	test_i18ngrep ! "Writing objects" err
 '
 
+test_expect_success 'progress messagesdo not go to non-tty (default -u)' '
+	ensure_fresh_upstream &&
+
+	# skip progress messages, since stderr is non-tty
+	git push -u >out 2>err &&
+	test_i18ngrep ! "Writing objects" err
+'
+
 test_expect_success 'progress messages go to non-tty (forced)' '
 	ensure_fresh_upstream &&
 
@@ -97,6 +181,14 @@ test_expect_success 'progress messages go to non-tty (forced)' '
 	test_i18ngrep "Writing objects" err
 '
 
+test_expect_success 'progress messages go to non-tty with default -u (forced)' '
+	ensure_fresh_upstream &&
+
+	# force progress messages to stderr, even though it is non-tty
+	git push -u --progress >out 2>err &&
+	test_i18ngrep "Writing objects" err
+'
+
 test_expect_success TTY 'push -q suppresses progress' '
 	ensure_fresh_upstream &&
 
@@ -104,6 +196,13 @@ test_expect_success TTY 'push -q suppresses progress' '
 	test_i18ngrep ! "Writing objects" err
 '
 
+test_expect_success TTY 'push -q suppresses progress (with default -u)' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push -u -q >out 2>err &&
+	test_i18ngrep ! "Writing objects" err
+'
+
 test_expect_success TTY 'push --no-progress suppresses progress' '
 	ensure_fresh_upstream &&
 
@@ -112,6 +211,14 @@ test_expect_success TTY 'push --no-progress suppresses progress' '
 	test_i18ngrep ! "Writing objects" err
 '
 
+test_expect_success TTY 'push --no-progress suppresses progress (default -u)' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push -u --no-progress >out 2>err &&
+	test_i18ngrep ! "Unpacking objects" err &&
+	test_i18ngrep ! "Writing objects" err
+'
+
 test_expect_success TTY 'quiet push' '
 	ensure_fresh_upstream &&
 
@@ -126,4 +233,11 @@ test_expect_success TTY 'quiet push -u' '
 	test_must_be_empty output
 '
 
+test_expect_success TTY 'quiet push -u (default -u)' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push --quiet -u --no-progress 2>&1 | tee output &&
+	test_must_be_empty output
+'
+
 test_done
-- 
2.17.1

Re: [PATCH v2 1/1] push: make '-u' have default arguments

[Eric Sunshine]

On Tue, Dec 7, 2021 at 4:11 PM Abhradeep Chakraborty
<chakrabortyabhradeep79@gmail.com> wrote:
> [...]
> Teach "git push -u" not to require repository and refspec.  When
> the user do not give what repository to push to, or which
> branch(es) to push, behave as if the default remote repository
> and a refspec (depending on the "push.default" configuration)
> are given.
> [...]
> Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>

This is not a proper review... just some superficial comments from
scanning my eye over the patch...

> diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
> @@ -60,6 +60,75 @@ test_expect_success 'push -u :topic_2' '
> +default_u_setup() {
> +       git checkout main
> +       remote=$(git config --get branch.main.remote)
> +       if [ ! -z "$remote" ]; then
> +               git branch --unset-upstream
> +       fi
> +       git config push.default $1
> +       git config remote.pushDefault upstream
> +}

A few issues...

* since callers of this function incorporate it into their &&-chains,
the body of the function itself should probably also have an intact
&&-chain

* use `test` rather than `[`

* `then` goes on its own line

* probably want to use test_config() here rather than raw `git config`

* `! -z` can be written more simply as `-n`

Taking the above into account, gives:

    default_u_setup() {
        git checkout main &&
        remote=$(git config --default '' --get branch.main.remote) &&
        if test -n "$remote"
        then
            git branch --unset-upstream
        fi &&
        test_config push.default $1 &&
        test_config remote.pushDefault upstream
    }

The `--default` ensures that `git config` will exit with a success
code which is important now that it's part of the &&-chain.
Alternatively, you could skip the dance of checking for
`branch.main.remote` and just call `git branch --unset-upstream`
unconditionally, but wrap it with test_might_fail() so it can be part
of the &&-chain without worrying about whether that command succeeds
or fails:

    default_u_setup() {
        git checkout main &&
        test_might_fail git branch --unset-upstream &&
        test_config push.default $1 &&
        test_config remote.pushDefault upstream
    }

> +test_expect_success 'push -u with push.default=simple' '
> +       default_u_setup simple &&
> +       git push -u &&
> +       check_config main upstream refs/heads/main &&
> +       git push -u upstream main:other &&
> +       git push -u &&
> +       check_config main upstream refs/heads/main
> +'
> +
> +test_expect_success 'push -u with push.default=current' '
> +       default_u_setup current &&
> +       git push -u &&
> +       check_config main upstream refs/heads/main &&
> +       git push -u upstream main:other &&
> +       git push -u &&
> +       check_config main upstream refs/heads/main
> +'
> +
> +test_expect_success 'push -u with push.default=upstream' '
> +       default_u_setup upstream &&
> +       git push -u &&
> +       check_config main upstream refs/heads/main &&
> +       git push -u upstream main:other &&
> +       git push -u &&
> +       check_config main upstream refs/heads/main
> +'

When a number of tests have nearly identical bodies like this, it is
sometimes clearer and more convenient to turn them into a for-loop
like this:

    for i in simple current upstream
    do
        test_expect_success "push -u with push.default=$i" '
            default_u_setup $i &&
            git push -u &&
            check_config main upstream refs/heads/main &&
            git push -u upstream main:other &&
            git push -u &&
            check_config main upstream refs/heads/main
        '
    done

> +check_empty_config() {
> +       test_expect_code 1 git config "branch.$1.remote"
> +       test_expect_code 1 git config "branch.$1.merge"
> +}

As above, because calls to this function are part of the &&-chain in
test bodies, it is important for the &&-chain to be intact in the
function too. It's especially important in this case since this
function is actually checking for specific conditions. As it's
currently written -- with a broken &&-chain -- if the first
test_expect_code() fails, we'll never know about it since that exit
code gets lost; only the exit code from the second test_expect_code()
has any bearing on the overall result of the test.

> +test_expect_success 'progress messagesdo not go to non-tty (default -u)' '

s/messagesdo/messages do/

> +       ensure_fresh_upstream &&
> +
> +       # skip progress messages, since stderr is non-tty
> +       git push -u >out 2>err &&
> +       test_i18ngrep ! "Writing objects" err
> +'

The captured stdout in `out` doesn't seem to be used, so it's probably
better to drop that redirection.

> +test_expect_success 'progress messages go to non-tty with default -u (forced)' '
> +       ensure_fresh_upstream &&
> +
> +       # force progress messages to stderr, even though it is non-tty
> +       git push -u --progress >out 2>err &&
> +       test_i18ngrep "Writing objects" err
> +'

Ditto. And repeat for the remaining tests.

Re: [PATCH v2 1/1] push: make '-u' have default argument

[Abhradeep Chakraborty]

Eric Sunshine <sunshine@sunshineco.com> wrote:

> As above, because calls to this function are part of the &&-chain in
> test bodies, it is important for the &&-chain to be intact in the
> function too. It's especially important in this case since this
> function is actually checking for specific conditions. As it's
> currently written -- with a broken &&-chain -- if the first
> test_expect_code() fails, we'll never know about it since that exit
> code gets lost; only the exit code from the second test_expect_code()
> has any bearing on the overall result of the test.

Thanks so much for all the suggestions! I am little beginner in shell
scripting. It would help me a lot!

> s/messagesdo/messages do/

oops :|

> The captured stdout in `out` doesn't seem to be used, so it's probably
> better to drop that redirection.

okay.

[PATCH v3 0/1] making --set-upstream have default arguments

[Abhradeep Chakraborty]

This patch series tries to make the 'set-upstream' option for
'git push' have default arguments.

In v0: argumentless 'git push -u' was blindly passing default remote
name and current branch's name as argv[0] and argv[1] respectively.
This was affecting `push.default` setting.

From v1: The default remote is still used for the <repository> value.
But <refspec> depends on the current push configurations. If
`push.default`='matching', it pushes to the upstream as it should and
sets upstream respectively. For other values of 'push.default', it
pushes to the remote branch with the same name as the current
branch and sets that branch as the upstream.

In this version, the tests are improved.

Abhradeep Chakraborty (1):
  push: make '-u' have default arguments

 Documentation/git-push.txt | 10 +++++
 builtin/push.c             | 11 ++++-
 t/t5523-push-upstream.sh   | 87 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+), 2 deletions(-)

Range-diff against v2:
1:  6567327900 ! 1:  376ba6cb8f push: make '-u' have default arguments
    @@ Documentation/git-push.txt: Specifying `--no-force-if-includes` disables this be
     +If `push.default` is set to `matching`, all remote branches to which
     +local branches pushed, will be set as upstream of respective local
     +branches. For all other values of `push.default`, current branch's
    -+`<refspec>` will be used as the `<refspec>`.
    ++refspec will be used as the `<refspec>`.
      
      --[no-]thin::
      	These options are passed to linkgit:git-send-pack[1]. A thin transfer
    @@ t/t5523-push-upstream.sh: test_expect_success 'push -u :topic_2' '
      '
      
     +default_u_setup() {
    -+	git checkout main
    -+	remote=$(git config --get branch.main.remote)
    -+	if [ ! -z "$remote" ]; then
    -+		git branch --unset-upstream
    -+	fi
    -+	git config push.default $1
    -+	git config remote.pushDefault upstream
    ++	git checkout main &&
    ++	test_might_fail	git branch --unset-upstream &&
    ++	test_config push.default $1 &&
    ++	test_config remote.pushDefault upstream
     +}
     +
    -+test_expect_success 'push -u with push.default=simple' '
    -+	default_u_setup simple &&
    -+	git push -u &&
    -+	check_config main upstream refs/heads/main &&
    -+	git push -u upstream main:other &&
    -+	git push -u &&
    -+	check_config main upstream refs/heads/main
    -+'
    -+
    -+test_expect_success 'push -u with push.default=current' '
    -+	default_u_setup current &&
    -+	git push -u &&
    -+	check_config main upstream refs/heads/main &&
    -+	git push -u upstream main:other &&
    -+	git push -u &&
    -+	check_config main upstream refs/heads/main
    -+'
    -+
    -+test_expect_success 'push -u with push.default=upstream' '
    -+	default_u_setup upstream &&
    -+	git push -u &&
    -+	check_config main upstream refs/heads/main &&
    -+	git push -u upstream main:other &&
    -+	git push -u &&
    -+	check_config main upstream refs/heads/main
    -+'
    ++for i in simple current upstream nothing
    ++do
    ++	test_expect_success 'push -u with push.default=$i' '
    ++		default_u_setup $i &&
    ++		git push -u &&
    ++		check_config main upstream refs/heads/main &&
    ++		git push -u upstream main:other &&
    ++		git push -u &&
    ++		check_config main upstream refs/heads/main
    ++	'
    ++done
     +
     +check_empty_config() {
    -+	test_expect_code 1 git config "branch.$1.remote"
    ++	test_expect_code 1 git config "branch.$1.remote" &&
     +	test_expect_code 1 git config "branch.$1.merge"
     +}
     +
    @@ t/t5523-push-upstream.sh: test_expect_success 'push -u :topic_2' '
     +	check_empty_config test_u
     +'
     +
    -+test_expect_success 'push -u with push.default=nothing' '
    -+	default_u_setup nothing &&
    -+	git push -u &&
    -+	check_config main upstream refs/heads/main &&
    -+	git push -u upstream main:other &&
    -+	git push -u &&
    -+	check_config main upstream refs/heads/main
    -+'
    -+
     +test_expect_success 'push -u --dry-run' '
     +	git checkout main &&
     +	git push -u upstream main:other &&
    @@ t/t5523-push-upstream.sh: test_expect_success TTY 'progress messages go to tty'
     +test_expect_success TTY 'progress messages go to tty with default -u' '
     +	ensure_fresh_upstream &&
     +
    -+	test_terminal git push -u >out 2>err &&
    ++	test_terminal git push -u 2>err &&
     +	test_i18ngrep "Writing objects" err
     +'
     +
    @@ t/t5523-push-upstream.sh: test_expect_success 'progress messages do not go to no
      	test_i18ngrep ! "Writing objects" err
      '
      
    -+test_expect_success 'progress messagesdo not go to non-tty (default -u)' '
    ++test_expect_success 'progress messages do not go to non-tty (default -u)' '
     +	ensure_fresh_upstream &&
     +
     +	# skip progress messages, since stderr is non-tty
    -+	git push -u >out 2>err &&
    ++	git push -u 2>err &&
     +	test_i18ngrep ! "Writing objects" err
     +'
     +
    @@ t/t5523-push-upstream.sh: test_expect_success 'progress messages go to non-tty (
     +	ensure_fresh_upstream &&
     +
     +	# force progress messages to stderr, even though it is non-tty
    -+	git push -u --progress >out 2>err &&
    ++	git push -u --progress 2>err &&
     +	test_i18ngrep "Writing objects" err
     +'
     +
    @@ t/t5523-push-upstream.sh: test_expect_success TTY 'push -q suppresses progress'
     +test_expect_success TTY 'push -q suppresses progress (with default -u)' '
     +	ensure_fresh_upstream &&
     +
    -+	test_terminal git push -u -q >out 2>err &&
    ++	test_terminal git push -u -q 2>err &&
     +	test_i18ngrep ! "Writing objects" err
     +'
     +
    @@ t/t5523-push-upstream.sh: test_expect_success TTY 'push --no-progress suppresses
     +test_expect_success TTY 'push --no-progress suppresses progress (default -u)' '
     +	ensure_fresh_upstream &&
     +
    -+	test_terminal git push -u --no-progress >out 2>err &&
    ++	test_terminal git push -u --no-progress 2>err &&
     +	test_i18ngrep ! "Unpacking objects" err &&
     +	test_i18ngrep ! "Writing objects" err
     +'
-- 
2.34.1

[PATCH v3 1/1] push: make '-u' have default arguments

[Abhradeep Chakraborty]

"git push -u" (set-upstream) requires where to push to and what
to push.  Often people push only the current branch to update
the branch of the same name at the 'origin' repository.  For
them, it would be convenient if "git push -u" without repository
or refspec, defaulted to push and set upstream to the branch as
configured by the "push.default" setting, of the remote repository
that is used by default.

Teach "git push -u" not to require repository and refspec.  When
the user do not give what repository to push to, or which
branch(es) to push, behave as if the default remote repository
and a refspec (depending on the "push.default" configuration)
are given.

If "push.default"=matching, push all the branches matched on both
remote and local side and set those remote branches as the upstream
of their respective local matched branches. Otherwise, set the
refspec to the refspec for current branch.

Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
---
 Documentation/git-push.txt | 10 +++++
 builtin/push.c             | 11 ++++-
 t/t5523-push-upstream.sh   | 87 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 2f25aa3a29..6fd474441f 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -375,6 +375,16 @@ Specifying `--no-force-if-includes` disables this behavior.
 	upstream (tracking) reference, used by argument-less
 	linkgit:git-pull[1] and other commands. For more information,
 	see `branch.<name>.merge` in linkgit:git-config[1].
++
+`-u` can also work with zero arguments( i.e. no `<repository>` and
+`<refspec>` are given). In that case, it tries to get the `<repository>`
+value from `branch.*.remote` configuration. If not found, it defaults to
+`origin`. If `remote.pushDefault` is set then it uses that instead. The
+value of `<refspec>` depends on the current `push.default` configuration.
+If `push.default` is set to `matching`, all remote branches to which
+local branches pushed, will be set as upstream of respective local
+branches. For all other values of `push.default`, current branch's
+refspec will be used as the `<refspec>`.
 
 --[no-]thin::
 	These options are passed to linkgit:git-send-pack[1]. A thin transfer
diff --git a/builtin/push.c b/builtin/push.c
index 4b026ce6c6..8bc206c9d8 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -202,11 +202,12 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na
 	return branch->merge[0]->src;
 }
 
-static void setup_default_push_refspecs(struct remote *remote)
+static void setup_default_push_refspecs(struct remote *remote, int flags)
 {
 	struct branch *branch;
 	const char *dst;
 	int same_remote;
+	int is_default_u = (flags & TRANSPORT_PUSH_SET_UPSTREAM);
 
 	switch (push_default) {
 	case PUSH_DEFAULT_MATCHING:
@@ -214,6 +215,8 @@ static void setup_default_push_refspecs(struct remote *remote)
 		return;
 
 	case PUSH_DEFAULT_NOTHING:
+		if (is_default_u)
+			break;
 		die(_("You didn't specify any refspecs to push, and "
 		    "push.default is \"nothing\"."));
 		return;
@@ -234,11 +237,15 @@ static void setup_default_push_refspecs(struct remote *remote)
 	case PUSH_DEFAULT_SIMPLE:
 		if (!same_remote)
 			break;
+		if (is_default_u)
+			break;
 		if (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))
 			die_push_simple(branch, remote);
 		break;
 
 	case PUSH_DEFAULT_UPSTREAM:
+		if (is_default_u)
+			break;
 		if (!same_remote)
 			die(_("You are pushing to remote '%s', which is not the upstream of\n"
 			      "your current branch '%s', without telling me what to push\n"
@@ -401,7 +408,7 @@ static int do_push(int flags,
 		if (remote->push.nr) {
 			push_refspec = &remote->push;
 		} else if (!(flags & TRANSPORT_PUSH_MIRROR))
-			setup_default_push_refspecs(remote);
+			setup_default_push_refspecs(remote, flags);
 	}
 	errs = 0;
 	url_nr = push_url_of_remote(remote, &url);
diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
index fdb4292056..ea5d1ae914 100755
--- a/t/t5523-push-upstream.sh
+++ b/t/t5523-push-upstream.sh
@@ -60,6 +60,48 @@ test_expect_success 'push -u :topic_2' '
 	check_config topic_2 upstream refs/heads/other2
 '
 
+default_u_setup() {
+	git checkout main &&
+	test_might_fail	git branch --unset-upstream &&
+	test_config push.default $1 &&
+	test_config remote.pushDefault upstream
+}
+
+for i in simple current upstream nothing
+do
+	test_expect_success 'push -u with push.default=$i' '
+		default_u_setup $i &&
+		git push -u &&
+		check_config main upstream refs/heads/main &&
+		git push -u upstream main:other &&
+		git push -u &&
+		check_config main upstream refs/heads/main
+	'
+done
+
+check_empty_config() {
+	test_expect_code 1 git config "branch.$1.remote" &&
+	test_expect_code 1 git config "branch.$1.merge"
+}
+
+test_expect_success 'push -u with push.default=matching' '
+	default_u_setup matching &&
+	git branch test_u &&
+	git branch test_u2 &&
+	git push upstream main:test_u2 &&
+	git push -u &&
+	check_config main upstream refs/heads/main &&
+	check_config test_u2 upstream refs/heads/test_u2 &&
+	check_empty_config test_u
+'
+
+test_expect_success 'push -u --dry-run' '
+	git checkout main &&
+	git push -u upstream main:other &&
+	git push -u --dry-run &&
+	check_config main upstream refs/heads/other
+'
+
 test_expect_success 'push -u --all' '
 	git branch all1 &&
 	git branch all2 &&
@@ -81,6 +123,13 @@ test_expect_success TTY 'progress messages go to tty' '
 	test_i18ngrep "Writing objects" err
 '
 
+test_expect_success TTY 'progress messages go to tty with default -u' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push -u 2>err &&
+	test_i18ngrep "Writing objects" err
+'
+
 test_expect_success 'progress messages do not go to non-tty' '
 	ensure_fresh_upstream &&
 
@@ -89,6 +138,14 @@ test_expect_success 'progress messages do not go to non-tty' '
 	test_i18ngrep ! "Writing objects" err
 '
 
+test_expect_success 'progress messages do not go to non-tty (default -u)' '
+	ensure_fresh_upstream &&
+
+	# skip progress messages, since stderr is non-tty
+	git push -u 2>err &&
+	test_i18ngrep ! "Writing objects" err
+'
+
 test_expect_success 'progress messages go to non-tty (forced)' '
 	ensure_fresh_upstream &&
 
@@ -97,6 +154,14 @@ test_expect_success 'progress messages go to non-tty (forced)' '
 	test_i18ngrep "Writing objects" err
 '
 
+test_expect_success 'progress messages go to non-tty with default -u (forced)' '
+	ensure_fresh_upstream &&
+
+	# force progress messages to stderr, even though it is non-tty
+	git push -u --progress 2>err &&
+	test_i18ngrep "Writing objects" err
+'
+
 test_expect_success TTY 'push -q suppresses progress' '
 	ensure_fresh_upstream &&
 
@@ -104,6 +169,13 @@ test_expect_success TTY 'push -q suppresses progress' '
 	test_i18ngrep ! "Writing objects" err
 '
 
+test_expect_success TTY 'push -q suppresses progress (with default -u)' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push -u -q 2>err &&
+	test_i18ngrep ! "Writing objects" err
+'
+
 test_expect_success TTY 'push --no-progress suppresses progress' '
 	ensure_fresh_upstream &&
 
@@ -112,6 +184,14 @@ test_expect_success TTY 'push --no-progress suppresses progress' '
 	test_i18ngrep ! "Writing objects" err
 '
 
+test_expect_success TTY 'push --no-progress suppresses progress (default -u)' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push -u --no-progress 2>err &&
+	test_i18ngrep ! "Unpacking objects" err &&
+	test_i18ngrep ! "Writing objects" err
+'
+
 test_expect_success TTY 'quiet push' '
 	ensure_fresh_upstream &&
 
@@ -126,4 +206,11 @@ test_expect_success TTY 'quiet push -u' '
 	test_must_be_empty output
 '
 
+test_expect_success TTY 'quiet push -u (default -u)' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push --quiet -u --no-progress 2>&1 | tee output &&
+	test_must_be_empty output
+'
+
 test_done
-- 
2.34.1

[PATCH v4 0/1] making --set-upstream have default arguments

[Abhradeep Chakraborty]

Often developers want to set a remote branch (i.e. upstream branch)
for the local branch according to their `push.default` settings. For
example, beginners often (may be most of the beginners) run

	git push -u origin <current_branch_name>

If the `push.default` configuration is set, people may want to set
the upstream to that branch that satisfies the `push.default`
configuration. For example, if the `push.default` is set to
'current', developer may want to do like this -

	git push -u <default_repo> <current_branch>

So, it would be great if 'git push -u' (i.e. without <repo> and
<refspec>) by default do this. If `push.default` is not set or has
a value other than 'matching', it would do this - 

	git push -u <default_repo> <current_branch>

And for `push.default`= 'matching', it would set all the remote
maching branch as upstream of their respective matching local
branches. E.g. if 'branch1' and 'branch2' branches both exist in
the local as well as remote repo, 'git push -u' would set the remote
'branch1' as the upstream of local 'branch1' branch and remote
'branch2' branch would be set as the upstream of local 'branch2'
branch.

Note, 'git push -u' for push.default=matching, already works. 

This patch series addresses this.

In v0: argumentless 'git push -u' was blindly passing default remote
name and current branch's name as argv[0] and argv[1] respectively.
This was affecting `push.default` setting.

From v1: The default remote is still used for the <repository> value.
But <refspec> depends on the current push configurations. If
`push.default`='matching', it pushes to the upstream as it should and
sets upstream respectively. For other values of 'push.default', it
pushes to the remote branch with the same name as the current
branch and sets that branch as the upstream.

In v2 and v3: various test cases were added and improved

In current version: tests for 'git push -u' with other options are
added. This includes '-f', '--prune', '-d', '--mirror'.


Abhradeep Chakraborty (1):
  push: make 'set-upstream' have dafault arguments

 Documentation/git-push.txt |  10 +++
 builtin/push.c             |  11 +++-
 t/t5523-push-upstream.sh   | 125 +++++++++++++++++++++++++++++++++++++
 3 files changed, 144 insertions(+), 2 deletions(-)

Range-diff against v3:
1:  64655de6ca ! 1:  d154c7d1f6 push: make '-u' have default arguments
    @@ Metadata
     Author: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
     
      ## Commit message ##
    -    push: make '-u' have default arguments
    +    push: make 'set-upstream' have dafault arguments
     
         "git push -u" (set-upstream) requires where to push to and what
         to push.  Often people push only the current branch to update
    @@ t/t5523-push-upstream.sh: test_expect_success 'push -u :topic_2' '
     +	test_config remote.pushDefault upstream
     +}
     +
    ++check_empty_config() {
    ++	test_expect_code 1 git config "branch.$1.remote" &&
    ++	test_expect_code 1 git config "branch.$1.merge"
    ++}
    ++
     +for i in simple current upstream nothing
     +do
     +	test_expect_success 'push -u with push.default=$i' '
    @@ t/t5523-push-upstream.sh: test_expect_success 'push -u :topic_2' '
     +		git push -u &&
     +		check_config main upstream refs/heads/main
     +	'
    ++
    ++	test_expect_success 'push -u -f with push.default=$i' '
    ++		default_u_setup $i &&
    ++		git push -u -f &&
    ++		check_config main upstream refs/heads/main
    ++	'
     +done
     +
    -+check_empty_config() {
    -+	test_expect_code 1 git config "branch.$1.remote" &&
    -+	test_expect_code 1 git config "branch.$1.merge"
    -+}
    ++for i in simple current upstream nothing matching
    ++do
    ++	test_expect_success 'push -u --prune with push.default=$i' '
    ++		default_u_setup $i &&
    ++		git push upstream main:test_u215 &&
    ++		git push -u --prune >out &&
    ++		check_config main upstream refs/heads/main &&
    ++		test_i18ngrep "[deleted]" out &&
    ++		test_i18ngrep ! "Branch '"'"'test_u215'"'"' set up to track" out
    ++	'
    ++
    ++	test_expect_success 'push -u --mirror with push.default=$i' '
    ++		default_u_setup $i &&
    ++		test_might_fail git branch mirror1 &&
    ++		test_might_fail git branch mirror2 &&
    ++		git push -u --mirror &&
    ++		check_config main upstream  refs/heads/main &&
    ++		check_config mirror1 upstream refs/heads/mirror1 &&
    ++		check_config mirror2 upstream refs/heads/mirror2
    ++	'
    ++done
     +
    -+test_expect_success 'push -u with push.default=matching' '
    -+	default_u_setup matching &&
    -+	git branch test_u &&
    -+	git branch test_u2 &&
    -+	git push upstream main:test_u2 &&
    -+	git push -u &&
    -+	check_config main upstream refs/heads/main &&
    -+	check_config test_u2 upstream refs/heads/test_u2 &&
    -+	check_empty_config test_u
    ++for i in '' '-f'
    ++do
    ++
    ++	test_expect_success 'push -u $i with push.default=matching' '
    ++		default_u_setup matching &&
    ++		test_might_fail git branch test_u &&
    ++		test_might_fail git branch test_u2 &&
    ++		git push upstream main:test_u2 &&
    ++		git push -u $i &&
    ++		check_config main upstream refs/heads/main &&
    ++		check_config test_u2 upstream refs/heads/test_u2 &&
    ++		check_empty_config test_u
    ++	'
    ++done
    ++
    ++test_expect_success 'push -u -d will fail' '
    ++	git checkout main &&
    ++	test_might_fail git branch --unset-upstream &&
    ++	test_must_fail git push -u -d
     +'
     +
     +test_expect_success 'push -u --dry-run' '
-- 
2.34.1

[PATCH v4 1/1] push: make 'set-upstream' have dafault arguments

[Abhradeep Chakraborty]

"git push -u" (set-upstream) requires where to push to and what
to push.  Often people push only the current branch to update
the branch of the same name at the 'origin' repository.  For
them, it would be convenient if "git push -u" without repository
or refspec, defaulted to push and set upstream to the branch as
configured by the "push.default" setting, of the remote repository
that is used by default.

Teach "git push -u" not to require repository and refspec.  When
the user do not give what repository to push to, or which
branch(es) to push, behave as if the default remote repository
and a refspec (depending on the "push.default" configuration)
are given.

If "push.default"=matching, push all the branches matched on both
remote and local side and set those remote branches as the upstream
of their respective local matched branches. Otherwise, set the
refspec to the refspec for current branch.

Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
---
 Documentation/git-push.txt |  10 +++
 builtin/push.c             |  11 +++-
 t/t5523-push-upstream.sh   | 125 +++++++++++++++++++++++++++++++++++++
 3 files changed, 144 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 2f25aa3a29..6fd474441f 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -375,6 +375,16 @@ Specifying `--no-force-if-includes` disables this behavior.
 	upstream (tracking) reference, used by argument-less
 	linkgit:git-pull[1] and other commands. For more information,
 	see `branch.<name>.merge` in linkgit:git-config[1].
++
+`-u` can also work with zero arguments( i.e. no `<repository>` and
+`<refspec>` are given). In that case, it tries to get the `<repository>`
+value from `branch.*.remote` configuration. If not found, it defaults to
+`origin`. If `remote.pushDefault` is set then it uses that instead. The
+value of `<refspec>` depends on the current `push.default` configuration.
+If `push.default` is set to `matching`, all remote branches to which
+local branches pushed, will be set as upstream of respective local
+branches. For all other values of `push.default`, current branch's
+refspec will be used as the `<refspec>`.
 
 --[no-]thin::
 	These options are passed to linkgit:git-send-pack[1]. A thin transfer
diff --git a/builtin/push.c b/builtin/push.c
index 4b026ce6c6..8bc206c9d8 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -202,11 +202,12 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na
 	return branch->merge[0]->src;
 }
 
-static void setup_default_push_refspecs(struct remote *remote)
+static void setup_default_push_refspecs(struct remote *remote, int flags)
 {
 	struct branch *branch;
 	const char *dst;
 	int same_remote;
+	int is_default_u = (flags & TRANSPORT_PUSH_SET_UPSTREAM);
 
 	switch (push_default) {
 	case PUSH_DEFAULT_MATCHING:
@@ -214,6 +215,8 @@ static void setup_default_push_refspecs(struct remote *remote)
 		return;
 
 	case PUSH_DEFAULT_NOTHING:
+		if (is_default_u)
+			break;
 		die(_("You didn't specify any refspecs to push, and "
 		    "push.default is \"nothing\"."));
 		return;
@@ -234,11 +237,15 @@ static void setup_default_push_refspecs(struct remote *remote)
 	case PUSH_DEFAULT_SIMPLE:
 		if (!same_remote)
 			break;
+		if (is_default_u)
+			break;
 		if (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))
 			die_push_simple(branch, remote);
 		break;
 
 	case PUSH_DEFAULT_UPSTREAM:
+		if (is_default_u)
+			break;
 		if (!same_remote)
 			die(_("You are pushing to remote '%s', which is not the upstream of\n"
 			      "your current branch '%s', without telling me what to push\n"
@@ -401,7 +408,7 @@ static int do_push(int flags,
 		if (remote->push.nr) {
 			push_refspec = &remote->push;
 		} else if (!(flags & TRANSPORT_PUSH_MIRROR))
-			setup_default_push_refspecs(remote);
+			setup_default_push_refspecs(remote, flags);
 	}
 	errs = 0;
 	url_nr = push_url_of_remote(remote, &url);
diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
index fdb4292056..c2d11c3f2a 100755
--- a/t/t5523-push-upstream.sh
+++ b/t/t5523-push-upstream.sh
@@ -60,6 +60,86 @@ test_expect_success 'push -u :topic_2' '
 	check_config topic_2 upstream refs/heads/other2
 '
 
+default_u_setup() {
+	git checkout main &&
+	test_might_fail	git branch --unset-upstream &&
+	test_config push.default $1 &&
+	test_config remote.pushDefault upstream
+}
+
+check_empty_config() {
+	test_expect_code 1 git config "branch.$1.remote" &&
+	test_expect_code 1 git config "branch.$1.merge"
+}
+
+for i in simple current upstream nothing
+do
+	test_expect_success 'push -u with push.default=$i' '
+		default_u_setup $i &&
+		git push -u &&
+		check_config main upstream refs/heads/main &&
+		git push -u upstream main:other &&
+		git push -u &&
+		check_config main upstream refs/heads/main
+	'
+
+	test_expect_success 'push -u -f with push.default=$i' '
+		default_u_setup $i &&
+		git push -u -f &&
+		check_config main upstream refs/heads/main
+	'
+done
+
+for i in simple current upstream nothing matching
+do
+	test_expect_success 'push -u --prune with push.default=$i' '
+		default_u_setup $i &&
+		git push upstream main:test_u215 &&
+		git push -u --prune >out &&
+		check_config main upstream refs/heads/main &&
+		test_i18ngrep "[deleted]" out &&
+		test_i18ngrep ! "Branch '"'"'test_u215'"'"' set up to track" out
+	'
+
+	test_expect_success 'push -u --mirror with push.default=$i' '
+		default_u_setup $i &&
+		test_might_fail git branch mirror1 &&
+		test_might_fail git branch mirror2 &&
+		git push -u --mirror &&
+		check_config main upstream  refs/heads/main &&
+		check_config mirror1 upstream refs/heads/mirror1 &&
+		check_config mirror2 upstream refs/heads/mirror2
+	'
+done
+
+for i in '' '-f'
+do
+
+	test_expect_success 'push -u $i with push.default=matching' '
+		default_u_setup matching &&
+		test_might_fail git branch test_u &&
+		test_might_fail git branch test_u2 &&
+		git push upstream main:test_u2 &&
+		git push -u $i &&
+		check_config main upstream refs/heads/main &&
+		check_config test_u2 upstream refs/heads/test_u2 &&
+		check_empty_config test_u
+	'
+done
+
+test_expect_success 'push -u -d will fail' '
+	git checkout main &&
+	test_might_fail git branch --unset-upstream &&
+	test_must_fail git push -u -d
+'
+
+test_expect_success 'push -u --dry-run' '
+	git checkout main &&
+	git push -u upstream main:other &&
+	git push -u --dry-run &&
+	check_config main upstream refs/heads/other
+'
+
 test_expect_success 'push -u --all' '
 	git branch all1 &&
 	git branch all2 &&
@@ -81,6 +161,13 @@ test_expect_success TTY 'progress messages go to tty' '
 	test_i18ngrep "Writing objects" err
 '
 
+test_expect_success TTY 'progress messages go to tty with default -u' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push -u 2>err &&
+	test_i18ngrep "Writing objects" err
+'
+
 test_expect_success 'progress messages do not go to non-tty' '
 	ensure_fresh_upstream &&
 
@@ -89,6 +176,14 @@ test_expect_success 'progress messages do not go to non-tty' '
 	test_i18ngrep ! "Writing objects" err
 '
 
+test_expect_success 'progress messages do not go to non-tty (default -u)' '
+	ensure_fresh_upstream &&
+
+	# skip progress messages, since stderr is non-tty
+	git push -u 2>err &&
+	test_i18ngrep ! "Writing objects" err
+'
+
 test_expect_success 'progress messages go to non-tty (forced)' '
 	ensure_fresh_upstream &&
 
@@ -97,6 +192,14 @@ test_expect_success 'progress messages go to non-tty (forced)' '
 	test_i18ngrep "Writing objects" err
 '
 
+test_expect_success 'progress messages go to non-tty with default -u (forced)' '
+	ensure_fresh_upstream &&
+
+	# force progress messages to stderr, even though it is non-tty
+	git push -u --progress 2>err &&
+	test_i18ngrep "Writing objects" err
+'
+
 test_expect_success TTY 'push -q suppresses progress' '
 	ensure_fresh_upstream &&
 
@@ -104,6 +207,13 @@ test_expect_success TTY 'push -q suppresses progress' '
 	test_i18ngrep ! "Writing objects" err
 '
 
+test_expect_success TTY 'push -q suppresses progress (with default -u)' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push -u -q 2>err &&
+	test_i18ngrep ! "Writing objects" err
+'
+
 test_expect_success TTY 'push --no-progress suppresses progress' '
 	ensure_fresh_upstream &&
 
@@ -112,6 +222,14 @@ test_expect_success TTY 'push --no-progress suppresses progress' '
 	test_i18ngrep ! "Writing objects" err
 '
 
+test_expect_success TTY 'push --no-progress suppresses progress (default -u)' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push -u --no-progress 2>err &&
+	test_i18ngrep ! "Unpacking objects" err &&
+	test_i18ngrep ! "Writing objects" err
+'
+
 test_expect_success TTY 'quiet push' '
 	ensure_fresh_upstream &&
 
@@ -126,4 +244,11 @@ test_expect_success TTY 'quiet push -u' '
 	test_must_be_empty output
 '
 
+test_expect_success TTY 'quiet push -u (default -u)' '
+	ensure_fresh_upstream &&
+
+	test_terminal git push --quiet -u --no-progress 2>&1 | tee output &&
+	test_must_be_empty output
+'
+
 test_done
-- 
2.34.1

Re: [PATCH v4 1/1] push: make 'set-upstream' have dafault arguments

[Junio C Hamano]

Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> writes:

> Teach "git push -u" not to require repository and refspec.  When
> the user do not give what repository to push to, or which
> branch(es) to push, behave as if the default remote repository
> and a refspec (depending on the "push.default" configuration)
> are given.

That means if the user says push.default==nothing, we should error
out "git push -u" as before, but that is not what the change to
setup_default_push_refspecs() function does, is it?

> -static void setup_default_push_refspecs(struct remote *remote)
> +static void setup_default_push_refspecs(struct remote *remote, int flags)
>  {
>  	struct branch *branch;
>  	const char *dst;
>  	int same_remote;
> +	int is_default_u = (flags & TRANSPORT_PUSH_SET_UPSTREAM);
>  
>  	switch (push_default) {
>  	case PUSH_DEFAULT_MATCHING:
> @@ -214,6 +215,8 @@ static void setup_default_push_refspecs(struct remote *remote)
>  		return;
>  
>  	case PUSH_DEFAULT_NOTHING:
> +		if (is_default_u)
> +			break;
>  		die(_("You didn't specify any refspecs to push, and "
>  		    "push.default is \"nothing\"."));
>  		return;
> @@ -234,11 +237,15 @@ static void setup_default_push_refspecs(struct remote *remote)
>  	case PUSH_DEFAULT_SIMPLE:
>  		if (!same_remote)
>  			break;
> +		if (is_default_u)
> +			break;
>  		if (strcmp(branch->refname, get_upstream_ref(branch, remote->name)))
>  			die_push_simple(branch, remote);
>  		break;
>  
>  	case PUSH_DEFAULT_UPSTREAM:
> +		if (is_default_u)
> +			break;
>  		if (!same_remote)
>  			die(_("You are pushing to remote '%s', which is not the upstream of\n"
>  			      "your current branch '%s', without telling me what to push\n"

So, I am not sure if many of the above changes are sensible.  The
first one certainly does not sound like sensible.

> diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
> index fdb4292056..c2d11c3f2a 100755
> --- a/t/t5523-push-upstream.sh
> +++ b/t/t5523-push-upstream.sh
> @@ -60,6 +60,86 @@ test_expect_success 'push -u :topic_2' '
>  	check_config topic_2 upstream refs/heads/other2
>  '
>  
> +default_u_setup() {

Style. (cf. Documentation/CodingGuildelines).

> +	git checkout main &&
> +	test_might_fail	git branch --unset-upstream &&
> +	test_config push.default $1 &&
> +	test_config remote.pushDefault upstream
> +}
> +
> +check_empty_config() {

Likewise.

> +	test_expect_code 1 git config "branch.$1.remote" &&
> +	test_expect_code 1 git config "branch.$1.merge"
> +}
> +
> +for i in simple current upstream nothing
> +do
> +	test_expect_success 'push -u with push.default=$i' '
> +		default_u_setup $i &&
> +		git push -u &&
> +		check_config main upstream refs/heads/main &&
> +		git push -u upstream main:other &&
> +		git push -u &&
> +		check_config main upstream refs/heads/main
> +	'
> +
> +	test_expect_success 'push -u -f with push.default=$i' '
> +		default_u_setup $i &&
> +		git push -u -f &&
> +		check_config main upstream refs/heads/main
> +	'
> +done
> +
> +for i in simple current upstream nothing matching
> +do
> +	test_expect_success 'push -u --prune with push.default=$i' '
> +		default_u_setup $i &&
> +		git push upstream main:test_u215 &&
> +		git push -u --prune >out &&
> +		check_config main upstream refs/heads/main &&
> +		test_i18ngrep "[deleted]" out &&
> +		test_i18ngrep ! "Branch '"'"'test_u215'"'"' set up to track" out
> +	'
> +
> +	test_expect_success 'push -u --mirror with push.default=$i' '
> +		default_u_setup $i &&
> +		test_might_fail git branch mirror1 &&
> +		test_might_fail git branch mirror2 &&
> +		git push -u --mirror &&
> +		check_config main upstream  refs/heads/main &&
> +		check_config mirror1 upstream refs/heads/mirror1 &&
> +		check_config mirror2 upstream refs/heads/mirror2
> +	'
> +done
> +
> +for i in '' '-f'
> +do
> +
> +	test_expect_success 'push -u $i with push.default=matching' '

Doesn't $i show in the output as-is here?  Quote the test title in
double-quotes, while using single-qoutes around the test body.

> +		default_u_setup matching &&
> +		test_might_fail git branch test_u &&
> +		test_might_fail git branch test_u2 &&
> +		git push upstream main:test_u2 &&
> +		git push -u $i &&
> +		check_config main upstream refs/heads/main &&
> +		check_config test_u2 upstream refs/heads/test_u2 &&
> +		check_empty_config test_u
> +	'
> +done
> +

Re: [PATCH v4 1/1] push: make 'set-upstream' have dafault arguments

[Abhradeep Chakraborty]

Junio C Hamano <gitster@pobox.com> wrote:

> That means if the user says push.default==nothing, we should error
> out "git push -u" as before, but that is not what the change to
> setup_default_push_refspecs() function does, is it?

Yeah. You're right. The current change does not throw error for
push.default=nothing. Because I thought that for all values of
`push.default` (except matching), 'git push -u' should create a
new branch with the same name as the current local branch. Now, It
seems that I was wrong.

> So, I am not sure if many of the above changes are sensible.  The
> first one certainly does not sound like sensible.

Actually, I didn't think deeply while commiting the changes. Today,
I think about it deeply and I realized the following points.

* if push.default='simple' or unspecified then it should not create
  a new branch on the remote. So, my proposed change of 'git push -u'
  for push.default='simple' is badly affecting the reason why
  push.default='simple' was built for.

* if push.default='nothing', It should throw error if no <refspec> is
  provided. Again, my proposed change is hurting it.

* For push.default=upstream, If an upstream is already defined then
  'git push -u' should only set that branch as the upstream of the
  local branch. This already works in git. But if an upstream is not
  provied, it should throw error. So, I am not sure whether 'git push
  -u' (with no upstream information) should create a new branch with
  the same name or not. What do you think about that?

* For push.default=matching, 'git push -u' should set all the existing
  matching branches as upstream of their respective matching local
  branches. It also already works. Same for 'push.default'=current also.

So, to put all in a nutshell, I think that the current behaviour of
'git push -u' is okay. It also seems that he/she who built the
setup_default_push_refspecs() was aware of this.

Sorry for the patch request and thanks for reviewing.

> Doesn't $i show in the output as-is here?  Quote the test title in
> double-quotes, while using single-qoutes around the test body.

Yeah. I observed this while testing. But had no idea why this happend
( as I am very beginner in shell scripting). I was waiting for the review
comment for it.

Thanks again for reviewing my patch request.

Re: [PATCH v4 1/1] push: make 'set-upstream' have dafault arguments

[Junio C Hamano]

Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> writes:

> * For push.default=upstream, If an upstream is already defined then
>   'git push -u' should only set that branch as the upstream of the
>   local branch. This already works in git. But if an upstream is not
>   provied, it should throw error. So, I am not sure whether 'git push
>   -u' (with no upstream information) should create a new branch with
>   the same name or not. What do you think about that?

I think erring on the side of caution is more sensible than blindly
assuming that the user wants a new branch with the same name.

Thank you for working on the topic and thinking its ramifications
through.