git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Sep 4, 2023, 10:16 UTC
Message-ID
<d9710161-ddb8-4b0a-9729-6b54cd56427d@gmail.com>
In-Reply-To
<xmqqlednuagl.fsf@gitster.g>
On 03/09/2023 05:50, Junio C Hamano wrote:
Show 14 quoted lines
> Junio C Hamano <gitster@pobox.com> 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.

I think that is a good idea and would help to mitigate the surprise that some users have expressed when --fork-point kicks and they didn't know about it. I think we may want to compare "branch_base" which holds the merge-base of HEAD and upstream with "restrict_revision" to decide when to warn.

Best Wishes
Phillip
Show 50 quoted lines
> 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"));
> 
Previous: Junio C HamanoNext: Wesley Schwengle
Message 14 of 23 in “builtin/rebase.c: Emit warning when rebasing without a forkpoint”
  1. 1/2 builtin/rebase.c: Emit warning when rebasing without a forkpointWesley Schwengle, Aug 19, 2023
  2. 1/2 builtin/rebase.c: Emit warning when rebasing without a forkpointWesley Schwengle, Aug 19, 2023
  3. Junio C HamanoAug 31, 2023
  4. Junio C HamanoAug 31, 2023
  5. Phillip WoodSep 1, 2023
  6. Junio C HamanoSep 1, 2023
  7. Emit warning when rebasing without a forkpointWesley Schwengle, Sep 2, 2023
  8. 2/3 builtin/rebase.c: Emit warning when rebasing without a forkpointWesley Schwengle, Sep 2, 2023
  9. Junio C HamanoSep 2, 2023
  10. WesleySep 3, 2023
  11. Junio C HamanoSep 3, 2023
  12. Wesley SchwengleSep 3, 2023
  13. Junio C HamanoSep 5, 2023
  14. Phillip WoodSep 4, 2023
  15. 1/3 rebase.c: Make a distiction between rebase.forkpoint and --fork-point argumentsWesley Schwengle, Sep 2, 2023
  16. 3/3 git-rebase.txt: Add deprecation notice to the --fork-point optionsWesley Schwengle, Sep 2, 2023
  17. Phillip WoodSep 1, 2023
  18. WesleySep 1, 2023
  19. Junio C HamanoSep 1, 2023
  20. WesleySep 2, 2023
  21. Junio C HamanoSep 2, 2023
  22. 2/2 git-rebase.txt: Add deprecation notice to the --fork-point optionsWesley Schwengle, Aug 19, 2023
  23. Wesley SchwengleAug 31, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.