From: Mark Jamsek <mark@jamsek.com>
Subject: Re: got: fix incorrect argc check when diffing blobs
To: Omar Polo <op@omarpolo.com>
Cc: Game of Trees <gameoftrees@openbsd.org>
Date: Mon, 29 Aug 2022 22:44:13 +1000

On 22-08-25 10:13PM, Omar Polo wrote:
> On 2022/08/26 01:44:05 +1000, Mark Jamsek <mark@jamsek.com> wrote:
> > We should check for more than two args as we have two blob ids:
> > 
> >   $ got diff 66173f9f4759b015a8 dae7c1789fc137c1b6927dca
> >   got: path arguments cannot be used when diffing blobs
> > 
> > With the below diff:
> > 
> >   $ got diff 66173f9f4759b015a8 dae7c1789fc137c1b6927dca
> >   blob - 66173f9f4759b015a8e2f0f9acc21194830f066d
> >   blob + dae7c1789fc137c1b6927dca65e7f44b457d1e34
> >   --- 66173f9f4759b015a8e2f0f9acc21194830f066d
> >   +++ dae7c1789fc137c1b6927dca65e7f44b457d1e34
> >   @@ -21,6 +21,14 @@ got:
> >      This must require an up-to-date and clean work tree to avoid unrelated
> >      changes from getting mixed in. Perform an implicit work tree base-commit
> >      bump after committing, like 'got rebase' and 'got histedit' do it.
> >   +- Respect the current umask when creating or changing files and directories
> >   +  in the work tree. This behaviour is already documented in got-worktree(5)
> >   +  but not actually implemented.
> >   +- When a clone fails the HEAD symref will always point to "refs/heads/main"
> >   +  (ie. the internal default HEAD symref of Got). Resuming a failed clone with
> >   +  'got fetch' is supposed to work. To make this easier, if the HEAD symref
> >   +  points to a non-existent reference it should be updated by 'got fetch'
> >   +  to match the HEAD symref sent by the server.
> >   
> >    network protocol:
> >    - add http(s) transport with libtls, speaking the two Git HTTP protocols
> 
> agreed, and the similar codepath for diffing objects has the `argc > 2' check.
> 
> ok for me.
> 
> should we add something like this in regress?

Yes, this is a good idea too. Do you want to commit this with the
related fix?

> diff /home/op/w/got
> commit - f0680473a7db1e5941bffdc2ab5f80ddec209122
> path + /home/op/w/got
> blob - 136f659b08eaf6408f6c79e05f6440fe7c754523
> file + regress/cmdline/diff.sh
> --- regress/cmdline/diff.sh
> +++ regress/cmdline/diff.sh
> @@ -385,6 +385,43 @@ test_diff_basic() {
>  		diff -u $testroot/stderr.expected $testroot/stderr
>  		return 1
>  	fi
> +
> +	# diff two blob ids, but first commit the changes to alpha
> +	(cd $testroot/wt && got commit -m 'edit' alpha >/dev/null)
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		echo "got commit failed" >&2
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	(cd $testroot/wt && got log -l1 -p alpha) > $testroot/log
> +	local bold="$(awk '/^blob \-/{print $3}' $testroot/log)"
> +	local bnew="$(awk '/^blob \+/{print $3}' $testroot/log)"
> +	sed -n	-e "s/^--- alpha/--- $bold/" \
> +		-e "s/^\+\+\+ alpha/+++ $bnew/" \
> +		-e '/^blob -/,$p' \
> +		$testroot/log > $testroot/diff.expected
> +
> +	(cd $testroot/wt && got diff $bold $bnew) > $testroot/diff
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		echo "diff failed unexpectedly" >&2
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	# got log adds an empty line at the bottom of a diff.
> +	echo >> $testroot/diff
> +
> +	cmp -s $testroot/diff.expected $testroot/diff
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		diff -u $testroot/diff.expected $testroot/diff
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
>  	test_done "$testroot" "$ret"
>  }
>  
> 

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68