Re: two-way merge corner case bug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> PS I wonder if the "initial_checkout" hack can just go away under this
>    new rule. Although we don't seem to use "o->reset" for the initial
>    checkout. Which seems kind of weird. I would think the initial
>    checkout would actually just be a oneway merge. Is the point to try
>    to leave any existing working tree contents untouched or something?

An initial checkout is *supposed* to happen in an empty working
tree, so if we code it not to overwrite an existing path in the
working tree, the user cannot lose possibly precious contents with
an mistaken initial checkout (they will instead appear as modified
relative to the index), while in the normal case we will write out
the contents from the HEAD through the index.  We could attempt "we
do not have to if the user behaves, but with this we could help
misbehaving users" if we used twoway merge for an initial checkout.

Having said that, I notice that in the normal codepath (e.g. "git
clone" without the "--no-checkout" option) we no longer use twoway
merge for the initial checkout.  Back when "git clone" was a
scripted Porcelain, I think we used to do a twoway read-tree.  It
may be that we broke it when "clone" was rewritten in C, but the
breakage is to the "we do not have to..." thing, so it may not be a
big deal.

The only case that matters in today's code is "git checkout"
(without any option or argument) immediately after "git clone -n", I
think.  The special casing for this initial checkout in twoway merge
is needed because we go from HEAD to HEAD in that case, and we do
not want to keep the artificial local removals from the index; we
start from not even having the $GIT_INDEX_FILE, so without the
special case all paths appear to have been "rm --cached", which is
usually not what the user would want to see ;-)

> ... My worry would be that somebody is
> using "--reset" but expecting the removal to be carried through
> (technically, "--reset" is documented as "-m but discard unmerged
> entries", but we are not really treating it that way here).

I've checked all in-tree uses of "read-tree --reset -u".

Nobody seems to use that combination, either from scripts or from C
(i.e. when opts.update==1 and opts.merge==1, opts.reset is not set)
with a twoway merge, other than "git am --abort/--skip".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]