On Sat, Oct 02, 2021 at 06:04:30PM +0200, Omar Polo wrote:
> Hello,
> 
> I think I've found an error in the way histedit manages diffs: if a
> commit adds a line *as the first one in the file*, after a histedit
> operation that line is mis-placed (even if the histedit operation for
> that commit was a plain 'pick'.)
> 
> I've found this while working with GNU-style ChangeLog files: news
> entries are added at the top of the file, and every time I did a
> histedit the ChangeLog was corrupted.
> 
> I'm attaching a diff with a test that exposes the problems, but I don't
> have any idea on how to actually fix it, sorry.  It fails with
> 
>   % ./histedit.sh -q -r /tmp
>   M  alpha
>   Created commit b13fd327b6b9f9d51827adcc4129d44218b48b91
>   U  alpha
>   Updated to refs/heads/master: dccc3ac37ab55c3347cf40ff6dff882cfe93d5bd
>   G  alpha
>   b13fd327b6b9 -> 831486a4e469: modified alpha on master
>   Switching work tree to refs/heads/master
>   --- /tmp/got-test-histedit_prepend_line-EdfGRt41/alpha.expected Sat Oct  2 18:32:14 2021
>   +++ /tmp/got-test-histedit_prepend_line-EdfGRt41/wt/alpha       Sat Oct  2 18:32:15 2021
>   @@ -1,2 +1,2 @@
>   -first line
>    alpha
>   +first line
>   test failed; leaving test data in /tmp/got-test-histedit_prepend_line-EdfGRt41
> 
> Cheers,
> 
> Omar Polo

This is a bug in the ed script output produced by our diff code,
which lives in a separate repository:
https://git.gameoftrees.org/gitweb/?p=diff.git;a=summary

Comparing ed-script output between OpenBSD diff(1) and our diff shows
the problem: When the changed file has inserted lines at the beginning
of the file, those lines need to be appended to the merge result after
line 0, not after line 1:

  $ cat /tmp/alpha
  alpha
  $ cat /tmp/alpha2
  first line
  alpha
  $ diff -e /tmp/alpha /tmp/alpha2
  0a
  first line
  .
  $ ~/src/diff/diff/obj/diff -e /tmp/alpha /tmp/alpha2
  1a1
  $

The patch below tweaks your test (which still failed with the bug
fixed because the final comparison was done against repo/alpha
instead of wt/alpha), and fixes the issue:

  $ ~/src/diff/diff/obj/diff -e /tmp/alpha /tmp/alpha2
  0a1
  $

I would commit this fix to diff.git first, then sync the fix over
to the got.git repository, then commit the new regression test.

ok?


diff 41f061b2f459318f3738f59d7676efccc4beb344 /home/stsp/src/got
blob - 1c9b6d17294abf78edffde84b7937856f2620bb1
file + lib/diff_output_edscript.c
--- lib/diff_output_edscript.c
+++ lib/diff_output_edscript.c
@@ -45,8 +45,10 @@ output_edscript_chunk(struct diff_output_info *outinfo
 		left_start = 0;
 	else if (left_len == 0 && cc->left.start > 0)
 		left_start = cc->left.start;
-	else
+	else if (cc->left.end > 0)
 		left_start = cc->left.start + 1;
+	else
+		left_start = cc->left.start;
 
 	right_len = cc->right.end - cc->right.start;
 	if (right_len < 0)
@@ -55,8 +57,10 @@ output_edscript_chunk(struct diff_output_info *outinfo
 		right_start = 0;
 	else if (right_len == 0 && cc->right.start > 0)
 		right_start = cc->right.start;
-	else
+	else if (cc->right.end > 0)
 		right_start = cc->right.start + 1;
+	else
+		right_start = cc->right.start;
 
 	if (left_len == 0) {
 		/* addition */
blob - 051f0bedccbe45d193bdeb08943bd0f9767a2f04
file + regress/cmdline/histedit.sh
--- regress/cmdline/histedit.sh
+++ regress/cmdline/histedit.sh
@@ -1917,6 +1917,62 @@ EOF
 	test_done "$testroot" "$ret"
 }
 
+test_histedit_prepend_line() {
+	local testroot=`test_init histedit_prepend_line`
+	local orig_commit=`git_show_head $testroot/repo`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+
+	ed "$testroot/wt/alpha" <<EOF >/dev/null 2>&1
+0i
+first line
+.
+wq
+EOF
+
+	cp $testroot/wt/alpha $testroot/content.expected
+
+	(cd $testroot/wt/ && got commit -m 'modified alpha on master' \
+		alpha > /dev/null)
+	ret="$?"
+	if [ "$?" != 0 ]; then
+		echo "got commit failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	local top_commit=`git_show_head $testroot/repo`
+	echo "pick $top_commit" > "$testroot/histedit-script"
+
+	(cd $testroot/wt/ && got update -c $orig_commit > /dev/null)
+	ret="$?"
+	if [ "$?" != 0 ]; then
+		echo "got update failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	(cd $testroot/wt && got histedit -F "$testroot/histedit-script" \
+		> /dev/null)
+	ret="$?"
+	if [ "$?" != 0 ]; then
+		echo "got histedit failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cp $testroot/wt/alpha $testroot/content
+	cmp -s $testroot/content.expected $testroot/content
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/content.expected $testroot/content
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	test_done "$testroot" $ret
+}
+
 test_parseargs "$@"
 run_test test_histedit_no_op
 run_test test_histedit_swap
@@ -1936,3 +1992,4 @@ run_test test_histedit_fold_add_delete
 run_test test_histedit_fold_only
 run_test test_histedit_fold_only_empty_logmsg
 run_test test_histedit_edit_only
+run_test test_histedit_prepend_line