[PATCH RFC] Move all dashed form git commands to libexecdir

[PATCH RFC] Move all dashed form git commands to libexecdir

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 We have talked about it for quite some time now. How about
 making it happen? I won't miss dashed form commands much :)

 A compromised approach could be keeping porcelain commands
 in bindir, only plumbings are moved to libexecdir. That would
 be less shock than this.

 config.mak.in |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/config.mak.in b/config.mak.in
index 11d256e..1db0338 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -11,7 +11,7 @@ TCLTK_PATH = @TCLTK_PATH@
 prefix = @prefix@
 exec_prefix = @exec_prefix@
 bindir = @bindir@
-#gitexecdir = @libexecdir@/git-core/
+gitexecdir = @libexecdir@/git-core/
 datarootdir = @datarootdir@
 template_dir = @datadir@/git-core/templates/
 
-- 
1.5.3.GIT

Re: [PATCH RFC] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 450 bytes --]

Hi,

On Tue, 27 Nov 2007, Nguyễn Thái Ngọc Duy wrote:

> diff --git a/config.mak.in b/config.mak.in

I don't use configure, and don't expect distros to use it either.  Maybe 
you want to change Makefile first, so that the hard-core "next" followers 
test it first?  Then, when everything is worked out and deemed to be 
stable, Junio can merge it to "master", and we can adjust the rpm spec and 
bug packagers to change their setup?

Ciao,
Dscho

Re: [PATCH RFC] Move all dashed form git commands to libexecdir

[Nicolas Pitre]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 390 bytes --]

On Tue, 27 Nov 2007, Nguyễn Thái Ngọc Duy wrote:

>  We have talked about it for quite some time now. How about
>  making it happen? I won't miss dashed form commands much :)
> 
>  A compromised approach could be keeping porcelain commands
>  in bindir, only plumbings are moved to libexecdir. That would
>  be less shock than this.

I think that would be an excellent idea.


Nicolas

[PATCH] Move all dashed form git commands to libexecdir

[Nguyễn Thái Ngoc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Both configure and make-only ways should work now

 Makefile      |    2 +-
 config.mak.in |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 313f9a2..377d7be 100644
--- a/Makefile
+++ b/Makefile
@@ -154,7 +154,7 @@ STRIP ?= strip
 
 prefix = $(HOME)
 bindir = $(prefix)/bin
-gitexecdir = $(bindir)
+gitexecdir = $(prefix)/libexec/git-core
 sharedir = $(prefix)/share
 template_dir = $(sharedir)/git-core/templates
 ifeq ($(prefix),/usr)
diff --git a/config.mak.in b/config.mak.in
index 11d256e..1db0338 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -11,7 +11,7 @@ TCLTK_PATH = @TCLTK_PATH@
 prefix = @prefix@
 exec_prefix = @exec_prefix@
 bindir = @bindir@
-#gitexecdir = @libexecdir@/git-core/
+gitexecdir = @libexecdir@/git-core/
 datarootdir = @datarootdir@
 template_dir = @datadir@/git-core/templates/
 
-- 
1.5.3.GIT

Re: [PATCH] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 199 bytes --]

Hi,

On Tue, 27 Nov 2007, Nguyễn Thái Ngoc Duy wrote:

>  Both configure and make-only ways should work now

I thought your plan was to put the non-porcelain into the libexecdir only?

Ciao,
Dscho

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jan Hudec]

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

On Tue, Nov 27, 2007 at 16:18:01 +0000, Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 27 Nov 2007, Nguyễn Thái Ngoc Duy wrote:
> 
> >  Both configure and make-only ways should work now
> 
> I thought your plan was to put the non-porcelain into the libexecdir only?

I had the impression that deprecating the dash notation for /all/ use was
approved some time ago. Though I don't want to search through the list
archives this late in the night to check it.

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Move all dashed form git commands to libexecdir

[Junio C Hamano]

Jan Hudec <bulb@ucw.cz> writes:

> On Tue, Nov 27, 2007 at 16:18:01 +0000, Johannes Schindelin wrote:
>> Hi,
>> 
>> On Tue, 27 Nov 2007, Nguyễn Thái Ngoc Duy wrote:
>> 
>> >  Both configure and make-only ways should work now
>> 
>> I thought your plan was to put the non-porcelain into the libexecdir only?
>
> I had the impression that deprecating the dash notation for /all/ use was
> approved some time ago. Though I don't want to search through the list
> archives this late in the night to check it.

Yes.  Moving the dash-form commands out of end user's PATH was something
distros and users have been allowed to do since forever, and strictly
speaking, using dash form from the command line was already deprecated
at that point.

If your script runs git-foo without first asking "git --exec-path" and
prepending it to the path, your script would not find git-foo if the
installation uses gitexecdir that is not on the usual $PATH, either.

I essentially just said that your patch is unnecessary, but at the same
time, your patch does not go far enough.  As Nico earlier pointed out,
we ship a sample rpm spec, which would also need to be updated.  We do
not ship a sample debian/rules anymore, thank $DEITY ;-)

Also, because we do not remove existing files from the installation
target directory when we do "make install", the commit log message
should carry a big fat warning that says "remove old installation of git
from your $(bindir) when you try this," for people who build from the
source, and we need to repeat the deprecation notice in bold red letters
in the Release Notes for perhaps git 1.6.0.

In case somebody is thinking about 36e5e70e0f40 (Start deprecating
"git-command" in favor of "git command"), that is a somewhat different
issue.  What Linus suggested is not installing git-foo link for built-in
commands _anywhere_ on the filesystem.  Not just "out of user's PATH".
That is not deprecating dash form but removing the support for it.  We
need to give ample time for users to adjust to such a change.

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jan Hudec]

On Tue, Nov 27, 2007 at 17:13:58 -0800, Junio C Hamano wrote:
> Jan Hudec <bulb@ucw.cz> writes:
> > On Tue, Nov 27, 2007 at 16:18:01 +0000, Johannes Schindelin wrote:
> >> On Tue, 27 Nov 2007, Nguyễn Thái Ngoc Duy wrote:
> >> 
> >> >  Both configure and make-only ways should work now
> >> 
> >> I thought your plan was to put the non-porcelain into the libexecdir only?
> >
> > I had the impression that deprecating the dash notation for /all/ use was
> > approved some time ago. Though I don't want to search through the list
> > archives this late in the night to check it.
> 
> [...]
>
> In case somebody is thinking about 36e5e70e0f40 (Start deprecating
> "git-command" in favor of "git command"), that is a somewhat different
> issue.  What Linus suggested is not installing git-foo link for built-in
> commands _anywhere_ on the filesystem.  Not just "out of user's PATH".
> That is not deprecating dash form but removing the support for it.  We
> need to give ample time for users to adjust to such a change.

Yes, that is what I said I recall seeing. Installing out of user's PATH is
a step towards not installing at all and the change suggests it has been
already accepted as a general direction for future. Or not?

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nguyen Thai Ngoc Duy]

On Nov 28, 2007 8:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
> In case somebody is thinking about 36e5e70e0f40 (Start deprecating
> "git-command" in favor of "git command"), that is a somewhat different
> issue.  What Linus suggested is not installing git-foo link for built-in
> commands _anywhere_ on the filesystem.  Not just "out of user's PATH".
> That is not deprecating dash form but removing the support for it.  We
> need to give ample time for users to adjust to such a change.

A little note on this one. I've been using git without builtin links
for a while with my git-box port. There are still some builtin fixups
needed. And because execv_git_cmd() always uses dash form, so it's
impossible to use vanilla git without builtin links.
-- 
Duy

Re: [PATCH] Move all dashed form git commands to libexecdir

[Junio C Hamano]

"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:

> On Nov 28, 2007 8:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> In case somebody is thinking about 36e5e70e0f40 (Start deprecating
>> "git-command" in favor of "git command"), that is a somewhat different
>> issue.  What Linus suggested is not installing git-foo link for built-in
>> commands _anywhere_ on the filesystem.  Not just "out of user's PATH".
>> That is not deprecating dash form but removing the support for it.  We
>> need to give ample time for users to adjust to such a change.
>
> A little note on this one. I've been using git without builtin links
> for a while with my git-box port. There are still some builtin fixups
> needed. And because execv_git_cmd() always uses dash form, so it's
> impossible to use vanilla git without builtin links.

Thanks for a heads up.

Would people agree with a rough roadmap like this?

 - v1.5.4 will ship with gitexecdir=$(bindir) in Makefile.  But the
   release notes for the version will warn users that:

   (1) using git-foo from the command line, and

   (2) using git-foo from your scripts without first prepending the
       return value of "git --exec-path" to the PATH

   is now officially deprecated (it has been deprecated for a long time
   since January 2006, v1.2.0~149) and upcoming v1.5.5 will ship with
   the default configuration that does not install git-foo form in
   user's PATH.

 - Post v1.5.4, start cooking gitexecdir=$(libexecdir)/git-core, aiming
   for inclusion in v1.5.5, perhaps in Mar-Feb 2008 timeframe.

 - The release notes for v1.5.5 will warn users that git-foo will be
   removed in v1.6.0 for many commands and it will be merely an accident
   if some of them still work.

 - Post v1.5.5, start cooking the change that does not install hardlinks
   for built-in commands, aiming for inclusion in v1.6.0, by the end of
   2008.

Re: [PATCH] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

Hi,

On Wed, 28 Nov 2007, Junio C Hamano wrote:

> "Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
> 
> > On Nov 28, 2007 8:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >> In case somebody is thinking about 36e5e70e0f40 (Start deprecating
> >> "git-command" in favor of "git command"), that is a somewhat different
> >> issue.  What Linus suggested is not installing git-foo link for built-in
> >> commands _anywhere_ on the filesystem.  Not just "out of user's PATH".
> >> That is not deprecating dash form but removing the support for it.  We
> >> need to give ample time for users to adjust to such a change.
> >
> > A little note on this one. I've been using git without builtin links
> > for a while with my git-box port. There are still some builtin fixups
> > needed. And because execv_git_cmd() always uses dash form, so it's
> > impossible to use vanilla git without builtin links.
> 
> Thanks for a heads up.
> 
> Would people agree with a rough roadmap like this?
> 
>  - v1.5.4 will ship with gitexecdir=$(bindir) in Makefile.  But the
>    release notes for the version will warn users that:
> 
>    (1) using git-foo from the command line, and
> 
>    (2) using git-foo from your scripts without first prepending the
>        return value of "git --exec-path" to the PATH
> 
>    is now officially deprecated (it has been deprecated for a long time
>    since January 2006, v1.2.0~149) and upcoming v1.5.5 will ship with
>    the default configuration that does not install git-foo form in
>    user's PATH.

Maybe we can squeeze a step in here where only porcelains are installed in 
the bindir?

FWIW I think that we should fix the problem with the builtins being called 
via their hard links.  But how?  As of now, libgit.a has no idea what the 
builtins are; this information is buried in git.c.

The fundamental problem is that we cannot move handle_internal_command() 
into libgit.a, because it has pointers to all builtin cmd_*() functions.

So maybe the best solution would be to try "git <command>" first, and then 
"git-<command>"?  But this means another exec() call :-(

Ciao,
Dscho

Re: [PATCH] Move all dashed form git commands to libexecdir

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> The fundamental problem is that we cannot move handle_internal_command() 
> into libgit.a, because it has pointers to all builtin cmd_*() functions.

What's wrong with having cmd_* functions in the library to begin with?

Re: [PATCH] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

Hi,

On Wed, 28 Nov 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > The fundamental problem is that we cannot move 
> > handle_internal_command() into libgit.a, because it has pointers to 
> > all builtin cmd_*() functions.
> 
> What's wrong with having cmd_* functions in the library to begin with?

I was considering it not desirable (after all, the library is meant to 
have common functions in it), but you're right...

Ciao,
Dscho

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jakub Narebski]

Nguyen Thai Ngoc Duy wrote:

> On Nov 28, 2007 8:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> In case somebody is thinking about 36e5e70e0f40 (Start deprecating
>> "git-command" in favor of "git command"), that is a somewhat different
>> issue.  What Linus suggested is not installing git-foo link for built-in
>> commands _anywhere_ on the filesystem.  Not just "out of user's PATH".
>> That is not deprecating dash form but removing the support for it.  We
>> need to give ample time for users to adjust to such a change.
> 
> A little note on this one. I've been using git without builtin links
> for a while with my git-box port. There are still some builtin fixups
> needed. And because execv_git_cmd() always uses dash form, so it's
> impossible to use vanilla git without builtin links.

By the way, what is the status of your git-box port?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] Move all dashed form git commands to libexecdir

[A Large Angry SCM]

Junio C Hamano wrote:
[...]
> Would people agree with a rough roadmap like this?
> 
>  - v1.5.4 will ship with gitexecdir=$(bindir) in Makefile.  But the
>    release notes for the version will warn users that:
> 
>    (1) using git-foo from the command line, and
> 
>    (2) using git-foo from your scripts without first prepending the
>        return value of "git --exec-path" to the PATH
> 
>    is now officially deprecated (it has been deprecated for a long time
>    since January 2006, v1.2.0~149) and upcoming v1.5.5 will ship with
>    the default configuration that does not install git-foo form in
>    user's PATH.
> 
>  - Post v1.5.4, start cooking gitexecdir=$(libexecdir)/git-core, aiming
>    for inclusion in v1.5.5, perhaps in Mar-Feb 2008 timeframe.
> 
>  - The release notes for v1.5.5 will warn users that git-foo will be
>    removed in v1.6.0 for many commands and it will be merely an accident
>    if some of them still work.
> 
>  - Post v1.5.5, start cooking the change that does not install hardlinks
>    for built-in commands, aiming for inclusion in v1.6.0, by the end of
>    2008.

So long as there remains the option in the Makefile to install the 
"dashed" commands in $(bindir) for those of us that wish it.

Re: [PATCH] Move all dashed form git commands to libexecdir

[Junio C Hamano]

A Large Angry SCM <gitzilla@gmail.com> writes:

> Junio C Hamano wrote:
> [...]
>> Would people agree with a rough roadmap like this?
>>
>>  - v1.5.4 will ship with gitexecdir=$(bindir) in Makefile.  But the
>>    release notes for the version will warn users that:
>>
>>    (1) using git-foo from the command line, and
>>
>>    (2) using git-foo from your scripts without first prepending the
>>        return value of "git --exec-path" to the PATH
>>
>>    is now officially deprecated (it has been deprecated for a long time
>>    since January 2006, v1.2.0~149) and upcoming v1.5.5 will ship with
>>    the default configuration that does not install git-foo form in
>>    user's PATH.
>>
>>  - Post v1.5.4, start cooking gitexecdir=$(libexecdir)/git-core, aiming
>>    for inclusion in v1.5.5, perhaps in Mar-Feb 2008 timeframe.
>>
>>  - The release notes for v1.5.5 will warn users that git-foo will be
>>    removed in v1.6.0 for many commands and it will be merely an accident
>>    if some of them still work.
>>
>>  - Post v1.5.5, start cooking the change that does not install hardlinks
>>    for built-in commands, aiming for inclusion in v1.6.0, by the end of
>>    2008.
>
> So long as there remains the option in the Makefile to install the
> "dashed" commands in $(bindir) for those of us that wish it.

Surely.

Actually they already have that option of going dashless themselves, so
in a sense it is not strictly necessary to make a big deal out of this.

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nguyen Thai Ngoc Duy]

On Nov 29, 2007 6:14 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
>
> > On Nov 28, 2007 8:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >> In case somebody is thinking about 36e5e70e0f40 (Start deprecating
> >> "git-command" in favor of "git command"), that is a somewhat different
> >> issue.  What Linus suggested is not installing git-foo link for built-in
> >> commands _anywhere_ on the filesystem.  Not just "out of user's PATH".
> >> That is not deprecating dash form but removing the support for it.  We
> >> need to give ample time for users to adjust to such a change.
> >
> > A little note on this one. I've been using git without builtin links
> > for a while with my git-box port. There are still some builtin fixups
> > needed. And because execv_git_cmd() always uses dash form, so it's
> > impossible to use vanilla git without builtin links.
>
> Thanks for a heads up.
>
> Would people agree with a rough roadmap like this?
>
>  - v1.5.4 will ship with gitexecdir=$(bindir) in Makefile.  But the
>    release notes for the version will warn users that:
>
>    (1) using git-foo from the command line, and
>
>    (2) using git-foo from your scripts without first prepending the
>        return value of "git --exec-path" to the PATH
>
>    is now officially deprecated (it has been deprecated for a long time
>    since January 2006, v1.2.0~149) and upcoming v1.5.5 will ship with
>    the default configuration that does not install git-foo form in
>    user's PATH.
>
>  - Post v1.5.4, start cooking gitexecdir=$(libexecdir)/git-core, aiming
>    for inclusion in v1.5.5, perhaps in Mar-Feb 2008 timeframe.
>
>  - The release notes for v1.5.5 will warn users that git-foo will be
>    removed in v1.6.0 for many commands and it will be merely an accident
>    if some of them still work.
>
>  - Post v1.5.5, start cooking the change that does not install hardlinks
>    for built-in commands, aiming for inclusion in v1.6.0, by the end of
>    2008.

There won't be a stage when only porcelain git-foos are in $(bindir)?
I could stop working on the relevant patch then.
-- 
Duy

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nicolas Pitre]

On Thu, 29 Nov 2007, Nguyen Thai Ngoc Duy wrote:

