Re: [PATCH 1/2] merge-recursive: prepare merge_recursive() to be called from builtins

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 11, 2008, 19:53 UTC
Message-ID: <7vzlnjwcog.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080811151303.GA11208@leksak.fem-net>

Stephan Beyer <s-beyer@gmx.net> writes:

> Hmm, I have the long-run vision that we have a nice libgit some day,
> with merge_recursive() being part of it.  And I'm a little unsure if
> libified functions should rely on environment variables.

I think the environment variable is the least of your worries.

I do not think anybody has vetted if it is safe to call merge_recursive() more than once in a single run of a process. Things to watch out for are the use of static variables (e.g. current_{file,directory}_set that are used for its (semi-broken) D/F conflict detection), its liberal use of die(), leaking of "virtual commit", to name a few. They are all perfectly fine programming constructs when we assume this is a one-shot "run and clean up with exit" program, but whoever wants to libify it needs to arrange them to be cleaned up inside the "library" without making the code too ugly nor one-shot use too expensive.

But such a clean-up may not be too bad as I initially feared, I suppose. After a cursory look in builtin-merge-recursive.c, at least it does not seem to mark objects with random object flags, expecting that nobody else will look at them after it exits --- which would have been very expensive to clean up after the fact.