From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got patch: fix applying on an empty file
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Tue, 23 Apr 2024 09:43:54 +0200

On Tue, Apr 23, 2024 at 09:38:43AM +0200, Omar Polo wrote:
> naddy found out that mpi's percpucaches diff fails to apply.  The issue
> is that sys/uvm/uvm_percpu.h is currently empty, so locate_hunk gets an
> EOF with match still being -1.

Yep, I also ran into it :)

> This is an error because if we reach the end of the file without having
> found a match, we can't apply the hunk.  Except if the file is empty :)
> 
> diff /home/op/w/got
> commit - 533c404ec30ef30690b8c41481cbdbbeeb8e2a5c
> path + /home/op/w/got
> blob - 8a971703968041dd421027fa38cfe23a737b0dee
> file + lib/patch.c
> --- lib/patch.c
> +++ lib/patch.c
> @@ -417,15 +417,18 @@ locate_hunk(FILE *orig, struct got_patch_hunk *h, off_
>  	int mangled = 0, match_lineno = -1;
>  
>  	for (;;) {
> +		(*lineno)++;
>  		linelen = getline(&line, &linesize, orig);
>  		if (linelen == -1) {
>  			if (ferror(orig))
>  				err = got_error_from_errno("getline");
> -			else if (match == -1)
> +			/* An EOF is fine iff the target file empty. */
> +			if (feof(orig) && match == -1 && h->old_lines != 0)

Comment should say: ... file is empty.

>  				err = got_error(GOT_ERR_HUNK_FAILED);
> +			match = 0;
> +			match_lineno = (*lineno)-1;
>  			break;
>  		}
> -		(*lineno)++;
>  
>  		if ((mode == ' ' && lines_eq(l, line, linelen, &mangled)) ||
>  		    (mode == '-' && lines_eq(l, line, linelen, &mangled)) ||
> blob - 705adb527c252d6e5694bcd07fc8aa74e35632de
> file + regress/cmdline/patch.sh
> --- regress/cmdline/patch.sh
> +++ regress/cmdline/patch.sh
> @@ -811,6 +811,41 @@ EOF
>  	test_done $testroot $ret
>  }
>  
> +test_patch_empty_file() {
> +	local testroot=`test_init patch_empty_file`
> +
> +	got checkout $testroot/repo $testroot/wt > /dev/null
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		test_done $testroot $ret
> +		return 1
> +	fi
> +
> +	> $testroot/wt/alpha

Could you write the above line like this?

  echo -n > $testroot/wt/alpha

Either way, ok by me.

> +	(cd "$testroot/wt" && got commit -m 'edit alpha' alpha) >/dev/null
> +	cat <<EOF >$testroot/wt/patch
> +--- alpha
> ++++ alpha
> +@@ -0,0 +1 @@
> ++alpha
> +EOF
> +
> +	(cd $testroot/wt && got patch patch) > $testroot/stdout
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		test_done $testroot $ret
> +		return 1
> +	fi
> +
> +	echo 'M  alpha' > $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
> +}
> +
>  test_patch_prefer_new_path() {
>  	local testroot=`test_init patch_orig`
>  
> @@ -2052,6 +2087,7 @@ run_test test_patch_nop
>  run_test test_patch_preserve_perm
>  run_test test_patch_create_dirs
>  run_test test_patch_with_offset
> +run_test test_patch_empty_file
>  run_test test_patch_prefer_new_path
>  run_test test_patch_no_newline
>  run_test test_patch_strip
> 
>