Download raw body.
rebase suffers '?' conflicts
On Wed, Apr 12, 2023 at 05:09:07PM +0200, Stefan Sperling wrote:
> On Thu, Apr 13, 2023 at 12:21:29AM +1000, Mark Jamsek wrote:
> > Nice! Thanks for the explanation, it all makes sense and is good to
> > know. I thought this was going to be a lot more work to fix.
>
> My fix is not quite correct. The contents of the file 'beta' are
> still from newbranch at the end of the test.
> I need to take another look at this.
Turns out there is another fairly trivial bug:
rebase -a is opening the wrong commit to diff against for checkout.
Files should be checked out from the commit we are moving the work
tree towards, rather than from the tip commit of the temporary branch
we are moving away from.
To fix the issue we can open the commit once the base ID has been
changed, instead of before. Now we see beta getting updated as part
of 'got rebase -a', as it should be.
The histedit abort command has the same bug.
The previously diagnosed issue is real regardless and affects a regression
test in update.sh. I have tweaked this fix slightly to only set the new
base commit ID if one is expected to be set on this file index entry.
Two diffs below:
-----------------------------------------------
update the base commit ID of unmodified files if the blob ID matches
This avoids having to run 'got update' after operations which run
checkout_files() and where a file happens to have the same blob ID
but its base commit differs from the commit we are updating to.
Found while investigating a bug report by naddy@
diff 859316d03c1ee64b2c2fd2787e727daadf60bcf3 4c67af7be6dc1185397d8ec2f63bff094bcce949
commit - 859316d03c1ee64b2c2fd2787e727daadf60bcf3
commit + 4c67af7be6dc1185397d8ec2f63bff094bcce949
blob - 9e87414b038cb6aa0ef965f162999f1108c5d82a
blob + cf84e01c6343a70a2374f95e81c6952c1ce51ad6
--- lib/worktree.c
+++ lib/worktree.c
@@ -1997,6 +1997,12 @@ update_blob(struct got_worktree *worktree,
memcmp(ie->blob_sha1, te->id.sha1,
SHA1_DIGEST_LENGTH) == 0) {
/* Different commit but the same blob. */
+ if (got_fileindex_entry_has_commit(ie)) {
+ /* Update the base commit ID of this file. */
+ memcpy(ie->commit_sha1,
+ worktree->base_commit_id->sha1,
+ sizeof(ie->commit_sha1));
+ }
err = sync_timestamps(worktree->root_fd,
path, status, ie, &sb);
if (err)
blob - 822844175d6069cb0ad59c5bd870b63d886b9990
blob + c08237c6cd1b14453b8e4f8e39789a3c6c017f1c
--- regress/cmdline/update.sh
+++ regress/cmdline/update.sh
@@ -1899,12 +1899,7 @@ test_update_modified_submodules() {
(cd $testroot/repo && git add repo2)
git_commit $testroot/repo -m "modified submodule link"
- # This update only records the new base commit. Otherwise it is a
- # no-op change because Got's file index does not track submodules.
- echo -n "Updated to refs/heads/master: " > $testroot/stdout.expected
- git_show_head $testroot/repo >> $testroot/stdout.expected
- echo >> $testroot/stdout.expected
-
+ echo "Already up-to-date" > $testroot/stdout.expected
(cd $testroot/wt && got update > $testroot/stdout)
cmp -s $testroot/stdout.expected $testroot/stdout
-----------------------------------------------
fix rebase/histedit -a leaving some files on the temporary branch
The commands 'got rebase -a' and 'got histedit -a' were checking out
files from the wrong commit. Make them check files out from the commit
we are switching the work tree to, as intended.
Avoids spurious merge conflicts when the work tree is later used for
another rebase operation. It also makes 'got update' right after
'rebase -a' a no-op, as it should be.
Problem found by naddy@ while rebasing jca's llvm15 branch
diff 4c67af7be6dc1185397d8ec2f63bff094bcce949 588fa0d487b8c4d088e9b536bad4bb6eeec34194
commit - 4c67af7be6dc1185397d8ec2f63bff094bcce949
commit + 588fa0d487b8c4d088e9b536bad4bb6eeec34194
blob - cf84e01c6343a70a2374f95e81c6952c1ce51ad6
blob + 7baaf97d3128b44b6690ee4e4a24bbbbd2d7c1c8
--- lib/worktree.c
+++ lib/worktree.c
@@ -7254,11 +7254,6 @@ got_worktree_rebase_abort(struct got_worktree *worktre
if (err)
return err;
- err = got_object_open_as_commit(&commit, repo,
- worktree->base_commit_id);
- if (err)
- goto done;
-
err = got_ref_open(&resolved, repo,
got_ref_get_symref_target(new_base_branch), 0);
if (err)
@@ -7281,6 +7276,11 @@ got_worktree_rebase_abort(struct got_worktree *worktre
if (err)
goto done;
+ err = got_object_open_as_commit(&commit, repo,
+ worktree->base_commit_id);
+ if (err)
+ goto done;
+
err = got_object_id_by_path(&tree_id, repo, commit,
worktree->path_prefix);
if (err)
@@ -7625,11 +7625,6 @@ got_worktree_histedit_abort(struct got_worktree *workt
if (err)
return err;
- err = got_object_open_as_commit(&commit, repo,
- worktree->base_commit_id);
- if (err)
- goto done;
-
err = got_ref_open(&resolved, repo,
got_ref_get_symref_target(branch), 0);
if (err)
@@ -7643,6 +7638,11 @@ got_worktree_histedit_abort(struct got_worktree *workt
if (err)
goto done;
+ err = got_object_open_as_commit(&commit, repo,
+ worktree->base_commit_id);
+ if (err)
+ goto done;
+
err = got_object_id_by_path(&tree_id, repo, commit,
worktree->path_prefix);
if (err)
blob - 6a4b676aff1b3de6b3384590c038eb5b5045981b
blob + bb762cd05b066923e9538c4bdf31982707dba134
--- regress/cmdline/rebase.sh
+++ regress/cmdline/rebase.sh
@@ -413,11 +413,16 @@ test_rebase_abort() {
local init_commit=`git_show_head $testroot/repo`
(cd $testroot/repo && git checkout -q -b newbranch)
- echo "modified alpha on branch" > $testroot/repo/alpha
- git_commit $testroot/repo -m "committing to alpha on newbranch"
+ echo "modified beta on branch" > $testroot/repo/beta
+ git_commit $testroot/repo -m "committing to beta on newbranch"
local orig_commit1=`git_show_head $testroot/repo`
local short_orig_commit1=`trim_obj_id 28 $orig_commit1`
+ echo "modified alpha on branch" > $testroot/repo/alpha
+ git_commit $testroot/repo -m "committing to alpha on newbranch"
+ local orig_commit2=`git_show_head $testroot/repo`
+ local short_orig_commit2=`trim_obj_id 28 $orig_commit2`
+
(cd $testroot/repo && git checkout -q master)
echo "modified alpha on master" > $testroot/repo/alpha
git_commit $testroot/repo -m "committing to alpha on master"
@@ -436,9 +441,17 @@ test_rebase_abort() {
(cd $testroot/wt && got rebase newbranch > $testroot/stdout \
2> $testroot/stderr)
- echo "C alpha" > $testroot/stdout.expected
+ new_commit1=$(cd $testroot/wt && got info beta | \
+ grep '^based on commit:' | cut -d' ' -f4)
+ local short_new_commit1=`trim_obj_id 28 $new_commit1`
+
+ echo "G beta" > $testroot/stdout.expected
+ echo -n "$short_orig_commit1 -> $short_new_commit1" \
+ >> $testroot/stdout.expected
+ echo ": committing to beta on newbranch" >> $testroot/stdout.expected
+ echo "C alpha" >> $testroot/stdout.expected
echo "Files with new merge conflicts: 1" >> $testroot/stdout.expected
- echo -n "$short_orig_commit1 -> merge conflict" \
+ echo -n "$short_orig_commit2 -> merge conflict" \
>> $testroot/stdout.expected
echo ": committing to alpha on newbranch" >> $testroot/stdout.expected
cmp -s $testroot/stdout.expected $testroot/stdout
@@ -461,12 +474,12 @@ test_rebase_abort() {
echo '<<<<<<<' > $testroot/content.expected
echo "modified alpha on master" >> $testroot/content.expected
- echo "||||||| 3-way merge base: commit $init_commit" \
+ echo "||||||| 3-way merge base: commit $orig_commit1" \
>> $testroot/content.expected
echo "alpha" >> $testroot/content.expected
echo "=======" >> $testroot/content.expected
echo "modified alpha on branch" >> $testroot/content.expected
- echo ">>>>>>> merged change: commit $orig_commit1" \
+ echo ">>>>>>> merged change: commit $orig_commit2" \
>> $testroot/content.expected
cat $testroot/wt/alpha > $testroot/content
cmp -s $testroot/content.expected $testroot/content
@@ -496,6 +509,7 @@ test_rebase_abort() {
echo "Switching work tree to refs/heads/master" \
> $testroot/stdout.expected
echo 'R alpha' >> $testroot/stdout.expected
+ echo 'U beta' >> $testroot/stdout.expected
echo "Rebase of refs/heads/newbranch aborted" \
>> $testroot/stdout.expected
@@ -519,13 +533,56 @@ test_rebase_abort() {
(cd $testroot/wt && got log -l3 -c newbranch \
| grep ^commit > $testroot/stdout)
- echo "commit $orig_commit1 (newbranch)" > $testroot/stdout.expected
+ echo "commit $orig_commit2 (newbranch)" > $testroot/stdout.expected
+ echo "commit $orig_commit1" >> $testroot/stdout.expected
echo "commit $init_commit" >> $testroot/stdout.expected
cmp -s $testroot/stdout.expected $testroot/stdout
ret=$?
if [ $ret -ne 0 ]; then
diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
fi
+
+ (cd $testroot/wt && got info .| \
+ grep '^based on commit:' | sort | uniq > $testroot/stdout)
+ echo "based on commit: $master_commit" > $testroot/stdout.expected
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ (cd $testroot/wt && got status > $testroot/stdout)
+ echo "? unversioned-file" > $testroot/stdout.expected
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ cat $testroot/wt/beta > $testroot/content
+ echo 'beta' > $testroot/content.expected
+ cmp -s $testroot/content.expected $testroot/content
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/content.expected $testroot/content
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # A subsequent update should be a no-op.
+ (cd $testroot/wt && got update > $testroot/stdout)
+ echo 'Already up-to-date' > $testroot/stdout.expected
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ fi
test_done "$testroot" "$ret"
}
rebase suffers '?' conflicts