Re: [PATCH] builtin-revert: Make use of merge_recursive()

From: Stephan Beyer <s-beyer@gmx.net>
Date: Aug 11, 2008, 23:47 UTC
Message-ID: <20080811234733.GB28340@leksak.fem-net>
In-Reply-To: <7v7ianuo8q.fsf@gitster.siamese.dyndns.org>

Hi,

Junio C Hamano wrote:

> I am not absolutely sure if the phoney commit-looking object that has
> nonsense SHA-1 created by make_virtual_commit() would have any unintended
> side effects to the rest of the system, but it does not look like it is
> even inserted into the global object hash table, so this should be Ok.

Hmm, the git-merge-recursive process that was run before, calls get_ref() and that also used make_virtual_commit() and thus this virtual_id. So if there is some danger regarding this, then it has also been there before my patch :-)

> That was the last piece of worry coming from me regarding this "call
> recursive internally" theme.

Well, for simple cherry-pick/revert your other worries can be ignored, because it only runs merge_recursive() _once_ and the die()s do not hurt. Wrong?

Show 7 quoted lines

> Would we need to consolidate this and Miklos's "call recursive internally
> from git-merge wrapper" by making them into three patches?
> I.e.
> 
>  (1) move bulk of code from builtin-merge-recursive.c to a new file
>      merge-recursive.c and introduce merge_recursive_helper() in there so
>      that both of you and cmd_merge_recursive() itself can call it;

I'd like to see a "libified" merge-recursive.c, but I wouldn't call the interesting function merge_recursive_helper(), I'd just take merge_recursive(). Of course those index locking could be done in it.

Looking at my sequencer code, I'd also be satisfied, if it takes SHAs as argument and no "struct commit *". But then this should be more generic, i.e. OBJ_TAG has to be handled correctly (builtin-revert does that at the beginning at parse-args()).

Hmm, then step (1) is ok. :-)

>  (2) make revert.c use merge_recursive_helper();
> 
>  (3) make builtin-merge.c use merge_recursive_helper().

This is ok then.

Thanks and regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F