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

Re: [RFC PATCH 1/1] rebase --onto: Skip previously applied commits

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Dec 12, 2022, 12:27 UTC
Message-ID
<221212.86cz8o6bwp.gmgdl@evledraar.gmail.com>
In-Reply-To
<20221212113516.27816-2-cristian.ciocaltea@collabora.com>
On Mon, Dec 12 2022, Cristian Ciocaltea wrote:
Show 35 quoted lines
> When rebase is used with '--onto <newbase>', the patches that might have
> been already applied on <newbase> are not detected, unless they resolve
> to an empty commit. When the related files contain additional changes
> merged, the rebase operation fails due to conflicts that require manual
> intervention.
>
> Ensure the '--onto' variant behaviour is consistent with the common
> rebase by dropping the already applied commits on the target branch.
>
> Note the current behavior is still reachable by using the
> '--reapply-cherry-picks' flag.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  builtin/rebase.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index b22768ca5b9f..2907c6db5cce 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1659,8 +1659,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  		strbuf_addstr(&buf, "...");
>  		strbuf_addstr(&buf, branch_name);
>  		options.onto_name = xstrdup(buf.buf);
> -	} else if (!options.onto_name)
> +	} else if (!options.onto_name) {
>  		options.onto_name = options.upstream_name;
> +	} else if (options.upstream) {
> +		options.restrict_revision = options.upstream;
> +		options.upstream = NULL;
> +	}
>  	if (strstr(options.onto_name, "...")) {
>  		if (get_oid_mb(options.onto_name, &branch_base) < 0) {
>  			if (keep_base)

When I apply this & run the tests e.g. t3418 will segfault, and t3403 seems to fail due to the new behavior not having adjusted the test.

Which would be my "C" for the "RFC" :)
I.e. try to get the tests working, and not segfaulting.

If this is a good idea UX wise (I haven't formed an opinion) the main thing that should inform that is having to decide on the various cases that the tests are checking for already.

Do I understand this correctly that we'll currently stop and requise a "git rebase --continue" if we have an empty patch with "--onto", but without "--onto" we'll just print a warning, and that you'd like "--onto" to be consistent with the non-onto case?

Previous: Cristian CiocalteaNext: Cristian Ciocaltea
Message 3 of 12 in “rebase --onto detection of already applied commits”
  1. 0/1 rebase --onto detection of already applied commitsCristian Ciocaltea, Dec 12, 2022
  2. 1/1 rebase --onto: Skip previously applied commitsCristian Ciocaltea, Dec 12, 2022
  3. Ævar Arnfjörð BjarmasonDec 12, 2022
  4. Cristian CiocalteaDec 12, 2022
  5. Junio C HamanoDec 13, 2022
  6. Cristian CiocalteaDec 13, 2022
  7. Junio C HamanoDec 13, 2022
  8. Phillip WoodDec 13, 2022
  9. Cristian CiocalteaDec 13, 2022
  10. Phillip WoodDec 15, 2022
  11. Cristian CiocalteaDec 15, 2022
  12. Phillip WoodDec 15, 2022

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.