Re: [RFC/PATCH 00/18] Add --index-only option to git merge

[Elijah Newren <newren@gmail.com>]

Hi,

On Fri, Apr 8, 2016 at 11:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Elijah Newren <newren@gmail.com> writes:
>
> The goal is stated rather vaguely--when you have a working tree and
> perform this "in index" merge, you would obviously update the index
> with the merge result and ...

Yes, I think you hit the nail on the head with your suggestions
elsewhere to (1) limit this to bare repository only, and/or (2) do the
"git merge --into master en/topic"

I'll fix this.

>> The core fix, to merge-recursive, was actually quite easy.  The
>> recursive merge logic already had the ability to ignore the working
>> directory and operate entirely on the index -- it needed to do this
>> when creating a virtual merge base, i.e. when o->call_depth > 0.
>
> You need to be careful here, though.  The creation of a virtual
> parent accepts (actually it wants to have) conflicted blobs as if
> that is a valid merge result--"git merge but only in index" probably
> does not want that behaviour.

Yes, precisely.  :-)

A close look at the merge-recursive code showed that the code to
handle the index-only behavior was already nicely separated from the
other things triggered by o->call_depth (e.g. the conflicted blobs
case you mention above, among others), making it really easy to modify
it so we could get the index-only behavior without the other stuff.

> I'd prefer these as a separate series if they are truly unrelated
> cleanups, so that we can quickly review and have them graduated
> without waiting for the remainder.  You can still submit the main
> series with a comment that says it applies on that separate clean-up
> series and the right things should happen ;-)

Will do.

> When you say "index only", I'd say people would understand that to
> be saying "as opposed to using and updating both the index and the
> working tree", so I do not think there is no confusion.
>
> An established convention to spell "index only" found in "apply" and
> "grep" is to say "--cached", though (cf. "git help cli").

I'm assuming your double negative was unintentional, i.e. that you do
not think there is any confusion.  Let me know if I misread.

The pointer is helpful, but I struggle a bit with the name '--cached'.
The past tense in that word suggests to me that it should be a
read-only operation on the cache (which works for grep), rather than a
write operation (such as for merge or apply).  It could also be
misconstrued in merge's case to think that the index is one of the
things being merged (rather than erroring out if the index doesn't
match HEAD).  I know apply already uses --cached, but if others don't
mind, I personally would rather not use it here.  Is this something
you feel strongly about, or are you okay with --index-only?

> If you have to assume, assume that there are people who use these
> programs in their scripts and workflows, because it is a relatively
> new development to make "git merge" directly calling into the
> recursive machinery bypassing these commands.

So, my question wasn't so much about whether the git-merge-* programs
are used, as to whether they should support all the same options that
e.g. "git merge -s recursive" does.  I'm not sure if having the old
merge-* programs support fewer options is considered good ("we want to
toss them eventually", or "they are less tested these days so we want
to take less risk with modifying them") or bad ("we don't want there
to be any difference in ability or behavior").

It should be easy to add the --index-only option to each of these, I'm
just unsure whether it matters or if it's wanted.

> I have a suspicion that this would become moot, as the conclusion
> may become that "git merge" that works only in index does not make
> sense unless you are in a bare repository---in which case, these
> external merge drivers has to be given a temporary working tree
> anyway, and they can keep writing their result out to what they
> consider is the working tree (i.e. via GIT_WORK_TREE or something).
> You read the result of them from that temporary working tree into
> the index before cleaning the temporary working tree.  That way,
> you do not have to touch them at all, no?  In fact, the temporary
> working tree trick may be applicable even when you are working in a
> repository with a tree working tree.

I'm a little confused; you seem to be suggesting git-merge-one-file
will always need to have a working tree of some sort, though I don't
see why.

git-merge-one-file doesn't currently read from the working tree,
except to check for untracked files in the way; it instead gets file
contents from git unpack-file, which pulls it from the object store.
git-merge-one-file currently records the merge resolution, if clean,
in both the index and the working copy, so my modifications are about
making it able to write to only the index. That's the only
modification I think it needs to avoid requiring a working tree at
all.

(git-merge-one-file does create some temporary files in the current
working directory via git unpack-file, but it doesn't need a full
working tree for that.  I should also make it a little cleaner by
having it not check for the presence of untracked-and-in-the-way files
in the --index-only case.)