* [RFC PATCH 0/1] making --set-upstream have default arguments @ 2021-12-02 14:43 Abhradeep Chakraborty 2021-12-02 14:43 ` [RFC PATCH 1/1] push: make '-u' " Abhradeep Chakraborty ` (2 more replies) 0 siblings, 3 replies; 21+ messages in thread From: Abhradeep Chakraborty @ 2021-12-02 14:43 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH 1/1] push: make '-u' have default arguments 2021-12-02 14:43 [RFC PATCH 0/1] making --set-upstream have default arguments Abhradeep Chakraborty @ 2021-12-02 14:43 ` Abhradeep Chakraborty 2021-12-02 18:24 ` Junio C Hamano 2021-12-03 11:32 ` [RFC PATCH 0/1] making --set-upstream " Philip Oakley 2021-12-07 18:22 ` [PATCH v2 " Abhradeep Chakraborty 2 siblings, 1 reply; 21+ messages in thread From: Abhradeep Chakraborty @ 2021-12-02 14:43 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 1/1] push: make '-u' have default arguments 2021-12-02 14:43 ` [RFC PATCH 1/1] push: make '-u' " Abhradeep Chakraborty @ 2021-12-02 18:24 ` Junio C Hamano 2021-12-03 8:14 ` Abhradeep Chakraborty 0 siblings, 1 reply; 21+ messages in thread From: Junio C Hamano @ 2021-12-02 18:24 UTC (permalink / raw) To: Abhradeep Chakraborty; +Cc: git 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 ;-) ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 1/1] push: make '-u' have default arguments 2021-12-02 18:24 ` Junio C Hamano @ 2021-12-03 8:14 ` Abhradeep Chakraborty 2021-12-03 17:29 ` Junio C Hamano 0 siblings, 1 reply; 21+ messages in thread From: Abhradeep Chakraborty @ 2021-12-03 8:14 UTC (permalink / raw) To: gitster; +Cc: git, 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 1/1] push: make '-u' have default arguments 2021-12-03 8:14 ` Abhradeep Chakraborty @ 2021-12-03 17:29 ` Junio C Hamano 2021-12-03 19:27 ` Abhradeep Chakraborty 0 siblings, 1 reply; 21+ messages in thread From: Junio C Hamano @ 2021-12-03 17:29 UTC (permalink / raw) To: Abhradeep Chakraborty; +Cc: git 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 1/1] push: make '-u' have default arguments 2021-12-03 17:29 ` Junio C Hamano @ 2021-12-03 19:27 ` Abhradeep Chakraborty 0 siblings, 0 replies; 21+ messages in thread From: Abhradeep Chakraborty @ 2021-12-03 19:27 UTC (permalink / raw) To: gitster; +Cc: git, 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 0/1] making --set-upstream have default arguments 2021-12-02 14:43 [RFC PATCH 0/1] making --set-upstream have default arguments Abhradeep Chakraborty 2021-12-02 14:43 ` [RFC PATCH 1/1] push: make '-u' " Abhradeep Chakraborty @ 2021-12-03 11:32 ` Philip Oakley 2021-12-03 16:03 ` Abhradeep Chakraborty 2021-12-03 17:28 ` Abhradeep Chakraborty 2021-12-07 18:22 ` [PATCH v2 " Abhradeep Chakraborty 2 siblings, 2 replies; 21+ messages in thread From: Philip Oakley @ 2021-12-03 11:32 UTC (permalink / raw) To: Abhradeep Chakraborty, git 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(-) > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 0/1] making --set-upstream have default arguments 2021-12-03 11:32 ` [RFC PATCH 0/1] making --set-upstream " Philip Oakley @ 2021-12-03 16:03 ` Abhradeep Chakraborty 2021-12-03 16:46 ` Philip Oakley 2021-12-03 17:28 ` Abhradeep Chakraborty 1 sibling, 1 reply; 21+ messages in thread From: Abhradeep Chakraborty @ 2021-12-03 16:03 UTC (permalink / raw) To: philipoakley; +Cc: git, 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 0/1] making --set-upstream have default arguments 2021-12-03 16:03 ` Abhradeep Chakraborty @ 2021-12-03 16:46 ` Philip Oakley 0 siblings, 0 replies; 21+ messages in thread From: Philip Oakley @ 2021-12-03 16:46 UTC (permalink / raw) To: Abhradeep Chakraborty; +Cc: git 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 0/1] making --set-upstream have default arguments 2021-12-03 11:32 ` [RFC PATCH 0/1] making --set-upstream " Philip Oakley 2021-12-03 16:03 ` Abhradeep Chakraborty @ 2021-12-03 17:28 ` Abhradeep Chakraborty 1 sibling, 0 replies; 21+ messages in thread From: Abhradeep Chakraborty @ 2021-12-03 17:28 UTC (permalink / raw) To: philipoakley; +Cc: git, 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! ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 0/1] making --set-upstream have default arguments 2021-12-02 14:43 [RFC PATCH 0/1] making --set-upstream have default arguments Abhradeep Chakraborty 2021-12-02 14:43 ` [RFC PATCH 1/1] push: make '-u' " Abhradeep Chakraborty 2021-12-03 11:32 ` [RFC PATCH 0/1] making --set-upstream " Philip Oakley @ 2021-12-07 18:22 ` Abhradeep Chakraborty 2021-12-07 18:23 ` [PATCH v2 1/1] push: make '-u' " Abhradeep Chakraborty 2021-12-09 10:15 ` [PATCH v3 0/1] making --set-upstream have default arguments Abhradeep Chakraborty 2 siblings, 2 replies; 21+ messages in thread From: Abhradeep Chakraborty @ 2021-12-07 18:22 UTC (permalink / raw) To: git; +Cc: Abhradeep Chakraborty, Junio C Hamano, Philip Oakley 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 1/1] push: make '-u' have default arguments 2021-12-07 18:22 ` [PATCH v2 " Abhradeep Chakraborty @ 2021-12-07 18:23 ` Abhradeep Chakraborty 2021-12-07 22:14 ` Eric Sunshine 2021-12-09 10:15 ` [PATCH v3 0/1] making --set-upstream have default arguments Abhradeep Chakraborty 1 sibling, 1 reply; 21+ messages in thread From: Abhradeep Chakraborty @ 2021-12-07 18:23 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/1] push: make '-u' have default arguments 2021-12-07 18:23 ` [PATCH v2 1/1] push: make '-u' " Abhradeep Chakraborty @ 2021-12-07 22:14 ` Eric Sunshine 2021-12-08 6:12 ` [PATCH v2 1/1] push: make '-u' have default argument Abhradeep Chakraborty 0 siblings, 1 reply; 21+ messages in thread From: Eric Sunshine @ 2021-12-07 22:14 UTC (permalink / raw) To: Abhradeep Chakraborty; +Cc: Git List 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/1] push: make '-u' have default argument 2021-12-07 22:14 ` Eric Sunshine @ 2021-12-08 6:12 ` Abhradeep Chakraborty 0 siblings, 0 replies; 21+ messages in thread From: Abhradeep Chakraborty @ 2021-12-08 6:12 UTC (permalink / raw) To: Eric Sunshine; +Cc: git, 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/1] making --set-upstream have default arguments 2021-12-07 18:22 ` [PATCH v2 " Abhradeep Chakraborty 2021-12-07 18:23 ` [PATCH v2 1/1] push: make '-u' " Abhradeep Chakraborty @ 2021-12-09 10:15 ` Abhradeep Chakraborty 2021-12-09 10:15 ` [PATCH v3 1/1] push: make '-u' " Abhradeep Chakraborty 2022-01-01 14:37 ` [PATCH v4 0/1] making --set-upstream " Abhradeep Chakraborty 1 sibling, 2 replies; 21+ messages in thread From: Abhradeep Chakraborty @ 2021-12-09 10:15 UTC (permalink / raw) To: git; +Cc: Abhradeep Chakraborty, Junio C Hamano, Philip Oakley, Eric Sunshine 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 1/1] push: make '-u' have default arguments 2021-12-09 10:15 ` [PATCH v3 0/1] making --set-upstream have default arguments Abhradeep Chakraborty @ 2021-12-09 10:15 ` Abhradeep Chakraborty 2022-01-01 14:37 ` [PATCH v4 0/1] making --set-upstream " Abhradeep Chakraborty 1 sibling, 0 replies; 21+ messages in thread From: Abhradeep Chakraborty @ 2021-12-09 10:15 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4 0/1] making --set-upstream have default arguments 2021-12-09 10:15 ` [PATCH v3 0/1] making --set-upstream have default arguments Abhradeep Chakraborty 2021-12-09 10:15 ` [PATCH v3 1/1] push: make '-u' " Abhradeep Chakraborty @ 2022-01-01 14:37 ` Abhradeep Chakraborty 2022-01-01 14:37 ` [PATCH v4 1/1] push: make 'set-upstream' have dafault arguments Abhradeep Chakraborty 1 sibling, 1 reply; 21+ messages in thread From: Abhradeep Chakraborty @ 2022-01-01 14:37 UTC (permalink / raw) To: git; +Cc: Abhradeep Chakraborty, Junio C Hamano, Philip Oakley, Eric Sunshine 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 1/1] push: make 'set-upstream' have dafault arguments 2022-01-01 14:37 ` [PATCH v4 0/1] making --set-upstream " Abhradeep Chakraborty @ 2022-01-01 14:37 ` Abhradeep Chakraborty 2022-01-04 3:46 ` Junio C Hamano 0 siblings, 1 reply; 21+ messages in thread From: Abhradeep Chakraborty @ 2022-01-01 14:37 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/1] push: make 'set-upstream' have dafault arguments 2022-01-01 14:37 ` [PATCH v4 1/1] push: make 'set-upstream' have dafault arguments Abhradeep Chakraborty @ 2022-01-04 3:46 ` Junio C Hamano 2022-01-04 13:28 ` Abhradeep Chakraborty 0 siblings, 1 reply; 21+ messages in thread From: Junio C Hamano @ 2022-01-04 3:46 UTC (permalink / raw) To: Abhradeep Chakraborty; +Cc: git 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 > + ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/1] push: make 'set-upstream' have dafault arguments 2022-01-04 3:46 ` Junio C Hamano @ 2022-01-04 13:28 ` Abhradeep Chakraborty 2022-01-04 20:35 ` Junio C Hamano 0 siblings, 1 reply; 21+ messages in thread From: Abhradeep Chakraborty @ 2022-01-04 13:28 UTC (permalink / raw) To: Junio C Hamano; +Cc: Abhradeep Chakraborty, git 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/1] push: make 'set-upstream' have dafault arguments 2022-01-04 13:28 ` Abhradeep Chakraborty @ 2022-01-04 20:35 ` Junio C Hamano 0 siblings, 0 replies; 21+ messages in thread From: Junio C Hamano @ 2022-01-04 20:35 UTC (permalink / raw) To: Abhradeep Chakraborty; +Cc: git 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2022-01-04 20:35 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-02 14:43 [RFC PATCH 0/1] making --set-upstream have default arguments Abhradeep Chakraborty 2021-12-02 14:43 ` [RFC PATCH 1/1] push: make '-u' " Abhradeep Chakraborty 2021-12-02 18:24 ` Junio C Hamano 2021-12-03 8:14 ` Abhradeep Chakraborty 2021-12-03 17:29 ` Junio C Hamano 2021-12-03 19:27 ` Abhradeep Chakraborty 2021-12-03 11:32 ` [RFC PATCH 0/1] making --set-upstream " Philip Oakley 2021-12-03 16:03 ` Abhradeep Chakraborty 2021-12-03 16:46 ` Philip Oakley 2021-12-03 17:28 ` Abhradeep Chakraborty 2021-12-07 18:22 ` [PATCH v2 " Abhradeep Chakraborty 2021-12-07 18:23 ` [PATCH v2 1/1] push: make '-u' " Abhradeep Chakraborty 2021-12-07 22:14 ` Eric Sunshine 2021-12-08 6:12 ` [PATCH v2 1/1] push: make '-u' have default argument Abhradeep Chakraborty 2021-12-09 10:15 ` [PATCH v3 0/1] making --set-upstream have default arguments Abhradeep Chakraborty 2021-12-09 10:15 ` [PATCH v3 1/1] push: make '-u' " Abhradeep Chakraborty 2022-01-01 14:37 ` [PATCH v4 0/1] making --set-upstream " Abhradeep Chakraborty 2022-01-01 14:37 ` [PATCH v4 1/1] push: make 'set-upstream' have dafault arguments Abhradeep Chakraborty 2022-01-04 3:46 ` Junio C Hamano 2022-01-04 13:28 ` Abhradeep Chakraborty 2022-01-04 20:35 ` Junio C Hamano
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.