[PATCH 0/2] advise about force-pushing as an alternative to reconciliation

[PATCH 0/2] advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

Many times now, I have seen novices do the following:

1. Start work on their own personal topic branch
2. Push the branch to origin
3. Rebase the branch onto origin/master
4. Try to push again, but Git says they need to pull
5. Pull and make a mess trying to reconcile the older topic branch with
   the rebased topic branch

Help avoid this mistake by giving advice that mentions force-pushing,
rather than assuming that the user always wants to do reconciliation.

Alex Henrie (2):
  remote: advise about force-pushing as an alternative to reconciliation
  push: advise about force-pushing as an alternative to reconciliation

 builtin/push.c | 22 +++++++++++++---------
 remote.c       |  3 ++-
 2 files changed, 15 insertions(+), 10 deletions(-)

-- 
2.41.0

[PATCH 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

Also, don't imply that `git pull` is only for merging.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 remote.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index a81f2e2f17..161d0cfe96 100644
--- a/remote.c
+++ b/remote.c
@@ -2323,7 +2323,8 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
 			base, ours, theirs);
 		if (advice_enabled(ADVICE_STATUS_HINTS))
 			strbuf_addstr(sb,
-				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
+				_("  (use \"git pull\" to reconcile your local branch with the remote branch,\n"
+				  "  or \"git push --force\" to overwrite the remote branch with your local branch)\n"));
 	}
 	free(base);
 	return 1;
-- 
2.41.0

[PATCH 2/2] push: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

Also, don't put `git pull` in an awkward parenthetical, because
`git pull` can always be used to reconcile branches and is the normal
way to do so.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 builtin/push.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 6f8a8dc711..9441c71bb0 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -301,21 +301,24 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
 
 static const char message_advice_pull_before_push[] =
 	N_("Updates were rejected because the tip of your current branch is behind\n"
-	   "its remote counterpart. Integrate the remote changes (e.g.\n"
-	   "'git pull ...') before pushing again.\n"
+	   "its remote counterpart. Use 'git pull' to integrate the remote changes\n"
+	   "before pushing again, or use 'git push --force' to delete the remote\n"
+	   "changes and replace them with your own.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_checkout_pull_push[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
-	   "counterpart. Check out this branch and integrate the remote changes\n"
-	   "(e.g. 'git pull ...') before pushing again.\n"
+	   "counterpart. Check out this branch and use 'git pull' to integrate the\n"
+	   "remote changes before pushing again, or use 'git push --force' to\n"
+	   "delete the remote changes and replace them with your own.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_ref_fetch_first[] =
 	N_("Updates were rejected because the remote contains work that you do\n"
 	   "not have locally. This is usually caused by another repository pushing\n"
-	   "to the same ref. You may want to first integrate the remote changes\n"
-	   "(e.g., 'git pull ...') before pushing again.\n"
+	   "to the same ref. Use 'git pull' to integrate the remote changes before\n"
+	   "pushing again, or use 'git push --force' to delete the remote changes\n"
+	   "and replace them with your own.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_ref_already_exists[] =
@@ -328,9 +331,10 @@ static const char message_advice_ref_needs_force[] =
 
 static const char message_advice_ref_needs_update[] =
 	N_("Updates were rejected because the tip of the remote-tracking\n"
-	   "branch has been updated since the last checkout. You may want\n"
-	   "to integrate those changes locally (e.g., 'git pull ...')\n"
-	   "before forcing an update.\n");
+	   "branch has been updated since the last checkout. Use 'git pull' to\n"
+	   "integrate the remote changes before pushing again, or use\n"
+	   "'git push --force' to delete the remote changes and replace them\n"
+	   "with your own.\n");
 
 static void advise_pull_before_push(void)
 {
-- 
2.41.0

Re: [PATCH 0/2] advise about force-pushing as an alternative to reconciliation

[Phillip Wood]

Hi Alex

On 02/07/2023 21:08, Alex Henrie wrote:
> Many times now, I have seen novices do the following:
> 
> 1. Start work on their own personal topic branch
> 2. Push the branch to origin
> 3. Rebase the branch onto origin/master
> 4. Try to push again, but Git says they need to pull
> 5. Pull and make a mess trying to reconcile the older topic branch with
>     the rebased topic branch
> 
> Help avoid this mistake by giving advice that mentions force-pushing,
> rather than assuming that the user always wants to do reconciliation.

I don't think we want to be advising users to force push. For the case 
you mention above I think it would be much safer to advise them to use

	git push --force-if-includes

In the absence of background fetches even

	git push --force-with-lease

is still safer than

	git push --force

Best Wishes

Phillip

> Alex Henrie (2):
>    remote: advise about force-pushing as an alternative to reconciliation
>    push: advise about force-pushing as an alternative to reconciliation
> 
>   builtin/push.c | 22 +++++++++++++---------
>   remote.c       |  3 ++-
>   2 files changed, 15 insertions(+), 10 deletions(-)
> 

Re: [PATCH 0/2] advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

On Mon, Jul 3, 2023 at 9:33 AM Phillip Wood <phillip.wood123@gmail.com> wrote:

> On 02/07/2023 21:08, Alex Henrie wrote:
> > Many times now, I have seen novices do the following:
> >
> > 1. Start work on their own personal topic branch
> > 2. Push the branch to origin
> > 3. Rebase the branch onto origin/master
> > 4. Try to push again, but Git says they need to pull
> > 5. Pull and make a mess trying to reconcile the older topic branch with
> >     the rebased topic branch
> >
> > Help avoid this mistake by giving advice that mentions force-pushing,
> > rather than assuming that the user always wants to do reconciliation.
>
> I don't think we want to be advising users to force push. For the case
> you mention above I think it would be much safer to advise them to use
>
>         git push --force-if-includes
>
> In the absence of background fetches even
>
>         git push --force-with-lease
>
> is still safer than
>
>         git push --force

Hi Phillip, thanks for the feedback. --force-with-lease would be fine.
I'll make that change in v2.

Regarding your other suggestion, --force-if-includes doesn't do
anything unless --force-with-lease is also specified, and I think
recommending that users always type --force-with-lease
--force-if-includes is a bit much to ask of them. It also could lead
to confusion if the user has decided to delete the local branch and
start over, and is now trying to push the new local branch over the
old one on the remote.

-Alex

[PATCH v2 0/2] advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

Many times now, I have seen novices do the following:

1. Start work on their own personal topic branch
2. Push the branch to origin
3. Rebase the branch onto origin/master
4. Try to push again, but Git says they need to pull
5. Pull and make a mess trying to reconcile the older topic branch with
   the rebased topic branch

Help avoid this mistake by giving advice that mentions force-pushing,
rather than assuming that the user always wants to do reconciliation.

Changes from v1:
- Recommend --force-with-lease instead of plain --force
- Consistently wrap messages to 72 characters

Alex Henrie (2):
  remote: advise about force-pushing as an alternative to reconciliation
  push: advise about force-pushing as an alternative to reconciliation

 builtin/push.c | 29 +++++++++++++++++------------
 remote.c       |  4 +++-
 2 files changed, 20 insertions(+), 13 deletions(-)

Range-diff against v1:
1:  48a9f6b1fa ! 1:  d0cb607225 remote: advise about force-pushing as an alternative to reconciliation
    @@ remote.c: int format_tracking_info(struct branch *branch, struct strbuf *sb,
      			strbuf_addstr(sb,
     -				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
     +				_("  (use \"git pull\" to reconcile your local branch with the remote branch,\n"
    -+				  "  or \"git push --force\" to overwrite the remote branch with your local branch)\n"));
    ++				  "  or \"git push --force-with-lease\" to overwrite the remote branch with\n"
    ++				  "  your local branch)\n"));
      	}
      	free(base);
      	return 1;
2:  0d47c23320 < -:  ---------- push: advise about force-pushing as an alternative to reconciliation
-:  ---------- > 2:  3295f0bb2b push: advise about force-pushing as an alternative to reconciliation
-- 
2.41.0

[PATCH v2 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

Also, don't imply that `git pull` is only for merging.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 remote.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index a81f2e2f17..009034ecde 100644
--- a/remote.c
+++ b/remote.c
@@ -2323,7 +2323,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
 			base, ours, theirs);
 		if (advice_enabled(ADVICE_STATUS_HINTS))
 			strbuf_addstr(sb,
-				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
+				_("  (use \"git pull\" to reconcile your local branch with the remote branch,\n"
+				  "  or \"git push --force-with-lease\" to overwrite the remote branch with\n"
+				  "  your local branch)\n"));
 	}
 	free(base);
 	return 1;
-- 
2.41.0

[PATCH v2 2/2] push: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

