git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path()
@ 2016-11-26 14:03 Mike Galbraith
  2016-11-26 16:56 ` Dennis Kaarsemaker
  2016-11-26 17:09 ` Jeff King
  0 siblings, 2 replies; 5+ messages in thread
From: Mike Galbraith @ 2016-11-26 14:03 UTC (permalink / raw)
  To: Jeff King; +Cc: git

Greetings,

git-daemon went broke on me post v2.9.3 due to binaries being installed
in /usr/lib/git, which is not in PATH.  Reverting 650c449250d7 fixes it
up, as does ln -s /usr/lib/git/git-daemon /usr/bin/git-daemon 'course,
but thought I should report it, since it used to work without that.

Process 18804 attached
restart_syscall(<... resuming interrupted call ...>) = 1
accept(4, {sa_family=AF_INET6, sin6_port=htons(44400), inet_pton(AF_INET6, "::1", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, [28]) = 5
dup(5)                                  = 6
pipe([7, 8])                            = 0
clone(Process 18830 attached
child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7f0e6061c9d0) = 18830
[pid 18830] set_robust_list(0x7f0e6061c9e0, 24 <unfinished ...>
[pid 18804] close(8 <unfinished ...>
[pid 18830] <... set_robust_list resumed> ) = 0
[pid 18804] <... close resumed> )       = 0
[pid 18804] read(7,  <unfinished ...>
[pid 18830] close(7)                    = 0
[pid 18830] fcntl(8, F_GETFD)           = 0
[pid 18830] fcntl(8, F_SETFD, FD_CLOEXEC) = 0
[pid 18830] dup2(5, 0)                  = 0
[pid 18830] close(5)                    = 0
[pid 18830] dup2(6, 1)                  = 1
[pid 18830] close(6)                    = 0
[pid 18830] execve("/usr/local/sbin/git-daemon", ["git-daemon", "--serve", "--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", "--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], ["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", "PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", "LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", "DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44400"]) = -1 ENOENT (No such file or directory)
[pid 18830] execve("/usr/local/bin/git-daemon", ["git-daemon", "--serve", "--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", "--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], ["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", "PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", "LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", "DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44400"]) = -1 ENOENT (No such file or directory)
[pid 18830] execve("/usr/sbin/git-daemon", ["git-daemon", "--serve", "--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", "--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], ["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", "PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", "LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", "DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44400"]) = -1 ENOENT (No such file or directory)
[pid 18830] execve("/usr/bin/git-daemon", ["git-daemon", "--serve", "--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", "--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], ["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", "PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", "LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", "DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44400"]) = -1 ENOENT (No such file or directory)
[pid 18830] execve("/sbin/git-daemon", ["git-daemon", "--serve", "--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", "--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], ["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", "PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", "LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", "DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44400"]) = -1 ENOENT (No such file or directory)
[pid 18830] execve("/bin/git-daemon", ["git-daemon", "--serve", "--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", "--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], ["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", "PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", "LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", "DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44400"]) = -1 ENOENT (No such file or directory)
[pid 18830] fstat(2, {st_dev=makedev(0, 6), st_ino=1029, st_mode=S_IFCHR|0666, st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=0, st_rdev=makedev(1, 3), st_atime=2016/11/26-00:42:47, st_mtime=2016/11/26-00:42:47, st_ctime=2016/11/26-00:42:47}) = 0
[pid 18830] ioctl(2, TCGETS, 0x7ffe6dbd09b0) = -1 ENOTTY (Inappropriate ioctl for device)
[pid 18830] mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f0e60632000
[pid 18830] write(2, "error: cannot run git-daemon: No"..., 56) = 56
[pid 18830] write(8, "\0", 1)           = 1
[pid 18804] <... read resumed> "\0", 1) = 1
[pid 18830] exit_group(127)             = ?
[pid 18804] wait4(18830,  <unfinished ...>
[pid 18830] +++ exited with 127 +++
<... wait4 resumed> [{WIFEXITED(s) && WEXITSTATUS(s) == 127}], 0, NULL) = 18830
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=18830, si_uid=1002, si_status=127, si_utime=0, si_stime=0} ---
rt_sigaction(SIGCHLD, {0x404e10, [CHLD], SA_RESTORER|SA_RESTART, 0x7f0e5f6a7140}, {0x404e10, [CHLD], SA_RESTORER|SA_RESTART, 0x7f0e5f6a7140}, 8) = 0
rt_sigreturn({mask=[]})                 = 18830
close(7)                                = 0
close(5)                                = 0
close(6)                                = 0
open("/etc/localtime", O_RDONLY|O_CLOEXEC) = 5
fstat(5, {st_dev=makedev(8, 5), st_ino=6031966, st_mode=S_IFREG|0644, st_nlink=2, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=8, st_size=2335, st_atime=2016/11/26-01:00:02, st_mtime=2016/11/08-18:09:53, st_ctime=2016/11/09-21:15:24}) = 0
fstat(5, {st_dev=makedev(8, 5), st_ino=6031966, st_mode=S_IFREG|0644, st_nlink=2, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=8, st_size=2335, st_atime=2016/11/26-01:00:02, st_mtime=2016/11/08-18:09:53, st_ctime=2016/11/09-21:15:24}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f0e60632000
read(5, "TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\t\0\0\0\t\0\0\0\0"..., 4096) = 2335
lseek(5, -1476, SEEK_CUR)               = 859
read(5, "TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\t\0\0\0\t\0\0\0\0"..., 4096) = 1476
close(5)                                = 0
munmap(0x7f0e60632000, 4096)            = 0
socket(PF_LOCAL, SOCK_DGRAM|SOCK_CLOEXEC, 0) = 5
connect(5, {sa_family=AF_LOCAL, sun_path="/dev/log"}, 110) = 0
sendto(5, "<27>Nov 26 14:25:55 git-daemon[1"..., 53, MSG_NOSIGNAL, NULL, 0) = 53
poll([{fd=3, events=POLLIN}, {fd=4, events=POLLIN}], 2, 4294967295

revert 650c449250d7

Process 18934 attached
restart_syscall(<... resuming interrupted call ...>) = 1
accept(4, {sa_family=AF_INET6, sin6_port=htons(44404), inet_pton(AF_INET6, "::1", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, [28]) = 5
dup(5)                                  = 6
pipe([7, 8])                            = 0
clone(Process 19010 attached
child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7f62fcb5e9d0) = 19010
[pid 18934] close(8 <unfinished ...>
[pid 19010] set_robust_list(0x7f62fcb5e9e0, 24) = 0
[pid 18934] <... close resumed> )       = 0
[pid 18934] read(7,  <unfinished ...>
[pid 19010] close(7)                    = 0
[pid 19010] fcntl(8, F_GETFD)           = 0
[pid 19010] fcntl(8, F_SETFD, FD_CLOEXEC) = 0
[pid 19010] dup2(5, 0)                  = 0
[pid 19010] close(5)                    = 0
[pid 19010] dup2(6, 1)                  = 1
[pid 19010] close(6)                    = 0
[pid 19010] execve("/usr/lib/git/git-daemon", ["/usr/lib/git/git-daemon", "--serve", "--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", "--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], ["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", "PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", "LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", "DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44404"] <unfinished ...>
[pid 18934] <... read resumed> "", 1)   = 0
[pid 19010] <... execve resumed> )      = 0
.... works


ln -s /usr/lib/git/git-daemon /usr/bin/git-daemon


Process 19862 attached
restart_syscall(<... resuming interrupted call ...>) = 1
accept(4, {sa_family=AF_INET6, sin6_port=htons(44412), inet_pton(AF_INET6, "::1", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, [28]) = 5
dup(5)                                  = 6
pipe([7, 8])                            = 0
clone(Process 19915 attached
child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7fc97962e9d0) = 19915
[pid 19862] close(8)                    = 0
[pid 19915] set_robust_list(0x7fc97962e9e0, 24 <unfinished ...>
[pid 19862] read(7,  <unfinished ...>
[pid 19915] <... set_robust_list resumed> ) = 0
[pid 19915] close(7)                    = 0
[pid 19915] fcntl(8, F_GETFD)           = 0
[pid 19915] fcntl(8, F_SETFD, FD_CLOEXEC) = 0
[pid 19915] dup2(5, 0)                  = 0
[pid 19915] close(5)                    = 0
[pid 19915] dup2(6, 1)                  = 1
[pid 19915] close(6)                    = 0
[pid 19915] execve("/usr/local/sbin/git-daemon", ["git-daemon", "--serve", "--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", "--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], ["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", "PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", "LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", "DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44412"]) = -1 ENOENT (No such file or directory)
[pid 19915] execve("/usr/local/bin/git-daemon", ["git-daemon", "--serve", "--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", "--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], ["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", "PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", "LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", "DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44412"]) = -1 ENOENT (No such file or directory)
[pid 19915] execve("/usr/sbin/git-daemon", ["git-daemon", "--serve", "--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", "--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], ["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", "PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", "LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", "DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44412"]) = -1 ENOENT (No such file or directory)
[pid 19915] execve("/usr/bin/git-daemon", ["git-daemon", "--serve", "--syslog", "--detach", "--reuseaddr", "--user=git", "--group=daemon", "--pid-file=/var/run/git-daemon.p"..., "--export-all", "--user-path"], ["CONSOLE=/dev/console", "LC_ALL=POSIX", "REDIRECT=/dev/tty7", "COLUMNS=320", "PATH=/usr/local/sbin:/usr/local/"..., "PWD=/", "LINES=90", "SHLVL=1", "LC_CTYPE=en_US.UTF-8", "_=/sbin/startproc", "PREVLEVEL=", "RUNLEVEL=5", "DAEMON=/usr/lib/git/git-daemon", "REMOTE_ADDR=[::1]", "REMOTE_PORT=44412"] <unfinished ...>
[pid 19862] <... read resumed> "", 1)   = 0
[pid 19915] <... execve resumed> )      = 0
.... works



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path()
  2016-11-26 14:03 git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path() Mike Galbraith
@ 2016-11-26 16:56 ` Dennis Kaarsemaker
  2016-11-26 17:09 ` Jeff King
  1 sibling, 0 replies; 5+ messages in thread
From: Dennis Kaarsemaker @ 2016-11-26 16:56 UTC (permalink / raw)
  To: Mike Galbraith, Jeff King; +Cc: git

Hi Mike,

On Sat, 2016-11-26 at 15:03 +0100, Mike Galbraith wrote:
> Greetings,
> 
> git-daemon went broke on me post v2.9.3 due to binaries being installed
> in /usr/lib/git, which is not in PATH.  Reverting 650c449250d7 fixes it
> up, as does ln -s /usr/lib/git/git-daemon /usr/bin/git-daemon 'course,
> but thought I should report it, since it used to work without that.

I don't know how you usually install git, but git-daemon is not
supposed to be in $PATH, the correct way to invoke the git daemon is
'git daemon' not 'git-daemon'

Having all subcommands of git as separate binaries in your $PATH is an
ancient git.git practice that stopped being used/supported quite a
while ago.

I don't know why this patch broke that obsolete practice, but hopefully
this can help you forward.

D.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path()
  2016-11-26 14:03 git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path() Mike Galbraith
  2016-11-26 16:56 ` Dennis Kaarsemaker
@ 2016-11-26 17:09 ` Jeff King
  2016-11-26 17:51   ` Mike Galbraith
  1 sibling, 1 reply; 5+ messages in thread
From: Jeff King @ 2016-11-26 17:09 UTC (permalink / raw)
  To: Mike Galbraith; +Cc: git

On Sat, Nov 26, 2016 at 03:03:48PM +0100, Mike Galbraith wrote:

> git-daemon went broke on me post v2.9.3 due to binaries being installed
> in /usr/lib/git, which is not in PATH.  Reverting 650c449250d7 fixes it
> up, as does ln -s /usr/lib/git/git-daemon /usr/bin/git-daemon 'course,
> but thought I should report it, since it used to work without that.

Generally /usr/lib/git _should_ be in your PATH, as it is added by the
git wrapper when you run "git daemon".

The only behavior difference caused by 650c449250d7 is that we replace
argv[0] with the output of git_extract_argv0_path(argv[0]), which will
give the basename, not a full path. So presumably you are running:

  /usr/lib/git/git-daemon

directly. I'm not sure that's even supposed to work these days, and it
was not just a happy accident that it did.

On the other hand, I am sympathetic that something used to work and now
doesn't. It probably wouldn't be that hard to work around it.

The reason for the behavior change is that one of the cmd_main()
functions was relying on the basename side-effect of the
extract_argv0_path function, so 650c449250d7 just feeds the munged
argv[0] to all of the programs. The cleanest fix would probably be
something like:

diff --git a/common-main.c b/common-main.c
index 44a29e8b1..c654f9555 100644
--- a/common-main.c
+++ b/common-main.c
@@ -33,7 +33,7 @@ int main(int argc, const char **argv)
 
 	git_setup_gettext();
 
-	argv[0] = git_extract_argv0_path(argv[0]);
+	git_extract_argv0_path(argv[0]);
 
 	restore_sigpipe_to_default();
 
diff --git a/git.c b/git.c
index bd66a2e0a..05986680c 100644
--- a/git.c
+++ b/git.c
@@ -730,6 +730,11 @@ int cmd_main(int argc, const char **argv)
 	cmd = argv[0];
 	if (!cmd)
 		cmd = "git-help";
+	else {
+		const char *base = find_last_dir_sep(cmd);
+		if (base)
+			cmd = base + 1;
+	}
 
 	trace_command_performance(argv);
 	trace_stdin();

-Peff

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path()
  2016-11-26 17:09 ` Jeff King
@ 2016-11-26 17:51   ` Mike Galbraith
  2016-11-27  4:31     ` [PATCH] common-main: stop munging argv[0] path Jeff King
  0 siblings, 1 reply; 5+ messages in thread
From: Mike Galbraith @ 2016-11-26 17:51 UTC (permalink / raw)
  To: Jeff King; +Cc: git

On Sat, 2016-11-26 at 12:09 -0500, Jeff King wrote:
> On Sat, Nov 26, 2016 at 03:03:48PM +0100, Mike Galbraith wrote:
> 
> > git-daemon went broke on me post v2.9.3 due to binaries being installed
> > in /usr/lib/git, which is not in PATH.  Reverting 650c449250d7 fixes it
> > up, as does ln -s /usr/lib/git/git-daemon /usr/bin/git-daemon 'course,
> > but thought I should report it, since it used to work without that.
> 
> Generally /usr/lib/git _should_ be in your PATH, as it is added by the
> git wrapper when you run "git daemon".
> 
> The only behavior difference caused by 650c449250d7 is that we replace
> argv[0] with the output of git_extract_argv0_path(argv[0]), which will
> give the basename, not a full path. So presumably you are running:
> 
>   /usr/lib/git/git-daemon
> 
> directly. I'm not sure that's even supposed to work these days, and it
> was not just a happy accident that it did.

Ah.  I'm using suse's rpm glue to package my modified source, and its
startup script still calls it directly, so wants some modernization.

> On the other hand, I am sympathetic that something used to work and now
> doesn't. It probably wouldn't be that hard to work around it.
> 
> The reason for the behavior change is that one of the cmd_main()
> functions was relying on the basename side-effect of the
> extract_argv0_path function, so 650c449250d7 just feeds the munged
> argv[0] to all of the programs. The cleanest fix would probably be
> something like:

That did fix it up, thanks.  I'll try twiddling the script instead.

> diff --git a/common-main.c b/common-main.c
> index 44a29e8b1..c654f9555 100644
> --- a/common-main.c
> +++ b/common-main.c
> @@ -33,7 +33,7 @@ int main(int argc, const char **argv)
>  
>  > 	> git_setup_gettext();
>  
> -> 	> argv[0] = git_extract_argv0_path(argv[0]);
> +> 	> git_extract_argv0_path(argv[0]);
>  
>  > 	> restore_sigpipe_to_default();
>  
> diff --git a/git.c b/git.c
> index bd66a2e0a..05986680c 100644
> --- a/git.c
> +++ b/git.c
> @@ -730,6 +730,11 @@ int cmd_main(int argc, const char **argv)
>  > 	> cmd = argv[0];
>  > 	> if (!cmd)
>  > 	> 	> cmd = "git-help";
> +> 	> else {
> +> 	> 	> const char *base = find_last_dir_sep(cmd);
> +> 	> 	> if (base)
> +> 	> 	> 	> cmd = base + 1;
> +> 	> }
>  
>  > 	> trace_command_performance(argv);
>  > 	> trace_stdin();
> 
> -Peff

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] common-main: stop munging argv[0] path
  2016-11-26 17:51   ` Mike Galbraith
@ 2016-11-27  4:31     ` Jeff King
  0 siblings, 0 replies; 5+ messages in thread
From: Jeff King @ 2016-11-27  4:31 UTC (permalink / raw)
  To: Mike Galbraith; +Cc: git

On Sat, Nov 26, 2016 at 06:51:11PM +0100, Mike Galbraith wrote:

> > On the other hand, I am sympathetic that something used to work and now
> > doesn't. It probably wouldn't be that hard to work around it.
> > 
> > The reason for the behavior change is that one of the cmd_main()
> > functions was relying on the basename side-effect of the
> > extract_argv0_path function, so 650c449250d7 just feeds the munged
> > argv[0] to all of the programs. The cleanest fix would probably be
> > something like:
> 
> That did fix it up, thanks.  I'll try twiddling the script instead.

Thanks for confirming. Here it is, with a commit message and a little
bit of polish.

-- >8 --
Subject: [PATCH] common-main: stop munging argv[0] path

Since 650c44925 (common-main: call git_extract_argv0_path(),
2016-07-01), the argv[0] that is seen in cmd_main() of
individual programs is always the basename of the
executable, as common-main strips off the full path. This
can produce confusing results for git-daemon, which wants to
re-exec itself.

For instance, if the program was originally run as
"/usr/lib/git/git-daemon", it will try just re-execing
"git-daemon", which will find the first instance in $PATH.
If git's exec-path has not been prepended to $PATH, we may
find the git-daemon from a different version (or no
git-daemon at all).

Normally this isn't a problem. Git commands are run as "git
daemon", the git wrapper puts the exec-path at the front of
$PATH, and argv[0] is already "daemon" anyway. But running
git-daemon via its full exec-path, while not really a
recommended method, did work prior to 650c44925. Let's make
it work again.

The real goal of 650c44925 was not to munge argv[0], but to
reliably set the argv0_path global. The only reason it
munges at all is that one caller, the git.c wrapper,
piggy-backed on that computation to find the command
basename.  Instead, let's leave argv[0] untouched in
common-main, and have git.c do its own basename computation.

While we're at it, let's drop the return value from
git_extract_argv0_path(). It was only ever used in this one
callsite, and its dual purposes is what led to this
confusion in the first place.

Note that by changing the interface, the compiler can
confirm for us that there are no other callers storing the
return value. But the compiler can't tell us whether any of
the cmd_main() functions (besides git.c) were relying on the
basename munging. However, we can observe that prior to
650c44925, no other cmd_main() functions did that munging,
and no new cmd_main() functions have been introduced since
then. So we can't be regressing any of those cases.

Signed-off-by: Jeff King <peff@peff.net>
---
 common-main.c |  2 +-
 exec_cmd.c    | 10 +++-------
 exec_cmd.h    |  2 +-
 git.c         |  5 +++++
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/common-main.c b/common-main.c
index 44a29e8b1..c654f9555 100644
--- a/common-main.c
+++ b/common-main.c
@@ -33,7 +33,7 @@ int main(int argc, const char **argv)
 
 	git_setup_gettext();
 
-	argv[0] = git_extract_argv0_path(argv[0]);
+	git_extract_argv0_path(argv[0]);
 
 	restore_sigpipe_to_default();
 
diff --git a/exec_cmd.c b/exec_cmd.c
index 9d5703a15..19ac2146d 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -38,21 +38,17 @@ char *system_path(const char *path)
 	return strbuf_detach(&d, NULL);
 }
 
-const char *git_extract_argv0_path(const char *argv0)
+void git_extract_argv0_path(const char *argv0)
 {
 	const char *slash;
 
 	if (!argv0 || !*argv0)
-		return NULL;
+		return;
 
 	slash = find_last_dir_sep(argv0);
 
-	if (slash) {
+	if (slash)
 		argv0_path = xstrndup(argv0, slash - argv0);
-		return slash + 1;
-	}
-
-	return argv0;
 }
 
 void git_set_argv_exec_path(const char *exec_path)
diff --git a/exec_cmd.h b/exec_cmd.h
index 1f6b43378..ff0b48048 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -4,7 +4,7 @@
 struct argv_array;
 
 extern void git_set_argv_exec_path(const char *exec_path);
-extern const char *git_extract_argv0_path(const char *path);
+extern void git_extract_argv0_path(const char *path);
 extern const char *git_exec_path(void);
 extern void setup_path(void);
 extern const char **prepare_git_cmd(struct argv_array *out, const char **argv);
diff --git a/git.c b/git.c
index e8b2baf2d..dce529fcb 100644
--- a/git.c
+++ b/git.c
@@ -654,6 +654,11 @@ int cmd_main(int argc, const char **argv)
 	cmd = argv[0];
 	if (!cmd)
 		cmd = "git-help";
+	else {
+		const char *slash = find_last_dir_sep(cmd);
+		if (slash)
+			cmd = slash + 1;
+	}
 
 	trace_command_performance(argv);
 
-- 
2.11.0.rc3.313.g1055eca


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-11-27  4:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-26 14:03 git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path() Mike Galbraith
2016-11-26 16:56 ` Dennis Kaarsemaker
2016-11-26 17:09 ` Jeff King
2016-11-26 17:51   ` Mike Galbraith
2016-11-27  4:31     ` [PATCH] common-main: stop munging argv[0] path Jeff King

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).