Subject: Re: [PATCH v2 2/3] builtin/rebase.c: Emit warning when rebasing without a forkpoint

From: Junio C Hamano <[email-masked]>
Date: Sat, 02 Sep 2023 21:50:18 -0700

Junio C Hamano <[email-masked]> writes:

> If you rewind to lose commits from the branch you are (re)building
> against, and what was rewound and discarded was part of the work you
> are building, whether it is on a local branch or on a remote branch
> that contains what you have already pushed, they will be discarded,
> it is by design, and it is a known deficiency with the fork-point
> heuristics.  How the fork-point heuristics breaks down is rather
> well known ...

Another tangent, this time very closely related to this topic, is
that it may be worth warning when the fork-point heuristics chooses
the base commit that is different from the original upstream,
regardless of how we ended up using fork-point heuristics.

Experienced users may not be confused when the heuristics kicks in
and when it does not (e.g. because they configured, because they
used the "lazy" form, or because they gave "--fork-point" from the
command line explicitly), but they still may get surprising results
if a reflog entry chosen to be used as the base by the heuristics is
not what they expected to be used, and can lose their work that way.
Imagine that you pushed your work to the remote that is a shared
repository, and then continued building on top of it, while others
rewound the remote branch to eject your work, and your "git fetch"
updated the remote-tracking branch.  You'll be pretty much in the
same situation you had in your reproduction recipe that rewound your
own local branch that you used to build your derived work on and
would lose your work the same way, if you do not notice that the
remote branch has been rewound (and the fork-point heuristics chose
a "wrong" commit from the reflog of your remote-tracking branch.

Perhaps something along the lines of this (not even compile tested,
though)...  It might even be useful to show a shortlog between the
.restrict_revision and .upstream, which is the list of commits that
is potentially lost, but that might turn out to be excessively loud
and noisy in the workflow of those who do benefit from the
fork-point heuristics because their project rewinds branches too
often and too wildly for them to manually keep track of.  I dunno.

 builtin/rebase.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git c/builtin/rebase.c w/builtin/rebase.c
index 50cb85751f..432a97e205 100644
--- c/builtin/rebase.c
+++ w/builtin/rebase.c
@@ -1721,9 +1721,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	if (keep_base && options.reapply_cherry_picks)
 		options.upstream = options.onto;
 
-	if (options.fork_point > 0)
+	if (options.fork_point > 0) {
 		options.restrict_revision =
 			get_fork_point(options.upstream_name, options.orig_head);
+		if (options.restrict_revision &&
+		    options.restrict_revision != options.upstream)
+			warning(_("fork-point heuristics using %s from the reflog of %s"),
+				oid_to_hex(&options.restrict_revision->object.oid),
+				options.upstream_name);
+	}
 
 	if (repo_read_index(the_repository) < 0)
 		die(_("could not read index"));