Also, don't put `git pull` in an awkward parenthetical, because
`git pull` can always be used to reconcile branches and is the normal
way to do so.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 builtin/push.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 6f8a8dc711..8e70046304 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -301,21 +301,25 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
 
 static const char message_advice_pull_before_push[] =
 	N_("Updates were rejected because the tip of your current branch is behind\n"
-	   "its remote counterpart. Integrate the remote changes (e.g.\n"
-	   "'git pull ...') before pushing again.\n"
+	   "its remote counterpart. Use 'git pull' to integrate the remote changes\n"
+	   "before pushing again, or use 'git push --force-with-lease' to delete the\n"
+	   "remote changes and replace them with your own.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_checkout_pull_push[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
-	   "counterpart. Check out this branch and integrate the remote changes\n"
-	   "(e.g. 'git pull ...') before pushing again.\n"
+	   "counterpart. Check out this branch and use 'git pull' to integrate the\n"
+	   "remote changes before pushing again, or use\n"
+	   "'git push --force-with-lease' to delete the remote changes and replace\n"
+	   "them with your own.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_ref_fetch_first[] =
-	N_("Updates were rejected because the remote contains work that you do\n"
-	   "not have locally. This is usually caused by another repository pushing\n"
-	   "to the same ref. You may want to first integrate the remote changes\n"
-	   "(e.g., 'git pull ...') before pushing again.\n"
+	N_("Updates were rejected because the remote contains work that you do not\n"
+	   "have locally. This is usually caused by another repository pushing to\n"
+	   "the same ref. Use 'git pull' to integrate the remote changes before\n"
+	   "pushing again, or use 'git push --force-with-lease' to delete the\n"
+	   "remote changes and replace them with your own.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_ref_already_exists[] =
@@ -327,10 +331,11 @@ static const char message_advice_ref_needs_force[] =
 	   "without using the '--force' option.\n");
 
 static const char message_advice_ref_needs_update[] =
-	N_("Updates were rejected because the tip of the remote-tracking\n"
-	   "branch has been updated since the last checkout. You may want\n"
-	   "to integrate those changes locally (e.g., 'git pull ...')\n"
-	   "before forcing an update.\n");
+	N_("Updates were rejected because the tip of the remote-tracking branch has\n"
+	   "been updated since the last checkout. Use 'git pull' to integrate the\n"
+	   "remote changes before pushing again, or use\n"
+	   "'git push --force-with-lease' to delete the remote changes and replace\n"
+	   "them with your own.\n");
 
 static void advise_pull_before_push(void)
 {
-- 
2.41.0

Re: [PATCH 0/2] advise about force-pushing as an alternative to reconciliation

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Alex
>
> On 02/07/2023 21:08, Alex Henrie wrote:
>> Many times now, I have seen novices do the following:
>> 1. Start work on their own personal topic branch
>> 2. Push the branch to origin

And did this succeed, or did this fail?  I'd assume that it failed,
because othrewise you would not be rebasing your work done in #1 on
top of what the central repository has.  Also ...

>> 3. Rebase the branch onto origin/master

... the user's better have done "git fetch" to update origin/master
before this step.  And that means this can just be done with "git
pull --rebase" (or you may already have configured pull to do so).

In any case, assuming that this was indeed the intention of the
user, i.e. the user never wanted to discard the changes made in the
central repository (presumably by others)...

>> 4. Try to push again, but Git says they need to pull

... if this happened, it is because somebody else pushed in the
meantime, right?  Then ...

>> 5. Pull and make a mess trying to reconcile the older topic branch with
>>     the rebased topic branch

... this means that somebody else's work was something that
overlapped with what you did in #1, and then you do want to clean up
the mess carefully, so that you do not lose the work by that
somebody else.  So ...

>> Help avoid this mistake by giving advice that mentions
>> force-pushing,

... why would it possibly be a good idea to suggest force pushing,
which discards other's work?  I do not quite understand.

> I don't think we want to be advising users to force push. For the case
> you mention above I think it would be much safer to advise them to use
>
> 	git push --force-if-includes
>
> In the absence of background fetches even
>
> 	git push --force-with-lease
>
> is still safer than
>
> 	git push --force

Absolutely.  git push --force-with-lease=$(git merge-base HEAD origin/master)
or something, perhaps, would be even better.

Thanks.

Re: [PATCH v2 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Junio C Hamano]

Alex Henrie <alexhenrie24@gmail.com> writes:

> Also, don't imply that `git pull` is only for merging.
>
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  remote.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/remote.c b/remote.c
> index a81f2e2f17..009034ecde 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2323,7 +2323,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
>  			base, ours, theirs);
>  		if (advice_enabled(ADVICE_STATUS_HINTS))
>  			strbuf_addstr(sb,
> -				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
> +				_("  (use \"git pull\" to reconcile your local branch with the remote branch,\n"
> +				  "  or \"git push --force-with-lease\" to overwrite the remote branch with\n"
> +				  "  your local branch)\n"));
>  	}
>  	free(base);
>  	return 1;

Use of --force-with-lease without which commit you assume to be at
the tip of their branch is just as risky as blind use of --force.

As I said in a separate message, I do not think "reconcile" and
"force" cannot both be sensible choices at the same time.  If the
user wants not to lose the work by themselves and by others,
reconciling would be the only sensible choice and forcing cannot be
a sane substitute for that (if the user knows what is at the tip of
central repository is wrong and wants to get rid of it, forcing
would be a very sensible choice, but then reconciling would not be a
subsitute for that in such a case---"merge --ours" does not count as
"reconciling").

So, I'd suggest to make it a bit more clear that they are not
alternatives in the message, and discourage forcing in the first
place by using not "overwrite" but a bit stronger word, like discard
or destroy.  e.g.

    To reconcile your local changes with the work at the remote, you
    can use 'git pull' and then 'git push'.  To discard the work at
    the remote and replace it with what you did (alone), you can use
    'git push --force'.

or something, perhaps.

Thanks.

Re: [PATCH 0/2] advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

On Tue, Jul 4, 2023 at 3:44 PM Junio C Hamano <gitster@pobox.com> wrote:

> > On 02/07/2023 21:08, Alex Henrie wrote:
> >> Many times now, I have seen novices do the following:
> >> 1. Start work on their own personal topic branch
> >> 2. Push the branch to origin
>
> And did this succeed, or did this fail?  I'd assume that it failed,
> because othrewise you would not be rebasing your work done in #1 on
> top of what the central repository has.  Also ...

It succeeded; the new branch was created on origin.

> >> 3. Rebase the branch onto origin/master
>
> ... the user's better have done "git fetch" to update origin/master
> before this step.  And that means this can just be done with "git
> pull --rebase" (or you may already have configured pull to do so).

Yes, the rebase was performed with `git pull -r origin master`. Other
work had been done on master while the topic branch was being worked
on.

> In any case, assuming that this was indeed the intention of the
> user, i.e. the user never wanted to discard the changes made in the
> central repository (presumably by others)...

The user did not want to discard anything from master, but they
absolutely did want to discard the obsolete version of their own
branch, which they made themself.

> >> 4. Try to push again, but Git says they need to pull
>
> ... if this happened, it is because somebody else pushed in the
> meantime, right?  Then ...

No one pushed between steps 3 and 4.

> >> 5. Pull and make a mess trying to reconcile the older topic branch with
> >>     the rebased topic branch
>
> ... this means that somebody else's work was something that
> overlapped with what you did in #1, and then you do want to clean up
> the mess carefully, so that you do not lose the work by that
> somebody else.  So ...

The conflicts came from trying to reconcile an older version of the
user's work with a rebased version of the user's work. The user
doesn't want to end up with a history that has two commits from the
same author with the same message.

> >> Help avoid this mistake by giving advice that mentions
> >> force-pushing,
>
> ... why would it possibly be a good idea to suggest force pushing,
> which discards other's work?  I do not quite understand.

The user was only trying to overwrite the old version of their own
branch, which no one cares about. This scenario comes up commonly when
using a single shared remote repository, or when using a forked remote
repository and running `git pull -r upstream master` to incorporate
changes from the upstream remote repository.

I hope that's more clear now. Please let me know if it's not.

-Alex

Re: [PATCH v2 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

On Tue, Jul 4, 2023 at 3:52 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Alex Henrie <alexhenrie24@gmail.com> writes:
>
> > Also, don't imply that `git pull` is only for merging.
> >
> > Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> > ---
> >  remote.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/remote.c b/remote.c
> > index a81f2e2f17..009034ecde 100644
> > --- a/remote.c
> > +++ b/remote.c
> > @@ -2323,7 +2323,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
> >                       base, ours, theirs);
> >               if (advice_enabled(ADVICE_STATUS_HINTS))
> >                       strbuf_addstr(sb,
> > -                             _("  (use \"git pull\" to merge the remote branch into yours)\n"));
> > +                             _("  (use \"git pull\" to reconcile your local branch with the remote branch,\n"
> > +                               "  or \"git push --force-with-lease\" to overwrite the remote branch with\n"
> > +                               "  your local branch)\n"));
> >       }
> >       free(base);
> >       return 1;
>
> Use of --force-with-lease without which commit you assume to be at
> the tip of their branch is just as risky as blind use of --force.
>
> As I said in a separate message, I do not think "reconcile" and
> "force" cannot both be sensible choices at the same time.  If the
> user wants not to lose the work by themselves and by others,
> reconciling would be the only sensible choice and forcing cannot be
> a sane substitute for that (if the user knows what is at the tip of
> central repository is wrong and wants to get rid of it, forcing
> would be a very sensible choice, but then reconciling would not be a
> subsitute for that in such a case---"merge --ours" does not count as
> "reconciling").
>
> So, I'd suggest to make it a bit more clear that they are not
> alternatives in the message, and discourage forcing in the first
> place by using not "overwrite" but a bit stronger word, like discard
> or destroy.  e.g.
>
>     To reconcile your local changes with the work at the remote, you
>     can use 'git pull' and then 'git push'.  To discard the work at
>     the remote and replace it with what you did (alone), you can use
>     'git push --force'.
>
> or something, perhaps.

I think we're splitting hairs about what the word "alternative" means.
The user has two ways to get their work onto the remote branch:
Reconcile it with what's already there, or delete what's already
there. I would call those two ways "alternatives" and that does not
imply that either one is always a sensible choice, but if you can
think of a word that's more clear, I'm happy to use it instead in
these commit messages.

At any rate, your proposed wording for the advice message in remote.c
is perfectly fine. Thanks, I'll use it in v3. Would you like the same
text in push.c, or was my proposed text there already OK?

-Alex

Re: [PATCH 0/2] advise about force-pushing as an alternative to reconciliation

[Junio C Hamano]

Alex Henrie <alexhenrie24@gmail.com> writes:

> I hope that's more clear now. Please let me know if it's not.

I think the description in the cover was prone to be misunderstood,
but I think I got it now.  Where you are pushing from your topic
branch is your "publishing" branch that only you would push into,
and the primary way you update it is by rebasing your local copy of
it on the updated 'master' branch to keep up with others' work
integrated into the shared 'master'.

In such a workflow, the way to update your "publishing" branch will
normally be to force push to overwrite.  And in this very narrow use
case, where nobody else is pushing into your "publishing" branch,
your remote-tracking branch would be always up-to-date with the
remote repository and use of --force-with-lease that does not say
which commit you expect there to be is safe.  In fact, you do not
even have to use --force-with-lease in such a use case, because its
additional safety (relative to --force) relies on the assumption
that you would be the only one who is pushing into the remote
repository to update that branch---and at that point, --force
without lease is just as good.

Thanks.

Re: [PATCH 0/2] advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

On Tue, Jul 4, 2023 at 11:30 PM Junio C Hamano <gitster@pobox.com> wrote:

> I think the description in the cover was prone to be misunderstood,
> but I think I got it now.  Where you are pushing from your topic
> branch is your "publishing" branch that only you would push into,
> and the primary way you update it is by rebasing your local copy of
> it on the updated 'master' branch to keep up with others' work
> integrated into the shared 'master'.

Right. It's a narrow case, but it's quite common. For example, GitHub
and GitLab keep permanent references to every version of a branch that
was pushed while a pull request was open for that branch. On those
platforms, force-pushing is analogous to emailing a new version of a
patchset.

> In such a workflow, the way to update your "publishing" branch will
> normally be to force push to overwrite.  And in this very narrow use
> case, where nobody else is pushing into your "publishing" branch,
> your remote-tracking branch would be always up-to-date with the
> remote repository and use of --force-with-lease that does not say
> which commit you expect there to be is safe.  In fact, you do not
> even have to use --force-with-lease in such a use case, because its
> additional safety (relative to --force) relies on the assumption
> that you would be the only one who is pushing into the remote
> repository to update that branch---and at that point, --force
> without lease is just as good.

OK, I'll switch back to --force in v3. Thanks for the feedback!

-Alex

[PATCH v3 0/2] advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

Many times now, I have seen novices do the following:

1. Start work on their own personal topic branch
2. Push the branch to origin
3. Rebase the branch onto origin/master
4. Try to push again, but Git says they need to pull
5. Pull and make a mess trying to reconcile the older topic branch with
   the rebased topic branch

Help avoid this mistake by giving advice that mentions force-pushing,
rather than assuming that the user always wants to do reconciliation.

Changes from v2:
- Switch back to recommending plain --force in these cases
- Use Junio's proposed wording in remote.c

Alex Henrie (2):
  remote: advise about force-pushing as an alternative to reconciliation
  push: advise about force-pushing as an alternative to reconciliation

 builtin/push.c | 27 +++++++++++++++------------
 remote.c       |  5 ++++-
 2 files changed, 19 insertions(+), 13 deletions(-)

