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? > 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@xxxxxxx>, PGP 0x6EDDD207FCC5040F -- 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
