Move all dashed form git commands to libexecdir
RFC PATCH 1/1 Move all dashed form git commands to libexecdir
---
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(-)
config.mak.in +1 -1
@@ -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/
Signature · 1 line
1.5.3.GIT
PATCH 1/1 Move all dashed form git commands to libexecdir
---
Both configure and make-only ways should work now
Makefile | 2 +- config.mak.in | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Makefile +1 -1
@@ -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)
config.mak.in +1 -1
@@ -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/
Signature · 1 line
1.5.3.GIT
Hi,
Quoted 1 line · Nguyễn Thái Ngoc Duy
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
Quoted 7 lines · Johannes Schindelin
Hi,
Quoted 1 line · Nguyễn Thái Ngoc Duy
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.
Signature · 1 line
Jan 'Bulb' Hudec <[email protected]>
Quoted 5 lines · Jan Hudec
Quoted 4 lines · Johannes Schindelin
Hi,
On Tue, 27 Nov 2007, Nguyễn Thái Ngoc Duy wrote:
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
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.
Quoted 12 lines · Junio C Hamano
Quoted 10 lines · Jan Hudec
Quoted 5 lines · Johannes Schindelin
Quoted 1 line · Nguyễn Thái Ngoc Duy
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.
Quoted 8 lines
[...]
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?
Signature · 1 line
Jan 'Bulb' Hudec <[email protected]>
Quoted 6 lines · Junio C Hamano
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.
Signature · 1 line
Duy
Quoted 8 lines · Nguyen Thai Ngoc Duy
Quoted 6 lines · Junio C Hamano
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.
Quoted 4 lines
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.
Junio C Hamano wrote:
[...]
Quoted 18 lines
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.
Quoted 7 lines
- 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.
Quoted 24 lines · A Large Angry SCM
Junio C Hamano wrote:
[...]Quoted 22 lines
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.
Quoted 6 lines
Quoted 3 lines
- 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
Makefileto 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.
Hi,
Quoted 24 lines · Junio C Hamano
Quoted 8 lines · Nguyen Thai Ngoc Duy
Quoted 6 lines · Junio C Hamano
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.Quoted 4 lines
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
Quoted 7 lines
(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
Quoted 2 lines · Johannes Schindelin
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?
Hi,
Quoted 7 lines · Junio C Hamano
Quoted 3 lines · Johannes Schindelin
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
Quoted 36 lines · Junio C Hamano
Quoted 8 lines · Nguyen Thai Ngoc Duy
Quoted 6 lines · Junio C Hamano
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.Quoted 4 lines
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.
Quoted 7 lines
- 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.
Signature · 1 line
Duy
Quoted 41 lines · Nguyen Thai Ngoc Duy
Quoted 40 lines · Junio C Hamano
Quoted 8 lines · Nguyen Thai Ngoc Duy
Quoted 6 lines · Junio C Hamano
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.Quoted 4 lines
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.
Quoted 6 lines
Quoted 3 lines
- 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
Quoted 8 lines · Nicolas Pitre
Quoted 2 lines · Nguyen Thai Ngoc Duy
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?
Quoted 9 lines · Junio C Hamano
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 (77cb17e9introduced 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.cTry 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
On 29. nov. 2007, at 23.36, Junio C Hamano wrote:
[...]
Quoted 7 lines
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 (
77cb17e9introduced 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 1/1 transport.c: call dash-less form of receive-pack and upload-pack on remote
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.
---
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(-)
transport.c +2 -2
@@ -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; }
Signature · 1 line
1.5.3.6.2088.g8c260
Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote
Quoted 8 lines · 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 <[email protected]>---
Quoted 7 lines
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] transport.c: call dash-less form of receive-pack and upload-pack on remote
Hi,
Quoted 12 lines · Junio C Hamano
Quoted 10 lines · 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 <[email protected]>---
On Fri, 30 Nov 2007, Eyvind Bernhardsen wrote:
Quoted 8 lines
Quoted 5 lines
> - 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
Quoted 5 lines · Johannes Schindelin
Quoted 2 lines
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(-)
exec_cmd.c +13 -21
@@ -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] transport.c: call dash-less form of receive-pack and upload-pack on remote
Hi,
Quoted 10 lines · Junio C Hamano
Quoted 5 lines · Johannes Schindelin
Quoted 2 lines
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 thestrbuf_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(-)
exec_cmd.c +12 -18
@@ -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 ; }
t/t0020-crlf.sh +1 -1
@@ -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
Hi,
Quoted 1 line · Johannes Schindelin
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
Quoted 7 lines · Johannes Schindelin
Quoted 1 line · Johannes Schindelin
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 ;-).
cover Call builtin functions directly, was Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote
In reply to Johannes SchindelinHi,
Quoted 7 lines · Johannes Schindelin
Quoted 1 line · Johannes Schindelin
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
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
Quoted 5 lines · Johannes Schindelin
Okay, I bit the apple and tried to move the builtins into the library, and rename
handle_internal_commandintoexecv_git_builtin(), moving it intoexec-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(-)
exec_cmd.c +12 -19
@@ -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 ; }
git.c +31 -1
@@ -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
shell.c +15 -11
@@ -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 0/3] Call builtin functions directly, was Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote
Hi,
Quoted 9 lines · Junio C Hamano
Quoted 5 lines · Johannes Schindelin
Okay, I bit the apple and tried to move the builtins into the library, and rename
handle_internal_commandintoexecv_git_builtin(), moving it intoexec-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
PATCH 1/3 Introduce release_all_objects()
In reply to Johannes SchindelinThe 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().
---
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(-)
alloc.c +30 -1
@@ -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 (); +}
blob.c +3 -0
@@ -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 ;
blob.h +1 -0
@@ -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);
cache.h +6 -0
@@ -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 */
commit.c +8 -0
@@ -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;
commit.h +2 -0
@@ -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);
object.c +24 -0
@@ -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) {
object.h +2 -0
@@ -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);
tag.c +8 -0
@@ -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;
tag.h +2 -0
@@ -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);
tree.c +6 -0
@@ -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.
tree.h +1 -0
@@ -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);
Signature · 1 line
1.5.3.6.2112.ge2263
PATCH 2/3 Include the objects needed for the builtin functions into libgit.a
In reply to Johannes SchindelinFor 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.
--- Makefile | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)
Makefile +9 -5
@@ -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
Signature · 1 line
1.5.3.6.2112.ge2263
PATCH 3/3 Introduce execv_git_builtin() and use it
In reply to Johannes SchindelinYou 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().
--- exec_cmd.c | 178 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ exec_cmd.h | 1 + git.c | 172 +--------------------------------------------------------- 3 files changed, 180 insertions(+), 171 deletions(-)
exec_cmd.c +178 -0
@@ -1,6 +1,8 @@#include "cache.h" #include "exec_cmd.h" #include "quote.h" +#include "builtin.h" +#include "object.h" #define MAX_ARGS 32extern 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 ]);
exec_cmd.h +1 -0
@@ -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, ...);
git.c +1 -171
196 lines not on this page · raw message
Signature · 1 line
1.5.3.6.2112.ge2263
Re: [PATCH 3/3] Introduce execv_git_builtin() and use it
Hi,
exec_cmd.c void setup_path(const char *cmd_path) · line 156
Quoted 1 line · Johannes Schindelin
exec_cmd.c +1
-0+ { "fmt", cmd_fmt },
Ah, well. This slipped in by mistake. Will resend in a few minutes.
Ciao,
Dscho
PATCH 3/3 Introduce execv_git_builtin() and use it
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().
---
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(-)
exec_cmd.c +176 -0
@@ -1,6 +1,8 @@#include "cache.h" #include "exec_cmd.h" #include "quote.h" +#include "builtin.h" +#include "object.h" #define MAX_ARGS 32extern 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 ]);
exec_cmd.h +1 -0
@@ -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, ...);
git.c +1 -169
194 lines not on this page · raw message
Signature · 1 line
1.5.3.6.2112.ge2263
Quoted 17 lines · Eyvind Bernhardsen
On 29. nov. 2007, at 23.36, Junio C Hamano wrote:
[...]
Quoted 9 lines
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 (77cb17e9introduced 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:
Quoted 6 lines
- 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.
Signature · 1 line
Duy
Hi,
Quoted 23 lines · Nguyen Thai Ngoc Duy
Quoted 21 lines · Eyvind Bernhardsen
On 29. nov. 2007, at 23.36, Junio C Hamano wrote:
[...]
Quoted 9 lines
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 (77cb17e9introduced 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.
Quoted 6 lines
Quoted 2 lines
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
Quoted 3 lines · Junio C Hamano
- 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
Quoted 17 lines · Jeff King
Quoted 3 lines · Junio C Hamano
- 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.
Quoted 4 lines
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.
Signature · 1 line
Duy
Quoted 7 lines · Nguyen Thai Ngoc Duy
Quoted 4 lines
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
Hi,
Quoted 10 lines · Jeff King
Quoted 7 lines · Nguyen Thai Ngoc Duy
Quoted 4 lines
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.
Quoted 6 lines
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
Quoted 8 lines · Johannes Schindelin
Quoted 2 lines
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
Quoted 4 lines · Jeff King
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
Quoted 8 lines · Linus Torvalds
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.
Quoted 3 lines · Junio C Hamano
- 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
Quoted 11 lines · Jeff King
Quoted 3 lines · Junio C Hamano
- 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.
Quoted 8 lines
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.
Quoted 5 lines · Junio C Hamano
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.
Quoted 7 lines
And I think the following from Linus makes sense.
Quoted 5 lines
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.
Quoted 6 lines
Quoted 3 lines
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:
Quoted 10 lines · Jeff King
Quoted 4 lines
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
Quoted 5 lines · Jeff King
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
Quoted 8 lines · Linus Torvalds
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.
Quoted 4 lines
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
Quoted 8 lines · Steffen Prohaska
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
Quoted 8 lines · Jeff King
Quoted 6 lines · Steffen Prohaska
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?
Quoted 7 lines
Quoted 2 lines
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
Quoted 5 lines · Santi Béjar
Quoted 1 line
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
Quoted 10 lines · Jeff King
Quoted 5 lines · Santi Béjar
Quoted 1 line
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
Quoted 5 lines · Linus Torvalds
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(-)
git.c +9 -0
@@ -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 ) {
Signature · 1 line
1.5.3.6.2064.g2e22f-dirty
Hi,
Quoted 32 lines · Jeff King
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(-)git.c +7
-0@@ -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;
git.c static int handle_alias(int *argcp, const char ***argv) · line 176
Quoted 8 lines
git.c +2
-0@@ -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
Quoted 12 lines · Johannes Schindelin
git.c static int handle_alias(int *argcp, const char ***argv) · line 176
Quoted 9 lines
git.c +2
-0@@ -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
Hi,
Quoted 15 lines · Jeff King
Quoted 12 lines · Johannes Schindelin
git.c static int handle_alias(int *argcp, const char ***argv) · line 176
Quoted 9 lines
git.c +2
-0@@ -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.
Quoted 2 lines
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
Quoted 11 lines · Johannes Schindelin
Quoted 11 lines
Quoted 10 lines
git.c static int handle_alias(int *argcp, const char ***argv) · line 176
Quoted 7 lines
git.c +2
-0alias_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.
Quoted 4 lines
Quoted 2 lines
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
Hi,
Quoted 18 lines · Jeff King
Quoted 11 lines · Johannes Schindelin
Quoted 11 lines
Quoted 10 lines
git.c static int handle_alias(int *argcp, const char ***argv) · line 176
Quoted 7 lines
git.c +2
-0alias_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.Quoted 4 lines
Quoted 2 lines
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...
Quoted 8 lines
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
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 8 lines · Nguyen Thai Ngoc Duy
Quoted 5 lines · Linus Torvalds
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).
Quoted 4 lines
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"?
Quoted 6 lines · Junio C Hamano
Quoted 4 lines
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
Quoted 10 lines · Junio C Hamano
Quoted 8 lines · Nguyen Thai Ngoc Duy
Quoted 5 lines · Linus Torvalds
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).
Quoted 7 lines
Quoted 4 lines
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"++
Signature · 1 line
Duy
Quoted 21 lines · Linus Torvalds
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)
Quoted 7 lines
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
Quoted 8 lines · Jeff King
Quoted 5 lines · Junio C Hamano
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.
Quoted 4 lines
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
Quoted 10 lines · Nicolas Pitre
Quoted 4 lines
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
Quoted 4 lines · Jeff King
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
Quoted 22 lines · Jeff King
Quoted 8 lines · Nicolas Pitre
Quoted 4 lines
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.
Quoted 4 lines
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.
Signature · 3 lines
Andreas Ericsson [email protected] OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231
Quoted 6 lines · Andreas Ericsson
Quoted 4 lines
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
Quoted 9 lines · Jeff King
On Fri, Nov 30, 2007 at 08:18:16AM +0100, Andreas Ericsson wrote:
...Quoted 2 lines
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?
Quoted 5 lines · Junio C Hamano
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).
Quoted 1 line
Newbie: Stupid inconsistency. Who suggested that?
Jeff [runs git-blame]: It must have been Junio! :)
-Peff
Quoted 8 lines · Jeff King
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).
Quoted 1 line
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.
Quoted 7 lines · Junio C Hamano
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
Hi,
Quoted 9 lines · Jeff King
Quoted 5 lines · Junio C Hamano
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
Quoted 3 lines · Johannes Schindelin
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
Hi,
Quoted 11 lines · Wincent Colaiuta
El 1/12/2007, a las 0:10, Johannes Schindelin escribi?:
Quoted 3 lines
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
Quoted 3 lines · Johannes Schindelin
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.
Signature · 8 lines
--|------------------------------------------------------ --| 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
Hi,
Quoted 7 lines · Pascal Obry
Johannes Schindelin a ?crit :
Quoted 2 lines
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.
Quoted 3 lines
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.
Quoted 2 lines
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
Quoted 11 lines · Jeff King
Quoted 3 lines · Junio C Hamano
- 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
Quoted 8 lines · Nicolas Pitre
Quoted 5 lines
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
Quoted 8 lines · Jeff King
Quoted 6 lines · Nicolas Pitre
Quoted 5 lines
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).
Quoted 6 lines
Quoted 2 lines
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
Quoted 6 lines · Nicolas Pitre
Quoted 3 lines
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).
Quoted 2 lines
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
Quoted 20 lines · Jeff King
Quoted 6 lines · Nicolas Pitre
Quoted 3 lines
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).
Quoted 2 lines
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).
Quoted 8 lines
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
Quoted 5 lines · Nicolas Pitre
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
Quoted 7 lines · Jeff King
Quoted 3 lines · Nicolas Pitre
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).
Junio C Hamano wrote:
[...]
Quoted 28 lines
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.
Quoted 7 lines
- 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.
Hi,
Quoted 34 lines · A Large Angry SCM
Junio C Hamano wrote:
[...]Quoted 32 lines
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.
Quoted 7 lines
Quoted 3 lines
- 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
Makefileto install the "dashed" versions of the commands for those that want it; and be able to set gitexecdir=$(binder) without editing theMakefile.
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
Quoted 3 lines · Johannes Schindelin
Hi,
On Thu, 29 Nov 2007, A Large Angry SCM wrote:
[...]
Quoted 5 lines
Quoted 3 lines
Again, there needs to remain support in the
Makefileto install the "dashed" versions of the commands for those that want it; and be able to set gitexecdir=$(binder) without editing theMakefile.Umm. Why? If there is no compelling reason (and you named none, but there were quite a few reasons against it), why should the
Makefilesupport the dash form after 1.5.6?
Not all shells support customized command completion (check the archives); "bang" history (check the archives); etc.
Quoted 3 lines · A Large Angry SCM
Again, there needs to remain support in the
Makefileto install the "dashed" versions of the commands for those that want it; and be able to set gitexecdir=$(binder) without editing theMakefile.
Well, if you want a "non standard" installation, maybe you just can edit a line or two in the Makefile?
Nicolas
Quoted 7 lines · Nicolas Pitre
Quoted 3 lines · A Large Angry SCM
Again, there needs to remain support in the
Makefileto install the "dashed" versions of the commands for those that want it; and be able to set gitexecdir=$(binder) without editing theMakefile.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?
Quoted 8 lines · A Large Angry SCM
Quoted 7 lines · Nicolas Pitre
Quoted 4 lines · A Large Angry SCM
Again, there needs to remain support in the
Makefileto install the "dashed"
versions of the commands for those that want it; and be able to set gitexecdir=$(binder) without editing theMakefile.
Quoted 4 lines
Quoted 2 lines
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"
Makefiletarget?
As long as it is not the default I don't object.
Nicolas
Quoted 7 lines · Junio C Hamano
- 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
Signature · 1 line
Duy
Quoted 8 lines · Nguyen Thai Ngoc Duy
Quoted 6 lines · Junio C Hamano
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.
Quoted 4 lines
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?
Signature · 3 lines
Jakub Narebski Warsaw, Poland ShadeHawk on #git
Hi,
config.mak.in
Quoted 1 line · Nguyễn Thái Ngọc Duy
config.mak.in +0
-0
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
Quoted 6 lines · Nguyễn Thái Ngọc Duy
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