Range-diff against v2:
1:  d0cb607225 < -:  ---------- remote: advise about force-pushing as an alternative to reconciliation
-:  ---------- > 1:  9cbf5f138e remote: advise about force-pushing as an alternative to reconciliation
2:  3295f0bb2b ! 2:  727e1f7636 push: advise about force-pushing as an alternative to reconciliation
    @@ builtin/push.c: static void setup_default_push_refspecs(int *flags, struct remot
     -	   "its remote counterpart. Integrate the remote changes (e.g.\n"
     -	   "'git pull ...') before pushing again.\n"
     +	   "its remote counterpart. Use 'git pull' to integrate the remote changes\n"
    -+	   "before pushing again, or use 'git push --force-with-lease' to delete the\n"
    -+	   "remote changes and replace them with your own.\n"
    ++	   "before pushing again, or use 'git push --force' to delete the remote\n"
    ++	   "changes and replace them with your own.\n"
      	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
      
      static const char message_advice_checkout_pull_push[] =
    @@ builtin/push.c: static void setup_default_push_refspecs(int *flags, struct remot
     -	   "counterpart. Check out this branch and integrate the remote changes\n"
     -	   "(e.g. 'git pull ...') before pushing again.\n"
     +	   "counterpart. Check out this branch and use 'git pull' to integrate the\n"
    -+	   "remote changes before pushing again, or use\n"
    -+	   "'git push --force-with-lease' to delete the remote changes and replace\n"
    -+	   "them with your own.\n"
    ++	   "remote changes before pushing again, or use 'git push --force' to delete\n"
    ++	   "the remote changes and replace them with your own.\n"
      	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
      
      static const char message_advice_ref_fetch_first[] =
    @@ builtin/push.c: static void setup_default_push_refspecs(int *flags, struct remot
     +	N_("Updates were rejected because the remote contains work that you do not\n"
     +	   "have locally. This is usually caused by another repository pushing to\n"
     +	   "the same ref. Use 'git pull' to integrate the remote changes before\n"
    -+	   "pushing again, or use 'git push --force-with-lease' to delete the\n"
    -+	   "remote changes and replace them with your own.\n"
    ++	   "pushing again, or use 'git push --force' to delete the remote changes\n"
    ++	   "and replace them with your own.\n"
      	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
      
      static const char message_advice_ref_already_exists[] =
    @@ builtin/push.c: static const char message_advice_ref_needs_force[] =
     -	   "before forcing an update.\n");
     +	N_("Updates were rejected because the tip of the remote-tracking branch has\n"
     +	   "been updated since the last checkout. Use 'git pull' to integrate the\n"
    -+	   "remote changes before pushing again, or use\n"
    -+	   "'git push --force-with-lease' to delete the remote changes and replace\n"
    -+	   "them with your own.\n");
    ++	   "remote changes before pushing again, or use 'git push --force' to delete\n"
    ++	   "the remote changes and replace them with your own.\n");
      
      static void advise_pull_before_push(void)
      {
-- 
2.41.0

[PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

Also, don't imply that `git pull` is only for merging.

Co-authored-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 remote.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index a81f2e2f17..1fe86f8b23 100644
--- a/remote.c
+++ b/remote.c
@@ -2323,7 +2323,10 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
 			base, ours, theirs);
 		if (advice_enabled(ADVICE_STATUS_HINTS))
 			strbuf_addstr(sb,
-				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
+				_("  (To reconcile your local changes with the work at the remote, you can\n"
+				  "  use 'git pull' and then 'git push'. To discard the work at the remote\n"
+				  "  and replace it with what you did (alone), you can use\n"
+				  "  'git push --force'.)\n"));
 	}
 	free(base);
 	return 1;
-- 
2.41.0

[PATCH v3 2/2] push: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

Also, don't put `git pull` in an awkward parenthetical, because
`git pull` can always be used to reconcile branches and is the normal
way to do so.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 builtin/push.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 6f8a8dc711..b2f0a64e7c 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -301,21 +301,24 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
 
 static const char message_advice_pull_before_push[] =
 	N_("Updates were rejected because the tip of your current branch is behind\n"
-	   "its remote counterpart. Integrate the remote changes (e.g.\n"
-	   "'git pull ...') before pushing again.\n"
+	   "its remote counterpart. Use 'git pull' to integrate the remote changes\n"
+	   "before pushing again, or use 'git push --force' to delete the remote\n"
+	   "changes and replace them with your own.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_checkout_pull_push[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
-	   "counterpart. Check out this branch and integrate the remote changes\n"
-	   "(e.g. 'git pull ...') before pushing again.\n"
+	   "counterpart. Check out this branch and use 'git pull' to integrate the\n"
+	   "remote changes before pushing again, or use 'git push --force' to delete\n"
+	   "the remote changes and replace them with your own.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_ref_fetch_first[] =
-	N_("Updates were rejected because the remote contains work that you do\n"
-	   "not have locally. This is usually caused by another repository pushing\n"
-	   "to the same ref. You may want to first integrate the remote changes\n"
-	   "(e.g., 'git pull ...') before pushing again.\n"
+	N_("Updates were rejected because the remote contains work that you do not\n"
+	   "have locally. This is usually caused by another repository pushing to\n"
+	   "the same ref. Use 'git pull' to integrate the remote changes before\n"
+	   "pushing again, or use 'git push --force' to delete the remote changes\n"
+	   "and replace them with your own.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_ref_already_exists[] =
@@ -327,10 +330,10 @@ static const char message_advice_ref_needs_force[] =
 	   "without using the '--force' option.\n");
 
 static const char message_advice_ref_needs_update[] =
-	N_("Updates were rejected because the tip of the remote-tracking\n"
-	   "branch has been updated since the last checkout. You may want\n"
-	   "to integrate those changes locally (e.g., 'git pull ...')\n"
-	   "before forcing an update.\n");
+	N_("Updates were rejected because the tip of the remote-tracking branch has\n"
+	   "been updated since the last checkout. Use 'git pull' to integrate the\n"
+	   "remote changes before pushing again, or use 'git push --force' to delete\n"
+	   "the remote changes and replace them with your own.\n");
 
 static void advise_pull_before_push(void)
 {
-- 
2.41.0

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Junio C Hamano]

Alex Henrie <alexhenrie24@gmail.com> writes:

> Also, don't imply that `git pull` is only for merging.
>
> Co-authored-by: Junio C Hamano <gitster@pobox.com>

I appreciate, but do not need, the credit; in any way, I didn't
co-author this one.

> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  remote.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/remote.c b/remote.c
> index a81f2e2f17..1fe86f8b23 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2323,7 +2323,10 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
>  			base, ours, theirs);
>  		if (advice_enabled(ADVICE_STATUS_HINTS))
>  			strbuf_addstr(sb,
> -				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
> +				_("  (To reconcile your local changes with the work at the remote, you can\n"
> +				  "  use 'git pull' and then 'git push'. To discard the work at the remote\n"
> +				  "  and replace it with what you did (alone), you can use\n"
> +				  "  'git push --force'.)\n"));
>  	}

Since wt-status.c:wt_longstatus_print_tracking() calls this
function, I would expect that this change would manifest as test
breakage in "git status" (or "git commit" whose commit log edit
buffer is examined) tests.  Are we lacking test coverage?

Thanks.

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Junio C Hamano]

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

>> diff --git a/remote.c b/remote.c
>> index a81f2e2f17..1fe86f8b23 100644
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -2323,7 +2323,10 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
>>  			base, ours, theirs);
>>  		if (advice_enabled(ADVICE_STATUS_HINTS))
>>  			strbuf_addstr(sb,
>> -				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
>> +				_("  (To reconcile your local changes with the work at the remote, you can\n"
>> +				  "  use 'git pull' and then 'git push'. To discard the work at the remote\n"
>> +				  "  and replace it with what you did (alone), you can use\n"
>> +				  "  'git push --force'.)\n"));
>>  	}
>
> Since wt-status.c:wt_longstatus_print_tracking() calls this
> function, I would expect that this change would manifest as test
> breakage in "git status" (or "git commit" whose commit log edit
> buffer is examined) tests.  Are we lacking test coverage?

The other callsite of format_tracking_info() is "git checkout".
When you start working on your own topic forked from upstream by
switching to it, if Git notices that your topic's base has become
behind (so that you would later need to merge or rebase to avoid
losing others' work), the "git pull" message is given to tell you
that it is OK if you want to catch up first before working on it.

But the new message does not fit well in the workflow.  It is
primarily targetted for the users who are about to push out.  They
are at the point where they are way before being ready to "discard
the work at the remote".

I guess the updated message in the context of "git status" has
exactly the same issue.  The user is about to make a commit, which
will later be pushed out.

So, while I agree that new users may need to be made aware of
situations where they should not afraid of overwriting the remote
repository by forcing a non-ff push, I am not sure if this is a good
advice message to convey it.

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

On Thu, Jul 6, 2023 at 2:40 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> >> diff --git a/remote.c b/remote.c
> >> index a81f2e2f17..1fe86f8b23 100644
> >> --- a/remote.c
> >> +++ b/remote.c
> >> @@ -2323,7 +2323,10 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
> >>                      base, ours, theirs);
> >>              if (advice_enabled(ADVICE_STATUS_HINTS))
> >>                      strbuf_addstr(sb,
> >> -                            _("  (use \"git pull\" to merge the remote branch into yours)\n"));
> >> +                            _("  (To reconcile your local changes with the work at the remote, you can\n"
> >> +                              "  use 'git pull' and then 'git push'. To discard the work at the remote\n"
> >> +                              "  and replace it with what you did (alone), you can use\n"
> >> +                              "  'git push --force'.)\n"));
> >>      }
> >
> > Since wt-status.c:wt_longstatus_print_tracking() calls this
> > function, I would expect that this change would manifest as test
> > breakage in "git status" (or "git commit" whose commit log edit
> > buffer is examined) tests.  Are we lacking test coverage?

Because I was only changing advice messages and not any functionality,
I didn't think to run the tests. They are indeed failing, sorry. I
will fix that in v4.

> The other callsite of format_tracking_info() is "git checkout".
> When you start working on your own topic forked from upstream by
> switching to it, if Git notices that your topic's base has become
> behind (so that you would later need to merge or rebase to avoid
> losing others' work), the "git pull" message is given to tell you
> that it is OK if you want to catch up first before working on it.
>
> But the new message does not fit well in the workflow.  It is
> primarily targetted for the users who are about to push out.  They
> are at the point where they are way before being ready to "discard
> the work at the remote".

If the branch is merely behind, format_tracking_info prints "(use "git
pull" to update your local branch)", which is perfectly reasonable.
The problem is only with the message that appears when the branches
are divergent, "(use "git pull" to merge the remote branch into
yours)", which is bad advice for the common GitHub/GitLab workflow
that expects force-pushing.

> I guess the updated message in the context of "git status" has
> exactly the same issue.  The user is about to make a commit, which
> will later be pushed out.
>
> So, while I agree that new users may need to be made aware of
> situations where they should not afraid of overwriting the remote
> repository by forcing a non-ff push, I am not sure if this is a good
> advice message to convey it.

For more context, the coworker who most recently had this problem
tried to pull because he looked at `git status` _after_ committing.
Git can't assume that certain commands go with certain workflows (at
least, not when it comes to divergent branches). Even if the user
switches to a different branch and switches back, the first branch
might be divergent simply because the user forgot to force-push before
switching off of it. So, let's please give the user all of the
information (two ways forward: reconcile or delete) and encourage them
to make the most appropriate decision for their particular workflow.

-Alex

[PATCH v4 0/2] advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

Many times now, I have seen novices do the following:

1. Start work on their own personal topic branch
2. Push the branch to origin
3. Rebase the branch onto origin/master
4. Try to push again, but Git says they need to pull
5. Pull and make a mess trying to reconcile the older topic branch with
   the rebased topic branch

Help avoid this mistake by giving advice that mentions force-pushing,
rather than assuming that the user always wants to do reconciliation.

Changes from v3:
- Update the tests
- Don't explicitly credit Junio

Alex Henrie (2):
  remote: advise about force-pushing as an alternative to reconciliation
  push: advise about force-pushing as an alternative to reconciliation

 builtin/push.c    |  27 +++++++-----
 remote.c          |   5 ++-
 t/t7508-status.sh | 110 ++++++++++++++++++++++++++++++++++++----------
 3 files changed, 107 insertions(+), 35 deletions(-)

Range-diff against v3:
1:  9cbf5f138e < -:  ---------- remote: advise about force-pushing as an alternative to reconciliation
-:  ---------- > 1:  9626721c13 remote: advise about force-pushing as an alternative to reconciliation
2:  727e1f7636 = 2:  209e86588a push: advise about force-pushing as an alternative to reconciliation
-- 
2.41.0

[PATCH v4 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

Also, don't imply that `git pull` is only for merging.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 remote.c          |   5 ++-
 t/t7508-status.sh | 110 ++++++++++++++++++++++++++++++++++++----------
 2 files changed, 92 insertions(+), 23 deletions(-)

diff --git a/remote.c b/remote.c
index a81f2e2f17..1fe86f8b23 100644
--- a/remote.c
+++ b/remote.c
@@ -2323,7 +2323,10 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
 			base, ours, theirs);
 		if (advice_enabled(ADVICE_STATUS_HINTS))
 			strbuf_addstr(sb,
-				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
+				_("  (To reconcile your local changes with the work at the remote, you can\n"
+				  "  use 'git pull' and then 'git push'. To discard the work at the remote\n"
+				  "  and replace it with what you did (alone), you can use\n"
+				  "  'git push --force'.)\n"));
 	}
 	free(base);
 	return 1;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 36567708f5..2a17e84793 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -92,7 +92,10 @@ test_expect_success 'status --column' '
 # On branch main
 # Your branch and '\''upstream'\'' have diverged,
 # and have 1 and 2 different commits each, respectively.