> On Nov 29, 2007 6:14 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >
> > "Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
> >
> > > On Nov 28, 2007 8:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > >> In case somebody is thinking about 36e5e70e0f40 (Start deprecating
> > >> "git-command" in favor of "git command"), that is a somewhat different
> > >> issue.  What Linus suggested is not installing git-foo link for built-in
> > >> commands _anywhere_ on the filesystem.  Not just "out of user's PATH".
> > >> That is not deprecating dash form but removing the support for it.  We
> > >> need to give ample time for users to adjust to such a change.
> > >
> > > A little note on this one. I've been using git without builtin links
> > > for a while with my git-box port. There are still some builtin fixups
> > > needed. And because execv_git_cmd() always uses dash form, so it's
> > > impossible to use vanilla git without builtin links.
> >
> > Thanks for a heads up.
> >
> > Would people agree with a rough roadmap like this?
> >
> >  - v1.5.4 will ship with gitexecdir=$(bindir) in Makefile.  But the
> >    release notes for the version will warn users that:
> >
> >    (1) using git-foo from the command line, and
> >
> >    (2) using git-foo from your scripts without first prepending the
> >        return value of "git --exec-path" to the PATH
> >
> >    is now officially deprecated (it has been deprecated for a long time
> >    since January 2006, v1.2.0~149) and upcoming v1.5.5 will ship with
> >    the default configuration that does not install git-foo form in
> >    user's PATH.
> >
> >  - Post v1.5.4, start cooking gitexecdir=$(libexecdir)/git-core, aiming
> >    for inclusion in v1.5.5, perhaps in Mar-Feb 2008 timeframe.
> >
> >  - The release notes for v1.5.5 will warn users that git-foo will be
> >    removed in v1.6.0 for many commands and it will be merely an accident
> >    if some of them still work.
> >
> >  - Post v1.5.5, start cooking the change that does not install hardlinks
> >    for built-in commands, aiming for inclusion in v1.6.0, by the end of
> >    2008.
> 
> There won't be a stage when only porcelain git-foos are in $(bindir)?
> I could stop working on the relevant patch then.

Well, I personally found your effort really nice.  I think Junio is 
overly cautious in this case, and I would prefer to see the number of 
git commands in the default path drop rather sooner than later.


Nicolas

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Wed, Nov 28, 2007 at 03:14:56PM -0800, Junio C Hamano wrote:

>  - Post v1.5.5, start cooking the change that does not install hardlinks
>    for built-in commands, aiming for inclusion in v1.6.0, by the end of
>    2008.

I am against this, unless it is configurable. I think the goal of
reducing user-visible commands is fine, and moving things to
$(libexecdir) is a good way of doing that.

However, I personally still think the 'git-foo' forms are valuable
(because fingers have already been trained, and because
non-bash-programmable completions understand them). And I don't mind
putting $(libexecdir)/git-core in my PATH to retain this behavior; it's
a one-time configuration tweak, and it helps new users with the
overwhelming command set.

But I don't see a point to removing the links entirely. The annoyance
factor for people who want git-* is much higher, and I don't see that it
actually buys us any help for new users (who will no longer care after
everything is hidden in $(libexecdir) anyway).

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nguyen Thai Ngoc Duy]

On Nov 29, 2007 10:08 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 28, 2007 at 03:14:56PM -0800, Junio C Hamano wrote:
>
> >  - Post v1.5.5, start cooking the change that does not install hardlinks
> >    for built-in commands, aiming for inclusion in v1.6.0, by the end of
> >    2008.
>
> I am against this, unless it is configurable. I think the goal of
> reducing user-visible commands is fine, and moving things to
> $(libexecdir) is a good way of doing that.
>
> However, I personally still think the 'git-foo' forms are valuable
> (because fingers have already been trained, and because
> non-bash-programmable completions understand them). And I don't mind
> putting $(libexecdir)/git-core in my PATH to retain this behavior; it's
> a one-time configuration tweak, and it helps new users with the
> overwhelming command set.
>
> But I don't see a point to removing the links entirely. The annoyance
> factor for people who want git-* is much higher, and I don't see that it
> actually buys us any help for new users (who will no longer care after
> everything is hidden in $(libexecdir) anyway).

Maybe only not install hardlinks on systems that do not support it
like Windows? git.exe duplication takes a lot of space.
-- 
Duy

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Fri, Nov 30, 2007 at 03:05:05AM +0700, Nguyen Thai Ngoc Duy wrote:

> > But I don't see a point to removing the links entirely. The annoyance
> > factor for people who want git-* is much higher, and I don't see that it
> > actually buys us any help for new users (who will no longer care after
> > everything is hidden in $(libexecdir) anyway).
> 
> Maybe only not install hardlinks on systems that do not support it
> like Windows? git.exe duplication takes a lot of space.

I think that is totally reasonable, as on those platforms there is
actually something to be gained from removing those hardlinks (you could
also of course make a very thin wrapper for "git-foo" that called "git
foo"; it would still be wasteful, but not as much as copying the whole
git.exe. But that is not worth doing unless people on Windows really
want the dash forms).

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

Hi,

On Thu, 29 Nov 2007, Jeff King wrote:

> On Fri, Nov 30, 2007 at 03:05:05AM +0700, Nguyen Thai Ngoc Duy wrote:
> 
> > > But I don't see a point to removing the links entirely. The annoyance
> > > factor for people who want git-* is much higher, and I don't see that it
> > > actually buys us any help for new users (who will no longer care after
> > > everything is hidden in $(libexecdir) anyway).
> > 
> > Maybe only not install hardlinks on systems that do not support it
> > like Windows? git.exe duplication takes a lot of space.
> 
> I think that is totally reasonable, as on those platforms there is
> actually something to be gained from removing those hardlinks (you could
> also of course make a very thin wrapper for "git-foo" that called "git
> foo"; it would still be wasteful, but not as much as copying the whole
> git.exe. But that is not worth doing unless people on Windows really
> want the dash forms).

Note that one big problem with a few platforms having dash forms and 
others not is that you _will_ get scripts and aliases that do not work 
everywhere.

Consistency is good.

Ciao,
Dscho

Re: [PATCH] Move all dashed form git commands to libexecdir

[Junio C Hamano]

Nicolas Pitre <nico@cam.org> writes:

> On Thu, 29 Nov 2007, Nguyen Thai Ngoc Duy wrote:
>
>> There won't be a stage when only porcelain git-foos are in $(bindir)?
>> I could stop working on the relevant patch then.
>
> Well, I personally found your effort really nice.  I think Junio is 
> overly cautious in this case, and I would prefer to see the number of 
> git commands in the default path drop rather sooner than later.

I agree with the first sentence.  And yes I am playing it safe, and at
the same time I do not think the "default" really matters as much as
people think.

If people are really serious about reducing the number of commands in
the path, I would expect fixes and bugreports saying "I am setting
gitexecdir different from bindir in _my_ installation when I build git,
and here are the things that does not work if I do so".  Within the span
of more than 20 months (77cb17e9 introduced gitexecdir in Jan 2006), I
do not think there was a single such report or patch, other than the
message from Nguyen that started this thread.

Which means one of two things (1) we got everything right and there is
nothing to fix, other than changing the default like Nguyen's patch
does, or (2) nobody is interested in moving git-foo out of their PATH
for _his_ own use, but pushing changes that would affect _other_ people
without testing.

I am of course hoping that (1) is the case.  And it could be that in
open-source settings often the silent majority is content with what's
already there, and that many people who are indeed interested in moving
git-foo out of their PATH are doing so happily without telling the
others of their success.

But it still worries me.

And people's scripts, especially old/unmaintained ones that google still
knows about, are worrysome too.  Didn't we just see a message that says
"git-update-cache in a script I picked up from google does not work" on
the list?

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Thu, Nov 29, 2007 at 10:19:16PM +0000, Johannes Schindelin wrote:

> > I think that is totally reasonable, as on those platforms there is
> > actually something to be gained from removing those hardlinks (you could
> 
> Note that one big problem with a few platforms having dash forms and 
> others not is that you _will_ get scripts and aliases that do not work 
> everywhere.
> 
> Consistency is good.

Yes, I am fine with the user having to go to extra lengths to use the
dash forms (like adding $(libexecdir) to their path), which I think
should address your consistency concern.

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Linus Torvalds]

On Thu, 29 Nov 2007, Jeff King wrote:
> 
> Yes, I am fine with the user having to go to extra lengths to use the
> dash forms (like adding $(libexecdir) to their path), which I think
> should address your consistency concern.

I agree. If we actually start moving the subcommands into a separate 
directory, I suspect scripts will be fixed up soon enough. Of course 
people *can* do it by just adding the path, but more likely, we'll just 
see people start doign "git xyz" instead of "git-xyz".

And from a consistency standpoint, that would be a *good* thing. There are 
many reasons why the git-xyz format *cannot* be the "consistent" form
(ranging from the flags like --bare and -p to just aliases), so 
encouraging people to move to "git xyz" is just a good idea.

Yeah, yeah, the man-pages need the "git-xyz" form, but on the other hand, 
rather than "man git-xyz", you can just do "git help xyz" instead, and now 
you're consistently avoiding the dash again!

			Linus

Re: [PATCH] Move all dashed form git commands to libexecdir

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> And from a consistency standpoint, that would be a *good* thing. There are 
> many reasons why the git-xyz format *cannot* be the "consistent" form
> (ranging from the flags like --bare and -p to just aliases), so 
> encouraging people to move to "git xyz" is just a good idea.
>
> Yeah, yeah, the man-pages need the "git-xyz" form, but on the other hand, 
> rather than "man git-xyz", you can just do "git help xyz" instead, and now 
> you're consistently avoiding the dash again!

Ok.  So here is a revised roadmap that a panda brain (that is not so
well working today due to fever) came up.

 - v1.5.4 will ship with gitexecdir=$(bindir) in Makefile.  But the
   release notes for the version will warn users that:

   (1) using git-foo from the command line, and

   (2) using git-foo from your scripts without first prepending the
       return value of "git --exec-path" to the PATH

   is now officially deprecated (it has been deprecated for a long time
   since January 2006, v1.2.0~149) and upcoming v1.5.5 will ship with
   the default configuration that does not install git-foo form in
   user's PATH.

   If further will warn users that git-foo form will be removed in
   v1.5.6 for many commands and it will be merely an accident if some of
   them still work after that.

 - Post v1.5.4, start cooking gitexecdir=$(libexecdir)/git-core, aiming
   for inclusion in v1.5.5, perhaps in Feb-Mar 2008 timeframe.  This
   will also affect the sample RPM spec and resulting RPM binary
   packages I will place on k.org, and I'll ask Gerrit to do the same on
   Debian side.  The official binary packaging of individual distros are
   not under my control, but if there is a handy list of people I can
   send this notice to for other distros, that would help this process.

 - The release notes for v1.5.5 will warn users again that git-foo will
   be removed in v1.5.6 for many commands and it will be merely an
   accident if some of them still work.

 - Post v1.5.5, start cooking the change that does not install hardlinks
   for built-in commands, aiming for inclusion in v1.5.6, in May-Jun
   2008 timeframe.

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Thu, Nov 29, 2007 at 04:13:29PM -0800, Junio C Hamano wrote:

>  - Post v1.5.5, start cooking the change that does not install hardlinks
>    for built-in commands, aiming for inclusion in v1.5.6, in May-Jun
>    2008 timeframe.

I am still against this step, for the reasons mentioned in the mails
leading up to the one you just quoted. I am fine with "does not install
hardlinks for builtin-commands on systems that don't support hardlinks"
(and of course all such hardlinks are in $(libexecdir)/git-core at this
point).

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nguyen Thai Ngoc Duy]

On Nov 30, 2007 7:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
>  - Post v1.5.4, start cooking gitexecdir=$(libexecdir)/git-core, aiming
>    for inclusion in v1.5.5, perhaps in Feb-Mar 2008 timeframe.  This
>    will also affect the sample RPM spec and resulting RPM binary
>    packages I will place on k.org, and I'll ask Gerrit to do the same on
>    Debian side.  The official binary packaging of individual distros are
>    not under my control, but if there is a handy list of people I can
>    send this notice to for other distros, that would help this process.

You can find Gentoo maintainers here:

http://sources.gentoo.org/viewcvs.py/gentoo-x86/dev-util/git/metadata.xml?rev=1.6&view=markup

-- 
Duy

Re: [PATCH] Move all dashed form git commands to libexecdir

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Nov 29, 2007 at 04:13:29PM -0800, Junio C Hamano wrote:
>
>>  - Post v1.5.5, start cooking the change that does not install hardlinks
>>    for built-in commands, aiming for inclusion in v1.5.6, in May-Jun
>>    2008 timeframe.
>
> I am still against this step, for the reasons mentioned in the mails
> leading up to the one you just quoted. I am fine with "does not install
> hardlinks for builtin-commands on systems that don't support hardlinks"
> (and of course all such hardlinks are in $(libexecdir)/git-core at this
> point).

I understand your point was primarily "git-a<tab>".  I think it has been
solved for bash and zsh but not for other shells.  I think possible and
sensible avenues are (1) punt -- cvs, svn nor hg people do not seem to
have problem with it, or (2) implement completion in your other favorite
shells.

And I think the following from Linus makes sense.

> And from a consistency standpoint, that would be a *good* thing. There are 
> many reasons why the git-xyz format *cannot* be the "consistent" form
> (ranging from the flags like --bare and -p to just aliases), so 
> encouraging people to move to "git xyz" is just a good idea.
>
> Yeah, yeah, the man-pages need the "git-xyz" form, but on the other hand, 
> rather than "man git-xyz", you can just do "git help xyz" instead, and now 
> you're consistently avoiding the dash again!

but I am feeling quite feverish today so I may be missing something
obvious.

Re: [PATCH] Move all dashed form git commands to libexecdir

[A Large Angry SCM]

Junio C Hamano wrote:
[...]
> Ok.  So here is a revised roadmap that a panda brain (that is not so
> well working today due to fever) came up.
> 
>  - v1.5.4 will ship with gitexecdir=$(bindir) in Makefile.  But the
>    release notes for the version will warn users that:
> 
>    (1) using git-foo from the command line, and
> 
>    (2) using git-foo from your scripts without first prepending the
>        return value of "git --exec-path" to the PATH
> 
>    is now officially deprecated (it has been deprecated for a long time
>    since January 2006, v1.2.0~149) and upcoming v1.5.5 will ship with
>    the default configuration that does not install git-foo form in
>    user's PATH.
> 
>    If further will warn users that git-foo form will be removed in
>    v1.5.6 for many commands and it will be merely an accident if some of
>    them still work after that.
> 
>  - Post v1.5.4, start cooking gitexecdir=$(libexecdir)/git-core, aiming
>    for inclusion in v1.5.5, perhaps in Feb-Mar 2008 timeframe.  This
>    will also affect the sample RPM spec and resulting RPM binary
>    packages I will place on k.org, and I'll ask Gerrit to do the same on
>    Debian side.  The official binary packaging of individual distros are
>    not under my control, but if there is a handy list of people I can
>    send this notice to for other distros, that would help this process.
> 
>  - The release notes for v1.5.5 will warn users again that git-foo will
>    be removed in v1.5.6 for many commands and it will be merely an
>    accident if some of them still work.
> 
>  - Post v1.5.5, start cooking the change that does not install hardlinks
>    for built-in commands, aiming for inclusion in v1.5.6, in May-Jun
>    2008 timeframe.

Again, there needs to remain support in the Makefile to install the 
"dashed" versions of the commands for those that want it; and be able to 
set gitexecdir=$(binder) without editing the Makefile.

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nicolas Pitre]

On Thu, 29 Nov 2007, Jeff King wrote:

> On Thu, Nov 29, 2007 at 04:13:29PM -0800, Junio C Hamano wrote:
> 
> >  - Post v1.5.5, start cooking the change that does not install hardlinks
> >    for built-in commands, aiming for inclusion in v1.5.6, in May-Jun
> >    2008 timeframe.
> 
> I am still against this step, for the reasons mentioned in the mails
> leading up to the one you just quoted. I am fine with "does not install
> hardlinks for builtin-commands on systems that don't support hardlinks"
> (and of course all such hardlinks are in $(libexecdir)/git-core at this
> point).

But only for porcelain, right?  You certainly don't need the dashed form 
of plumbing commands?


Nicolas

Re: [PATCH] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

Hi,

On Thu, 29 Nov 2007, A Large Angry SCM wrote:

> Junio C Hamano wrote:
> [...]
> > Ok.  So here is a revised roadmap that a panda brain (that is not so
> > well working today due to fever) came up.
> > 
> >  - v1.5.4 will ship with gitexecdir=$(bindir) in Makefile.  But the
> >    release notes for the version will warn users that:
> > 
> >    (1) using git-foo from the command line, and
> > 
> >    (2) using git-foo from your scripts without first prepending the
> >        return value of "git --exec-path" to the PATH
> > 
> >    is now officially deprecated (it has been deprecated for a long time
> >    since January 2006, v1.2.0~149) and upcoming v1.5.5 will ship with
> >    the default configuration that does not install git-foo form in
> >    user's PATH.
> > 
> >    If further will warn users that git-foo form will be removed in
> >    v1.5.6 for many commands and it will be merely an accident if some of
> >    them still work after that.
> > 
> >  - Post v1.5.4, start cooking gitexecdir=$(libexecdir)/git-core, aiming
> >    for inclusion in v1.5.5, perhaps in Feb-Mar 2008 timeframe.  This
> >    will also affect the sample RPM spec and resulting RPM binary
> >    packages I will place on k.org, and I'll ask Gerrit to do the same on
> >    Debian side.  The official binary packaging of individual distros are
> >    not under my control, but if there is a handy list of people I can
> >    send this notice to for other distros, that would help this process.
> > 
> >  - The release notes for v1.5.5 will warn users again that git-foo will
> >    be removed in v1.5.6 for many commands and it will be merely an
> >    accident if some of them still work.
> > 
> >  - Post v1.5.5, start cooking the change that does not install hardlinks
> >    for built-in commands, aiming for inclusion in v1.5.6, in May-Jun
> >    2008 timeframe.
> 
> Again, there needs to remain support in the Makefile to install the "dashed"
> versions of the commands for those that want it; and be able to set
> gitexecdir=$(binder) without editing the Makefile.

Umm.  Why?  If there is no compelling reason (and you named none, but 
there were quite a few reasons against it), why should the Makefile 
support the dash form after 1.5.6?

Ciao,
Dscho

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Thu, Nov 29, 2007 at 04:49:26PM -0800, Junio C Hamano wrote:

> I understand your point was primarily "git-a<tab>".  I think it has been
> solved for bash and zsh but not for other shells.  I think possible and
> sensible avenues are (1) punt -- cvs, svn nor hg people do not seem to
> have problem with it, or (2) implement completion in your other favorite
> shells.

My point is that (2) is already implemented for every program (shell or
no) which understands filename completion, and there is a proposal for
taking it away. I would consider that, except I haven't see any claimed
advantages except that the hardlinks are awful under Windows.

