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

[Alex Henrie <alexhenrie24@gmail.com>]

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