-#   (use "git pull" to merge the remote branch into yours)
+#   (To reconcile your local changes with the work at the remote, you can
+#   use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+#   and replace it with what you did (alone), you can use
+#   '\''git push --force'\''.)
 #
 # Changes to be committed:
 #   (use "git restore --staged <file>..." to unstage)
@@ -123,7 +126,10 @@ cat >expect <<\EOF
 # On branch main
 # Your branch and 'upstream' have diverged,
 # and have 1 and 2 different commits each, respectively.
-#   (use "git pull" to merge the remote branch into yours)
+#   (To reconcile your local changes with the work at the remote, you can
+#   use 'git pull' and then 'git push'. To discard the work at the remote
+#   and replace it with what you did (alone), you can use
+#   'git push --force'.)
 #
 # Changes to be committed:
 #   (use "git restore --staged <file>..." to unstage)
@@ -270,7 +276,10 @@ test_expect_success 'status with gitignore' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -335,7 +344,10 @@ test_expect_success 'status with gitignore (nothing untracked)' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -405,7 +417,10 @@ test_expect_success 'status -uno' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -467,7 +482,10 @@ test_expect_success 'status -unormal' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -522,7 +540,10 @@ test_expect_success 'status -uall' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -582,7 +603,10 @@ test_expect_success 'status with relative paths' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -650,7 +674,10 @@ test_expect_success TTY 'status with color.ui' '
 On branch <GREEN>main<RESET>
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -773,7 +800,10 @@ test_expect_success 'status without relative paths' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -847,7 +877,10 @@ test_expect_success 'dry-run of partial commit excluding new file in index' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -901,7 +934,10 @@ test_expect_success 'status submodule summary is disabled by default' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -958,7 +994,10 @@ test_expect_success 'status submodule summary' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -1017,7 +1056,10 @@ test_expect_success 'status submodule summary (clean submodule): commit' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes not staged for commit:
   (use "git add <file>..." to update what will be committed)
@@ -1065,7 +1107,10 @@ test_expect_success 'commit --dry-run submodule summary (--amend)' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --source=HEAD^1 --staged <file>..." to unstage)
@@ -1117,7 +1162,10 @@ test_expect_success '--ignore-submodules=untracked suppresses submodules with un
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -1226,7 +1274,10 @@ test_expect_success "--ignore-submodules=untracked doesn't suppress submodules w
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -1283,7 +1334,10 @@ test_expect_success "--ignore-submodules=untracked doesn't suppress submodule su
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -1364,7 +1418,10 @@ cat > expect << EOF
 ; On branch main
 ; Your branch and 'upstream' have diverged,
 ; and have 2 and 2 different commits each, respectively.
-;   (use "git pull" to merge the remote branch into yours)
+;   (To reconcile your local changes with the work at the remote, you can
+;   use 'git pull' and then 'git push'. To discard the work at the remote
+;   and replace it with what you did (alone), you can use
+;   'git push --force'.)
 ;
 ; Changes to be committed:
 ;   (use "git restore --staged <file>..." to unstage)
@@ -1412,7 +1469,10 @@ test_expect_success "--ignore-submodules=all suppresses submodule summary" '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes not staged for commit:
   (use "git add <file>..." to update what will be committed)
@@ -1438,7 +1498,10 @@ test_expect_success '.gitmodules ignore=all suppresses unstaged submodule summar
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -1558,7 +1621,10 @@ test_expect_success 'git commit --dry-run will show a staged but ignored submodu
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (To reconcile your local changes with the work at the remote, you can
+  use '\''git pull'\'' and then '\''git push'\''. To discard the work at the remote
+  and replace it with what you did (alone), you can use
+  '\''git push --force'\''.)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
-- 
2.41.0

[PATCH v4 2/2] push: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

Also, don't put `git pull` in an awkward parenthetical, because
`git pull` can always be used to reconcile branches and is the normal
way to do so.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 builtin/push.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 6f8a8dc711..b2f0a64e7c 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -301,21 +301,24 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
 
 static const char message_advice_pull_before_push[] =
 	N_("Updates were rejected because the tip of your current branch is behind\n"
-	   "its remote counterpart. Integrate the remote changes (e.g.\n"
-	   "'git pull ...') before pushing again.\n"
+	   "its remote counterpart. Use 'git pull' to integrate the remote changes\n"
+	   "before pushing again, or use 'git push --force' to delete the remote\n"
+	   "changes and replace them with your own.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_checkout_pull_push[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
-	   "counterpart. Check out this branch and integrate the remote changes\n"
-	   "(e.g. 'git pull ...') before pushing again.\n"
+	   "counterpart. Check out this branch and use 'git pull' to integrate the\n"
+	   "remote changes before pushing again, or use 'git push --force' to delete\n"
+	   "the remote changes and replace them with your own.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_ref_fetch_first[] =
-	N_("Updates were rejected because the remote contains work that you do\n"
-	   "not have locally. This is usually caused by another repository pushing\n"
-	   "to the same ref. You may want to first integrate the remote changes\n"
-	   "(e.g., 'git pull ...') before pushing again.\n"
+	N_("Updates were rejected because the remote contains work that you do not\n"
+	   "have locally. This is usually caused by another repository pushing to\n"
+	   "the same ref. Use 'git pull' to integrate the remote changes before\n"
+	   "pushing again, or use 'git push --force' to delete the remote changes\n"
+	   "and replace them with your own.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_ref_already_exists[] =
@@ -327,10 +330,10 @@ static const char message_advice_ref_needs_force[] =
 	   "without using the '--force' option.\n");
 
 static const char message_advice_ref_needs_update[] =
-	N_("Updates were rejected because the tip of the remote-tracking\n"
-	   "branch has been updated since the last checkout. You may want\n"
-	   "to integrate those changes locally (e.g., 'git pull ...')\n"
-	   "before forcing an update.\n");
+	N_("Updates were rejected because the tip of the remote-tracking branch has\n"
+	   "been updated since the last checkout. Use 'git pull' to integrate the\n"
+	   "remote changes before pushing again, or use 'git push --force' to delete\n"
+	   "the remote changes and replace them with your own.\n");
 
 static void advise_pull_before_push(void)
 {
-- 
2.41.0

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Phillip Wood]

Hi Alex

On 06/07/2023 05:01, Alex Henrie wrote:
> Also, don't imply that `git pull` is only for merging.

While the cover letter gives some background for the reason behind this 
change the commit message does not explain why the proposed changes are 
desirable.

> @@ -2323,7 +2323,10 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
>   			base, ours, theirs);
>   		if (advice_enabled(ADVICE_STATUS_HINTS))
>   			strbuf_addstr(sb,
> -				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
> +				_("  (To reconcile your local changes with the work at the remote, you can\n"

This is a welcome improvement but I think it would be better to say 
"integrate" rather than "reconcile" to keep the wording aligned with the 
advice is builtin/push.c.

> +				  "  use 'git pull' and then 'git push'. To discard the work at the remote\n"
> +				  "  and replace it with what you did (alone), you can use\n"
> +				  "  'git push --force'.)\n"));

I share Junio's concerns about giving this advice after "git status" or 
"git checkout" especially as we don't know if our remote tracking ref 
accurately reflects the current state of the remote branch.

Best Wishes

Phillip

>   	}
>   	free(base);
>   	return 1;

Re: [PATCH v3 2/2] push: advise about force-pushing as an alternative to reconciliation

[Phillip Wood]

Hi Alex

On 06/07/2023 05:01, Alex Henrie wrote:
> Also, don't put `git pull` in an awkward parenthetical, because
> `git pull` can always be used to reconcile branches and is the normal
> way to do so.

This message would also benefit from adding explanation as to why this 
change is desirable.

> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>   builtin/push.c | 27 +++++++++++++++------------
>   1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/builtin/push.c b/builtin/push.c
> index 6f8a8dc711..b2f0a64e7c 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -301,21 +301,24 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
>   
>   static const char message_advice_pull_before_push[] =
>   	N_("Updates were rejected because the tip of your current branch is behind\n"
> -	   "its remote counterpart. Integrate the remote changes (e.g.\n"
> -	   "'git pull ...') before pushing again.\n"
> +	   "its remote counterpart. Use 'git pull' to integrate the remote changes\n"

This is much clearer than "(e.g. 'git pull ...')"

> +	   "before pushing again, or use 'git push --force' to delete the remote\n"
> +	   "changes and replace them with your own.\n"

I think it would be good to give a bit more context here as to when 
force pushing is a good idea. For example something like

     If you have rebased the branch since you last integrated remote
     changes then you can use
     'git push --force-with-lease=<branch-ref> --force-if-includes' to
     safely replace the remote branch.

     If you have deleted and then recreated the branch since you last
     integrated remote changes then you can use 'git push +<branch>' to
     replace the remote. Note that if anyone else has pushed work to
     this branch it will be deleted.

It makes the advice longer  but the user get a specific suggestion for 
their current situation rather than a generic suggestion to delete the 
remote changes without discussing the implications. In this case we know 
that it was the current branch that was rejected and so should fill in 
the branch name in the advice as well.

My main issue with the changes in this series is that they seem to 
assume the user is (a) pushing a single branch and (b) they are the only 
person who works on that branch. That is a common but narrow case where 
force pushing is perfectly sensible but there are many other scenarios 
where suggesting "push --force" would not be a good idea.

Best Wishes

Phillip

>   	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
>   
>   static const char message_advice_checkout_pull_push[] =
>   	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
> -	   "counterpart. Check out this branch and integrate the remote changes\n"
> -	   "(e.g. 'git pull ...') before pushing again.\n"
> +	   "counterpart. Check out this branch and use 'git pull' to integrate the\n"
> +	   "remote changes before pushing again, or use 'git push --force' to delete\n"
> +	   "the remote changes and replace them with your own.\n"
>   	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
>   
>   static const char message_advice_ref_fetch_first[] =
> -	N_("Updates were rejected because the remote contains work that you do\n"
> -	   "not have locally. This is usually caused by another repository pushing\n"
> -	   "to the same ref. You may want to first integrate the remote changes\n"
> -	   "(e.g., 'git pull ...') before pushing again.\n"
> +	N_("Updates were rejected because the remote contains work that you do not\n"
> +	   "have locally. This is usually caused by another repository pushing to\n"
> +	   "the same ref. Use 'git pull' to integrate the remote changes before\n"
> +	   "pushing again, or use 'git push --force' to delete the remote changes\n"
> +	   "and replace them with your own.\n"
>   	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
>   
>   static const char message_advice_ref_already_exists[] =
> @@ -327,10 +330,10 @@ static const char message_advice_ref_needs_force[] =
>   	   "without using the '--force' option.\n");
>   
>   static const char message_advice_ref_needs_update[] =
> -	N_("Updates were rejected because the tip of the remote-tracking\n"
> -	   "branch has been updated since the last checkout. You may want\n"
> -	   "to integrate those changes locally (e.g., 'git pull ...')\n"
> -	   "before forcing an update.\n");
> +	N_("Updates were rejected because the tip of the remote-tracking branch has\n"
> +	   "been updated since the last checkout. Use 'git pull' to integrate the\n"
> +	   "remote changes before pushing again, or use 'git push --force' to delete\n"
> +	   "the remote changes and replace them with your own.\n");
>   
>   static void advise_pull_before_push(void)
>   {

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Junio C Hamano]

Alex Henrie <alexhenrie24@gmail.com> writes:

>> When you start working on your own topic forked from upstream by
>> switching to it, if Git notices that your topic's base has become
>> behind (so that you would later need to merge or rebase to avoid
>> losing others' work), the "git pull" message is given to tell you
>> that it is OK if you want to catch up first before working on it.
>>
>> But the new message does not fit well in the workflow.  It is
>> primarily targetted for the users who are about to push out.  They
>> are at the point where they are way before being ready to "discard
>> the work at the remote".
>
> If the branch is merely behind, format_tracking_info prints "(use "git
> pull" to update your local branch)", which is perfectly reasonable.

Correct.  The message you are changing is not the "your topic has
become behind" case, and it is exactly why I said "your topic's base
has become behind", i.e. your upstream has diverged.

> The problem is only with the message that appears when the branches
> are divergent, "(use "git pull" to merge the remote branch into
> yours)", which is bad advice for the common GitHub/GitLab workflow
> that expects force-pushing.

We are in agreement in that "you must always reconcile" is not a
good message in general to give, but I do not think "git checkout"
and "git status" are good places to give the new advice "depending
on your workflow, you do not necessarily have to pay attention to
what the upstream has and just overwrite it may be good".  That is
about how to "push", but the user is a few steps before they are
ready to start thinking about how to "push" when they get this
message.

These places in "checkout" and "status", where the message is given,
were perfectly good places to say "by the way, you are divergent and
even long before you are ready to push your work out, you may want
to refresh your work to work better with the updated upstream",
which was the "use git pull to reconcile" message was all about.

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Junio C Hamano]

Alex Henrie <alexhenrie24@gmail.com> writes:

> So, let's please give the user all of the
> information (two ways forward: reconcile or delete) and encourage them
> to make the most appropriate decision for their particular workflow.

It may be OK to do so in "git status".

It does not make any sense in "git checkout" to talk about "you can
force push".  That happens AFTER the work is done, and a message
that tells them BEFORE they start the work and asking them to
remember doing the right thing is an unnecessary noise.

I would rather see us toning the message down, e.g. "Your branches
have diverged. **IF** you intend to eventually reconcile the work on
the remote with yours, you could use `git pull` to do so now" is all
we should say.  If they do not want to keep the work on the remote,
at the point of seeing "you have diverged", there is nothing they
need to do.  There is no need to talk about "push --force" and force
the user to remember that they have to do so later.  When they try
"git push", an appropriate message should be given anyway, but that
is not the message you are touching in this patch.

For that matter, it does not make ANY sense to give "you can pull to
reconcile" message in the comment you are editing the log message
while running "git commit".  It would be the most inconvenient time
to do so.  So it might be necessary to first tweak the code so that
different messages depending on the codepath are shown, perhaps by
teaching format_tracking_info() who is calling.

Thanks.

Re: [PATCH v3 2/2] push: advise about force-pushing as an alternative to reconciliation

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Alex
>
> On 06/07/2023 05:01, Alex Henrie wrote:
>> Also, don't put `git pull` in an awkward parenthetical, because
>> `git pull` can always be used to reconcile branches and is the normal
>> way to do so.
>
> This message would also benefit from adding explanation as to why this
> change is desirable.

Yes, at least some essence from the lengthy discussion we had in the
review threads for the expected use cases deserve to be summarized
to help future developers who run "git log" (and "git blame") to
find this commit.

Unlike the [1/2] step, where the commands like "status" and
"checkout" that are detached far away from the actual "push" are
affected, this is exactly about "push has failed, now what"
situation, where a change from "you must reconcile" to "if you want
to reconcile, you could do this, but it may be that discarding the
work on the other side is the right thing, if that is just a stale
copy of what you are pushing" is very much welcome.

> It makes the advice longer  but the user get a specific suggestion for
> their current situation rather than a generic suggestion to delete the
> remote changes without discussing the implications. In this case we
> know that it was the current branch that was rejected and so should
> fill in the branch name in the advice as well.
>
> My main issue with the changes in this series is that they seem to
> assume the user is (a) pushing a single branch and (b) they are the
> only person who works on that branch. That is a common but narrow case
> where force pushing is perfectly sensible but there are many other
> scenarios where suggesting "push --force" would not be a good idea.

Yup.  Thanks for a review.

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

On Fri, Jul 7, 2023 at 11:52 AM Junio C Hamano <gitster@pobox.com> wrote:

> I would rather see us toning the message down, e.g. "Your branches
> have diverged. **IF** you intend to eventually reconcile the work on
> the remote with yours, you could use `git pull` to do so now" is all
> we should say.  If they do not want to keep the work on the remote,
> at the point of seeing "you have diverged", there is nothing they
> need to do.  There is no need to talk about "push --force" and force
> the user to remember that they have to do so later.  When they try
> "git push", an appropriate message should be given anyway, but that
> is not the message you are touching in this patch.

I would be satisfied with toning down this message as you suggest;
you're right that we don't necessarily have to mention force-pushing
here. To keep the message short, we could just replace "(use "git
pull" to merge the remote branch into yours)" with "(use "git pull" if
you want to integrate the remote branch into yours)".

> For that matter, it does not make ANY sense to give "you can pull to
> reconcile" message in the comment you are editing the log message
> while running "git commit".  It would be the most inconvenient time
> to do so.  So it might be necessary to first tweak the code so that
> different messages depending on the codepath are shown, perhaps by
> teaching format_tracking_info() who is calling.

I agree, showing this message in the middle of `git commit` is not
ideal. However, that's a separate issue that can be fixed later; it's
not part of the problem I'm trying to solve in this series.

Thanks for the feedback,

-Alex

Re: [PATCH v3 2/2] push: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

On Fri, Jul 7, 2023 at 2:49 AM Phillip Wood <phillip.wood123@gmail.com> wrote:

> This message would also benefit from adding explanation as to why this
> change is desirable.

OK, in v5 I'll add more explanation to the commit messages, including
points brought up in this discussion.

> >   static const char message_advice_pull_before_push[] =
> >       N_("Updates were rejected because the tip of your current branch is behind\n"
> > -        "its remote counterpart. Integrate the remote changes (e.g.\n"
> > -        "'git pull ...') before pushing again.\n"
> > +        "its remote counterpart. Use 'git pull' to integrate the remote changes\n"
>
> This is much clearer than "(e.g. 'git pull ...')"
>
> > +        "before pushing again, or use 'git push --force' to delete the remote\n"
> > +        "changes and replace them with your own.\n"
>
> I think it would be good to give a bit more context here as to when
> force pushing is a good idea. For example something like
>
>      If you have rebased the branch since you last integrated remote
>      changes then you can use
>      'git push --force-with-lease=<branch-ref> --force-if-includes' to
>      safely replace the remote branch.
>
>      If you have deleted and then recreated the branch since you last
>      integrated remote changes then you can use 'git push +<branch>' to
>      replace the remote. Note that if anyone else has pushed work to
>      this branch it will be deleted.
>
> It makes the advice longer  but the user get a specific suggestion for
> their current situation rather than a generic suggestion to delete the
> remote changes without discussing the implications. In this case we know
> that it was the current branch that was rejected and so should fill in
> the branch name in the advice as well.

Even if we could fill in <branch-ref> automatically, it's too much to
ask the user to type out --force-with-lease=<branch-ref>
--force-if-includes. Mentioning `git push --force` with a fat warning
about how it only makes sense in a narrow (but common) case would be
enough to make users aware of it while deterring them from abusing it.
The advice already refers the user to the man page for more
information, which includes a discussion of --force-with-lease and
--force-if-includes as alternatives to plain --force.

> My main issue with the changes in this series is that they seem to
> assume the user is (a) pushing a single branch and (b) they are the only
> person who works on that branch. That is a common but narrow case where
> force pushing is perfectly sensible but there are many other scenarios
> where suggesting "push --force" would not be a good idea.

The goal of the series is not to assume that the user's situation is
that narrow but common case, but rather to not assume that the user's
situation is not that case. The most important thing is to make the
user aware that integration/reconciliation is not the only possible
way forward.

Thanks for the feedback,

-Alex

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Junio C Hamano]

Alex Henrie <alexhenrie24@gmail.com> writes:

> I agree, showing this message in the middle of `git commit` is not
> ideal. However, that's a separate issue that can be fixed later; it's
> not part of the problem I'm trying to solve in this series.

That is debatable.  Even "by the way you can pull and reconcile
early before you have fully finished working on the topic and are
ready to push back" is irrelevant during `git commit`.  "Reconciling
the differences is not the only way to deal with divergence; you may
decide to simply discard what they have with push --force" is even
less relevant at that time.  So it seems to be very much an integral
part of the problem you are tackling, at least to me.

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

On Sat, Jul 8, 2023 at 7:38 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Alex Henrie <alexhenrie24@gmail.com> writes:
>
> > I agree, showing this message in the middle of `git commit` is not
> > ideal. However, that's a separate issue that can be fixed later; it's
> > not part of the problem I'm trying to solve in this series.
>
> That is debatable.  Even "by the way you can pull and reconcile
> early before you have fully finished working on the topic and are
> ready to push back" is irrelevant during `git commit`.  "Reconciling
> the differences is not the only way to deal with divergence; you may
> decide to simply discard what they have with push --force" is even
> less relevant at that time.  So it seems to be very much an integral
> part of the problem you are tackling, at least to me.