I am proposing to have those dashed forms on platforms where it makes
sense, but to treat them as second-class citizens (by putting them in
$(libexecdir), which will address consistency problems.

> And I think the following from Linus makes sense.
> 
> > And from a consistency standpoint, that would be a *good* thing. There are 
> > many reasons why the git-xyz format *cannot* be the "consistent" form
> > (ranging from the flags like --bare and -p to just aliases), so 
> > encouraging people to move to "git xyz" is just a good idea.
> >
> > Yeah, yeah, the man-pages need the "git-xyz" form, but on the other hand, 
> > rather than "man git-xyz", you can just do "git help xyz" instead, and now 
> > you're consistently avoiding the dash again!
> 
> but I am feeling quite feverish today so I may be missing something
> obvious.

I thought Linus' point was that moving the subcommands was sufficient
for dealing with the consistency issue (i.e., all scripts would move to
"git foo" and only those people who really wanted to would put the
dashed forms in their path). From the same email you quoted, but just
above:

> On Thu, 29 Nov 2007, Jeff King wrote:
> >
> > Yes, I am fine with the user having to go to extra lengths to use the
> > dash forms (like adding $(libexecdir) to their path), which I think
> > should address your consistency concern.
> 
> I agree. If we actually start moving the subcommands into a separate
> directory, I suspect scripts will be fixed up soon enough. Of course
> people *can* do it by just adding the path, but more likely, we'll just
> see people start doign "git xyz" instead of "git-xyz".

But now we are arguing about what Linus meant like it is scripture. ;)

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Thu, Nov 29, 2007 at 07:52:01PM -0500, Nicolas Pitre wrote:

> > I am still against this step, for the reasons mentioned in the mails
> > leading up to the one you just quoted. I am fine with "does not install
> > hardlinks for builtin-commands on systems that don't support hardlinks"
> > (and of course all such hardlinks are in $(libexecdir)/git-core at this
> > point).
> 
> But only for porcelain, right?  You certainly don't need the dashed form 
> of plumbing commands?

In principle, yes, though one man's porcelain is another man's plumbing,
so determining the correct set is hard (and why bother if they are all
hidden from mere mortals, anyway?).

Perhaps because I am actively working on git, I tend to use a fair
number of plumbing commands for under-the-hood inspection and
experimentation.

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nicolas Pitre]

On Thu, 29 Nov 2007, A Large Angry SCM wrote:

> Again, there needs to remain support in the Makefile to install the "dashed"
> versions of the commands for those that want it; and be able to set
> gitexecdir=$(binder) without editing the Makefile.

Well, if you want a "non standard" installation, maybe you just can edit 
a line or two in the Makefile?


Nicolas

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nicolas Pitre]

On Thu, 29 Nov 2007, Jeff King wrote:

> On Thu, Nov 29, 2007 at 04:49:26PM -0800, Junio C Hamano wrote:
> 
> > I understand your point was primarily "git-a<tab>".  I think it has been
> > solved for bash and zsh but not for other shells.  I think possible and
> > sensible avenues are (1) punt -- cvs, svn nor hg people do not seem to
> > have problem with it, or (2) implement completion in your other favorite
> > shells.
> 
> My point is that (2) is already implemented for every program (shell or
> no) which understands filename completion, and there is a proposal for
> taking it away. I would consider that, except I haven't see any claimed
> advantages except that the hardlinks are awful under Windows.

Weren't enough complaints about Git having waaaaaaaaaaay too many 
commands?  Didn't those complaints come about often enough already?

	$ git-[tab]
	Display all 135 possibilities? (y or n)


Nicolas

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Thu, Nov 29, 2007 at 08:13:04PM -0500, Nicolas Pitre wrote:

> > My point is that (2) is already implemented for every program (shell or
> > no) which understands filename completion, and there is a proposal for
> > taking it away. I would consider that, except I haven't see any claimed
> > advantages except that the hardlinks are awful under Windows.
> 
> Weren't enough complaints about Git having waaaaaaaaaaay too many 
> commands?  Didn't those complaints come about often enough already?
> 
> 	$ git-[tab]
> 	Display all 135 possibilities? (y or n)

Go back and read the thread to which you are responding. I am _not_
arguing against moving those commands to $(libexecdir) where no sane
user will ever see them. That change addresses the issue you are talking
about.

I _am_ arguing against removing them entirely, for those of us who want
to go to the trouble of enabling this (by putting a non-standard entry
into our PATH). Because the issue you are talking about will already
have been dealt with, it is no longer a compelling reason to remove the
hardlinks entirely.

The only reason I have heard to remove them entirely is that Windows
doesn't properly support hardlinks, which I addressed in my other mails
(and to which I have seen no rebuttal).

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nicolas Pitre]

On Thu, 29 Nov 2007, Jeff King wrote:

> On Thu, Nov 29, 2007 at 07:52:01PM -0500, Nicolas Pitre wrote:
> 
> > > I am still against this step, for the reasons mentioned in the mails
> > > leading up to the one you just quoted. I am fine with "does not install
> > > hardlinks for builtin-commands on systems that don't support hardlinks"
> > > (and of course all such hardlinks are in $(libexecdir)/git-core at this
> > > point).
> > 
> > But only for porcelain, right?  You certainly don't need the dashed form 
> > of plumbing commands?
> 
> In principle, yes, though one man's porcelain is another man's plumbing,
> so determining the correct set is hard (and why bother if they are all
> hidden from mere mortals, anyway?).

That would be a good reason not to bother determining which set to 
preserve and remove them all then.

Sure you'll miss the dashed form for, say, one week? After that your 
fingers should be retrained.


Nicolas

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Thu, Nov 29, 2007 at 08:19:38PM -0500, Nicolas Pitre wrote:

> > In principle, yes, though one man's porcelain is another man's plumbing,
> > so determining the correct set is hard (and why bother if they are all
> > hidden from mere mortals, anyway?).
> 
> That would be a good reason not to bother determining which set to 
> preserve and remove them all then.

It clearly argues for putting all in the same boat, yes (but obviously
we disagree on which boat).

> Sure you'll miss the dashed form for, say, one week? After that your 
> fingers should be retrained.

Perhaps, although that doesn't address my other point, about non-bash
program in the world which already does filename completion (in my case,
I am specifically thinking about vim's ":r!", but surely emacs users
must have a similar issue).

But that is just talking about the disadvantages; you can argue that
they are small, but they are clearly non-zero. More importantly, what
are the _advantages_ of removing the hardlinks (and if you haven't read
the other message I just sent you, I am talking not about putting
hardlinks into a non-PATH directory, but about removing them entirely
once they are already in that alternate directory)? If there aren't any
advantages, or they are also small, then it makes sense to keep the
hardlinks.

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nicolas Pitre]

On Thu, 29 Nov 2007, Jeff King wrote:

> On Thu, Nov 29, 2007 at 08:19:38PM -0500, Nicolas Pitre wrote:
> 
> > > In principle, yes, though one man's porcelain is another man's plumbing,
> > > so determining the correct set is hard (and why bother if they are all
> > > hidden from mere mortals, anyway?).
> > 
> > That would be a good reason not to bother determining which set to 
> > preserve and remove them all then.
> 
> It clearly argues for putting all in the same boat, yes (but obviously
> we disagree on which boat).
> 
> > Sure you'll miss the dashed form for, say, one week? After that your 
> > fingers should be retrained.
> 
> Perhaps, although that doesn't address my other point, about non-bash
> program in the world which already does filename completion (in my case,
> I am specifically thinking about vim's ":r!", but surely emacs users
> must have a similar issue).
> 
> But that is just talking about the disadvantages; you can argue that
> they are small, but they are clearly non-zero. More importantly, what
> are the _advantages_ of removing the hardlinks (and if you haven't read
> the other message I just sent you, I am talking not about putting
> hardlinks into a non-PATH directory, but about removing them entirely
> once they are already in that alternate directory)? If there aren't any
> advantages, or they are also small, then it makes sense to keep the
> hardlinks.

So what you want is for the dashed hardlinks to exist _inside_ the 
libexec directory, even if most people won't "see" them due to that 
libexec directory not being in the shell path, right?

If that is what you mean then I personally don't care at all.


Nicolas

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Thu, Nov 29, 2007 at 08:33:39PM -0500, Nicolas Pitre wrote:

> So what you want is for the dashed hardlinks to exist _inside_ the 
> libexec directory, even if most people won't "see" them due to that 
> libexec directory not being in the shell path, right?
> 
> If that is what you mean then I personally don't care at all.

That is exactly what I mean.

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[A Large Angry SCM]

Johannes Schindelin wrote:
> Hi,
> 
> On Thu, 29 Nov 2007, A Large Angry SCM wrote:
[...]
>> Again, there needs to remain support in the Makefile to install the "dashed"
>> versions of the commands for those that want it; and be able to set
>> gitexecdir=$(binder) without editing the Makefile.
> 
> Umm.  Why?  If there is no compelling reason (and you named none, but 
> there were quite a few reasons against it), why should the Makefile 
> support the dash form after 1.5.6?

Not all shells support customized command completion (check the 
archives); "bang" history (check the archives); etc.

Re: [PATCH] Move all dashed form git commands to libexecdir

[A Large Angry SCM]

Nicolas Pitre wrote:
> On Thu, 29 Nov 2007, A Large Angry SCM wrote:
> 
>> Again, there needs to remain support in the Makefile to install the "dashed"
>> versions of the commands for those that want it; and be able to set
>> gitexecdir=$(binder) without editing the Makefile.
> 
> Well, if you want a "non standard" installation, maybe you just can edit 
> a line or two in the Makefile?

Why do you object to a "install-dashed" Makefile target?

Re: [PATCH] Move all dashed form git commands to libexecdir

[A Large Angry SCM]

Jeff King wrote:
> On Thu, Nov 29, 2007 at 08:33:39PM -0500, Nicolas Pitre wrote:
> 
>> So what you want is for the dashed hardlinks to exist _inside_ the 
>> libexec directory, even if most people won't "see" them due to that 
>> libexec directory not being in the shell path, right?
>>
>> If that is what you mean then I personally don't care at all.
> 
> That is exactly what I mean.

And that also works for me when I set libexec=$(bindir).

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nicolas Pitre]

On Thu, 29 Nov 2007, A Large Angry SCM wrote:

> Nicolas Pitre wrote:
> > On Thu, 29 Nov 2007, A Large Angry SCM wrote:
> > 
> > > Again, there needs to remain support in the Makefile to install the
> > > "dashed"
> > > versions of the commands for those that want it; and be able to set
> > > gitexecdir=$(binder) without editing the Makefile.
> > 
> > Well, if you want a "non standard" installation, maybe you just can edit a
> > line or two in the Makefile?
> 
> Why do you object to a "install-dashed" Makefile target?

As long as it is not the default I don't object.


Nicolas

Re: [PATCH] Move all dashed form git commands to libexecdir

[Linus Torvalds]

On Thu, 29 Nov 2007, Jeff King wrote:
> 
> I thought Linus' point was that moving the subcommands was sufficient
> for dealing with the consistency issue (i.e., all scripts would move to
> "git foo" and only those people who really wanted to would put the
> dashed forms in their path). 

Yes. I meant that we might as well keep all the git-xyz forms around, but 
_only_ in the libexec directory (and make sure that the libexec directory 
by default is *not* a binary directory), so that they'd normally not be 
visible.

So then, people who want to use the old-fashioned git-xyz forms, because 
they just really hate white-space or whatever, could choose to do one of 
two things:
 - either just change the default libexec directory to be the same as the 
   binary install directory, and have all the git-xyz things in the same 
   place they've always been.
 - or just add $(gitlibexec) into their path.

but the default (which is what 99% of all people use) would be to not 
show them. I also think that it makes sense to avoid wasting diskspace 
with duplicate files, so in situations where you don't have hardlinks, 
just don't install the git-xyz files at all by default (and again, maybe 
we can have a option to the installer to do it for people who really are 
very attached to the git-xyz format, and prefer to waste even a lot of 
disk on it)

So I just think that the whole idiotic complaint that some people have 
(that whole "git-<tab><tab>" shows "Display all 144 possibilities?" and 
people are somehow using that as an argument that git is "complex") should 
be something we strive to undo. I think the complaint is insane (because 
the answer is "well, nobody forces you to _use_ all the power and scripts 
we give you!"), but still, it's a complaint, so let's just assume the user 
is right, and try to fix it.

So when you do "git-<tab><tab>" it should just beep at you and not show 
anything at all by default. And when you do "git <tab><tab>", we should 
make sure that the bash expansion (or zsh or whatever) shows a nice 
collection of common plumbing, not soemthing really scary.

			Linus

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nicolas Pitre]

On Thu, 29 Nov 2007, Linus Torvalds wrote:

