Re: [PATCH] Make git-revert & git-cherry-pick a builtin

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 1, 2007, 22:24 UTC
Message-ID: <Pine.LNX.4.63.0703012318020.22628@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <20070301215123.GB9254@spearce.org>

Hi,

On Thu, 1 Mar 2007, Shawn O. Pearce wrote:

Show 19 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > > +static int merge_recursive(const char *base_sha1,
> > > > +		const char *head_sha1, const char *head_name,
> > > > +		const char *next_sha1, const char *next_name)
> > > > +{
> > > > + ...
> > > > +}
> > > 
> > > Somehow I would have expected you to call merge-recursive not spawn, 
> > > but this is saner ;-).
> > 
> > I briefly considered this, until I realized that merge-recursive is no 
> > builtin yet!
> 
> Heh, I expected the same thing as Junio when I first read through the 
> patch.  But then I realized that doing so right now might not be a great 
> idea; normally if merge-recursive aborts we say "fix up and commit" and 
> do a few things still in cherry-pick/revert before giving the shell back 
> to the user.

IIRC merge-recursive does not quit early with an exit(), rather returns the exit status from the main() routine. So, it would be a matter of making merge-recursive a builtin, separating the main() method in merge_recursive() and an option parsing cmd_merge_recursive(), and call it from revert.

I admit that I was a little too uneasy to do that, what with the object flags and all, but it _should_ work. After all, merge-recursive is _recursive_, so we had to make sure that everything was properly cleaned up after each iteration.

Show 7 quoted lines

> > Speaking of this issue: Would it be conceivable to make the 
> > standalones into builtins? (This adds a dependency on libcurl to core 
> > Git programs, but I could live with that... It would make a builtin 
> > fetch easier, too.)
> 
> Please don't make libcurl required.  I don't build with it on Solaris, 
> because its not there, and I don't need it there.

I don't mean to make it required. I mean to link git$X to it. That would mean that even a silly git-rev-parse dynamically links to libcurl, iff curl support is enabled...

Ciao, Dscho