I thought we just agreed that we don't need to mention force-pushing
in this particular message? I guess you're saying that we'd still be
over-encouraging `git pull` if we don't remove this message from `git
commit` altogether?

-Alex

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Junio C Hamano]

Alex Henrie <alexhenrie24@gmail.com> writes:

> On Sat, Jul 8, 2023 at 7:38 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Alex Henrie <alexhenrie24@gmail.com> writes:
>>
>> > I agree, showing this message in the middle of `git commit` is not
>> > ideal. However, that's a separate issue that can be fixed later; it's
>> > not part of the problem I'm trying to solve in this series.
>>
>> That is debatable.  Even "by the way you can pull and reconcile
>> early before you have fully finished working on the topic and are
>> ready to push back" is irrelevant during `git commit`.  "Reconciling
>> the differences is not the only way to deal with divergence; you may
>> decide to simply discard what they have with push --force" is even
>> less relevant at that time.  So it seems to be very much an integral
>> part of the problem you are tackling, at least to me.
>
> I thought we just agreed that we don't need to mention force-pushing
> in this particular message? I guess you're saying that we'd still be
> over-encouraging `git pull` if we don't remove this message from `git
> commit` altogether?

I do not think so.

I was saying that, when the user during `git commit` is wondering
what to write in the log message of the commit they are working on
(which may not yet make the current branch ready to be pushed to or
integrated with the remote), the user is not ready to even choose
between "forcing push to overwrite" and "integrate and then push".

It can be fixed later, but it is a part of "how to avoid giving
confusing message to users, especially the new ones" theme.  After
all, "do not make it sound like they always have to integrate" is
how you started this journey, no?

Thanks.

Re: [PATCH v3 2/2] push: advise about force-pushing as an alternative to reconciliation

[Phillip Wood]

Hi Alex

On 08/07/2023 19:56, Alex Henrie wrote:
> On Fri, Jul 7, 2023 at 2:49 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>> +        "before pushing again, or use 'git push --force' to delete the remote\n"
>>> +        "changes and replace them with your own.\n"
>>
>> I think it would be good to give a bit more context here as to when
>> force pushing is a good idea. For example something like
>>
>>       If you have rebased the branch since you last integrated remote
>>       changes then you can use
>>       'git push --force-with-lease=<branch-ref> --force-if-includes' to
>>       safely replace the remote branch.
>>
>>       If you have deleted and then recreated the branch since you last
>>       integrated remote changes then you can use 'git push +<branch>' to
>>       replace the remote. Note that if anyone else has pushed work to
>>       this branch it will be deleted.
>>
>> It makes the advice longer  but the user get a specific suggestion for
>> their current situation rather than a generic suggestion to delete the
>> remote changes without discussing the implications. In this case we know
>> that it was the current branch that was rejected and so should fill in
>> the branch name in the advice as well.
> 
> Even if we could fill in <branch-ref> automatically, it's too much to
> ask the user to type out --force-with-lease=<branch-ref>
> --force-if-includes.

Can't they just copy and paste the command from the advice message? Even 
if the user does not copy and paste it is not that hard to type it out 
with the benefit of the shell's tab completion. You're basically saying 
this combination of options is unusable in practice because it is too 
much effort to type them. We could look to see if we can make it less 
unwieldy by changing push to allow --force-if-includes=ref imply 
--force-with-lease for instance.

> Mentioning `git push --force` with a fat warning
> about how it only makes sense in a narrow (but common) case would be
> enough to make users aware of it while deterring them from abusing it.

Having a warning in the advice message would definitely help

> The advice already refers the user to the man page for more
> information, which includes a discussion of --force-with-lease and
> --force-if-includes as alternatives to plain --force.

It is good to mention the man page in the advice but we shouldn't assume 
that users will actually go and read it before running the suggested 
command.

>> My main issue with the changes in this series is that they seem to
>> assume the user is (a) pushing a single branch and (b) they are the only
>> person who works on that branch. That is a common but narrow case where
>> force pushing is perfectly sensible but there are many other scenarios
>> where suggesting "push --force" would not be a good idea.
> 
> The goal of the series is not to assume that the user's situation is
> that narrow but common case, but rather to not assume that the user's
> situation is not that case. The most important thing is to make the
> user aware that integration/reconciliation is not the only possible
> way forward.

Thanks for clarifying, that is the sort of thing that should be in the 
commit message.

Best Wishes

Phillip

> Thanks for the feedback,
> 
> -Alex

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

On Mon, Jul 10, 2023 at 6:55 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Alex Henrie <alexhenrie24@gmail.com> writes:
>
> > On Sat, Jul 8, 2023 at 7:38 PM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> Alex Henrie <alexhenrie24@gmail.com> writes:
> >>
> >> > I agree, showing this message in the middle of `git commit` is not
> >> > ideal. However, that's a separate issue that can be fixed later; it's
> >> > not part of the problem I'm trying to solve in this series.
> >>
> >> That is debatable.  Even "by the way you can pull and reconcile
> >> early before you have fully finished working on the topic and are
> >> ready to push back" is irrelevant during `git commit`.  "Reconciling
> >> the differences is not the only way to deal with divergence; you may
> >> decide to simply discard what they have with push --force" is even
> >> less relevant at that time.  So it seems to be very much an integral
> >> part of the problem you are tackling, at least to me.
> >
> > I thought we just agreed that we don't need to mention force-pushing
> > in this particular message? I guess you're saying that we'd still be
> > over-encouraging `git pull` if we don't remove this message from `git
> > commit` altogether?
>
> I do not think so.
>
> I was saying that, when the user during `git commit` is wondering
> what to write in the log message of the commit they are working on
> (which may not yet make the current branch ready to be pushed to or
> integrated with the remote), the user is not ready to even choose
> between "forcing push to overwrite" and "integrate and then push".
>
> It can be fixed later, but it is a part of "how to avoid giving
> confusing message to users, especially the new ones" theme.  After
> all, "do not make it sound like they always have to integrate" is
> how you started this journey, no?

To me, one of those things is bad advice, and the other is irrelevant
advice. They're both confusing, but one of them is more likely to
cause trouble than the other.

Omitting this message from `git commit` isn't technically difficult,
my main worry is that that change will be picked to death in code
review and it will hold up the more important changes. I have to find
time for Git outside of work and I'm already feeling pretty burned out
trying to communicate the problem and integrate the feedback on this
series so far. Even so, because it's important to you, and because I
appreciate your willingness to work with me on this problem, I'm
willing to take a stab at fixing both the bad advice and the
irrelevant advice in the same series.

Just to be sure that we're on the same page, when I said "I thought we
just agreed that we don't need to mention force-pushing..." and you
replied "I do not think so", were you only saying that you think that
changes to `git commit` are essential, or were you also saying that we
have not come to an agreement about whether to include force-pushing
advice in this message?

Thanks,

-Alex

Re: [PATCH v3 2/2] push: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

On Tue, Jul 11, 2023 at 12:33 PM Phillip Wood <phillip.wood123@gmail.com> wrote:

> On 08/07/2023 19:56, Alex Henrie wrote:
> > On Fri, Jul 7, 2023 at 2:49 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> >>> +        "before pushing again, or use 'git push --force' to delete the remote\n"
> >>> +        "changes and replace them with your own.\n"
> >>
> >> I think it would be good to give a bit more context here as to when
> >> force pushing is a good idea. For example something like
> >>
> >>       If you have rebased the branch since you last integrated remote
> >>       changes then you can use
> >>       'git push --force-with-lease=<branch-ref> --force-if-includes' to
> >>       safely replace the remote branch.
> >>
> >>       If you have deleted and then recreated the branch since you last
> >>       integrated remote changes then you can use 'git push +<branch>' to
> >>       replace the remote. Note that if anyone else has pushed work to
> >>       this branch it will be deleted.
> >>
> >> It makes the advice longer  but the user get a specific suggestion for
> >> their current situation rather than a generic suggestion to delete the
> >> remote changes without discussing the implications. In this case we know
> >> that it was the current branch that was rejected and so should fill in
> >> the branch name in the advice as well.
> >
> > Even if we could fill in <branch-ref> automatically, it's too much to
> > ask the user to type out --force-with-lease=<branch-ref>
> > --force-if-includes.
>
> Can't they just copy and paste the command from the advice message? Even
> if the user does not copy and paste it is not that hard to type it out
> with the benefit of the shell's tab completion. You're basically saying
> this combination of options is unusable in practice because it is too
> much effort to type them. We could look to see if we can make it less
> unwieldy by changing push to allow --force-if-includes=ref imply
> --force-with-lease for instance.

Yes, `git push --force-with-lease=<branch-ref> --force-if-includes` is
cryptic and unwieldy, and even asking users to copy and paste a
command is a bit much. If that's what's presented as the alternative
to integration via `git pull`, it could make users who want to
overwrite the remote branch think that force-pushing isn't what they
want because what they want is conceptually very simple, so they
expect it to have a simple user interface.

It's possible that improvements will be made to this user interface in
the future, but that's definitely not something that I'm going to
tackle. I just want Git to give decent advice about what is available
right now. If we can't agree on what specific command to recommend,
maybe we can at least agree to tone down these messages to not sound
so prescriptive. Just changing "Use 'git pull' to integrate..." to
"You can use 'git pull' to integrate...' would be a big improvement.

Thanks,

-Alex

Re: [PATCH v3 2/2] push: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

On Tue, Jul 11, 2023 at 10:47 PM Alex Henrie <alexhenrie24@gmail.com> wrote:
>
> On Tue, Jul 11, 2023 at 12:33 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> > On 08/07/2023 19:56, Alex Henrie wrote:
> > > On Fri, Jul 7, 2023 at 2:49 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> > >>> +        "before pushing again, or use 'git push --force' to delete the remote\n"
> > >>> +        "changes and replace them with your own.\n"
> > >>
> > >> I think it would be good to give a bit more context here as to when
> > >> force pushing is a good idea. For example something like
> > >>
> > >>       If you have rebased the branch since you last integrated remote
> > >>       changes then you can use
> > >>       'git push --force-with-lease=<branch-ref> --force-if-includes' to
> > >>       safely replace the remote branch.
> > >>
> > >>       If you have deleted and then recreated the branch since you last
> > >>       integrated remote changes then you can use 'git push +<branch>' to
> > >>       replace the remote. Note that if anyone else has pushed work to
> > >>       this branch it will be deleted.
> > >>
> > >> It makes the advice longer  but the user get a specific suggestion for
> > >> their current situation rather than a generic suggestion to delete the
> > >> remote changes without discussing the implications. In this case we know
> > >> that it was the current branch that was rejected and so should fill in
> > >> the branch name in the advice as well.
> > >
> > > Even if we could fill in <branch-ref> automatically, it's too much to
> > > ask the user to type out --force-with-lease=<branch-ref>
> > > --force-if-includes.
> >
> > Can't they just copy and paste the command from the advice message? Even
> > if the user does not copy and paste it is not that hard to type it out
> > with the benefit of the shell's tab completion. You're basically saying
> > this combination of options is unusable in practice because it is too
> > much effort to type them. We could look to see if we can make it less
> > unwieldy by changing push to allow --force-if-includes=ref imply
> > --force-with-lease for instance.
>
> Yes, `git push --force-with-lease=<branch-ref> --force-if-includes` is
> cryptic and unwieldy, and even asking users to copy and paste a
> command is a bit much. If that's what's presented as the alternative
> to integration via `git pull`, it could make users who want to
> overwrite the remote branch think that force-pushing isn't what they
> want because what they want is conceptually very simple, so they
> expect it to have a simple user interface.
>
> It's possible that improvements will be made to this user interface in
> the future, but that's definitely not something that I'm going to
> tackle. I just want Git to give decent advice about what is available
> right now. If we can't agree on what specific command to recommend,
> maybe we can at least agree to tone down these messages to not sound
> so prescriptive. Just changing "Use 'git pull' to integrate..." to
> "You can use 'git pull' to integrate...' would be a big improvement.

Whoops, I accidentally quoted my own proposed text as if it were the
current text. The current text is in fact "Integrate the remote
changes..." which is stronger still.

-Alex

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Junio C Hamano]

Alex Henrie <alexhenrie24@gmail.com> writes:

> Just to be sure that we're on the same page, when I said "I thought we
> just agreed that we don't need to mention force-pushing..." and you
> replied "I do not think so", were you only saying that you think that
> changes to `git commit` are essential, or were you also saying that we
> have not come to an agreement about whether to include force-pushing
> advice in this message?

None of the above ;-)

With that "I do not think so", I meant that I do not agree with "I
guess you're saying that we'd still be over-encouraging `git pull`"
that was in your message.  In the message you were responding to, I
was saying that the time the user runs `git commit` is not a good
time for the user to decide how to eventually update the remote
target, and it does not matter which one we encourage more between
"`git pull [--rebase]` then `git push`" and "`git push --force`".

I am fine dropping patch [1/2]; we would not be touching output from
"git status", "git commit", or "git checkout", and "we should not
talk about 'git pull' (or how the eventual remote update should go,
for that matter) when we notice that the base of the user's branch
has become stale" becomes totally out of the scope of this topic.  I
think that we all are in agreement that [2/2] is the more important
part of this topic, as it more directly improves the guidance for
the end-users when their "push" triggers the non-ff check.

Thanks.

Re: [PATCH v3 1/2] remote: advise about force-pushing as an alternative to reconciliation

[Alex Henrie]

On Wed, Jul 12, 2023 at 9:18 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Alex Henrie <alexhenrie24@gmail.com> writes:
>
> > Just to be sure that we're on the same page, when I said "I thought we
> > just agreed that we don't need to mention force-pushing..." and you
> > replied "I do not think so", were you only saying that you think that
> > changes to `git commit` are essential, or were you also saying that we
> > have not come to an agreement about whether to include force-pushing
> > advice in this message?
>
> None of the above ;-)
>
> With that "I do not think so", I meant that I do not agree with "I
> guess you're saying that we'd still be over-encouraging `git pull`"
> that was in your message.  In the message you were responding to, I
> was saying that the time the user runs `git commit` is not a good
> time for the user to decide how to eventually update the remote
> target, and it does not matter which one we encourage more between
> "`git pull [--rebase]` then `git push`" and "`git push --force`".
>
> I am fine dropping patch [1/2]; we would not be touching output from
> "git status", "git commit", or "git checkout", and "we should not
> talk about 'git pull' (or how the eventual remote update should go,
> for that matter) when we notice that the base of the user's branch
> has become stale" becomes totally out of the scope of this topic.  I
> think that we all are in agreement that [2/2] is the more important
> part of this topic, as it more directly improves the guidance for
> the end-users when their "push" triggers the non-ff check.

Thanks for the clarification. This all started because of the message
in `git status`, so despite it being the less important message, I
feel pretty strongly that that message does need to be toned down
slightly. There's also the problem of that message assuming that `git
pull` will do a merge when it can do either a merge or a rebase,
depending on the user's Git config.

I've already written a patch to suppress the irrelevant advice in `git
commit`, so I might as well send it. I'm hoping that we can agree to
make a few tweaks to these advice messages without going as far as I
originally proposed.

-Alex

[PATCH v5 0/3] don't imply that integration is always required before pushing

[Alex Henrie]

Many times now, I have seen novices do the following:

1. Start work on their own personal topic branch
2. Push the branch to origin
3. Rebase the branch onto origin/master
4. Try to push again, but Git says they need to pull
5. Pull and make a mess trying to reconcile the older topic branch with
   the rebased topic branch

Help avoid this mistake by giving somewhat more general advice that does
not assume that the user always wants to do reconciliation.

Changes from v4:
- Don't show divergent branch advice in the middle of `git commit`
- Soften the advice, but don't specifically mention force-pushing

Alex Henrie (3):
  wt-status: don't show divergence advice when committing
  remote: don't imply that integration is always required before pushing
  push: don't imply that integration is always required before pushing

 builtin/checkout.c |  2 +-
 builtin/push.c     | 24 +++++++++++------------
 remote.c           |  8 +++++---
 remote.h           |  3 ++-
 t/t7508-status.sh  | 48 ++++++++++++++++++++++------------------------
 wt-status.c        |  3 ++-
 6 files changed, 45 insertions(+), 43 deletions(-)

Range-diff against v4:
1:  9626721c13 < -:  ---------- remote: advise about force-pushing as an alternative to reconciliation
-:  ---------- > 1:  e84989c4a6 wt-status: don't show divergence advice when committing
-:  ---------- > 2:  9bb643df7e remote: don't imply that integration is always required before pushing
2:  209e86588a ! 3:  5ff9ecb51b push: advise about force-pushing as an alternative to reconciliation
    @@ Metadata
     Author: Alex Henrie <alexhenrie24@gmail.com>
     
      ## Commit message ##
    -    push: advise about force-pushing as an alternative to reconciliation
    +    push: don't imply that integration is always required before pushing
     
    -    Also, don't put `git pull` in an awkward parenthetical, because
    -    `git pull` can always be used to reconcile branches and is the normal
    -    way to do so.
    +    In a narrow but common case, the user is the only author of a branch and
    +    doesn't mind overwriting the corresponding branch on the remote. This
    +    workflow is especially common on GitHub, GitLab, and Gerrit, which keep
    +    a permanent record of every version of a branch that is pushed while a
    +    pull request is open for that branch. On those platforms, force-pushing
    +    is encouraged and is analogous to emailing a new version of a patchset.
    +
    +    When giving advice about divergent branches, tell the user about
    +    `git pull`, but don't unconditionally instruct the user to do it. A less
    +    prescriptive message will help prevent users from thinking that they are
    +    required to create an integrated history instead of simply replacing the
    +    previous history. Also, don't put `git pull` in an awkward
    +    parenthetical, because `git pull` can always be used to reconcile
    +    branches and is the normal way to do so.
    +
    +    Due to the difficulty of knowing which command for force-pushing is best
    +    suited to the user's situation, no specific advice is given about
    +    force-pushing. Instead, the user is directed to the Git documentation to
    +    read about possible ways forward that do not involve integration.
     
         Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
     
    @@ builtin/push.c: static void setup_default_push_refspecs(int *flags, struct remot
      	N_("Updates were rejected because the tip of your current branch is behind\n"
     -	   "its remote counterpart. Integrate the remote changes (e.g.\n"
     -	   "'git pull ...') before pushing again.\n"
    -+	   "its remote counterpart. Use 'git pull' to integrate the remote changes\n"
    -+	   "before pushing again, or use 'git push --force' to delete the remote\n"
    -+	   "changes and replace them with your own.\n"
    ++	   "its remote counterpart. If you want to integrate the remote changes,\n"
    ++	   "use 'git pull' before pushing again.\n"
      	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
      
      static const char message_advice_checkout_pull_push[] =
      	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
     -	   "counterpart. Check out this branch and integrate the remote changes\n"
     -	   "(e.g. 'git pull ...') before pushing again.\n"
    -+	   "counterpart. Check out this branch and use 'git pull' to integrate the\n"
    -+	   "remote changes before pushing again, or use 'git push --force' to delete\n"
    -+	   "the remote changes and replace them with your own.\n"
    ++	   "counterpart. If you want to integrate the remote changes, use 'git pull'\n"
    ++	   "before pushing again.\n"
      	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
      
      static const char message_advice_ref_fetch_first[] =
    @@ builtin/push.c: static void setup_default_push_refspecs(int *flags, struct remot
     -	   "(e.g., 'git pull ...') before pushing again.\n"
     +	N_("Updates were rejected because the remote contains work that you do not\n"
     +	   "have locally. This is usually caused by another repository pushing to\n"
    -+	   "the same ref. Use 'git pull' to integrate the remote changes before\n"
    -+	   "pushing again, or use 'git push --force' to delete the remote changes\n"
    -+	   "and replace them with your own.\n"
    ++	   "the same ref. If you want to integrate the remote changes, use\n"
    ++	   "'git pull' before pushing again.\n"
      	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
      
      static const char message_advice_ref_already_exists[] =
    @@ builtin/push.c: static const char message_advice_ref_needs_force[] =
     -	   "to integrate those changes locally (e.g., 'git pull ...')\n"
     -	   "before forcing an update.\n");
     +	N_("Updates were rejected because the tip of the remote-tracking branch has\n"
    -+	   "been updated since the last checkout. Use 'git pull' to integrate the\n"
    -+	   "remote changes before pushing again, or use 'git push --force' to delete\n"
    -+	   "the remote changes and replace them with your own.\n");
    ++	   "been updated since the last checkout. If you want to integrate the\n"
    ++	   "remote changes, use 'git pull' before pushing again.\n"
    ++	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
      
      static void advise_pull_before_push(void)
      {
-- 
2.41.0

[PATCH v5 1/3] wt-status: don't show divergence advice when committing

[Alex Henrie]

When the user is in the middle of making a commit, they are not yet at
the point where they are ready to think about integrating their local
branch with the corresponding remote branch or force-pushing over the
remote branch. Don't include advice on how to deal with divergent
branches in the commit template, to avoid giving the impression that the
divergence needs to be dealt with immediately. Similar advice will be
printed when it is most relevant, that is, if the user does try to push
without first reconciling the two branches.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 builtin/checkout.c |  2 +-
 remote.c           |  6 ++++--
 remote.h           |  3 ++-
 t/t7508-status.sh  | 10 ++++------
 wt-status.c        |  3 ++-
 5 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index fd6ee8c272..c278c2169d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -916,7 +916,7 @@ static void report_tracking(struct branch_info *new_branch_info)
 	struct strbuf sb = STRBUF_INIT;
 	struct branch *branch = branch_get(new_branch_info->name);
 
-	if (!format_tracking_info(branch, &sb, AHEAD_BEHIND_FULL))
+	if (!format_tracking_info(branch, &sb, AHEAD_BEHIND_FULL, 1))
 		return;
 	fputs(sb.buf, stdout);
 	strbuf_release(&sb);
diff --git a/remote.c b/remote.c
index a81f2e2f17..d79aae0d76 100644
--- a/remote.c
+++ b/remote.c
@@ -2258,7 +2258,8 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
  * Return true when there is anything to report, otherwise false.
  */
 int format_tracking_info(struct branch *branch, struct strbuf *sb,
-			 enum ahead_behind_flags abf)
+			 enum ahead_behind_flags abf,
+			 int show_divergence_advice)
 {
 	int ours, theirs, sti;
 	const char *full_base;
@@ -2321,7 +2322,8 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
 			       "respectively.\n",
 			   ours + theirs),
 			base, ours, theirs);
-		if (advice_enabled(ADVICE_STATUS_HINTS))
+		if (show_divergence_advice &&
+		    advice_enabled(ADVICE_STATUS_HINTS))
 			strbuf_addstr(sb,
 				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
 	}
diff --git a/remote.h b/remote.h
index 929c7c676d..cdc8b1db42 100644
--- a/remote.h
+++ b/remote.h
@@ -380,7 +380,8 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
 		       const char **upstream_name, int for_push,
 		       enum ahead_behind_flags abf);
 int format_tracking_info(struct branch *branch, struct strbuf *sb,
-			 enum ahead_behind_flags abf);
+			 enum ahead_behind_flags abf,
+			 int show_divergence_advice);
 
 struct ref *get_local_heads(void);
 /*
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 36567708f5..845af287d7 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -847,7 +847,6 @@ test_expect_success 'dry-run of partial commit excluding new file in index' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -1013,7 +1012,7 @@ test_expect_success 'status -s submodule summary' '
 '
 
 test_expect_success 'status submodule summary (clean submodule): commit' '
-	cat >expect <<EOF &&
+	cat >expect-status <<EOF &&
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
@@ -1033,12 +1032,13 @@ Untracked files:
 
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
+	sed "/git pull/d" expect-status > expect-commit &&
 	git commit -m "commit submodule" &&
 	git config status.submodulesummary 10 &&
 	test_must_fail git commit --dry-run >output &&
-	test_cmp expect output &&
+	test_cmp expect-commit output &&
 	git status >output &&
-	test_cmp expect output
+	test_cmp expect-status output
 '
 
 cat >expect <<EOF
@@ -1065,7 +1065,6 @@ test_expect_success 'commit --dry-run submodule summary (--amend)' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
 
 Changes to be committed:
   (use "git restore --source=HEAD^1 --staged <file>..." to unstage)
@@ -1558,7 +1557,6 @@ test_expect_success 'git commit --dry-run will show a staged but ignored submodu
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
diff --git a/wt-status.c b/wt-status.c
index bcd0ef8044..e3e3732ea2 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1186,7 +1186,8 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
 
 	t_begin = getnanotime();
 
-	if (!format_tracking_info(branch, &sb, s->ahead_behind_flags))
+	if (!format_tracking_info(branch, &sb, s->ahead_behind_flags,
+				  !s->commit_template))
 		return;
 
 	if (advice_enabled(ADVICE_STATUS_AHEAD_BEHIND_WARNING) &&
-- 
2.41.0

[PATCH v5 2/3] remote: don't imply that integration is always required before pushing

[Alex Henrie]

In a narrow but common case, the user is the only author of a branch and
doesn't mind overwriting the corresponding branch on the remote. This
workflow is especially common on GitHub, GitLab, and Gerrit, which keep
a permanent record of every version of a branch that is pushed while a
pull request is open for that branch. On those platforms, force-pushing
is encouraged and is analogous to emailing a new version of a patchset.

When giving advice about divergent branches, tell the user about
`git pull`, but don't unconditionally instruct the user to do it. A less
prescriptive message will help prevent users from thinking that they are
required to create an integrated history instead of simply replacing the
previous history. Likewise, don't imply that `git pull` is only for
merging.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 remote.c          |  2 +-
 t/t7508-status.sh | 38 +++++++++++++++++++-------------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/remote.c b/remote.c
index d79aae0d76..71019564d5 100644
--- a/remote.c
+++ b/remote.c
@@ -2325,7 +2325,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
 		if (show_divergence_advice &&
 		    advice_enabled(ADVICE_STATUS_HINTS))
 			strbuf_addstr(sb,
-				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
+				_("  (use \"git pull\" if you want to integrate the remote branch with yours)\n"));
 	}
 	free(base);
 	return 1;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 845af287d7..6928fd89f5 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -92,7 +92,7 @@ test_expect_success 'status --column' '
 # On branch main
 # Your branch and '\''upstream'\'' have diverged,
 # and have 1 and 2 different commits each, respectively.
-#   (use "git pull" to merge the remote branch into yours)
+#   (use "git pull" if you want to integrate the remote branch with yours)
 #
 # Changes to be committed:
 #   (use "git restore --staged <file>..." to unstage)
@@ -123,7 +123,7 @@ cat >expect <<\EOF
 # On branch main
 # Your branch and 'upstream' have diverged,
 # and have 1 and 2 different commits each, respectively.
-#   (use "git pull" to merge the remote branch into yours)
+#   (use "git pull" if you want to integrate the remote branch with yours)
 #
 # Changes to be committed:
 #   (use "git restore --staged <file>..." to unstage)
@@ -270,7 +270,7 @@ test_expect_success 'status with gitignore' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -335,7 +335,7 @@ test_expect_success 'status with gitignore (nothing untracked)' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -405,7 +405,7 @@ test_expect_success 'status -uno' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -467,7 +467,7 @@ test_expect_success 'status -unormal' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -522,7 +522,7 @@ test_expect_success 'status -uall' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -582,7 +582,7 @@ test_expect_success 'status with relative paths' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -650,7 +650,7 @@ test_expect_success TTY 'status with color.ui' '
 On branch <GREEN>main<RESET>
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -773,7 +773,7 @@ test_expect_success 'status without relative paths' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -900,7 +900,7 @@ test_expect_success 'status submodule summary is disabled by default' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -957,7 +957,7 @@ test_expect_success 'status submodule summary' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 1 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -1016,7 +1016,7 @@ test_expect_success 'status submodule summary (clean submodule): commit' '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes not staged for commit:
   (use "git add <file>..." to update what will be committed)
@@ -1116,7 +1116,7 @@ test_expect_success '--ignore-submodules=untracked suppresses submodules with un
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -1225,7 +1225,7 @@ test_expect_success "--ignore-submodules=untracked doesn't suppress submodules w
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -1282,7 +1282,7 @@ test_expect_success "--ignore-submodules=untracked doesn't suppress submodule su
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
@@ -1363,7 +1363,7 @@ cat > expect << EOF
 ; On branch main
 ; Your branch and 'upstream' have diverged,
 ; and have 2 and 2 different commits each, respectively.
-;   (use "git pull" to merge the remote branch into yours)
+;   (use "git pull" if you want to integrate the remote branch with yours)
 ;
 ; Changes to be committed:
 ;   (use "git restore --staged <file>..." to unstage)
@@ -1411,7 +1411,7 @@ test_expect_success "--ignore-submodules=all suppresses submodule summary" '
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes not staged for commit:
   (use "git add <file>..." to update what will be committed)
@@ -1437,7 +1437,7 @@ test_expect_success '.gitmodules ignore=all suppresses unstaged submodule summar
 On branch main
 Your branch and '\''upstream'\'' have diverged,
 and have 2 and 2 different commits each, respectively.
-  (use "git pull" to merge the remote branch into yours)
+  (use "git pull" if you want to integrate the remote branch with yours)
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
-- 
2.41.0

[PATCH v5 3/3] push: don't imply that integration is always required before pushing

[Alex Henrie]

In a narrow but common case, the user is the only author of a branch and
doesn't mind overwriting the corresponding branch on the remote. This
workflow is especially common on GitHub, GitLab, and Gerrit, which keep
a permanent record of every version of a branch that is pushed while a
pull request is open for that branch. On those platforms, force-pushing
is encouraged and is analogous to emailing a new version of a patchset.

When giving advice about divergent branches, tell the user about
`git pull`, but don't unconditionally instruct the user to do it. A less
prescriptive message will help prevent users from thinking that they are
required to create an integrated history instead of simply replacing the
previous history. Also, don't put `git pull` in an awkward
parenthetical, because `git pull` can always be used to reconcile
branches and is the normal way to do so.

Due to the difficulty of knowing which command for force-pushing is best
suited to the user's situation, no specific advice is given about
force-pushing. Instead, the user is directed to the Git documentation to
read about possible ways forward that do not involve integration.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 builtin/push.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 6f8a8dc711..61a251c50a 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -301,21 +301,21 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
 
 static const char message_advice_pull_before_push[] =
 	N_("Updates were rejected because the tip of your current branch is behind\n"
-	   "its remote counterpart. Integrate the remote changes (e.g.\n"
-	   "'git pull ...') before pushing again.\n"
+	   "its remote counterpart. If you want to integrate the remote changes,\n"
+	   "use 'git pull' before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_checkout_pull_push[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
-	   "counterpart. Check out this branch and integrate the remote changes\n"
-	   "(e.g. 'git pull ...') before pushing again.\n"
+	   "counterpart. If you want to integrate the remote changes, use 'git pull'\n"
+	   "before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_ref_fetch_first[] =
-	N_("Updates were rejected because the remote contains work that you do\n"
-	   "not have locally. This is usually caused by another repository pushing\n"
-	   "to the same ref. You may want to first integrate the remote changes\n"
-	   "(e.g., 'git pull ...') before pushing again.\n"
+	N_("Updates were rejected because the remote contains work that you do not\n"
+	   "have locally. This is usually caused by another repository pushing to\n"
+	   "the same ref. If you want to integrate the remote changes, use\n"
+	   "'git pull' before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static const char message_advice_ref_already_exists[] =
@@ -327,10 +327,10 @@ static const char message_advice_ref_needs_force[] =
 	   "without using the '--force' option.\n");
 
 static const char message_advice_ref_needs_update[] =
-	N_("Updates were rejected because the tip of the remote-tracking\n"
-	   "branch has been updated since the last checkout. You may want\n"
-	   "to integrate those changes locally (e.g., 'git pull ...')\n"
-	   "before forcing an update.\n");
+	N_("Updates were rejected because the tip of the remote-tracking branch has\n"
+	   "been updated since the last checkout. If you want to integrate the\n"
+	   "remote changes, use 'git pull' before pushing again.\n"
+	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
 static void advise_pull_before_push(void)
 {
-- 
2.41.0

Re: [PATCH v5 0/3] don't imply that integration is always required before pushing

[Phillip Wood]

Hi Alex

On 13/07/2023 05:41, Alex Henrie wrote:
> Many times now, I have seen novices do the following:
> 
> 1. Start work on their own personal topic branch
> 2. Push the branch to origin
> 3. Rebase the branch onto origin/master
> 4. Try to push again, but Git says they need to pull
> 5. Pull and make a mess trying to reconcile the older topic branch with
>     the rebased topic branch
> 
> Help avoid this mistake by giving somewhat more general advice that does
> not assume that the user always wants to do reconciliation.
> 
> Changes from v4:
> - Don't show divergent branch advice in the middle of `git commit`
> - Soften the advice, but don't specifically mention force-pushing

All three patches look fine to me, they are definitely an improvement on 
the current advice.

Thanks for working on this,

Phillip

> Alex Henrie (3):
>    wt-status: don't show divergence advice when committing
>    remote: don't imply that integration is always required before pushing
>    push: don't imply that integration is always required before pushing
> 
>   builtin/checkout.c |  2 +-
>   builtin/push.c     | 24 +++++++++++------------
>   remote.c           |  8 +++++---
>   remote.h           |  3 ++-
>   t/t7508-status.sh  | 48 ++++++++++++++++++++++------------------------
>   wt-status.c        |  3 ++-
>   6 files changed, 45 insertions(+), 43 deletions(-)
> 
> Range-diff against v4:
> 1:  9626721c13 < -:  ---------- remote: advise about force-pushing as an alternative to reconciliation
> -:  ---------- > 1:  e84989c4a6 wt-status: don't show divergence advice when committing
> -:  ---------- > 2:  9bb643df7e remote: don't imply that integration is always required before pushing
> 2:  209e86588a ! 3:  5ff9ecb51b push: advise about force-pushing as an alternative to reconciliation
>      @@ Metadata
>       Author: Alex Henrie <alexhenrie24@gmail.com>
>       
>        ## Commit message ##
>      -    push: advise about force-pushing as an alternative to reconciliation
>      +    push: don't imply that integration is always required before pushing
>       
>      -    Also, don't put `git pull` in an awkward parenthetical, because
>      -    `git pull` can always be used to reconcile branches and is the normal
>      -    way to do so.
>      +    In a narrow but common case, the user is the only author of a branch and
>      +    doesn't mind overwriting the corresponding branch on the remote. This
>      +    workflow is especially common on GitHub, GitLab, and Gerrit, which keep
>      +    a permanent record of every version of a branch that is pushed while a
>      +    pull request is open for that branch. On those platforms, force-pushing
>      +    is encouraged and is analogous to emailing a new version of a patchset.
>      +
>      +    When giving advice about divergent branches, tell the user about
>      +    `git pull`, but don't unconditionally instruct the user to do it. A less
>      +    prescriptive message will help prevent users from thinking that they are
>      +    required to create an integrated history instead of simply replacing the
>      +    previous history. Also, don't put `git pull` in an awkward
>      +    parenthetical, because `git pull` can always be used to reconcile
>      +    branches and is the normal way to do so.
>      +
>      +    Due to the difficulty of knowing which command for force-pushing is best
>      +    suited to the user's situation, no specific advice is given about
>      +    force-pushing. Instead, the user is directed to the Git documentation to
>      +    read about possible ways forward that do not involve integration.
>       
>           Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
>       
>      @@ builtin/push.c: static void setup_default_push_refspecs(int *flags, struct remot
>        	N_("Updates were rejected because the tip of your current branch is behind\n"
>       -	   "its remote counterpart. Integrate the remote changes (e.g.\n"
>       -	   "'git pull ...') before pushing again.\n"
>      -+	   "its remote counterpart. Use 'git pull' to integrate the remote changes\n"
>      -+	   "before pushing again, or use 'git push --force' to delete the remote\n"
>      -+	   "changes and replace them with your own.\n"
>      ++	   "its remote counterpart. If you want to integrate the remote changes,\n"
>      ++	   "use 'git pull' before pushing again.\n"
>        	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
>        
>        static const char message_advice_checkout_pull_push[] =
>        	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
>       -	   "counterpart. Check out this branch and integrate the remote changes\n"
>       -	   "(e.g. 'git pull ...') before pushing again.\n"
>      -+	   "counterpart. Check out this branch and use 'git pull' to integrate the\n"
>      -+	   "remote changes before pushing again, or use 'git push --force' to delete\n"
>      -+	   "the remote changes and replace them with your own.\n"
>      ++	   "counterpart. If you want to integrate the remote changes, use 'git pull'\n"
>      ++	   "before pushing again.\n"
>        	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
>        
>        static const char message_advice_ref_fetch_first[] =
>      @@ builtin/push.c: static void setup_default_push_refspecs(int *flags, struct remot
>       -	   "(e.g., 'git pull ...') before pushing again.\n"
>       +	N_("Updates were rejected because the remote contains work that you do not\n"
>       +	   "have locally. This is usually caused by another repository pushing to\n"
>      -+	   "the same ref. Use 'git pull' to integrate the remote changes before\n"
>      -+	   "pushing again, or use 'git push --force' to delete the remote changes\n"
>      -+	   "and replace them with your own.\n"
>      ++	   "the same ref. If you want to integrate the remote changes, use\n"
>      ++	   "'git pull' before pushing again.\n"
>        	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
>        
>        static const char message_advice_ref_already_exists[] =
>      @@ builtin/push.c: static const char message_advice_ref_needs_force[] =
>       -	   "to integrate those changes locally (e.g., 'git pull ...')\n"
>       -	   "before forcing an update.\n");
>       +	N_("Updates were rejected because the tip of the remote-tracking branch has\n"
>      -+	   "been updated since the last checkout. Use 'git pull' to integrate the\n"
>      -+	   "remote changes before pushing again, or use 'git push --force' to delete\n"
>      -+	   "the remote changes and replace them with your own.\n");
>      ++	   "been updated since the last checkout. If you want to integrate the\n"
>      ++	   "remote changes, use 'git pull' before pushing again.\n"
>      ++	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
>        
>        static void advise_pull_before_push(void)
>        {

Re: [PATCH v5 0/3] don't imply that integration is always required before pushing

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Alex
>
> On 13/07/2023 05:41, Alex Henrie wrote:
>> Many times now, I have seen novices do the following:
>> 1. Start work on their own personal topic branch
>> 2. Push the branch to origin
>> 3. Rebase the branch onto origin/master
>> 4. Try to push again, but Git says they need to pull
>> 5. Pull and make a mess trying to reconcile the older topic branch with
>>     the rebased topic branch
>> Help avoid this mistake by giving somewhat more general advice that
>> does
>> not assume that the user always wants to do reconciliation.
>> Changes from v4:
>> - Don't show divergent branch advice in the middle of `git commit`
>> - Soften the advice, but don't specifically mention force-pushing
>
> All three patches look fine to me, they are definitely an improvement
> on the current advice.
>
> Thanks for working on this,
>
> Phillip

Thanks, both.  Will queue after taking another look.