> Yes. I meant that we might as well keep all the git-xyz forms around, but 
> _only_ in the libexec directory (and make sure that the libexec directory 
> by default is *not* a binary directory), so that they'd normally not be 
> visible.
> 
> So then, people who want to use the old-fashioned git-xyz forms, because 
> they just really hate white-space or whatever, could choose to do one of 
> two things:
>  - either just change the default libexec directory to be the same as the 
>    binary install directory, and have all the git-xyz things in the same 
>    place they've always been.
>  - or just add $(gitlibexec) into their path.
> 
> but the default (which is what 99% of all people use) would be to not 
> show them. I also think that it makes sense to avoid wasting diskspace 
> with duplicate files, so in situations where you don't have hardlinks, 
> just don't install the git-xyz files at all by default (and again, maybe 
> we can have a option to the installer to do it for people who really are 
> very attached to the git-xyz format, and prefer to waste even a lot of 
> disk on it)
> 
> So I just think that the whole idiotic complaint that some people have 
> (that whole "git-<tab><tab>" shows "Display all 144 possibilities?" and 
> people are somehow using that as an argument that git is "complex") should 
> be something we strive to undo. I think the complaint is insane (because 
> the answer is "well, nobody forces you to _use_ all the power and scripts 
> we give you!"), but still, it's a complaint, so let's just assume the user 
> is right, and try to fix it.

Absolutely!

And despite Junio's appearance of some cowardliness on this issue :-)
I think we should have this right now instead of later. Like Junio said 
himself, this was planned for a while already, and poorly maintained 
external scripts are already failing due to other reasons anyway.


Nicolas

Re: [PATCH] Move all dashed form git commands to libexecdir

[Steffen Prohaska]

On Nov 30, 2007, at 2:17 AM, Jeff King wrote:

>
> The only reason I have heard to remove them entirely is that Windows
> doesn't properly support hardlinks, which I addressed in my other  
> mails
> (and to which I have seen no rebuttal).

We don't have a problem with hardlinks for git-* commands in
msysgit.  The msysgit installer already creates hardlinks if
installing on NTFS.  On non-NTFS partitions, though, it needs
to copy the files.

Note, this doesn't mean we love hardlinks in msysgit.  Actually,
msys does _not_ support them in its Unix emulation layer.  So,
for daily work git does not have hardlinks.  For examples, the
test script skip all hardlink related tests.  The installer
handles hardlinks using the Windows API directly and not using
the Unix emulation layer.

We already have a setup that supports exporting only git and
gitk to the a Windows Command Prompt (not the bash that comes
with msysgit).  If you choose this setup, a directory that
contains only these two commands will be added to the system
wide PATH. We use this indirection to hide all the the Unix
tools that are included in msysgit (and needed by git) from
the Windows Command Prompt.

	Steffen

Re: [PATCH] Move all dashed form git commands to libexecdir

[Steffen Prohaska]

On Nov 30, 2007, at 3:29 AM, Linus Torvalds wrote:

> So I just think that the whole idiotic complaint that some people have
> (that whole "git-<tab><tab>" shows "Display all 144 possibilities?"  
> and
> people are somehow using that as an argument that git is "complex")  
> should
> be something we strive to undo. I think the complaint is insane  
> (because
> the answer is "well, nobody forces you to _use_ all the power and  
> scripts
> we give you!"), but still, it's a complaint, so let's just assume  
> the user
> is right, and try to fix it.
>
> So when you do "git-<tab><tab>" it should just beep at you and not  
> show
> anything at all by default. And when you do "git <tab><tab>", we  
> should
> make sure that the bash expansion (or zsh or whatever) shows a nice
> collection of common plumbing, not soemthing really scary.


What will happen to gitk?

Shouldn't it be included in the nice collection?  gitk is
an essential command.  Then, following your reasoning,
"git <tab><tab>" should recommend it, no?

Note, "git gui" already works.  gitk would really be the last
git "command" that can't be accessed through "git <command>"

	Steffen

Re: [PATCH] Move all dashed form git commands to libexecdir

[Andreas Ericsson]

Jeff King wrote:
> On Thu, Nov 29, 2007 at 08:13:04PM -0500, Nicolas Pitre wrote:
> 
>>> My point is that (2) is already implemented for every program (shell or
>>> no) which understands filename completion, and there is a proposal for
>>> taking it away. I would consider that, except I haven't see any claimed
>>> advantages except that the hardlinks are awful under Windows.
>> Weren't enough complaints about Git having waaaaaaaaaaay too many 
>> commands?  Didn't those complaints come about often enough already?
>>
>> 	$ git-[tab]
>> 	Display all 135 possibilities? (y or n)
> 
> Go back and read the thread to which you are responding. I am _not_
> arguing against moving those commands to $(libexecdir) where no sane
> user will ever see them. That change addresses the issue you are talking
> about.
> 
> I _am_ arguing against removing them entirely, for those of us who want
> to go to the trouble of enabling this (by putting a non-standard entry
> into our PATH). Because the issue you are talking about will already
> have been dealt with, it is no longer a compelling reason to remove the
> hardlinks entirely.
> 
> The only reason I have heard to remove them entirely is that Windows
> doesn't properly support hardlinks, which I addressed in my other mails
> (and to which I have seen no rebuttal).
> 

It would provide a ui inconsistency between platforms. Several people
pointed that out. It's decidedly a Bad Thing.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH] Move all dashed form git commands to libexecdir

[Wincent Colaiuta]

El 29/11/2007, a las 23:36, Junio C Hamano escribió:

> If people are really serious about reducing the number of commands in
> the path, I would expect fixes and bugreports saying "I am setting
> gitexecdir different from bindir in _my_ installation when I build  
> git,
> and here are the things that does not work if I do so".  Within the  
> span
> of more than 20 months (77cb17e9 introduced gitexecdir in Jan 2006), I
> do not think there was a single such report or patch, other than the
> message from Nguyen that started this thread.

One reason why there have been no reports is probably because 99% of  
people have never heard of gitexecdir nor know what it does.

$ git grep gitexecdir | awk -F : '{print $1}' | uniq
Makefile
config.mak.in
git-gui/Makefile
git-gui/macosx/AppMain.tcl
git.c

Try googling for "site:git.or.cz gitexecdir"

Basically, seems the only way you could know about it is if you've  
heard it mentioned on this mailing list and decided to study the  
Makefile.

Cheers,
Wincent

Re: [PATCH] Move all dashed form git commands to libexecdir

[Eyvind Bernhardsen]

On 29. nov. 2007, at 23.36, Junio C Hamano wrote:

[...]

> If people are really serious about reducing the number of commands in
> the path, I would expect fixes and bugreports saying "I am setting
> gitexecdir different from bindir in _my_ installation when I build  
> git,
> and here are the things that does not work if I do so".  Within the  
> span
> of more than 20 months (77cb17e9 introduced gitexecdir in Jan 2006), I
> do not think there was a single such report or patch, other than the
> message from Nguyen that started this thread.

I'm setting gitexecdir different from bindir in my installation, and  
here are the things that don't work:

- When pushing to my system over ssh, git-receive-pack and git-upload- 
pack are expected to be in $PATH.  I resolved the problem by putting  
symlinks in /usr/local/bin.

I haven't seen any other problems, but then again, I only use git  
plumbing commands and my own scripts.

Eyvind

[PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

[Johannes Schindelin]

Since we plan to move the dash-form (git-<whatever>) into an execdir, it
make sense to prepare our git protocol users for it.

Noticed by Eyvind Bernhardsen.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Fri, 30 Nov 2007, Eyvind Bernhardsen wrote:

	> - When pushing to my system over ssh, git-receive-pack and
	> git-upload-pack are expected to be in $PATH.  I resolved the 
	> problem by putting symlinks in /usr/local/bin.

	How about this?  (I only compile-tested it...)

 transport.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/transport.c b/transport.c
index 50db980..7bd0846 100644
--- a/transport.c
+++ b/transport.c
@@ -736,10 +736,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		ret->disconnect = disconnect_git;
 
 		data->thin = 1;
-		data->uploadpack = "git-upload-pack";
+		data->uploadpack = "git upload-pack";
 		if (remote && remote->uploadpack)
 			data->uploadpack = remote->uploadpack;
-		data->receivepack = "git-receive-pack";
+		data->receivepack = "git receive-pack";
 		if (remote && remote->receivepack)
 			data->receivepack = remote->receivepack;
 	}
-- 
1.5.3.6.2088.g8c260

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nguyen Thai Ngoc Duy]

On Nov 30, 2007 6:28 PM, Eyvind Bernhardsen
<eyvind-git-list@orakel.ntnu.no> wrote:
> On 29. nov. 2007, at 23.36, Junio C Hamano wrote:
>
> [...]
>
> > If people are really serious about reducing the number of commands in
> > the path, I would expect fixes and bugreports saying "I am setting
> > gitexecdir different from bindir in _my_ installation when I build
> > git,
> > and here are the things that does not work if I do so".  Within the
> > span
> > of more than 20 months (77cb17e9 introduced gitexecdir in Jan 2006), I
> > do not think there was a single such report or patch, other than the
> > message from Nguyen that started this thread.
>
> I'm setting gitexecdir different from bindir in my installation, and
> here are the things that don't work:
>
> - When pushing to my system over ssh, git-receive-pack and git-upload-
> pack are expected to be in $PATH.  I resolved the problem by putting
> symlinks in /usr/local/bin.
>
> I haven't seen any other problems, but then again, I only use git
> plumbing commands and my own scripts.

You remind me my experience of making every external C-based command
builtin. There is another case: git-merge. It calls  something like
git-merge-$strategy.

-- 
Duy

Re: [PATCH] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

Hi,

On Fri, 30 Nov 2007, Nguyen Thai Ngoc Duy wrote:

> On Nov 30, 2007 6:28 PM, Eyvind Bernhardsen
> <eyvind-git-list@orakel.ntnu.no> wrote:
> > On 29. nov. 2007, at 23.36, Junio C Hamano wrote:
> >
> > [...]
> >
> > > If people are really serious about reducing the number of commands in
> > > the path, I would expect fixes and bugreports saying "I am setting
> > > gitexecdir different from bindir in _my_ installation when I build
> > > git,
> > > and here are the things that does not work if I do so".  Within the
> > > span
> > > of more than 20 months (77cb17e9 introduced gitexecdir in Jan 2006), I
> > > do not think there was a single such report or patch, other than the
> > > message from Nguyen that started this thread.
> >
> > I'm setting gitexecdir different from bindir in my installation, and
> > here are the things that don't work:
> >
> > - When pushing to my system over ssh, git-receive-pack and git-upload-
> > pack are expected to be in $PATH.  I resolved the problem by putting
> > symlinks in /usr/local/bin.
> >
> > I haven't seen any other problems, but then again, I only use git
> > plumbing commands and my own scripts.
> 
> You remind me my experience of making every external C-based command
> builtin. There is another case: git-merge. It calls  something like
> git-merge-$strategy.

Good catch!

Ciao,
Dscho

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Fri, Nov 30, 2007 at 08:18:16AM +0100, Andreas Ericsson wrote:

>> The only reason I have heard to remove them entirely is that Windows
>> doesn't properly support hardlinks, which I addressed in my other mails
>> (and to which I have seen no rebuttal).
>>
> It would provide a ui inconsistency between platforms. Several people
> pointed that out. It's decidedly a Bad Thing.

Which, as I said, I have already addressed (and which Linus has also
expanded upon in this thread). Since those hardlinks would be hidden
from users who did not go to some trouble to find them, there will not
be inconsistency problems. Scripts will have to either go to some effort
to change their PATH, or simply use the correct form, and I expect them
to do the latter.

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Fri, Nov 30, 2007 at 06:51:35AM +0100, Steffen Prohaska wrote:

> What will happen to gitk?
>
> Shouldn't it be included in the nice collection?  gitk is
> an essential command.  Then, following your reasoning,
> "git <tab><tab>" should recommend it, no?
>
> Note, "git gui" already works.  gitk would really be the last
> git "command" that can't be accessed through "git <command>"

Except for qgit, tig, etc. The only difference being that gitk actually
ships with git.

But I am not opposed to having some "git foo" form for gitk.

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Santi Béjar]

On Nov 30, 2007 4:12 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Nov 30, 2007 at 06:51:35AM +0100, Steffen Prohaska wrote:
>
> > What will happen to gitk?
> >
> > Shouldn't it be included in the nice collection?  gitk is
> > an essential command.  Then, following your reasoning,
> > "git <tab><tab>" should recommend it, no?
> >
> > Note, "git gui" already works.  gitk would really be the last
> > git "command" that can't be accessed through "git <command>"
>
> Except for qgit, tig, etc. The only difference being that gitk actually
> ships with git.
>
> But I am not opposed to having some "git foo" form for gitk.

In mercurial "hg view" is actually an old version of gitk modified for hg.

And as "git view" it could be added to the "git help" list.

Santi

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Fri, Nov 30, 2007 at 04:28:21PM +0100, Santi Béjar wrote:

> > But I am not opposed to having some "git foo" form for gitk.
> 
> In mercurial "hg view" is actually an old version of gitk modified for hg.
> 
> And as "git view" it could be added to the "git help" list.

Unfortunately, there is already a "gitview" program similar to gitk,
although it never made it out of contrib/.

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Linus Torvalds]

On Fri, 30 Nov 2007, Jeff King wrote:
>
> On Fri, Nov 30, 2007 at 04:28:21PM +0100, Santi Béjar wrote:
> > > But I am not opposed to having some "git foo" form for gitk.
> > 
> > In mercurial "hg view" is actually an old version of gitk modified for hg.
> > 
> > And as "git view" it could be added to the "git help" list.
> 
> Unfortunately, there is already a "gitview" program similar to gitk,
> although it never made it out of contrib/.

Well, different people will want different viewers *anyway* (ie some will 
prefer qgit etc), so how about making "git view" be something that 
literally acts as a built-in alias that just defaults to running gitk (if 
for no other reason than the fact that gitk is the one that ships with 
git, and simply has most users).

There's a few other things that I think we could consider to be good 
built-in aliases: things like "git cat" being an alias for "git -p 
cat-file -p" etc.

The only difference between a "built-in alias" and a "built-in command" 
would be:
 - the alias has never even had the "git-xyz" format, and never will
 - the alias can be overridden by user aliases unlike "real" git commands

Hmm?

That way we can hide away gitk too (although I do suspect that we might as 
well just leave gitk in the path - it's different in naming from all the 
git-xyz commands anyway)

		Linus

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Fri, Nov 30, 2007 at 07:50:47AM -0800, Linus Torvalds wrote:

> Well, different people will want different viewers *anyway* (ie some will 
> prefer qgit etc), so how about making "git view" be something that 
> literally acts as a built-in alias that just defaults to running gitk (if 
> for no other reason than the fact that gitk is the one that ships with 
> git, and simply has most users).

I think that is a good idea, and here's a patch.

-- >8 --
Support builtin aliases

Builtin aliases are "default" alias values that can be
overridden by user-configured aliases.

For example, the first such alias is "view", an alias for
gitk. A user with no further configuration can run
"git view" to use gitk. However, they can also set the
config option "alias.view" to "!tig" to run tig.
---
 git.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/git.c b/git.c
index f220284..95296aa 100644
--- a/git.c
+++ b/git.c
@@ -151,6 +151,13 @@ static int split_cmdline(char *cmdline, const char ***argv)
 	return count;
 }
 
