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

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: [bug] histedit and the first line of a file
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Fri, 08 Oct 2021 10:12:12 +0200

Download raw body.

Thread
Sorry for the delay!

Stefan Sperling <stsp@stsp.name> writes:

> On Sat, Oct 02, 2021 at 06:04:30PM +0200, Omar Polo wrote:
>> [...]
>
> 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:

ops :D

>   $ ~/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?

The regress suite is passing and I did some more manual test but I
couldn't reproduce the bug.

FWIW the patch reads fine

Thanks! :D

>
> 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