From: Junio C Hamano Date: Fri, 06 May 2011 22:49:15 GMT Subject: Re: [PATCH] Teach checkout the -n|--dry-run option Message-ID: <7vhb97xx5g.fsf@alter.siamese.dyndns.org> In-Reply-To: <4DC47242.6060205@web.de> Jens Lehmann 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 > @@ -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".