+static char *builtin_alias(const char *cmd)
+{
+	if (!strcmp(cmd, "view"))
+		return xstrdup("!gitk");
+	return NULL;
+}
+
 static int handle_alias(int *argcp, const char ***argv)
 {
 	int nongit = 0, envchanged = 0, ret = 0, saved_errno = errno;
@@ -162,6 +169,8 @@ static int handle_alias(int *argcp, const char ***argv)
 
 	alias_command = (*argv)[0];
 	git_config(git_alias_config);
+	if (!alias_string)
+		alias_string = builtin_alias(alias_command);
 	if (alias_string) {
 		if (alias_string[0] == '!') {
 			if (*argcp > 1) {
-- 
1.5.3.6.2064.g2e22f-dirty

Re: [PATCH] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

Hi,

On Fri, 30 Nov 2007, Jeff King wrote:

> Support builtin aliases
> 
> Builtin aliases are "default" alias values that can be
> overridden by user-configured aliases.
> 
> For example, the first such alias is "view", an alias for
> gitk. A user with no further configuration can run
> "git view" to use gitk. However, they can also set the
> config option "alias.view" to "!tig" to run tig.
> ---
>  git.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/git.c b/git.c
> index f220284..95296aa 100644
> --- a/git.c
> +++ b/git.c
> @@ -151,6 +151,13 @@ static int split_cmdline(char *cmdline, const char ***argv)
>  	return count;
>  }
>  
> +static char *builtin_alias(const char *cmd)
> +{
> +	if (!strcmp(cmd, "view"))
> +		return xstrdup("!gitk");
> +	return NULL;
> +}
> +
>  static int handle_alias(int *argcp, const char ***argv)
>  {
>  	int nongit = 0, envchanged = 0, ret = 0, saved_errno = errno;
> @@ -162,6 +169,8 @@ static int handle_alias(int *argcp, const char ***argv)
>  
>  	alias_command = (*argv)[0];
>  	git_config(git_alias_config);
> +	if (!alias_string)
> +		alias_string = builtin_alias(alias_command);
>  	if (alias_string) {
>  		if (alias_string[0] == '!') {
>  			if (*argcp > 1) {

Didn't you mean to put this _before_ the git_config() call?  As you wrote 
it, the "soft" alias overrides the user-specified one.

Ciao,
Dscho

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Fri, Nov 30, 2007 at 06:28:50PM +0000, Johannes Schindelin wrote:

> > @@ -162,6 +169,8 @@ static int handle_alias(int *argcp, const char ***argv)
> >  
> >  	alias_command = (*argv)[0];
> >  	git_config(git_alias_config);
> > +	if (!alias_string)
> > +		alias_string = builtin_alias(alias_command);
> >  	if (alias_string) {
> >  		if (alias_string[0] == '!') {
> >  			if (*argcp > 1) {
> 
> Didn't you mean to put this _before_ the git_config() call?  As you wrote 
> it, the "soft" alias overrides the user-specified one.

No. The "if (!alias_string)" means we only do the lookup if no user
alias was found. Try it.

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Nov 30, 2007 at 08:18:16AM +0100, Andreas Ericsson wrote:
> ...
>> It would provide a ui inconsistency between platforms. Several people
>> pointed that out. It's decidedly a Bad Thing.
>
> Which, as I said, I have already addressed (and which Linus has also
> expanded upon in this thread). Since those hardlinks would be hidden
> from users who did not go to some trouble to find them, there will not
> be inconsistency problems.

I already can see exchanges in the user community after such a change
you propose would happen:

 Newbie: Ay! why doesn't git-commit work anymore?

 Jeff: Stupid Junio and Linus decided that you should not use dash form
       but say "git commit" instead.

 Newbie: But my fingers are trained and I like the "git-<tab>"
         completion.

 Jeff: If you really like that, here is a hidden trick.  Add
       /usr/libexec/git-core/ to your PATH.

 Newbie: Ah, that worked, thanks.

 A few days later...

 Newbie: Jeff, your trick does not work for my coworker.  He also has
         the latest git.  His installation does not even have that
         directory!  What gives?

 Jeff: Ah, sorry, that trick works for some platforms but not others.

 Newbie: Stupid inconsistency.  Who suggested that?

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Fri, Nov 30, 2007 at 12:01:13PM -0800, Junio C Hamano wrote:

> I already can see exchanges in the user community after such a change
> you propose would happen:
> [...]
>  Jeff: If you really like that, here is a hidden trick.  Add
>        /usr/libexec/git-core/ to your PATH.

What if I promise not to tell anyone? :)

Anyway, I don't think it will be a problem. You think it might. But I
suspect neither of us has anything more than a gut feeling to argue
with. And now I have registered my complaint, so you can do what you
think is best.

I can, of course, always make my own hardlinks (which is really the same
thing, except the "trick" is slightly harder to perform and perhaps less
socially acceptable; OTOH, if such a trick is common, perhaps it means
taking away the dash forms wasn't such a good idea after all).

>  Newbie: Stupid inconsistency.  Who suggested that?

Jeff [runs git-blame]: It must have been Junio! :)

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

Hi,

On Fri, 30 Nov 2007, Jeff King wrote:

> On Fri, Nov 30, 2007 at 06:28:50PM +0000, Johannes Schindelin wrote:
> 
> > > @@ -162,6 +169,8 @@ static int handle_alias(int *argcp, const char ***argv)
> > >  
> > >  	alias_command = (*argv)[0];
> > >  	git_config(git_alias_config);
> > > +	if (!alias_string)
> > > +		alias_string = builtin_alias(alias_command);
> > >  	if (alias_string) {
> > >  		if (alias_string[0] == '!') {
> > >  			if (*argcp > 1) {
> > 
> > Didn't you mean to put this _before_ the git_config() call?  As you wrote 
> > it, the "soft" alias overrides the user-specified one.
> 
> No. The "if (!alias_string)" means we only do the lookup if no user
> alias was found. Try it.

Ah.  To me, that was rather easy to miss, though...

Ciao,
Dscho

Re: [PATCH] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

Hi,

On Fri, 30 Nov 2007, Jeff King wrote:

> On Fri, Nov 30, 2007 at 12:01:13PM -0800, Junio C Hamano wrote:
> 
> > I already can see exchanges in the user community after such a change
> > you propose would happen:
> > [...]
> >  Jeff: If you really like that, here is a hidden trick.  Add
> >        /usr/libexec/git-core/ to your PATH.
> 
> What if I promise not to tell anyone? :)

By the same reasoning you can invade an unsuspecting country, saying that 
everybody will be better off afterwards.  But the risk is high, not 
because of the probability, but because of the cost to pay if it does not 
work out.

Really, I'd rather have this be done right.  So I am quite happy with 
Junio being "girly" (which I would have called cautious and nice-to-users, 
as well as considerate, though).

In the end I would be so much happier not to have hard links at all, 
and it seems that all the "easy" SCMs out there are quite well off without 
hard links, too.

To me, it is mighty annoying anybody brings up that "144 commands" 
argument Linus was referring to, and if there is _any_ way to shut up 
those bikeshedders, I am all for it.

But if that is not possible, so be it.

Ciao,
Dscho

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Fri, Nov 30, 2007 at 11:05:50PM +0000, Johannes Schindelin wrote:

> > > >  	alias_command = (*argv)[0];
> > > >  	git_config(git_alias_config);
> > > > +	if (!alias_string)
> > > > +		alias_string = builtin_alias(alias_command);
> > > >  	if (alias_string) {
> > > >  		if (alias_string[0] == '!') {
> > > >  			if (*argcp > 1) {
> > > 
> > > Didn't you mean to put this _before_ the git_config() call?  As you wrote 
> > > it, the "soft" alias overrides the user-specified one.
> > 
> > No. The "if (!alias_string)" means we only do the lookup if no user
> > alias was found. Try it.
> 
> Ah.  To me, that was rather easy to miss, though...

I don't particularly care if it is re-written as:

  alias_string = builtin_alias(alias_command);
  git_config(git_alias_config);

which should be equivalent.  I wrote it the original way to avoid doing
the O(n) search through builtin aliases when it was unnecessary, but
obviously this isn't a performance critical code path.

-Peff

Re: [PATCH] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

Hi,

On Fri, 30 Nov 2007, Jeff King wrote:

> On Fri, Nov 30, 2007 at 11:05:50PM +0000, Johannes Schindelin wrote:
> 
> > > > >  	alias_command = (*argv)[0];
> > > > >  	git_config(git_alias_config);
> > > > > +	if (!alias_string)
> > > > > +		alias_string = builtin_alias(alias_command);
> > > > >  	if (alias_string) {
> > > > >  		if (alias_string[0] == '!') {
> > > > >  			if (*argcp > 1) {
> > > > 
> > > > Didn't you mean to put this _before_ the git_config() call?  As you wrote 
> > > > it, the "soft" alias overrides the user-specified one.
> > > 
> > > No. The "if (!alias_string)" means we only do the lookup if no user
> > > alias was found. Try it.
> > 
> > Ah.  To me, that was rather easy to miss, though...
> 
> I don't particularly care if it is re-written as:
> 
>   alias_string = builtin_alias(alias_command);
>   git_config(git_alias_config);
> 
> which should be equivalent.  I wrote it the original way to avoid doing
> the O(n) search through builtin aliases when it was unnecessary, but
> obviously this isn't a performance critical code path.

Actually, I felt/feel quite dumb missing it.

Ciao,
Dscho

Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Since we plan to move the dash-form (git-<whatever>) into an execdir, it
> make sense to prepare our git protocol users for it.
>
> Noticed by Eyvind Bernhardsen.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> 	On Fri, 30 Nov 2007, Eyvind Bernhardsen wrote:
>
> 	> - When pushing to my system over ssh, git-receive-pack and
> 	> git-upload-pack are expected to be in $PATH.  I resolved the 
> 	> problem by putting symlinks in /usr/local/bin.
>
> 	How about this?  (I only compile-tested it...)

I only eyeball-tested it and looks Okay, but that does not assure us
much ;-).  Is this change easy to test using local transport?

Re: [PATCH] Move all dashed form git commands to libexecdir

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I can, of course, always make my own hardlinks (which is really the same
> thing, except the "trick" is slightly harder to perform and perhaps less
> socially acceptable; OTOH, if such a trick is common, perhaps it means
> taking away the dash forms wasn't such a good idea after all).
>
>>  Newbie: Stupid inconsistency.  Who suggested that?
>
> Jeff [runs git-blame]: It must have been Junio! :)

You found a bug in git-blame, then ;-).  I think it should report Jeff.

As Windows ports need to have their own difference _anyway_, I
personally do not think it is a big deal if the Makefile I ship
continues to install the dashed form in gitexecdir, and Windows ports
omit the hardlinks if they feel copies are wasteful.

However, that would introduce hard-to-track platform dependent bugs
(e.g. "git-receive-pack" is asked for by "git-send-pack", but the other
side does not have such a program anywhere).  So my preference at this
point is to move things out of PATH first (without removing the
hardlinks), deal with possible fallout from that move.

And after things stablize, discuss to either remove the hardlinks from
all installations, or to keep them in all installations.  I do not think
"it's this way here but that way there" is a good thing in general.

We do have "git-foo.exe" vs "git-foo" difference and there are some
existing code (most notably, help.c::list_commands_in_dir()) that need
to be aware of it.  Let's try not to make things any worse.

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Fri, Nov 30, 2007 at 06:37:12PM -0800, Junio C Hamano wrote:

> side does not have such a program anywhere).  So my preference at this
> point is to move things out of PATH first (without removing the
> hardlinks), deal with possible fallout from that move.
> 
> And after things stablize, discuss to either remove the hardlinks from
> all installations, or to keep them in all installations.  I do not think
> "it's this way here but that way there" is a good thing in general.

I think that it is a sensible course of action.

-Peff

Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

[Johannes Schindelin]

Hi,

On Fri, 30 Nov 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Since we plan to move the dash-form (git-<whatever>) into an execdir, it
> > make sense to prepare our git protocol users for it.
> >
> > Noticed by Eyvind Bernhardsen.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >
> > 	On Fri, 30 Nov 2007, Eyvind Bernhardsen wrote:
> >
> > 	> - When pushing to my system over ssh, git-receive-pack and
> > 	> git-upload-pack are expected to be in $PATH.  I resolved the 
> > 	> problem by putting symlinks in /usr/local/bin.
> >
> > 	How about this?  (I only compile-tested it...)
> 
> I only eyeball-tested it and looks Okay, but that does not assure us
> much ;-).  Is this change easy to test using local transport?

Seems like it breaks down with git-shell.  But then, I think that we just 
have to fix execv_git_cmd() to call builtins via "git" instead.

Ciao,
Dscho

Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> I only eyeball-tested it and looks Okay, but that does not assure us
>> much ;-).  Is this change easy to test using local transport?
>
> Seems like it breaks down with git-shell.  But then, I think that we just 
> have to fix execv_git_cmd() to call builtins via "git" instead.

So in execv_git_cmd(), instead of doing the strbuf_addf(), we do
something like this (and drop your patch)?

---
 exec_cmd.c |   34 +++++++++++++---------------------
 1 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 2d0a758..2920335 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -65,32 +65,24 @@ void setup_path(const char *cmd_path)
 
 int execv_git_cmd(const char **argv)
 {
-	struct strbuf cmd;
-	const char *tmp;
-
-	strbuf_init(&cmd, 0);
-	strbuf_addf(&cmd, "git-%s", argv[0]);
-
-	/*
-	 * argv[0] must be the git command, but the argv array
-	 * belongs to the caller, and may be reused in
-	 * subsequent loop iterations. Save argv[0] and
-	 * restore it on error.
-	 */
-	tmp = argv[0];
-	argv[0] = cmd.buf;
-
-	trace_argv_printf(argv, -1, "trace: exec:");
+	int i;
+	const char **args;
+
+	for (i = 0; argv[i]; i++)
+		;
+	args = xcalloc(i + 1, sizeof(*args));
+	for (i = 0; argv[i]; i++)
+		args[i+1] = argv[i];
+	args[0] = "git";
+	args[i+1] = NULL;
+	trace_argv_printf(args, -1, "trace: exec:");
 
 	/* execvp() can only ever return if it fails */
-	execvp(cmd.buf, (char **)argv);
+	execvp(args[0], (char **)args);
 
 	trace_printf("trace: exec failed: %s\n", strerror(errno));
 
-	argv[0] = tmp;
-
-	strbuf_release(&cmd);
-
+	free(args);
 	return -1;
 }
 

Re: [PATCH] Move all dashed form git commands to libexecdir

[Junio C Hamano]

"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:

> On Nov 30, 2007 10:50 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>> Well, different people will want different viewers *anyway* (ie some will
>> prefer qgit etc), so how about making "git view" be something that
>> literally acts as a built-in alias that just defaults to running gitk (if
>> for no other reason than the fact that gitk is the one that ships with
>> git, and simply has most users).
>
> We already have "git show", now we gonna get "git view", git trainers
> may have hard time explaining this one shows you a particular object
> while the other one shows you history. How about "git lshistory" (from
> clearcase land) or git show --history?

Heh, we have "bisect visualize".  How about "git visualize"?

Re: [PATCH] Move all dashed form git commands to libexecdir

[Jeff King]

On Sat, Dec 01, 2007 at 11:32:27AM -0800, Junio C Hamano wrote:

> > We already have "git show", now we gonna get "git view", git trainers
> > may have hard time explaining this one shows you a particular object
> > while the other one shows you history. How about "git lshistory" (from
> > clearcase land) or git show --history?
> 
> Heh, we have "bisect visualize".  How about "git visualize"?

Yes, in retrospect "view" is probably not the best. "lshistory" just
looks awful, and I think it's wrong for an option to "git show" to
change it from a terminal application into a GUI application.

"visualize" is actually pretty good, except that it would be painful to
type. On the other hand, I will probably still just type "gitk". I
actually think these sorts of aliases may be most useful for user-facing
scripts to say "and now show the dataset in the user's graphical history
browser of choice."

-Peff

Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

[Johannes Schindelin]

Hi,

On Sat, 1 Dec 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> I only eyeball-tested it and looks Okay, but that does not assure us
> >> much ;-).  Is this change easy to test using local transport?
> >
> > Seems like it breaks down with git-shell.  But then, I think that we just 
> > have to fix execv_git_cmd() to call builtins via "git" instead.
> 
> So in execv_git_cmd(), instead of doing the strbuf_addf(), we do
> something like this (and drop your patch)?

You know what is really funny?  I have this in my stash:

-- snip --
 exec_cmd.c      |   30 ++++++++++++------------------
 t/t0020-crlf.sh |    2 +-
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 2d0a758..7d022a2 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -65,31 +65,25 @@ void setup_path(const char *cmd_path)
 
 int execv_git_cmd(const char **argv)
 {
-	struct strbuf cmd;
-	const char *tmp;
+	int i;
+	char **new_argv;
 
-	strbuf_init(&cmd, 0);
-	strbuf_addf(&cmd, "git-%s", argv[0]);
+	for (i = 0; argv[i]; i++)
+		; /* do nothing */
+	new_argv = xmalloc((i + 2) * sizeof(*new_argv));
+	new_argv[0] = "git";
+	for (i = 0; argv[i]; i++)
+		new_argv[i + 1] = (char *)argv[i];
+	new_argv[i] = NULL;
 
-	/*
-	 * argv[0] must be the git command, but the argv array
-	 * belongs to the caller, and may be reused in
-	 * subsequent loop iterations. Save argv[0] and
-	 * restore it on error.
-	 */
-	tmp = argv[0];
-	argv[0] = cmd.buf;
-
-	trace_argv_printf(argv, -1, "trace: exec:");
+	trace_argv_printf((const char **)new_argv, -1, "trace: exec:");
 
 	/* execvp() can only ever return if it fails */
-	execvp(cmd.buf, (char **)argv);
+	execvp(new_argv[0], new_argv);
 
 	trace_printf("trace: exec failed: %s\n", strerror(errno));
 
-	argv[0] = tmp;
-
-	strbuf_release(&cmd);
+	free(new_argv);
 
 	return -1;
 }
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 62bc4bb..275379d 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -36,7 +36,7 @@ test_expect_success setup '
 
 	for w in Some extra lines here; do echo $w; done >>one &&
 	git diff >patch.file &&
-	patched=`git hash-object --stdin <one` &&
+	patched=`GIT_TRACE=2 git hash-object --stdin <one` &&
 	git read-tree --reset -u HEAD &&
 
 	echo happy.
-- snap --

Which looks awfully like your patch (except that I called it new_argv, I 
think).

Now you might ask why there is such a funny patch to t0020?  Well, the 
patch does not work as-is.

Will investigate right now,
Dscho

Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

[Johannes Schindelin]

Hi,

On Sat, 1 Dec 2007, Johannes Schindelin wrote:

> Will investigate right now,

The problem is that "git <command>" will call execv_git_cmd() for 
non-builtins, which in turn will execute "git <command>", ... ad 
infinitum.

Ciao,
Dscho

Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Sat, 1 Dec 2007, Johannes Schindelin wrote:
>
>> Will investigate right now,
>
> The problem is that "git <command>" will call execv_git_cmd() for 
> non-builtins, which in turn will execute "git <command>", ... ad 
> infinitum.

Ok, then the ".. then try the external ones" in git.c needs to do the
execvp itself, which should not be a big deal.

Very funny ;-).

[PATCH 0/3] Call builtin functions directly, was Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

[Johannes Schindelin]

Hi,

On Sat, 1 Dec 2007, Johannes Schindelin wrote:

> On Sat, 1 Dec 2007, Johannes Schindelin wrote:
> 
> > Will investigate right now,
> 
> The problem is that "git <command>" will call execv_git_cmd() for 
> non-builtins, which in turn will execute "git <command>", ... ad 
> infinitum.

Okay, I bit the apple and tried to move the builtins into the library, and 
rename handle_internal_command into execv_git_builtin(), moving it into 
exec-cmd.c.

Big mistake.

Why?  Because there is at least one caller, git-bundle, which relies on 
execv_git_cmd() _not_ reusing all those "nice" one-shot static variables, 
like for example the object hashmap and the objects themselves.

Now, it seems that we can get away for the moment with just introducing an 
object release mechanism and calling that in execv_git_builtin() before 
calling a builtin function, because the existing callers do not rely on 
more than a cleanup of the objects.

But it is hairy, since it is such an essential part of git.  And since I 
was utterly tired while preparing this patch series.  So I suggest maybe 
putting this into pu, but no further for the moment.  I will use the 
patched git in the next days, though, to catch breakages (hopefully).

Ciao,
Dscho

[PATCH 1/3] Introduce release_all_objects()

[Johannes Schindelin]

The new function release_all_objects() can be used to flush the object
cache.  This will be needed for the upcoming change in execv_git_cmd(),
which should call the builtin functions directly instead of calling
execvp().

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Guess how surprised I was when "free(commit);" would work the
	first time, but not the second...

	ATM I use an ugly way to cope with the static "nr" variables
	in alloc.c: I just do not free() the last block.

	It might be a better idea to refactor the (quite ugly) code in
	alloc.c to have global structures a la

		#define BLOCKING 1024

		struct object_block {
			size_t struct_size;
			int nr;
			/* first (uint32_t *) is pointer to previous block */
			void *block;
		};

		static void *alloc_node(struct object_block *block)
		{
			if (!block || block->nr >= BLOCKING) {
				void *next = xmalloc(sizeof(void *)
					+ BLOCKING * block->struct_size);
				*(void **)next = block->block;
				block->block = next;
				block->nr = 0;
			}
			return block->block + sizeof(void *)
				+ block->struct_size * block->nr++;
		}

		static void release_nodes(struct object_block *block)
		{
			while (block->block) {
				void *previous = *(void **)block->block;
				free(block->block);
				block->block = previous;
			}
			block->nr = 0; /* not strictly necessary */
		}

		#define DEFINE_ALLOCATOR(type)				\
		static object_block type##s = { sizeof(struct type) };	\
		struct type *alloc_##type##_node(void)			\
		{							\
			return alloc_node(&type##s);			\
		}

		DEFINE_ALLOCATOR(object)
		DEFINE_ALLOCATOR(blob)
		DEFINE_ALLOCATOR(tree)
		DEFINE_ALLOCATOR(commit)
		DEFINE_ALLOCATOR(tag)

	but I am waaay too tired to brush this up, test it and submit it
	(hint, hint).

 alloc.c  |   31 ++++++++++++++++++++++++++++++-
 blob.c   |    3 +++
 blob.h   |    1 +
 cache.h  |    6 ++++++
 commit.c |    8 ++++++++
 commit.h |    2 ++
 object.c |   24 ++++++++++++++++++++++++
 object.h |    2 ++
 tag.c    |    8 ++++++++
 tag.h    |    2 ++
 tree.c   |    6 ++++++
 tree.h   |    1 +
 12 files changed, 93 insertions(+), 1 deletions(-)

diff --git a/alloc.c b/alloc.c
index 216c23a..8c5e5e0 100644
--- a/alloc.c
+++ b/alloc.c
@@ -20,6 +20,7 @@
 
 #define DEFINE_ALLOCATOR(name, type)				\
 static unsigned int name##_allocs;				\
+static void *last_alloced_##name;				\
 void *alloc_##name##_node(void)					\
 {								\
 	static int nr;						\
@@ -28,13 +29,32 @@ void *alloc_##name##_node(void)					\
 								\
 	if (!nr) {						\
 		nr = BLOCKING;					\
-		block = xmalloc(BLOCKING * sizeof(type));	\
+		struct {					\
+			void *previous;				\
+			type block[BLOCKING];			\
+		} *buf = xmalloc(sizeof(*buf));			\
+		buf->previous = last_alloced_##name;		\
+		last_alloced_##name = buf;			\
+		block = buf->block;				\
 	}							\
 	nr--;							\
 	name##_allocs++;					\
 	ret = block++;						\
 	memset(ret, 0, sizeof(type));				\
 	return ret;						\
+}								\
+								\
+void release_all_##name##_nodes(void)				\
+{								\
+	void *buf = last_alloced_##name;			\
+	if (!buf)						\
+		return;						\
+	buf = *(void **)buf;					\
+	while (buf) {						\
+		void *next = *(void **)buf;			\
+		free(buf);					\
+		buf = next;					\
+	}							\
 }
 
 union any_object {
@@ -74,3 +94,12 @@ void alloc_report(void)
 	REPORT(commit);
 	REPORT(tag);
 }
+
+void release_all_nodes(void)
+{
+	release_all_blob_nodes();
+	release_all_tree_nodes();
+	release_all_commit_nodes();
+	release_all_tag_nodes();
+	release_all_object_nodes();
+}
diff --git a/blob.c b/blob.c
index bd7d078..63756e6 100644
--- a/blob.c
+++ b/blob.c
@@ -18,6 +18,9 @@ struct blob *lookup_blob(const unsigned char *sha1)
 	return (struct blob *) obj;
 }
 
+void release_blob(struct blob *blob) {
+}
+
 int parse_blob_buffer(struct blob *item, void *buffer, unsigned long size)
 {
 	item->object.parsed = 1;
diff --git a/blob.h b/blob.h
index ea5d9e9..7560671 100644
--- a/blob.h
+++ b/blob.h
@@ -10,6 +10,7 @@ struct blob {
 };
 
 struct blob *lookup_blob(const unsigned char *sha1);
+void release_blob(struct blob *blob);
 
 int parse_blob_buffer(struct blob *item, void *buffer, unsigned long size);
 
diff --git a/cache.h b/cache.h
index 4e59646..cc50f1c 100644
--- a/cache.h
+++ b/cache.h
@@ -613,6 +613,12 @@ extern void *alloc_tree_node(void);
 extern void *alloc_commit_node(void);
 extern void *alloc_tag_node(void);
 extern void *alloc_object_node(void);
+extern void release_all_blob_nodes(void);
+extern void release_all_tree_nodes(void);
+extern void release_all_commit_nodes(void);
+extern void release_all_tag_nodes(void);
+extern void release_all_object_nodes(void);
+extern void release_all_nodes(void);
 extern void alloc_report(void);
 
 /* trace.c */
diff --git a/commit.c b/commit.c
index f074811..59c2236 100644
--- a/commit.c
+++ b/commit.c
@@ -48,6 +48,14 @@ struct commit *lookup_commit(const unsigned char *sha1)
 	return check_commit(obj, sha1, 0);
 }
 
+void release_commit(struct commit *commit)
+{
+	if (commit->parents)
+		free_commit_list(commit->parents);
+	if (commit->buffer)
+		free(commit->buffer);
+}
+
 static unsigned long parse_commit_date(const char *buf)
 {
 	unsigned long date;
diff --git a/commit.h b/commit.h
index 10e2b5d..363b9fb 100644
--- a/commit.h
+++ b/commit.h
@@ -21,6 +21,7 @@ struct commit {
 	char *buffer;
 };
 
+
 extern int save_commit_buffer;
 extern const char *commit_type;
 
@@ -35,6 +36,7 @@ struct commit *lookup_commit(const unsigned char *sha1);
 struct commit *lookup_commit_reference(const unsigned char *sha1);
 struct commit *lookup_commit_reference_gently(const unsigned char *sha1,
 					      int quiet);
+void release_commit(struct commit *commit);
 
 int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size);
 
diff --git a/object.c b/object.c
index 16793d9..f217122 100644
--- a/object.c
+++ b/object.c
@@ -192,6 +192,30 @@ struct object *parse_object(const unsigned char *sha1)
 	return NULL;
 }
 
+void release_all_objects(void)
+{
+	int i;
+	for (i = 0; i < obj_hash_size; i++)
+		if (obj_hash[i]) {
+			switch (obj_hash[i]->type) {
+			case OBJ_BLOB:
+				release_blob((struct blob *)obj_hash[i]);
+				break;
+			case OBJ_COMMIT:
+				release_commit((struct commit *)obj_hash[i]);
+				break;
+			/*case OBJ_TREE:
+				release_tree((struct tree *)obj_hash[i]);
+				break;
+			case OBJ_TAG:
+				release_tag((struct tag *)obj_hash[i]);
+				break;*/
+			}
+			obj_hash[i] = NULL;
+		}
+	release_all_nodes();
+}
+
 struct object_list *object_list_insert(struct object *item,
 				       struct object_list **list_p)
 {
diff --git a/object.h b/object.h
index 397bbfa..ad6184c 100644
--- a/object.h
+++ b/object.h
@@ -44,6 +44,8 @@ extern unsigned int get_max_object_index(void);
 extern struct object *get_indexed_object(unsigned int);
 extern struct object_refs *lookup_object_refs(struct object *);
 
+extern void release_all_objects(void);
+
 /** Internal only **/
 struct object *lookup_object(const unsigned char *sha1);
 
diff --git a/tag.c b/tag.c
index f62bcdd..8bc6840 100644
--- a/tag.c
+++ b/tag.c
@@ -33,6 +33,14 @@ struct tag *lookup_tag(const unsigned char *sha1)
         return (struct tag *) obj;
 }
 
+void release_tag(struct tag *tag)
+{
+	if (tag->tag)
+		free(tag->tag);
+	if (tag->signature)
+		free(tag->signature);
+}
+
 int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 {
 	int typelen, taglen;
diff --git a/tag.h b/tag.h
index 7a0cb00..fbc6048 100644
--- a/tag.h
+++ b/tag.h
@@ -12,6 +12,8 @@ struct tag {
 	char *signature; /* not actually implemented */
 };
 
+void release_tag(struct tag *tag);
+
 extern struct tag *lookup_tag(const unsigned char *sha1);
 extern int parse_tag_buffer(struct tag *item, void *data, unsigned long size);
 extern int parse_tag(struct tag *item);
diff --git a/tree.c b/tree.c
index 8c0819f..ee99bc6 100644
--- a/tree.c
+++ b/tree.c
@@ -202,6 +202,12 @@ struct tree *lookup_tree(const unsigned char *sha1)
 	return (struct tree *) obj;
 }
 
+void release_tree(struct tree *tree)
+{
+	if (tree->buffer)
+		free(tree->buffer);
+}
+
 /*
  * NOTE! Tree refs to external git repositories
  * (ie gitlinks) do not count as real references.
diff --git a/tree.h b/tree.h
index dd25c53..f8372a2 100644
--- a/tree.h
+++ b/tree.h
@@ -12,6 +12,7 @@ struct tree {
 };
 
 struct tree *lookup_tree(const unsigned char *sha1);
+void release_tree(struct tree *tree);
 
 int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size);
 
-- 
1.5.3.6.2112.ge2263

[PATCH 2/3] Include the objects needed for the builtin functions into libgit.a

[Johannes Schindelin]

For the upcoming change in execv_git_cmd() to call builtin functions
directly, it is necessary to be able to access the builtins, so
move the corresponding objects into libgit.a.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Makefile |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index f000a5e..f9a62eb 100644
--- a/Makefile
+++ b/Makefile
@@ -314,7 +314,8 @@ LIB_OBJS = \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
 	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
-	transport.o bundle.o walker.o parse-options.o
+	transport.o bundle.o walker.o parse-options.o \
+	$(BUILTIN_OBJS)
 
 BUILTIN_OBJS = \
 	builtin-add.o \
@@ -785,12 +786,12 @@ strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
 git.o: git.c common-cmds.h GIT-CFLAGS
-	$(QUIET_CC)$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
+	$(QUIET_CC)$(CC) \
 		$(ALL_CFLAGS) -c $(filter %.c,$^)
 
 git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \
-		$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)
+		$(ALL_LDFLAGS) $(LIBS)
 
 help.o: common-cmds.h
 
@@ -894,7 +895,10 @@ git.o git.spec \
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
 exec_cmd.o: exec_cmd.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' $<
+	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) \
+		-DGIT_EXEC_PATH='"$(gitexecdir_SQ)"' \
+		-DGIT_VERSION='"$(GIT_VERSION)"' \
+		$<
 builtin-init-db.o: builtin-init-db.c GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"' $<
 
@@ -920,7 +924,7 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
 
-$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
+$(LIB_OBJS): $(LIB_H)
 $(patsubst git-%$X,%.o,$(PROGRAMS)): $(LIB_H) $(wildcard */*.h)
 builtin-revert.o builtin-runstatus.o wt-status.o: wt-status.h
 
-- 
1.5.3.6.2112.ge2263

[PATCH 3/3] Introduce execv_git_builtin() and use it

[Johannes Schindelin]

You can call execv_git_builtin() to execute a builtin command.  This
function is a reborn handle_internal_command() from git.c, and has the
semantics of execv_git_cmd(), i.e. it exits with the exit code of the
builtin if there is a matching builtin, but it avoids the real
execvp() call.

This function calls release_all_objects() and discard_cache() to start
from a clean slate (this, along with the calculation of argc, is the
only difference from a straight code move).

The test suite passes, which at least does not contradict the
hypothesis that this is good enough.  However, it might be
necessary to de-initialize more global variables.

The function execv_git_cmd() and git.c's main() function were changed
to take advantage of execv_git_builtin().

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 exec_cmd.c |  178 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 exec_cmd.h |    1 +
 git.c      |  172 +---------------------------------------------------------
 3 files changed, 180 insertions(+), 171 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 2d0a758..ac21181 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -1,6 +1,8 @@
 #include "cache.h"
 #include "exec_cmd.h"
 #include "quote.h"
+#include "builtin.h"
+#include "object.h"
 #define MAX_ARGS	32
 
 extern char **environ;
@@ -63,11 +65,187 @@ void setup_path(const char *cmd_path)
 	strbuf_release(&new_path);
 }
 
+const char git_usage_string[] =
+	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
+
+const char git_version_string[] = GIT_VERSION;
+
+#define RUN_SETUP	(1<<0)
+#define USE_PAGER	(1<<1)
+/*
+ * require working tree to be present -- anything uses this needs
+ * RUN_SETUP for reading from the configuration file.
+ */
+#define NEED_WORK_TREE	(1<<2)
+
+struct cmd_struct {
+	const char *cmd;
+	int (*fn)(int, const char **, const char *);
+	int option;
+};
+
+static int run_command(struct cmd_struct *p, int argc, const char **argv)
+{
+	int status;
+	struct stat st;
+	const char *prefix;
+
+	prefix = NULL;
+	if (p->option & RUN_SETUP)
+		prefix = setup_git_directory();
+	if (p->option & USE_PAGER)
+		setup_pager();
+	if (p->option & NEED_WORK_TREE)
+		setup_work_tree();
+
+	trace_argv_printf(argv, argc, "trace: built-in: git");
+
+	status = p->fn(argc, argv, prefix);
+	if (status)
+		return status & 0xff;
+
+	/* Somebody closed stdout? */
+	if (fstat(fileno(stdout), &st))
+		return 0;
+	/* Ignore write errors for pipes and sockets.. */
+	if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode))
+		return 0;
+
+	/* Check for ENOSPC and EIO errors.. */
+	if (fflush(stdout))
+		die("write failure on standard output: %s", strerror(errno));
+	if (ferror(stdout))
+		die("unknown write failure on standard output");
+	if (fclose(stdout))
+		die("close failed on standard output: %s", strerror(errno));
+	return 0;
+}
+
+int execv_git_builtin(const char **argv)
+{
+	const char *cmd = argv[0];
+	static struct cmd_struct commands[] = {
+		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
+		{ "annotate", cmd_annotate, RUN_SETUP },
+		{ "apply", cmd_apply },
+		{ "archive", cmd_archive },
+		{ "blame", cmd_blame, RUN_SETUP },
+		{ "branch", cmd_branch, RUN_SETUP },
+		{ "bundle", cmd_bundle },
+		{ "cat-file", cmd_cat_file, RUN_SETUP },
+		{ "checkout-index", cmd_checkout_index,
+			RUN_SETUP | NEED_WORK_TREE},
+		{ "check-ref-format", cmd_check_ref_format },
+		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
+		{ "cherry", cmd_cherry, RUN_SETUP },
+		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
+		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
+		{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
+		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
+		{ "config", cmd_config },
+		{ "count-objects", cmd_count_objects, RUN_SETUP },
+		{ "describe", cmd_describe, RUN_SETUP },
+		{ "diff", cmd_diff },
+		{ "diff-files", cmd_diff_files },
+		{ "diff-index", cmd_diff_index, RUN_SETUP },
+		{ "diff-tree", cmd_diff_tree, RUN_SETUP },
+		{ "fast-export", cmd_fast_export, RUN_SETUP },
+		{ "fetch", cmd_fetch, RUN_SETUP },
+		{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
+		{ "fetch--tool", cmd_fetch__tool, RUN_SETUP },
+		{ "fmt", cmd_fmt },
+		{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
+		{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
+		{ "format-patch", cmd_format_patch, RUN_SETUP },
+		{ "fsck", cmd_fsck, RUN_SETUP },
+		{ "fsck-objects", cmd_fsck, RUN_SETUP },
+		{ "gc", cmd_gc, RUN_SETUP },
+		{ "get-tar-commit-id", cmd_get_tar_commit_id },
+		{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },
+		{ "help", cmd_help },
+#ifndef NO_CURL
+		{ "http-fetch", cmd_http_fetch, RUN_SETUP },
+#endif
+		{ "init", cmd_init_db },
+		{ "init-db", cmd_init_db },
+		{ "log", cmd_log, RUN_SETUP | USE_PAGER },
+		{ "ls-files", cmd_ls_files, RUN_SETUP },
+		{ "ls-tree", cmd_ls_tree, RUN_SETUP },
+		{ "ls-remote", cmd_ls_remote },
+		{ "mailinfo", cmd_mailinfo },
+		{ "mailsplit", cmd_mailsplit },
+		{ "merge-base", cmd_merge_base, RUN_SETUP },
+		{ "merge-file", cmd_merge_file },
+		{ "merge-ours", cmd_merge_ours, RUN_SETUP },
+		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
+		{ "name-rev", cmd_name_rev, RUN_SETUP },
+		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
+		{ "peek-remote", cmd_ls_remote },
+		{ "pickaxe", cmd_blame, RUN_SETUP },
+		{ "prune", cmd_prune, RUN_SETUP },
+		{ "prune-packed", cmd_prune_packed, RUN_SETUP },
+		{ "push", cmd_push, RUN_SETUP },
+		{ "read-tree", cmd_read_tree, RUN_SETUP },
+		{ "reflog", cmd_reflog, RUN_SETUP },
+		{ "repo-config", cmd_config },
+		{ "rerere", cmd_rerere, RUN_SETUP },
+		{ "reset", cmd_reset, RUN_SETUP },
+		{ "rev-list", cmd_rev_list, RUN_SETUP },
+		{ "rev-parse", cmd_rev_parse },
+		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
+		{ "rm", cmd_rm, RUN_SETUP },
+		{ "send-pack", cmd_send_pack, RUN_SETUP },
+		{ "shortlog", cmd_shortlog, RUN_SETUP | USE_PAGER },
+		{ "show-branch", cmd_show_branch, RUN_SETUP },
+		{ "show", cmd_show, RUN_SETUP | USE_PAGER },
+		{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
+		{ "stripspace", cmd_stripspace },
+		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
+		{ "tag", cmd_tag, RUN_SETUP },
+		{ "tar-tree", cmd_tar_tree },
+		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
+		{ "update-index", cmd_update_index, RUN_SETUP },
+		{ "update-ref", cmd_update_ref, RUN_SETUP },
+		{ "upload-archive", cmd_upload_archive },
+		{ "verify-tag", cmd_verify_tag, RUN_SETUP },
+		{ "version", cmd_version },
+		{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
+		{ "write-tree", cmd_write_tree, RUN_SETUP },
+		{ "verify-pack", cmd_verify_pack },
+		{ "show-ref", cmd_show_ref, RUN_SETUP },
+		{ "pack-refs", cmd_pack_refs, RUN_SETUP },
+	};
+	int i;
+
+	/* Turn "git cmd --help" into "git help cmd" */
+	if (argv[0] && argv[1] && !strcmp(argv[1], "--help")) {
+		argv[1] = argv[0];
+		argv[0] = cmd = "help";
+	}
+
+	for (i = 0; i < ARRAY_SIZE(commands); i++) {
+		int argc;
+		struct cmd_struct *p = commands+i;
+		if (strcmp(p->cmd, cmd))
+			continue;
+		release_all_objects();
+		discard_cache();
+		for (argc = 0; argv[argc]; argc++)
+			; /* do nothing */
+		exit(run_command(p, argc, argv));
+	}
+	return -1;
+}
+
 int execv_git_cmd(const char **argv)
 {
 	struct strbuf cmd;
 	const char *tmp;
 
+	/* Try builtin first... */
+	execv_git_builtin(argv);
+
+	/* ... and then external commands */
 	strbuf_init(&cmd, 0);
 	strbuf_addf(&cmd, "git-%s", argv[0]);
 
diff --git a/exec_cmd.h b/exec_cmd.h
index a892355..bb15425 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -4,6 +4,7 @@
 extern void git_set_argv_exec_path(const char *exec_path);
 extern const char* git_exec_path(void);
 extern void setup_path(const char *);
+extern int execv_git_builtin(const char **argv); /* NULL terminated */
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);
 
diff --git a/git.c b/git.c
index 38b01ca..1523c4a 100644
--- a/git.c
+++ b/git.c
@@ -3,9 +3,6 @@
 #include "cache.h"
 #include "quote.h"
 
-const char git_usage_string[] =
-	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
-
 static int handle_options(const char*** argv, int* argc, int* envchanged)
 {
 	int handled = 0;
@@ -231,169 +228,6 @@ static int handle_alias(int *argcp, const char ***argv)
 	return ret;
 }
 
-const char git_version_string[] = GIT_VERSION;
-
-#define RUN_SETUP	(1<<0)
-#define USE_PAGER	(1<<1)
-/*
- * require working tree to be present -- anything uses this needs
- * RUN_SETUP for reading from the configuration file.
- */
-#define NEED_WORK_TREE	(1<<2)
-
-struct cmd_struct {
-	const char *cmd;
-	int (*fn)(int, const char **, const char *);
-	int option;
-};
-
-static int run_command(struct cmd_struct *p, int argc, const char **argv)
-{
-	int status;
-	struct stat st;
-	const char *prefix;
-
-	prefix = NULL;
-	if (p->option & RUN_SETUP)
-		prefix = setup_git_directory();
-	if (p->option & USE_PAGER)
-		setup_pager();
-	if (p->option & NEED_WORK_TREE)
-		setup_work_tree();
-
-	trace_argv_printf(argv, argc, "trace: built-in: git");
-
-	status = p->fn(argc, argv, prefix);
-	if (status)
-		return status & 0xff;
-
-	/* Somebody closed stdout? */
-	if (fstat(fileno(stdout), &st))
-		return 0;
-	/* Ignore write errors for pipes and sockets.. */
-	if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode))
-		return 0;
-
-	/* Check for ENOSPC and EIO errors.. */
-	if (fflush(stdout))
-		die("write failure on standard output: %s", strerror(errno));
-	if (ferror(stdout))
-		die("unknown write failure on standard output");
-	if (fclose(stdout))
-		die("close failed on standard output: %s", strerror(errno));
-	return 0;
-}
-
-static void handle_internal_command(int argc, const char **argv)
-{
-	const char *cmd = argv[0];
-	static struct cmd_struct commands[] = {
-		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
-		{ "annotate", cmd_annotate, RUN_SETUP },
-		{ "apply", cmd_apply },
-		{ "archive", cmd_archive },
-		{ "blame", cmd_blame, RUN_SETUP },
-		{ "branch", cmd_branch, RUN_SETUP },
-		{ "bundle", cmd_bundle },
-		{ "cat-file", cmd_cat_file, RUN_SETUP },
-		{ "checkout-index", cmd_checkout_index,
-			RUN_SETUP | NEED_WORK_TREE},
-		{ "check-ref-format", cmd_check_ref_format },
-		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
-		{ "cherry", cmd_cherry, RUN_SETUP },
-		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
-		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
-		{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
-		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
-		{ "config", cmd_config },
-		{ "count-objects", cmd_count_objects, RUN_SETUP },
-		{ "describe", cmd_describe, RUN_SETUP },
-		{ "diff", cmd_diff },
-		{ "diff-files", cmd_diff_files },
-		{ "diff-index", cmd_diff_index, RUN_SETUP },
-		{ "diff-tree", cmd_diff_tree, RUN_SETUP },
-		{ "fast-export", cmd_fast_export, RUN_SETUP },
-		{ "fetch", cmd_fetch, RUN_SETUP },
-		{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
-		{ "fetch--tool", cmd_fetch__tool, RUN_SETUP },
-		{ "fmt", cmd_fmt },
-		{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
-		{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
-		{ "format-patch", cmd_format_patch, RUN_SETUP },
-		{ "fsck", cmd_fsck, RUN_SETUP },
-		{ "fsck-objects", cmd_fsck, RUN_SETUP },
-		{ "gc", cmd_gc, RUN_SETUP },
-		{ "get-tar-commit-id", cmd_get_tar_commit_id },
-		{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },
-		{ "help", cmd_help },
-#ifndef NO_CURL
-		{ "http-fetch", cmd_http_fetch, RUN_SETUP },
-#endif
-		{ "init", cmd_init_db },
-		{ "init-db", cmd_init_db },
-		{ "log", cmd_log, RUN_SETUP | USE_PAGER },
-		{ "ls-files", cmd_ls_files, RUN_SETUP },
-		{ "ls-tree", cmd_ls_tree, RUN_SETUP },
-		{ "ls-remote", cmd_ls_remote },
-		{ "mailinfo", cmd_mailinfo },
-		{ "mailsplit", cmd_mailsplit },
-		{ "merge-base", cmd_merge_base, RUN_SETUP },
-		{ "merge-file", cmd_merge_file },
-		{ "merge-ours", cmd_merge_ours, RUN_SETUP },
-		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
-		{ "name-rev", cmd_name_rev, RUN_SETUP },
-		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
-		{ "peek-remote", cmd_ls_remote },
-		{ "pickaxe", cmd_blame, RUN_SETUP },
-		{ "prune", cmd_prune, RUN_SETUP },
-		{ "prune-packed", cmd_prune_packed, RUN_SETUP },
-		{ "push", cmd_push, RUN_SETUP },
-		{ "read-tree", cmd_read_tree, RUN_SETUP },
-		{ "reflog", cmd_reflog, RUN_SETUP },
-		{ "repo-config", cmd_config },
-		{ "rerere", cmd_rerere, RUN_SETUP },
-		{ "reset", cmd_reset, RUN_SETUP },
-		{ "rev-list", cmd_rev_list, RUN_SETUP },
-		{ "rev-parse", cmd_rev_parse },
-		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
-		{ "rm", cmd_rm, RUN_SETUP },
-		{ "send-pack", cmd_send_pack, RUN_SETUP },
-		{ "shortlog", cmd_shortlog, RUN_SETUP | USE_PAGER },
-		{ "show-branch", cmd_show_branch, RUN_SETUP },
-		{ "show", cmd_show, RUN_SETUP | USE_PAGER },
-		{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
-		{ "stripspace", cmd_stripspace },
-		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
-		{ "tag", cmd_tag, RUN_SETUP },
-		{ "tar-tree", cmd_tar_tree },
-		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
-		{ "update-index", cmd_update_index, RUN_SETUP },
-		{ "update-ref", cmd_update_ref, RUN_SETUP },
-		{ "upload-archive", cmd_upload_archive },
-		{ "verify-tag", cmd_verify_tag, RUN_SETUP },
-		{ "version", cmd_version },
-		{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
-		{ "write-tree", cmd_write_tree, RUN_SETUP },
-		{ "verify-pack", cmd_verify_pack },
-		{ "show-ref", cmd_show_ref, RUN_SETUP },
-		{ "pack-refs", cmd_pack_refs, RUN_SETUP },
-	};
-	int i;
-
-	/* Turn "git cmd --help" into "git help cmd" */
-	if (argc > 1 && !strcmp(argv[1], "--help")) {
-		argv[1] = argv[0];
-		argv[0] = cmd = "help";
-	}
-
-	for (i = 0; i < ARRAY_SIZE(commands); i++) {
-		struct cmd_struct *p = commands+i;
-		if (strcmp(p->cmd, cmd))
-			continue;
-		exit(run_command(p, argc, argv));
-	}
-}
-
 int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] ? argv[0] : "git-help";
@@ -425,7 +259,7 @@ int main(int argc, const char **argv)
 	if (!prefixcmp(cmd, "git-")) {
 		cmd += 4;
 		argv[0] = cmd;
-		handle_internal_command(argc, argv);
+		execv_git_builtin(argv);
 		die("cannot handle %s internally", cmd);
 	}
 
@@ -453,10 +287,6 @@ int main(int argc, const char **argv)
 	setup_path(cmd_path);
 
 	while (1) {
-		/* See if it's an internal command */
-		handle_internal_command(argc, argv);
-
-		/* .. then try the external ones */
 		execv_git_cmd(argv);
 
 		/* It could be an alias -- this works around the insanity
-- 
1.5.3.6.2112.ge2263

Re: [PATCH 3/3] Introduce execv_git_builtin() and use it

[Johannes Schindelin]

Hi,

On Sun, 2 Dec 2007, Johannes Schindelin wrote:

> +		{ "fmt", cmd_fmt },

Ah, well.  This slipped in by mistake.  Will resend in a few minutes.

Ciao,
Dscho

[REPLACEMENT PATCH 3/3] Introduce execv_git_builtin() and use it

[Johannes Schindelin]

You can call execv_git_builtin() to execute a builtin command.  This 
function is a reborn handle_internal_command() from git.c, and has the 
semantics of execv_git_cmd(), i.e. it exits with the exit code of the 
builtin if there is a matching builtin, but it avoids the real execvp() 
call.

This function calls release_all_objects() and discard_cache() to start 
from a clean slate (this, along with the calculation of argc, is the only 
difference from a straight code move).

The test suite passes, which at least does not contradict the hypothesis 
that this is good enough.  However, it might be necessary to de-initialize 
more global variables.

The function execv_git_cmd() and git.c's main() function were changed to 
take advantage of execv_git_builtin().

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Sun, 2 Dec 2007, Johannes Schindelin wrote:

	> Hi,
	> 
	> On Sun, 2 Dec 2007, Johannes Schindelin wrote:
	> 
	> > +		{ "fmt", cmd_fmt },
	> 
	> Ah, well.  This slipped in by mistake.  Will resend in a few 
	> minutes.

	Here we go.

 exec_cmd.c |  176 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 exec_cmd.h |    1 +
 git.c      |  170 +---------------------------------------------------------
 3 files changed, 178 insertions(+), 169 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 2d0a758..745b951 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -1,6 +1,8 @@
 #include "cache.h"
 #include "exec_cmd.h"
 #include "quote.h"
+#include "builtin.h"
+#include "object.h"
 #define MAX_ARGS	32
 
 extern char **environ;
@@ -63,11 +65,185 @@ void setup_path(const char *cmd_path)
 	strbuf_release(&new_path);
 }
 
+const char git_usage_string[] =
+	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
+
+const char git_version_string[] = GIT_VERSION;
+
+#define RUN_SETUP	(1<<0)
+#define USE_PAGER	(1<<1)
+/*
+ * require working tree to be present -- anything uses this needs
+ * RUN_SETUP for reading from the configuration file.
+ */
+#define NEED_WORK_TREE	(1<<2)
+
+struct cmd_struct {
+	const char *cmd;
+	int (*fn)(int, const char **, const char *);
+	int option;
+};
+
+static int run_command(struct cmd_struct *p, int argc, const char **argv)
+{
+	int status;
+	struct stat st;
+	const char *prefix;
+
+	prefix = NULL;
+	if (p->option & RUN_SETUP)
+		prefix = setup_git_directory();
+	if (p->option & USE_PAGER)
+		setup_pager();
+	if (p->option & NEED_WORK_TREE)
+		setup_work_tree();
+
+	trace_argv_printf(argv, argc, "trace: built-in: git");
+
+	status = p->fn(argc, argv, prefix);
+	if (status)
+		return status & 0xff;
+
+	/* Somebody closed stdout? */
+	if (fstat(fileno(stdout), &st))
+		return 0;
+	/* Ignore write errors for pipes and sockets.. */
+	if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode))
+		return 0;
+
+	/* Check for ENOSPC and EIO errors.. */
+	if (fflush(stdout))
+		die("write failure on standard output: %s", strerror(errno));
+	if (ferror(stdout))
+		die("unknown write failure on standard output");
+	if (fclose(stdout))
+		die("close failed on standard output: %s", strerror(errno));
+	return 0;
+}
+
+int execv_git_builtin(const char **argv)
+{
+	const char *cmd = argv[0];
+	static struct cmd_struct commands[] = {
+		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
+		{ "annotate", cmd_annotate, RUN_SETUP },
+		{ "apply", cmd_apply },
+		{ "archive", cmd_archive },
+		{ "blame", cmd_blame, RUN_SETUP },
+		{ "branch", cmd_branch, RUN_SETUP },
+		{ "bundle", cmd_bundle },
+		{ "cat-file", cmd_cat_file, RUN_SETUP },
+		{ "checkout-index", cmd_checkout_index,
+			RUN_SETUP | NEED_WORK_TREE},
+		{ "check-ref-format", cmd_check_ref_format },
+		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
+		{ "cherry", cmd_cherry, RUN_SETUP },
+		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
+		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
+		{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
+		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
+		{ "config", cmd_config },
+		{ "count-objects", cmd_count_objects, RUN_SETUP },
+		{ "describe", cmd_describe, RUN_SETUP },
+		{ "diff", cmd_diff },
+		{ "diff-files", cmd_diff_files },
+		{ "diff-index", cmd_diff_index, RUN_SETUP },
+		{ "diff-tree", cmd_diff_tree, RUN_SETUP },
+		{ "fetch", cmd_fetch, RUN_SETUP },
+		{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
+		{ "fetch--tool", cmd_fetch__tool, RUN_SETUP },
+		{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
+		{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
+		{ "format-patch", cmd_format_patch, RUN_SETUP },
+		{ "fsck", cmd_fsck, RUN_SETUP },
+		{ "fsck-objects", cmd_fsck, RUN_SETUP },
+		{ "gc", cmd_gc, RUN_SETUP },
+		{ "get-tar-commit-id", cmd_get_tar_commit_id },
+		{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },
+		{ "help", cmd_help },
+#ifndef NO_CURL
+		{ "http-fetch", cmd_http_fetch, RUN_SETUP },
+#endif
+		{ "init", cmd_init_db },
+		{ "init-db", cmd_init_db },
+		{ "log", cmd_log, RUN_SETUP | USE_PAGER },
+		{ "ls-files", cmd_ls_files, RUN_SETUP },
+		{ "ls-tree", cmd_ls_tree, RUN_SETUP },
+		{ "ls-remote", cmd_ls_remote },
+		{ "mailinfo", cmd_mailinfo },
+		{ "mailsplit", cmd_mailsplit },
+		{ "merge-base", cmd_merge_base, RUN_SETUP },
+		{ "merge-file", cmd_merge_file },
+		{ "merge-ours", cmd_merge_ours, RUN_SETUP },
+		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
+		{ "name-rev", cmd_name_rev, RUN_SETUP },
+		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
+		{ "peek-remote", cmd_ls_remote },
+		{ "pickaxe", cmd_blame, RUN_SETUP },
+		{ "prune", cmd_prune, RUN_SETUP },
+		{ "prune-packed", cmd_prune_packed, RUN_SETUP },
+		{ "push", cmd_push, RUN_SETUP },
+		{ "read-tree", cmd_read_tree, RUN_SETUP },
+		{ "reflog", cmd_reflog, RUN_SETUP },
+		{ "repo-config", cmd_config },
+		{ "rerere", cmd_rerere, RUN_SETUP },
+		{ "reset", cmd_reset, RUN_SETUP },
+		{ "rev-list", cmd_rev_list, RUN_SETUP },
+		{ "rev-parse", cmd_rev_parse },
+		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
+		{ "rm", cmd_rm, RUN_SETUP },
+		{ "send-pack", cmd_send_pack, RUN_SETUP },
+		{ "shortlog", cmd_shortlog, RUN_SETUP | USE_PAGER },
+		{ "show-branch", cmd_show_branch, RUN_SETUP },
+		{ "show", cmd_show, RUN_SETUP | USE_PAGER },
+		{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
+		{ "stripspace", cmd_stripspace },
+		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
+		{ "tag", cmd_tag, RUN_SETUP },
+		{ "tar-tree", cmd_tar_tree },
+		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
+		{ "update-index", cmd_update_index, RUN_SETUP },
+		{ "update-ref", cmd_update_ref, RUN_SETUP },
+		{ "upload-archive", cmd_upload_archive },
+		{ "verify-tag", cmd_verify_tag, RUN_SETUP },
+		{ "version", cmd_version },
+		{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
+		{ "write-tree", cmd_write_tree, RUN_SETUP },
+		{ "verify-pack", cmd_verify_pack },
+		{ "show-ref", cmd_show_ref, RUN_SETUP },
+		{ "pack-refs", cmd_pack_refs, RUN_SETUP },
+	};
+	int i;
+
+	/* Turn "git cmd --help" into "git help cmd" */
+	if (argv[0] && argv[1] && !strcmp(argv[1], "--help")) {
+		argv[1] = argv[0];
+		argv[0] = cmd = "help";
+	}
+
+	for (i = 0; i < ARRAY_SIZE(commands); i++) {
+		int argc;
+		struct cmd_struct *p = commands+i;
+		if (strcmp(p->cmd, cmd))
+			continue;
+		release_all_objects();
+		discard_cache();
+		for (argc = 0; argv[argc]; argc++)
+			; /* do nothing */
+		exit(run_command(p, argc, argv));
+	}
+	return -1;
+}
+
 int execv_git_cmd(const char **argv)
 {
 	struct strbuf cmd;
 	const char *tmp;
 
+	/* Try builtin first... */
+	execv_git_builtin(argv);
+
+	/* ... and then external commands */
 	strbuf_init(&cmd, 0);
 	strbuf_addf(&cmd, "git-%s", argv[0]);
 
diff --git a/exec_cmd.h b/exec_cmd.h
index a892355..bb15425 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -4,6 +4,7 @@
 extern void git_set_argv_exec_path(const char *exec_path);
 extern const char* git_exec_path(void);
 extern void setup_path(const char *);
+extern int execv_git_builtin(const char **argv); /* NULL terminated */
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);
 
diff --git a/git.c b/git.c
index 95296aa..1523c4a 100644
--- a/git.c
+++ b/git.c
@@ -3,9 +3,6 @@
 #include "cache.h"
 #include "quote.h"
 
-const char git_usage_string[] =
-	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
-
 static int handle_options(const char*** argv, int* argc, int* envchanged)
 {
 	int handled = 0;
@@ -231,167 +228,6 @@ static int handle_alias(int *argcp, const char ***argv)
 	return ret;
 }
 
-const char git_version_string[] = GIT_VERSION;
-
-#define RUN_SETUP	(1<<0)
-#define USE_PAGER	(1<<1)
-/*
- * require working tree to be present -- anything uses this needs
- * RUN_SETUP for reading from the configuration file.
- */
-#define NEED_WORK_TREE	(1<<2)
-
-struct cmd_struct {
-	const char *cmd;
-	int (*fn)(int, const char **, const char *);
-	int option;
-};
-
-static int run_command(struct cmd_struct *p, int argc, const char **argv)
-{
-	int status;
-	struct stat st;
-	const char *prefix;
-
-	prefix = NULL;
-	if (p->option & RUN_SETUP)
-		prefix = setup_git_directory();
-	if (p->option & USE_PAGER)
-		setup_pager();
-	if (p->option & NEED_WORK_TREE)
-		setup_work_tree();
-
-	trace_argv_printf(argv, argc, "trace: built-in: git");
-
-	status = p->fn(argc, argv, prefix);
-	if (status)
-		return status & 0xff;
-
-	/* Somebody closed stdout? */
-	if (fstat(fileno(stdout), &st))
-		return 0;
-	/* Ignore write errors for pipes and sockets.. */
-	if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode))
-		return 0;
-
-	/* Check for ENOSPC and EIO errors.. */
-	if (fflush(stdout))
-		die("write failure on standard output: %s", strerror(errno));
-	if (ferror(stdout))
-		die("unknown write failure on standard output");
-	if (fclose(stdout))
-		die("close failed on standard output: %s", strerror(errno));
-	return 0;
-}
-
-static void handle_internal_command(int argc, const char **argv)
-{
-	const char *cmd = argv[0];
-	static struct cmd_struct commands[] = {
-		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
-		{ "annotate", cmd_annotate, RUN_SETUP },
-		{ "apply", cmd_apply },
-		{ "archive", cmd_archive },
-		{ "blame", cmd_blame, RUN_SETUP },
-		{ "branch", cmd_branch, RUN_SETUP },
-		{ "bundle", cmd_bundle },
-		{ "cat-file", cmd_cat_file, RUN_SETUP },
-		{ "checkout-index", cmd_checkout_index,
-			RUN_SETUP | NEED_WORK_TREE},
-		{ "check-ref-format", cmd_check_ref_format },
-		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
-		{ "cherry", cmd_cherry, RUN_SETUP },
-		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
-		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
-		{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
-		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
-		{ "config", cmd_config },
-		{ "count-objects", cmd_count_objects, RUN_SETUP },
-		{ "describe", cmd_describe, RUN_SETUP },
-		{ "diff", cmd_diff },
-		{ "diff-files", cmd_diff_files },
-		{ "diff-index", cmd_diff_index, RUN_SETUP },
-		{ "diff-tree", cmd_diff_tree, RUN_SETUP },
-		{ "fetch", cmd_fetch, RUN_SETUP },
-		{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
-		{ "fetch--tool", cmd_fetch__tool, RUN_SETUP },
-		{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
-		{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
-		{ "format-patch", cmd_format_patch, RUN_SETUP },
-		{ "fsck", cmd_fsck, RUN_SETUP },
-		{ "fsck-objects", cmd_fsck, RUN_SETUP },
-		{ "gc", cmd_gc, RUN_SETUP },
-		{ "get-tar-commit-id", cmd_get_tar_commit_id },
-		{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },
-		{ "help", cmd_help },
-#ifndef NO_CURL
-		{ "http-fetch", cmd_http_fetch, RUN_SETUP },
-#endif
-		{ "init", cmd_init_db },
-		{ "init-db", cmd_init_db },
-		{ "log", cmd_log, RUN_SETUP | USE_PAGER },
-		{ "ls-files", cmd_ls_files, RUN_SETUP },
-		{ "ls-tree", cmd_ls_tree, RUN_SETUP },
-		{ "ls-remote", cmd_ls_remote },
-		{ "mailinfo", cmd_mailinfo },
-		{ "mailsplit", cmd_mailsplit },
-		{ "merge-base", cmd_merge_base, RUN_SETUP },
-		{ "merge-file", cmd_merge_file },
-		{ "merge-ours", cmd_merge_ours, RUN_SETUP },
-		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
-		{ "name-rev", cmd_name_rev, RUN_SETUP },
-		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
-		{ "peek-remote", cmd_ls_remote },
-		{ "pickaxe", cmd_blame, RUN_SETUP },
-		{ "prune", cmd_prune, RUN_SETUP },
-		{ "prune-packed", cmd_prune_packed, RUN_SETUP },
-		{ "push", cmd_push, RUN_SETUP },
-		{ "read-tree", cmd_read_tree, RUN_SETUP },
-		{ "reflog", cmd_reflog, RUN_SETUP },
-		{ "repo-config", cmd_config },
-		{ "rerere", cmd_rerere, RUN_SETUP },
-		{ "reset", cmd_reset, RUN_SETUP },
-		{ "rev-list", cmd_rev_list, RUN_SETUP },
-		{ "rev-parse", cmd_rev_parse },
-		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
-		{ "rm", cmd_rm, RUN_SETUP },
-		{ "send-pack", cmd_send_pack, RUN_SETUP },
-		{ "shortlog", cmd_shortlog, RUN_SETUP | USE_PAGER },
-		{ "show-branch", cmd_show_branch, RUN_SETUP },
-		{ "show", cmd_show, RUN_SETUP | USE_PAGER },
-		{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
-		{ "stripspace", cmd_stripspace },
-		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
-		{ "tag", cmd_tag, RUN_SETUP },
-		{ "tar-tree", cmd_tar_tree },
-		{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
-		{ "update-index", cmd_update_index, RUN_SETUP },
-		{ "update-ref", cmd_update_ref, RUN_SETUP },
-		{ "upload-archive", cmd_upload_archive },
-		{ "verify-tag", cmd_verify_tag, RUN_SETUP },
-		{ "version", cmd_version },
-		{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
-		{ "write-tree", cmd_write_tree, RUN_SETUP },
-		{ "verify-pack", cmd_verify_pack },
-		{ "show-ref", cmd_show_ref, RUN_SETUP },
-		{ "pack-refs", cmd_pack_refs, RUN_SETUP },
-	};
-	int i;
-
-	/* Turn "git cmd --help" into "git help cmd" */
-	if (argc > 1 && !strcmp(argv[1], "--help")) {
-		argv[1] = argv[0];
-		argv[0] = cmd = "help";
-	}
-
-	for (i = 0; i < ARRAY_SIZE(commands); i++) {
-		struct cmd_struct *p = commands+i;
-		if (strcmp(p->cmd, cmd))
-			continue;
-		exit(run_command(p, argc, argv));
-	}
-}
-
 int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] ? argv[0] : "git-help";
@@ -423,7 +259,7 @@ int main(int argc, const char **argv)
 	if (!prefixcmp(cmd, "git-")) {
 		cmd += 4;
 		argv[0] = cmd;
-		handle_internal_command(argc, argv);
+		execv_git_builtin(argv);
 		die("cannot handle %s internally", cmd);
 	}
 
@@ -451,10 +287,6 @@ int main(int argc, const char **argv)
 	setup_path(cmd_path);
 
 	while (1) {
-		/* See if it's an internal command */
-		handle_internal_command(argc, argv);
-
-		/* .. then try the external ones */
 		execv_git_cmd(argv);
 
 		/* It could be an alias -- this works around the insanity
-- 
1.5.3.6.2112.ge2263

Re: [PATCH 0/3] Call builtin functions directly, was Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Okay, I bit the apple and tried to move the builtins into the library, and 
> rename handle_internal_command into execv_git_builtin(), moving it into 
> exec-cmd.c.
>
> Big mistake.

I really feel this should not go in.  Anything called exec _should_
assure the callers that the new command will start from a clean slate,
and the way to give that assurance is by actually doing exec(), not
introducing "clean-up" functions for random things we can think of (like
cached objects) and risking of forgetting some others.  I do not think
the complexity is worth it.

The first step we have decided to take is to move git-foo form out of
users' PATH.  This would reduce the cluttered PATH problem, and it means
not all of external commands have to become built-ins on a single flag
day.  I also think it has always been a nice touch that we allowed users
to drop their own custom git-foo script to their path and call "git foo"
as if it is part of the official git suite, so spawning commands in
git-foo form needs to be supported via GIT_EXEC_PATH even if everything
eventually becomes built-in.

So I would prefer doing something like this instead for v1.5.5 (see
the top of updated release notes for 1.5.4 for deprecation notice).

 * execv_git_cmd() function will exec "git" with the given subcommand
   and its arguments;

 * The command dispatcher of git potty itself will first try the
   built-ins, and then try externals in dash form (which cannot be done
   with execv_git_cmd() anymore), and then aliases.

 * Just to be nice, we allow git-shell to treat "git foo arg" as if
   "git-foo arg" was given, but it continues to use execv_git_cmd(), and
   starts from a clean slate.

---
 exec_cmd.c |   31 ++++++++++++-------------------
 git.c      |   32 +++++++++++++++++++++++++++++++-
 shell.c    |   26 +++++++++++++++-----------
 3 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 2d0a758..10b2908 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -65,32 +65,25 @@ void setup_path(const char *cmd_path)
 
 int execv_git_cmd(const char **argv)
 {
-	struct strbuf cmd;
-	const char *tmp;
-
-	strbuf_init(&cmd, 0);
-	strbuf_addf(&cmd, "git-%s", argv[0]);
+	int argc;
+	const char **nargv;
 
-	/*
-	 * argv[0] must be the git command, but the argv array
-	 * belongs to the caller, and may be reused in
-	 * subsequent loop iterations. Save argv[0] and
-	 * restore it on error.
-	 */
-	tmp = argv[0];
-	argv[0] = cmd.buf;
+	for (argc = 0; argv[argc]; argc++)
+		; /* just counting */
+	nargv = xmalloc(sizeof(*nargv) * (argc + 2));
 
-	trace_argv_printf(argv, -1, "trace: exec:");
+	nargv[0] = "git";
+	for (argc = 0; argv[argc]; argc++)
+		nargv[argc + 1] = argv[argc];
+	nargv[argc + 1] = NULL;
+	trace_argv_printf(nargv, -1, "trace: exec:");
 
 	/* execvp() can only ever return if it fails */
-	execvp(cmd.buf, (char **)argv);
+	execvp("git", (char **)nargv);
 
 	trace_printf("trace: exec failed: %s\n", strerror(errno));
 
-	argv[0] = tmp;
-
-	strbuf_release(&cmd);
-
+	free(nargv);
 	return -1;
 }
 
diff --git a/git.c b/git.c
index 01bbbc7..d690426 100644
--- a/git.c
+++ b/git.c
@@ -382,6 +382,36 @@ static void handle_internal_command(int argc, const char **argv)
 	}
 }
 
+static void execv_dashed_external(const char **argv)
+{
+	struct strbuf cmd;
+	const char *tmp;
+
+	strbuf_init(&cmd, 0);
+	strbuf_addf(&cmd, "git-%s", argv[0]);
+
+	/*
+	 * argv[0] must be the git command, but the argv array
+	 * belongs to the caller, and may be reused in
+	 * subsequent loop iterations. Save argv[0] and
+	 * restore it on error.
+	 */
+	tmp = argv[0];
+	argv[0] = cmd.buf;
+
+	trace_argv_printf(argv, -1, "trace: exec:");
+
+	/* execvp() can only ever return if it fails */
+	execvp(cmd.buf, (char **)argv);
+
+	trace_printf("trace: exec failed: %s\n", strerror(errno));
+
+	argv[0] = tmp;
+
+	strbuf_release(&cmd);
+}
+
+
 int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] ? argv[0] : "git-help";
@@ -445,7 +475,7 @@ int main(int argc, const char **argv)
 		handle_internal_command(argc, argv);
 
 		/* .. then try the external ones */
-		execv_git_cmd(argv);
+		execv_dashed_external(argv);
 
 		/* It could be an alias -- this works around the insanity
 		 * of overriding "git log" with "git show" by having
diff --git a/shell.c b/shell.c
index 9826109..729797c 100644
--- a/shell.c
+++ b/shell.c
@@ -19,17 +19,13 @@ static int do_generic_cmd(const char *me, char *arg)
 	return execv_git_cmd(my_argv);
 }
 
-static int do_cvs_cmd(const char *me, char *arg)
+static int do_cvs_cmd(void)
 {
 	const char *cvsserver_argv[3] = {
 		"cvsserver", "server", NULL
 	};
 
-	if (!arg || strcmp(arg, "server"))
-		die("git-cvsserver only handles server: %s", arg);
-
 	setup_path(NULL);
-
 	return execv_git_cmd(cvsserver_argv);
 }
 
@@ -40,7 +36,6 @@ static struct commands {
 } cmd_list[] = {
 	{ "git-receive-pack", do_generic_cmd },
 	{ "git-upload-pack", do_generic_cmd },
-	{ "cvs", do_cvs_cmd },
 	{ NULL },
 };
 
@@ -49,15 +44,24 @@ int main(int argc, char **argv)
 	char *prog;
 	struct commands *cmd;
 
+	/*
+	 * Special hack to pretend to be a CVS server
+	 */
 	if (argc == 2 && !strcmp(argv[1], "cvs server"))
-		argv--;
-	/* We want to see "-c cmd args", and nothing else */
-	else if (argc != 3 || strcmp(argv[1], "-c"))
+		exit(do_cvs_cmd());
+
+	/*
+	 * We do not accept anything but "-c" followed by "cmd arg",
+	 * where "cmd" is a very limited subset of git commands.
+	 */
+	if (argc != 3 || strcmp(argv[1], "-c"))
 		die("What do you think I am? A shell?");
 
 	prog = argv[2];
-	argv += 2;
-	argc -= 2;
+	if (!strncmp(prog, "git", 3) && isspace(prog[3]))
+		/* Accept "git foo" as if the caller said "git-foo". */
+		prog[3] = '-';
+
 	for (cmd = cmd_list ; cmd->name ; cmd++) {
 		int len = strlen(cmd->name);
 		char *arg;

Re: [PATCH] Move all dashed form git commands to libexecdir

[Nguyen Thai Ngoc Duy]

On Dec 2, 2007 2:32 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
>
> > On Nov 30, 2007 10:50 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >
> >> Well, different people will want different viewers *anyway* (ie some will
> >> prefer qgit etc), so how about making "git view" be something that
> >> literally acts as a built-in alias that just defaults to running gitk (if
> >> for no other reason than the fact that gitk is the one that ships with
> >> git, and simply has most users).
> >
> > We already have "git show", now we gonna get "git view", git trainers
> > may have hard time explaining this one shows you a particular object
> > while the other one shows you history. How about "git lshistory" (from
> > clearcase land) or git show --history?
>
> Heh, we have "bisect visualize".  How about "git visualize"?
>

"git visualize"++


-- 
Duy

Re: [PATCH 0/3] Call builtin functions directly, was Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote

[Johannes Schindelin]

Hi,

On Sat, 1 Dec 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Okay, I bit the apple and tried to move the builtins into the library, 
> > and rename handle_internal_command into execv_git_builtin(), moving it 
> > into exec-cmd.c.
> >
> > Big mistake.
> 
> I really feel this should not go in.

Hmm.

My rationale was not only avoiding an exec() (which I would find worth it 
on its own), but because this would be a non-verbal step towards 
libification.

Ciao,
Dscho

Re: [PATCH] Move all dashed form git commands to libexecdir

[Wincent Colaiuta]

El 1/12/2007, a las 0:10, Johannes Schindelin escribió:

> To me, it is mighty annoying anybody brings up that "144 commands"
> argument Linus was referring to, and if there is _any_ way to shut up
> those bikeshedders, I am all for it.

This is not a bikeshed argument and it is not an "idiotic  
complaint" (to use Linus' phrase). It is a legitimate concern and a  
*real* UI problem.

You and Linus don't care that there are 140+ Git commands and I  
imagine that you know exactly what each of them does.

I don't really care either because although I don't know what every  
single command does, I know what the 20 or 30 commands I personally  
need for my own workflow do.

The problem is for *newcomers* to Git who sit down for the first time  
and ask themselves, "Now, how do I...?". This is not an idiotic  
complaint but a legitimate concern about a real UI problem.

Honestly, Johannes, do you think the following is a good UI?

$ git
Display all 148 possibilities? (y or n)
git*                     git-cvsimport*           git-local- 
fetch*         git-peek-remote*         git-show-index*
git-add*                 git-cvsserver*           git- 
log*                 git-prune*               git-show-ref*
git-add--interactive*    git-daemon*              git-lost- 
found*          git-prune-packed*        git-ssh-fetch*
git-am*                  git-describe*            git-ls- 
files*            git-pull*                git-ssh-pull*
git-annotate*            git-diff*                git-ls- 
remote*           git-push*                git-ssh-push*
git-apply*               git-diff-files*          git-ls- 
tree*             git-quiltimport*         git-ssh-upload*
git-applymbox            git-diff-index*          git- 
mailinfo*            git-read-tree*           git-stash*
git-applypatch           git-diff-tree*           git- 
mailsplit*           git-rebase*              git-status*
git-archimport*          git-fast-import*         git- 
merge*               git-rebase--interactive* git-stripspace*
git-archive*             git-fetch*               git-merge- 
base*          git-receive-pack*        git-submodule*
git-bisect*              git-fetch--tool*         git-merge- 
file*          git-reflog*              git-svn*
git-blame*               git-fetch-pack*          git-merge- 
index*         git-relink*              git-svnimport
git-branch*              git-filter-branch*       git-merge- 
octopus*       git-remote*              git-symbolic-ref*
git-bundle*              git-fmt-merge-msg*       git-merge-one- 
file*      git-repack*              git-tag*
git-cat-file*            git-for-each-ref*        git-merge- 
ours*          git-repo-config*         git-tar-tree*
git-check-attr*          git-format-patch*        git-merge- 
recursive*     git-request-pull*        git-unpack-file*
git-check-ref-format*    git-fsck*                git-merge- 
resolve*       git-rerere*              git-unpack-objects*
git-checkout*            git-fsck-objects*        git-merge- 
stupid*        git-reset*               git-update-index*
git-checkout-index*      git-gc*                  git-merge- 
subtree*       git-rev-list*            git-update-ref*
git-cherry*              git-get-tar-commit-id*   git-merge- 
tree*          git-rev-parse*           git-update-server-info*
git-cherry-pick*         git-grep*                git- 
mergetool*           git-revert*              git-upload-archive*
git-citool               git-gui/                 git- 
mktag*               git-rm*                  git-upload-pack*
git-clean*               git-hash-object*         git- 
mktree*              git-runstatus*           git-var*
git-clone*               git-http-fetch*          git- 
mv*                  git-send-email*          git-verify-pack*
git-commit*              git-http-push*           git-name- 
rev*            git-send-pack*           git-verify-tag*
git-commit-tree*         git-imap-send*           git-pack- 
objects*        git-sh-setup*            git-whatchanged*
git-config*              git-index-pack*          git-pack- 
redundant*      git-shell*               git-write-tree*
git-convert-objects      git-init*                git-pack- 
refs*           git-shortlog*            gitk
git-count-objects*       git-init-db*             git-parse- 
remote*        git-show*
git-cvsexportcommit*     git-instaweb*            git-patch- 
id*            git-show-branch*

Would you argue that this screenshot shows a sane UI?

<http://wincent.com/tmp/git-ui.png>

Note: I am not complaining about the *number* of commands; I myself  
find them highly useful for scripting. I am saying that the  
*visibility* of those commands is the problem. That's why I support  
the efforts to move most of this stuff out of the default PATH. We're  
not talking about getting rid of it, just about putting it somewhere  
more appropriate in order to correct what is basically a *hideous*  
user interface for the beginner.

Cheers,
Wincent

Re: [PATCH] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

Hi,

On Sun, 2 Dec 2007, Wincent Colaiuta wrote:

> El 1/12/2007, a las 0:10, Johannes Schindelin escribi?:
> 
> > To me, it is mighty annoying anybody brings up that "144 commands" 
> > argument Linus was referring to, and if there is _any_ way to shut up 
> > those bikeshedders, I am all for it.
> 
> This is not a bikeshed argument and it is not an "idiotic complaint" (to 
> use Linus' phrase). It is a legitimate concern and a *real* UI problem.
> 
> You and Linus don't care that there are 140+ Git commands and I imagine 
> that you know exactly what each of them does.

Okay, how many executables are there in your /usr/bin/?  Here there are 
2973.

Guess what.  I am not intimidated by that number.

Ciao,
Dscho

Re: [PATCH] Move all dashed form git commands to libexecdir

[Pascal Obry]

Johannes Schindelin a écrit :
> Okay, how many executables are there in your /usr/bin/?  Here there are 
> 2973.
> Guess what.  I am not intimidated by that number.

Good, and look in /usr/bin, all those 2973 binary are all disconnected.

Here we are speaking about a tool as a whole : Git.

And I agree that hiding some of them will probably help new comers. We
can also argue that a new comers should read some documentation :)

After all I'm not sure what's the right move !

At least let me say something constructive :) I'm a new comer to Git.
I've read many documentations before grabbing the system and I've not
been impressed by the number of binaries in /usr/bin... Because I've
almost never looked there. Most of the time I'm using "git <tab>" and
the bash completion feature is just right for me.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

Re: [PATCH] Move all dashed form git commands to libexecdir

[Johannes Schindelin]

Hi,

On Sun, 2 Dec 2007, Pascal Obry wrote:

> Johannes Schindelin a ?crit :
> > Okay, how many executables are there in your /usr/bin/?  Here there 
> > are 2973. Guess what.  I am not intimidated by that number.
> 
> Good, and look in /usr/bin, all those 2973 binary are all disconnected.
> 
> Here we are speaking about a tool as a whole : Git.

No, we are speaking about different commands, such as commit, fetch, push, 
etc.

I refuse to believe that you cannot see the equivalence.

> I've read many documentations before grabbing the system and I've not
> been impressed by the number of binaries in /usr/bin... Because I've
> almost never looked there.

Exactly my point.

> Most of the time I'm using "git <tab>" and the bash completion feature 
> is just right for me.

Bash completion is really something fine.

But even without, I do not see a problem: many cvs users used only three 
out of 32 commands (most CVS users I personally know/knew only called add, 
commit and update).  You could even see all 32 commands when calling the 
clunky command line "cvs --help-commands".  I am convinced we're already 
more user-friendly than that.

Ciao,
Dscho