"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: three bugs related to merge commits
To:
James Cook <falsifian@falsifian.org>
Cc:
gameoftrees@openbsd.org
Date:
Fri, 14 Apr 2023 15:52:57 +0200

Download raw body.

Thread
On Mon, Mar 13, 2023 at 12:01:42AM +0000, James Cook wrote:
> I recently noticed the following bugs. I just verified that they still
> happen with the lastest got, commit 264c43dd.
> 
> Two of them have scripts to reproduce; the other's pretty simple.
 
I would like to fix these issues but haven't found time to take
a closer look yet, sorry.

Would it be possible to convert your reproducers into tests that can
be run with the test suite (which is also based on shell scripts)?
That would be my first step anyway when looking at fixing these issues.
We will want to be sure that the bugs won't resurface at a later time
once we've fixed them.

> ** First bug: got ci doesn't properly check for updated-ness, and
>    clobbers merge commit with surprising results
> 
> A script to reproduce follows. First, though, a description. We set up
> the following four commits, where a commit "branch" from another branch
> got merged into the main branch.
> 
> merge
> |   \
> 1   branch
> |  /
> init
> 
> Now, in another worktree pointing to the "1" commit, we make a change
> and run "got ci". This should fail because the worktree is not updated.
> Instead, the merge commit is forgotten, and we end up with:
> 
> 2
> |
> 1
> |
> init
> 
> As fallout from this strange situation, two weird things happen:
> - The commit "2" includes the changes from the commit "branch", even
>   though they weren't in the work tree when we ran "got ci" to create
>   that commit.
> - In the worktree pointing to the commit "merge", got up refuses to run,
>   saying "got: work tree's head reference now points to a different
>   branch; new head reference and/or update -b required"
> 
> ---- BEGIN script ----
> #!/bin/sh
> set -e
> 
> mkdir bug;cd bug
> 
> # Create a repo with an initial commit.
> gotadmin init repo.git
> mkdir init;touch init/f
> got import -m init -r repo.git init
> 
> # For convenience, we'll use three work trees.
> for name in branch merge linear; do got checkout repo.git $name; done
> 
> # Make a branch with one new commit.
> cd branch
> got branch aux
> echo 1 > on_branch
> got add on_branch
> got commit -m branch
> 
> # Commit to the main branch; put that in the other two work trees.
> cd ../linear
> echo 1 > f
> got commit -m 1
> cd ../merge
> got up
> 
> # In the worktree called "merge", add a merge commit to main.
> got merge aux
> 
> # In the worktree called "linear", add a commit *without updating*.
> cd ../linear
> echo 2 > f
> got commit -m 2
> 
> # First problem:
> #
> # Examine the commit that was created in the worktree at bug/linear.
> # Surprisingly, it adds the file "on_branch", but does not include
> # either the commit called "branch" or the merge commit in its history.
> 
> # Second problem:
> cd ../merge
> got up
> # got complains: "got: work tree's head reference now points to a different branch; new head reference and/or update -b required"
> ---- END script ----
> 
> ** Second bug: can't send after merging origin/main
> 
> This one's simpler. An argument could be made it's not a bug, but the
> behaviour is certainly different from git.
> 
> - Clone a remote repo.
> - Make a change in the remote repo.
> - Separately, make a change in your local clone.
> - In the local clone, run got fetch then got merge origin/main.
> - Run got send.
> 
> got send says:
> 	got: refs/heads/main: fetch and rebase required
> Coming from git, my expectation is that I can send the new commit since
> it includes the remote head in its history. If the goal of "got merge"
> is only to support vendor branches, then this behaviour might be
> reasonable, since it indicates a departure from that work flow: the
> remote main should appear in the "linear history" ignoring second merge
> children. Still, though, a different error message would help.
> 
> ** Third bug: Can't merge a branch with identical content.
> 
> Create two branches A and B, where A includes all of B's changes, but as
> different commit objects. Then try to merge B into A. Got complains:
> 	got: merge of refs/heads/aux cannot proceed: no changes to commit
> 
> I do occasionally want to create merge commits like this. For example,
> it happened while I was cleaning up after I encountered the first bug
> above: I ended up with divergent commits making some of the same
> changes, and wanted to incorporate all of them into my commit graph for
> a historical record.
> 
> Here's a script to reproduce; for simplicity, the changes are not just a
> subset but exactly the same.
> ---- BEGIN script ----
> #!/bin/sh
> set -e
> 
> mkdir bug;cd bug
> 
> # Create and check out a repo with an initial commit.
> gotadmin init repo.git
> mkdir init;touch init/f
> got import -m init -r repo.git init
> got checkout repo.git; cd repo
> 
> # Create a branch with a commit.
> got branch aux
> echo 1 > f
> got ci -m branch
> 
> # Commit the same change (but different commit object) to main.
> got up -b main
> echo 1 > f
> got ci -m main
> 
> # Try to merge.
> got merge aux
> ---- END script ----
> 
> -- 
> James
> 
>