Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Branch reference is erroneously moved on git move #249

Closed
KevinWuWon opened this issue Dec 29, 2021 · 5 comments · Fixed by #251
Closed

Branch reference is erroneously moved on git move #249

KevinWuWon opened this issue Dec 29, 2021 · 5 comments · Fixed by #251
Assignees
Labels
bug Something isn't working

Comments

@KevinWuWon
Copy link

KevinWuWon commented Dec 29, 2021

Description of the bug

Set up:

$ git init branchless-test
$ cd branchless-test
$ git branchless init --main-branch master
$ echo a > file
$ git add file
$ git commit -a -m 'file = a'
$ git checkout --detach
$ echo b > file
$ git commit -a -m 'file = b'
$ echo c > file
$ git commit -a -m 'file = c'
$ git checkout master

At this point, git sl says:

❯ git sl
◆ 90f135b4 32s (master) file = a
┃
◯ cd85daf6 9s file = b
┃
◯ 34bf98fe 1s file = c

Trigger the bug by moving some commits to the current checkout with git move without -d.

git move -s 34bf98fe --merge

We are checked out to a named branch and we move a commit here that triggers a merge conflict. Resolve the merge conflict with:

$ echo c > file
$ git add file
$ git rebase --continue

Expected behavior

I expected the branch reference to not move. That master still points to the same commit, like this:

◆ 90f135b4 9m (master) file = a
┣━┓
┃ ◯ cd85daf6 9m file = b
┃
◯ c3e02195 1m file = c

Actual behavior

master moved to the end of the chain of rebased commits.

◆ 90f135b4 9m file = a
┣━┓
┃ ◯ cd85daf6 9m file = b
┃
◯ c3e02195 1m (master) file = c

I noticed that I can work around it by specifying the -d parameter to git move. I can specify anything that refers to the current commit. ie., any of these will give me the behavior I expect: -d HEAD, -d master, or -d 90f135b4.

Version of git-branchless

0.3.8

Version of rustc

No response

@KevinWuWon KevinWuWon added the bug Something isn't working label Dec 29, 2021
@KevinWuWon
Copy link
Author

KevinWuWon commented Dec 29, 2021

I wondered if this was working as intended. But I don't think it is; I think it's a bug. Because what if you're rebasing a non-linear tree?

◇ 90f135b4 16m file = a
┣━┓
┃ ◯ cd85daf6 16m file = b
┃ ┣━┓
┃ ┃ ◯ 797e3943 24s file = d
┃ ┃
┃ ◯ cdbf4a24 10s file = e
┃
◆ f23cc614 50s (master) file = z

If you type:

$ git move -s cd85daf6 --merge

Where should master end up? The only noncontroversial answer would be that it shouldn't move. But instead, I get this:

◇ 90f135b4 17m file = a
┃
◆ f23cc614 1m file = z
┃
◇ 6ecfc811 3s (master) file = b
┣━┓
┃ ◯ 494bd44f 2s file = d
┃
◯ 9f474441 1s file = e

@arxanas
Copy link
Owner

arxanas commented Dec 29, 2021

Thanks for the detailed report, as always! I was able to reproduce this. I'm sure it's some kind of silly bug given that it works properly with -d HEAD; they should be equivalent.

@arxanas
Copy link
Owner

arxanas commented Dec 30, 2021

Actually, I wasn't able to reproduce this by passing -d explicitly. I get the be behavior either way. This makes more sense to me, as it's probably a bug with the generation of the on-disk rebase plan.

The default behavior of git rebase is to move the currently checked-out branch along with each commit. git-branchless tries to inhibit this behavior, but evidently it doesn't work right in this case.

@arxanas
Copy link
Owner

arxanas commented Jan 2, 2022

This should be fixed by #251. Let me know if you still see this behavior. As I was testing this, I also found some other misbehaving cases involving merge conflict resolution, which should now also be fixed. Thanks for your report!

@KevinWuWon
Copy link
Author

Thanks for the quick fix! I'll try it out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants