Re: [WIP PATCH] Teach read-tree the -n|--dry-run option

From: Junio C Hamano <gitster@pobox.com>
Date: May 17, 2011, 22:47 UTC
Message-ID: <7vpqnheygi.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4DD2CFD4.7060508@web.de>

Jens Lehmann <Jens.Lehmann@web.de> writes:

Show 9 quoted lines

>> I think the change to the implementation would be trivial, but you would
>> probably want to update the tests in 1002 (remove the one at the end that
>> checks -u/-n incompatibility, and perhaps make sure "-n -u" combination
>> does not touch anything as advertised).
>
> I added two convenience functions read_tree_u_must_[succeed|fail]() which
> work pretty much the way read_tree_must_[succeed|fail]() do but with the
> added functionality that they check git diff-files before and after the
> read-tree -n too.

Sounds sensible.

Show 6 quoted lines

> The only exceptions where I could not make that replacement without
> breaking a test were:
>
> * t1001:381 (#25) new file conflicts with untracked directory
>   This should have been detected by verify_absent_1(), but that function
>   is left early when opts->update is 0. Will look into that.

Thanks. This is the kind of corner case I was worried about when I said "we might have a case where read-tree -m succeeds but read-tree -m -u doesn't". Fixing it would mean that ...

Show 9 quoted lines

> @@ -183,6 +184,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
>  		die("--exclude-per-directory is meaningless unless -u");
>  	if (opts.merge && !opts.index_only)
>  		setup_work_tree();
> +	if (opts.update && dry_run)
> +		opts.update = 0;
>
>  	if (opts.merge) {
>  		if (stage < 2)

... this hunk must go, right?

Show 15 quoted lines

> diff --git a/t/t1001-read-tree-m-2way.sh b/t/t1001-read-tree-m-2way.sh
> index 680d992..54e69ce 100755
> --- a/t/t1001-read-tree-m-2way.sh
> +++ b/t/t1001-read-tree-m-2way.sh
> @@ -48,6 +48,25 @@ check_cache_at () {
>  	esac
>  }
>
> +read_tree_must_succeed () {
> +...
> +}
> +
> +read_tree_u_must_succeed () {
> +...
> +}

Now at this point wouldn't it make more sense to move these helpers to lib-read-tree.sh or something and .-source it from all nine tests?