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

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

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Sep 1, 2023, 13:19 UTC
Message-ID
<6127b570-5e9b-404f-9802-9135a1c9f31f@gmail.com>
In-Reply-To
<20230819203528.562156-2-wesleys@opperschaap.net>
Hi Wesley
On 19/08/2023 21:34, Wesley Schwengle wrote:
Show 32 quoted lines
> When commit d1e894c6d7 (Document `rebase.forkpoint` in rebase man page,
> 2021-09-16) was submitted there was a discussion on if the forkpoint
> behaviour of `git rebase' was sane. In my experience this wasn't sane.
> Git rebase doesn't work if you don't have an upstream branch configured
> (or something that says `merge = refs/heads/master' in the git config).
> The behaviour of `git rebase' was that if you supply an upstream on the
> command line that it behaves as if `--no-forkpoint' was supplied and if
> you don't supply an upstream, it behaves as if `--forkpoint' was
> supplied. This can result in a loss of commits if you don't know that
> and if you don't know about `git reflog' or have other copies of your
> changes. This can be seen with the following reproduction path:
> 
>      mkdir reproduction
>      cd reproduction
>      git init .
>      echo "commit a" > file.txt
>      git add file.txt
>      git commit -m "First commit" file.txt
>      echo "commit b" >> file.txt
>      git commit -m "Second commit" file.txt
> 
>      git switch -c foo
>      echo "commit c" >> file.txt"
>      git commit -m "Third commit" file.txt
>      git branch --set-upstream-to=master
> 
>      git status
>      On branch foo
>      Your branch is ahead of 'master' by 1 commit.
> 
>      git switch master
>      git merge foo

Here "git merge" fast-forwards I think, if instead it created a merge commit there would be no problem as the tip of branch "foo" would not end up in master's reflog.

Show 6 quoted lines
>      git reset --hard HEAD^
>      git switch foo
>      Switched to branch 'foo'
>      Your branch is ahead of 'master' by 1 commit.
> 
>      git log --format='%C(yellow)%h%Creset %Cgreen%s%Creset'
For a reproduction recipe I think "git log --oneline" would suffice.
Show 12 quoted lines
>      5f427e3 Third commit
>      03ad791 Second commit
>      411e6d4 First commit
> 
>      git rebase
>      git status
>      On branch foo
>      Your branch is up to date with 'master'.
> 
>      git log --format='%C(yellow)%h%Creset %Cgreen%s%Creset'
>      03ad791 Second commit
>      411e6d4 First commit

Thanks for the detailed reproduction recipe, I think it would be helpful to summarize what's happening in the commit message, especially as it seems to depend on "git merge" fast-forwarding. Do you often merge a branch into it's upstream and then reset the upstream branch?

I tend to agree with Junio that the current default is pretty reasonable. Looking through the links from the cover letter it seems that the current behavior came from a desire for

	git fetch && git rebase
to behave like
	git pull --rebase

I think the commit message for any change to the default should address why that is undesirable. Also we should consider what problems may arise from not defaulting to --fork-point when rebasing on an upstream branch that has itself been rebased or rewound.

Best Wishes
Phillip
Previous: Wesley SchwengleNext: Wesley
Message 17 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.