Re: [PATCH] Teach checkout the -n|--dry-run option

From: Junio C Hamano <gitster@pobox.com>
Date: May 6, 2011, 22:49 UTC
Message-ID: <7vhb97xx5g.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4DC47242.6060205@web.de>

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

> This option can be used to determine if checking out a branch or commit
> would touch files which are modified in the work tree. It doesn't change
> the work tree contents or the index, so it can only tell if the checkout
> would succeed using trivial merges.

I hated this patch when I saw it the first time. I thought you were joking and let it pass. But now you brought it again, I hate it even more now.

I do not see a merit in contaminating the checkout Porcelain with a half baked [*1*] "dry-run" code, when your ultimate motive to use it in submodule script and exercise only a small portion of checkout codepaths. You are only interested in branch switching, and not other things the Porcelain can do, like checking out a path out of the index or a tree, or re-checking out a conflicted path out of the index.

In other words, can't the check you need in submodule be scripted around the specific plumbing responsible for the branch switching, which is:

    $ git read-tree -m HEAD $other_branch

Show 9 quoted lines

> @@ -397,7 +400,7 @@ static int merge_working_tree(struct checkout_opts *opts,
>
>  		/* 2-way merge to the new branch */
>  		topts.initial_checkout = is_cache_unborn();
> -		topts.update = 1;
> +		topts.update = !opts->dry_run;
>  		topts.merge = 1;
>  		topts.gently = opts->merge && old->commit;
>  		topts.verbose_update = !opts->quiet;

What you are doing in this part of your patch is exactly that two-tree form of the "read-tree -m", no?

[Footnote]

*1* Look at how many places you say "are incompatible" when you really mean "I didn't bother to think deeply enough about implementing this combination".