Re: [PATCH v3] checkout: eliminate unnecessary merge for trivial checkout

[Junio C Hamano <gitster@pobox.com>]

"Ben Peart" <peartben@gmail.com> writes:

> The fact that "git checkout -b NEW" updates the index and as a
> result reflects any changes in the sparse-checkout and the issue 
> Junio pointed out earlier about not calling show_local_changes 
> at the end of merge_working_tree are the only difference in behavior
> I am aware of.  Both of these are easily rectified.
>
> That said, given we are skipping huge amounts of work by no longer 
> merging the commit trees, generating a new index, and merging the 
> local modifications in the working tree, it is possible that there are
> other behavior changes I'm just not aware of.

That is OK.  It is not ok to leave such bugs at the end of the
development before the topic is merged to 'master' to be delivered
to the end users, but you do not have to fight alone to produce a
perfect piece of code with your first attempt.  That's what the
reviews and testing period are for.

If you are shooting for the same behaviour, then that is much better
than "make 'checkout -b NEW' be equivalent to a sequence of
update-ref && symbolic-ref, which is different from others", which
was the second explanation you gave earlier.  I am much happier with
that goal.

But if that is the case, I really do not see any point of singling
out "-b NEW" case.  The following property MUST be kept:

 (1) "git checkout -b NEW", "git checkout", "git checkout HEAD^0"
     and "git checkout HEAD" (no other parameters to any of them)
     ought to give identical index and working tree.  It is too
     confusing to leave subtly different results that will lead to
     hard to diagnose bugs for only one of them.

That would make the "do we skip unpack_trees() call?" decision a lot
simpler to make, I would suspect.  We only need to see "are the two
trees we would fed unpack_trees() the same as HEAD's tree?" and do
not have to look at new_branch and other irrelevant things at all.
What happens in the ref namespace is immaterial, as making or
skipping an unpack_trees() call would not affect anything other than
the resulting index and the working tree.  If we want to keep that
sparse-checkout wart, we would also need to see if the control file
sparse-checkout keeps in $GIT_DIR/ exists, but the result will be
much simpler set of rules, and would hopefully help remove the "the
optimization kicks in following logic that is an unreviewable-mess"
issue.