* trustExitCode doesn't apply to vimdiff mergetool
From: Dun Peal @ 2016-11-27 2:44 UTC (permalink / raw)
To: Git ML
I'm using vimdiff as my mergetool, and have the following lines in ~/.gitconfig:
[merge]
tool = vimdiff
[mergetool "vimdiff"]
trustExitCode = true
My understanding from the docs is that this sets
mergetool.vimdiff.trustExitCode to true, thereby concluding that a
merge hasn't been successful if vimdiff's exit code is non-zero.
Unfortunately, when I exit Vim using `:cq` - which returns code 1 -
the merge is still presumed to have succeeded.
Is there a way to accomplish the desired effect, such that exiting
vimdiff with a non-zero code would prevent git from resolving the
conflict in the merged file?
^ permalink raw reply
* Re: git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path()
From: Mike Galbraith @ 2016-11-26 17:51 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161126170933.6tge6j5etuchqy33@sigill.intra.peff.net>
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
* Re: git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path()
From: Jeff King @ 2016-11-26 17:09 UTC (permalink / raw)
To: Mike Galbraith; +Cc: git
In-Reply-To: <1480169028.3830.24.camel@gmail.com>
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
* Re: git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path()
From: Dennis Kaarsemaker @ 2016-11-26 16:56 UTC (permalink / raw)
To: Mike Galbraith, Jeff King; +Cc: git
In-Reply-To: <1480169028.3830.24.camel@gmail.com>
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
* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Jeff King @ 2016-11-26 16:19 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Junio C Hamano, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1611261320050.117539@virtualbox>
On Sat, Nov 26, 2016 at 01:22:28PM +0100, Johannes Schindelin wrote:
> In other words, GIT_CONFIG_PARAMETERS is *explicitly scrubbed* from the
> environment when we run our tests (by the code block between the "before"
> and the "after" statements in the diff above).
Sorry if I wasn't clear. I meant to modify t7800 to run the tests twice,
once with the existing script and once with the builtin. I.e., to set
the variable after test-lib.sh has done its scrubbing, and then use a
loop or similar to go through the tests twice.
If you want to control it from outside the test script, you'd need
something like:
if test "$GIT_TEST_DIFFTOOL" = "builtin"
then
GIT_CONFIG_PARAMETERS=...
fi
-Peff
^ permalink raw reply
* git-daemon regression: 650c449250d7 common-main: call git_extract_argv0_path()
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
* RE: [char-misc-next] mei: request async autosuspend at the end of enumeration
From: Winkler, Tomas @ 2016-11-26 13:02 UTC (permalink / raw)
To: jnareb@gmail.com, Jeff King, Jiri Slaby,
Greg KH (gregkh@linuxfoundation.org), Ben Hutchings
Cc: Matthieu Moy, git@vger.kernel.org, Usyskin, Alexander,
linux-kernel@vger.kernel.org
In-Reply-To: <e11d28d3-c1b5-2c04-643f-0b3bd96cb4d3@gmail.com>
>
> W dniu 25.11.2016 o 04:14, Jeff King pisze:
> > On Thu, Nov 24, 2016 at 10:37:14PM +0000, Winkler, Tomas wrote:
> >
> >>>>> Cc: <stable@vger.kernel.org> # 4.4+
> >>>>
> >>>> Looks like git send-email is not able to parse this address
> >>>> correctly though this is suggested format by
> Documentation/stable_kernel_rules.txt.
> >>>> Create wrong address If git parsers is used : 'stable@vger.kernel.org#4.4+'
> [...]
>
> > The patch just brings parity to the Mail::Address behavior and git's
> > fallback parser, so that you don't end up with the broken
> > stable@vger.kernel.org#4.4+ address. Instead, that content goes into
> > the name part of the address.
> >
> > It sounds like you want the "# 4.4+" to be dropped entirely in the
> > rfc822 header. It looks like send-email used to do that, but stopped
> > in
> > b1c8a11c8 (send-email: allow multiple emails using --cc, --to and
> > --bcc, 2015-06-30).
> >
> > So perhaps there are further fixes required, but it's hard to know.
> > The input isn't a valid rfc822 header, so it's not entirely clear what
> > the output is supposed to be. I can buy either "drop it completely" or
> > "stick it in the name field of the cc header" as reasonable.
>
> Well, we could always convert it to email address comment, converting for
> example the following trailer:
>
> Cc: John Doe <john@example.com> # comment
>
> to the following address:
>
> John Doe <john@example.com> (comment)
>
> Just FYI. Though I'm not sure how well this would work...
>
Yep, it actually looks as right place to put this kind of info,
though I'm not on the receiving side.
I'm not sure if and how is this used by stable maintainers.
Thanks
Tomas
^ permalink raw reply
* Re: [PATCH v2 2/2] Avoid a segmentation fault with renaming merges
From: Johannes Schindelin @ 2016-11-26 12:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Markus Klein
In-Reply-To: <d1571a25e8f3860a2867b00994d4d6938aa602ec.1480164459.git.johannes.schindelin@gmx.de>
Hi,
On Sat, 26 Nov 2016, Johannes Schindelin wrote:
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 9041c2f149..609061f58a 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -235,6 +235,8 @@ static int add_cacheinfo(struct merge_options *o,
> struct cache_entry *nce;
>
> nce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);
> + if (!nce)
> + return err(o, _("addinfo: '%s' is not up-to-date"), path);
> if (nce != ce)
> ret = add_cache_entry(nce, options);
> }
BTW I was not quite sure why we need to refresh the cache entry here, and
1335d76e45 (merge: avoid "safer crlf" during recording of merge results,
2016-07-08) has a commit message for which I need some time to wrap my
head around.
Also, an error here may be overkill. Maybe we should simply change the "if
(nce != ce)" to an "if (nce && nce != ce)" here, as a locally-modified
file will give a nicer message later, anyway.
Dunno,
Dscho
^ permalink raw reply
* [PATCH v2 2/2] Avoid a segmentation fault with renaming merges
From: Johannes Schindelin @ 2016-11-26 12:48 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Markus Klein
In-Reply-To: <cover.1480164459.git.johannes.schindelin@gmx.de>
Under very particular circumstances, merge-recursive's `add_cacheinfo()`
function gets a `NULL` returned from `refresh_cache_entry()` without
expecting it, and subsequently passes it to `add_cache_entry()` which
consequently crashes.
Let's not crash.
This fixes https://github.com/git-for-windows/git/issues/952
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
merge-recursive.c | 2 ++
t/t3501-revert-cherry-pick.sh | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 9041c2f149..609061f58a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -235,6 +235,8 @@ static int add_cacheinfo(struct merge_options *o,
struct cache_entry *nce;
nce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);
+ if (!nce)
+ return err(o, _("addinfo: '%s' is not up-to-date"), path);
if (nce != ce)
ret = add_cache_entry(nce, options);
}
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index d7b4251234..4f2a263b63 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick "-" works with arguments' '
test_cmp expect actual
'
-test_expect_failure 'cherry-pick works with dirty renamed file' '
+test_expect_success 'cherry-pick works with dirty renamed file' '
test_commit to-rename &&
git checkout -b unrelated &&
test_commit unrelated &&
--
2.11.0.rc3.windows.1
^ permalink raw reply related
* [PATCH v2 1/2] cherry-pick: demonstrate a segmentation fault
From: Johannes Schindelin @ 2016-11-26 12:48 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Markus Klein
In-Reply-To: <cover.1480164459.git.johannes.schindelin@gmx.de>
In https://github.com/git-for-windows/git/issues/952, a complicated
scenario was described that leads to a segmentation fault in
cherry-pick.
It boils down to a certain code path involving a renamed file that is
dirty, for which `refresh_cache_entry()` returns `NULL`, and that
`NULL` not being handled properly.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t3501-revert-cherry-pick.sh | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 394f0005a1..d7b4251234 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -141,4 +141,16 @@ test_expect_success 'cherry-pick "-" works with arguments' '
test_cmp expect actual
'
+test_expect_failure 'cherry-pick works with dirty renamed file' '
+ test_commit to-rename &&
+ git checkout -b unrelated &&
+ test_commit unrelated &&
+ git checkout @{-1} &&
+ git mv to-rename.t renamed &&
+ test_tick &&
+ git commit -m renamed &&
+ echo modified >renamed &&
+ git cherry-pick refs/heads/unrelated
+'
+
test_done
--
2.11.0.rc3.windows.1
^ permalink raw reply related
* [PATCH v2 0/2] Fix segmentation fault with cherry-pick
From: Johannes Schindelin @ 2016-11-26 12:47 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Markus Klein
In-Reply-To: <cover.1480091758.git.johannes.schindelin@gmx.de>
The culprit is actually not cherry-pick, but a special code path that
expects refresh_cache_entry() not to return NULL. And the fix is to
teach it to handle NULL there.
This bug was brought to my attention by Markus Klein via
https://github.com/git-for-windows/git/issues/952.
Changes since v1:
- changed test title
- avoided ambiguous refname in test
Johannes Schindelin (2):
cherry-pick: demonstrate a segmentation fault
Avoid a segmentation fault with renaming merges
merge-recursive.c | 2 ++
t/t3501-revert-cherry-pick.sh | 12 ++++++++++++
2 files changed, 14 insertions(+)
base-commit: e2b2d6a172b76d44cb7b1ddb12ea5bfac9613a44
Published-As: https://github.com/dscho/git/releases/tag/cherry-pick-segfault-v2
Fetch-It-Via: git fetch https://github.com/dscho/git cherry-pick-segfault-v2
Interdiff vs v1:
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 8e21840f11..4f2a263b63 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick "-" works with arguments' '
test_cmp expect actual
'
-test_expect_success 'cherry-pick fails gracefully with dirty renamed file' '
+test_expect_success 'cherry-pick works with dirty renamed file' '
test_commit to-rename &&
git checkout -b unrelated &&
test_commit unrelated &&
@@ -150,7 +150,7 @@ test_expect_success 'cherry-pick fails gracefully with dirty renamed file' '
test_tick &&
git commit -m renamed &&
echo modified >renamed &&
- git cherry-pick unrelated
+ git cherry-pick refs/heads/unrelated
'
test_done
--
2.11.0.rc3.windows.1
^ permalink raw reply
* Re: [PATCH 1/2] cherry-pick: demonstrate a segmentation fault
From: Johannes Schindelin @ 2016-11-26 12:47 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Markus Klein
In-Reply-To: <89ffd6eaf4e1e121426c84f31dfc9c289f2a948b.1480091758.git.johannes.schindelin@gmx.de>
Hi,
On Fri, 25 Nov 2016, Johannes Schindelin wrote:
> +test_expect_failure 'cherry-pick fails gracefully with dirty renamed file' '
Woops. This title is wrong. It should say instead: 'cherry-pick succeeds
with unrelated renamed, dirty file'.
> + test_commit to-rename &&
> + git checkout -b unrelated &&
> + test_commit unrelated &&
> + git checkout @{-1} &&
> + git mv to-rename.t renamed &&
> + test_tick &&
> + git commit -m renamed &&
> + echo modified >renamed &&
> + git cherry-pick unrelated
And this actually warns about an ambiguous refname.
Will send out v2 in a moment.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Johannes Schindelin @ 2016-11-26 12:22 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <20161125174721.f35mzc276kdwakzm@sigill.intra.peff.net>
Hi Peff,
On Fri, 25 Nov 2016, Jeff King wrote:
> On Fri, Nov 25, 2016 at 06:41:23PM +0100, Johannes Schindelin wrote:
>
> > > Ah, I didn't realize that was a requirement. If this is going to be part
> > > of a release and real end-users are going to see it, that does make me
> > > think the config option is the better path (than the presence of some
> > > file), as it's our standard way of tweaking run-time behavior.
> >
> > So how do you easily switch back and forth between testing the old vs the
> > new difftool via the test suite?
>
> If it's for a specific tool, I'd consider teaching the test suite to run
> the whole script twice: once with the flag set and once without.
>
> That is sometimes more complicated, though, if the script creates many
> sub-repos. An environment variable might be more natural. If you already
> support flipping the default via config, you can probably do:
>
> GIT_CONFIG_PARAMETERS="'difftool.usebuiltin=true'"
> export GIT_CONFIG_PARAMETERS
Except that that does not work, of course. To figure out why, apply this
diff:
-- snip --
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 17f3008277..27159f65f3 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -10,6 +10,9 @@ Testing basic diff tool invocation
. ./test-lib.sh
+echo "config $(git config difftool.usebuiltin)." >&2
+exit 1
+
difftool_test_setup ()
{
test_config diff.tool test-tool &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9980a46133..0ddeded92b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -86,6 +86,7 @@ EDITOR=:
# /usr/xpg4/bin/sh and /bin/ksh to bail out. So keep the unsets
# deriving from the command substitution clustered with the other
# ones.
+echo "before $(git config difftool.usebuiltin)." >&2
unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
my @env = keys %ENV;
my $ok = join("|", qw(
@@ -104,6 +105,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e
'
my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
print join("\n", @vars);
')
+echo "after $(git config difftool.usebuiltin)." >&2
unset XDG_CONFIG_HOME
unset GITPERLLIB
GIT_AUTHOR_EMAIL=author@example.com
-- snap --
and then weep at this output:
GIT_CONFIG_PARAMETERS="'difftool.usebuiltin=true'"; export GIT_CONFIG_PARAMETERS; bash t7800-difftool.sh -i -v -x
before true.
after .
Initialized empty Git repository in
/home/virtualbox/git/git-for-windows/t/trash
directory.t7800-difftool/.git/
config .
FATAL: Unexpected exit with code 1
In other words, GIT_CONFIG_PARAMETERS is *explicitly scrubbed* from the
environment when we run our tests (by the code block between the "before"
and the "after" statements in the diff above).
Ciao,
Dscho
^ permalink raw reply related
* Re: What's cooking in git.git (Nov 2016, #05; Wed, 23)
From: Duy Nguyen @ 2016-11-26 9:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqk2btlr3x.fsf@gitster.mtv.corp.google.com>
On Thu, Nov 24, 2016 at 6:21 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * nd/rebase-forget (2016-10-28) 1 commit
> - rebase: add --forget to cleanup rebase, leave HEAD untouched
>
> "git rebase" learned "--forget" option, which allows a user to
> remove the metadata left by an earlier "git rebase" that was
> manually aborted without using "git rebase --abort".
>
> Waiting for a reroll.
The reroll was http://public-inbox.org/git/%3C20161112020041.2335-1-pclouds@gmail.com%3E/
--
Duy
^ permalink raw reply
* Re: [PATCH v3 2/2] difftool: implement the functionality in the builtin
From: Jakub Narębski @ 2016-11-25 21:24 UTC (permalink / raw)
To: Johannes Schindelin, git
Cc: Junio C Hamano, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <ac91e4818cfb5c5af6b5874662dbeb61cde1f69d.1480019834.git.johannes.schindelin@gmx.de>
W dniu 24.11.2016 o 21:55, Johannes Schindelin pisze:
> The current version of the builtin difftool does not, however, make full
> use of the internals but instead chooses to spawn a couple of Git
> processes, still, to make for an easier conversion. There remains a lot
> of room for improvement, left for a later date.
[...]
> Sadly, the speedup is more noticable on Linux than on Windows: a quick
> test shows that t7800-difftool.sh runs in (2.183s/0.052s/0.108s)
> (real/user/sys) in a Linux VM, down from (6.529s/3.112s/0.644s), while
> on Windows, it is (36.064s/2.730s/7.194s), down from
> (47.637s/2.407s/6.863s). The culprit is most likely the overhead
> incurred from *still* having to shell out to mergetool-lib.sh and
> difftool--helper.sh.
Does this mean that our shell-based testsuite is not well suited to be
benchmark suite for comparing performance on MS Windows?
Or does it mean that "builtin-difftool" spawning Git processes is the
problem?
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> builtin/difftool.c | 670 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 669 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/difftool.c b/builtin/difftool.c
> index 53870bb..3480920 100644
> --- a/builtin/difftool.c
> +++ b/builtin/difftool.c
> @@ -11,9 +11,608 @@
> *
> * Copyright (C) 2016 Johannes Schindelin
> */
> +#include "cache.h"
> #include "builtin.h"
> #include "run-command.h"
> #include "exec_cmd.h"
> +#include "parse-options.h"
> +#include "argv-array.h"
> +#include "strbuf.h"
> +#include "lockfile.h"
> +#include "dir.h"
> +
> +static char *diff_gui_tool;
> +static int trust_exit_code;
> +
> +static const char *const builtin_difftool_usage[] = {
> + N_("git difftool [<options>] [<commit> [<commit>]] [--] [<path>...]"),
> + NULL
> +};
> +
> +static int difftool_config(const char *var, const char *value, void *cb)
> +{
> + if (!strcmp(var, "diff.guitool")) {
Shouldn't you also read other configuration variables, like "diff.tool",
and it's mergetool fallbacks ("merge.guitool", "merge.tool")?
> + diff_gui_tool = xstrdup(value);
> + return 0;
> + }
> +
> + if (!strcmp(var, "difftool.trustexitcode")) {
> + trust_exit_code = git_config_bool(var, value);
> + return 0;
> + }
Why you do not need to check "difftool.prompt"? And "mergetool.*" fallbacks?
> +
> + return git_default_config(var, value, cb);
> +}
> +
> +static int print_tool_help(void)
> +{
> + const char *argv[] = { "mergetool", "--tool-help=diff", NULL };
> + return run_command_v_opt(argv, RUN_GIT_CMD);
This looks a bit strange to me, but I guess this is to avoid recursively
invoking ourself, and { "legacy-difftool", "--tool-help", NULL }; isn't
that much better.
> +}
> +
> +static int parse_index_info(char *p, int *mode1, int *mode2,
> + struct object_id *oid1, struct object_id *oid2,
> + char *status)
There are only two hard things in Computer Science:
cache invalidation and naming things.
-- Phil Karlton
Why did you name function that parses "diff --raw" output (aka "diff-tree"
output) parse_index_info() instead of parse_raw_diff() or parse_diff_tree()?
I went searching for `update-index --index-info` formats...
This is not that important, because this function is file-static; though
future developers would thank you for more descriptive naming.
ADDED: Disregard this, I see this function is about index (?) part of
raw diff, that is only a part of "git diff --raw -z" output. Though...
> +{
> + if (*p != ':')
> + return error("expected ':', got '%c'", *p);
> + *mode1 = (int)strtol(p + 1, &p, 8);
> + if (*p != ' ')
> + return error("expected ' ', got '%c'", *p);
Nitpicking.
I guess because this error shouldn't really happen, and because current
implementation is transient, we don't need to worry about better error
messages (was it problem with parsing, or was it unexpected character).
For example '10064x', or '10064\n' would fail parse, but it is not
space that we were expecting...
> + *mode2 = (int)strtol(p + 1, &p, 8);
> + if (*p != ' ')
> + return error("expected ' ', got '%c'", *p);
> + if (get_oid_hex(++p, oid1))
> + return error("expected object ID, got '%s'", p + 1);
> + p += GIT_SHA1_HEXSZ;
> + if (*p != ' ')
> + return error("expected ' ', got '%c'", *p);
> + if (get_oid_hex(++p, oid2))
> + return error("expected object ID, got '%s'", p + 1);
> + p += GIT_SHA1_HEXSZ;
> + if (*p != ' ')
> + return error("expected ' ', got '%c'", *p);
> + *status = *++p;
> + if (!status || p[1])
> + return error("unexpected trailer: '%s'", p);
> + return 0;
> +}
> +
> +/*
> + * Remove any trailing slash from $workdir
> + * before starting to avoid double slashes in symlink targets.
> + */
Err... that's not what add_path() does, in its current implementation.
It doesn't remove trailing slashes, but it checks if there is trailing
slash, and if there isn't, it adds it as separator before adding path.
Or was it original comment from the Perl implementation? It look
like this, with '$workdir'... If it is meant to be straight copy
of comment from legacy-difftool, a note would be nice.
> +static void add_path(struct strbuf *buf, size_t base_len, const char *path)
Naming: I think strbuf_addpath() would be a better name, but I guess
it is a matter of taste.
> +{
> + strbuf_setlen(buf, base_len);
> + if (buf->len && buf->buf[buf->len - 1] != '/')
> + strbuf_addch(buf, '/');
> + strbuf_addstr(buf, path);
> +}
> +
> +/*
> + * Determine whether we can simply reuse the file in the worktree.
> + */
> +static int use_wt_file(const char *workdir, const char *name,
Should it be 'name' or 'pathname'?
> + struct object_id *oid)
> +{
> + struct strbuf buf = STRBUF_INIT;
> + struct stat st;
> + int use = 0;
> +
> + strbuf_addstr(&buf, workdir);
> + add_path(&buf, buf.len, name);
With proposed rename, it would IMVVVHO looks better
+ strbuf_addstr(&buf, workdir);
+ strbuf_addpath(&buf, buf.len, name);
But that is a matter of taste (again, the function is file-local).
> +
> + if (!lstat(buf.buf, &st) && !S_ISLNK(st.st_mode)) {
> + struct object_id wt_oid;
> + int fd = open(buf.buf, O_RDONLY);
> +
> + if (!index_fd(wt_oid.hash, fd, &st, OBJ_BLOB, name, 0)) {
> + if (is_null_oid(oid)) {
> + oidcpy(oid, &wt_oid);
> + use = 1;
> + } else if (!oidcmp(oid, &wt_oid))
> + use = 1;
> + }
> + }
> +
> + strbuf_release(&buf);
> +
> + return use;
> +}
[...]
> +static int ensure_leading_directories(char *path)
> +{
> + switch (safe_create_leading_directories(path)) {
> + case SCLD_OK:
> + case SCLD_EXISTS:
> + return 0;
> + default:
> + return error(_("could not create leading directories "
> + "of '%s'"), path);
> + }
> +}
Nice function, I wonder if it would be useful in other places.
> +
> +static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
> + int argc, const char **argv)
> +{
> + char tmpdir[PATH_MAX];
> + struct strbuf info = STRBUF_INIT, lpath = STRBUF_INIT;
> + struct strbuf rpath = STRBUF_INIT, buf = STRBUF_INIT;
Nitpicking.
To be symmetric, it could be reordered like this:
+ struct strbuf info = STRBUF_INIT, buf = STRBUF_INIT;
+ struct strbuf lpath = STRBUF_INIT, rpath = STRBUF_INIT;
See: lpath, rpath; ldir, rdir; ldir_len, rdir_len.
> + struct strbuf ldir = STRBUF_INIT, rdir = STRBUF_INIT;
> + struct strbuf wtdir = STRBUF_INIT;
> + size_t ldir_len, rdir_len, wtdir_len;
[...]
> + argv_array_pushl(&child.args, "diff", "--raw", "--no-abbrev", "-z",
> + NULL);
> + for (i = 0; i < argc; i++)
> + argv_array_push(&child.args, argv[i]);
> + if (start_command(&child))
> + die("could not obtain raw diff");
> + fp = xfdopen(child.out, "r");
> +
> + /* Build index info for left and right sides of the diff */
> + i = 0;
> + while (!strbuf_getline_nul(&info, fp)) {
> + int lmode, rmode;
> + struct object_id loid, roid;
> + char status;
> + const char *src_path, *dst_path;
> + size_t src_path_len, dst_path_len;
> +
> + if (starts_with(info.buf, "::"))
> + die(N_("combined diff formats('-c' and '--cc') are "
> + "not supported in\n"
> + "directory diff mode('-d' and '--dir-diff')."));
> +
> + if (parse_index_info(info.buf, &lmode, &rmode, &loid, &roid,
> + &status))
After rename it would read as:
+ if (parse_raw_diff(info.buf, &lmode, &rmode, &loid, &roid,
+ &status))
Though now I see that you parse here index information of raw diff
(I think)... so disregard my musings.
> + break;
> + if (strbuf_getline_nul(&lpath, fp))
> + break;
> + src_path = lpath.buf;
> + src_path_len = lpath.len;
> +
> + i++;
> + if (status != 'C' && status != 'R') {
> + dst_path = src_path;
> + dst_path_len = src_path_len;
> + } else {
> + if (strbuf_getline_nul(&rpath, fp))
> + break;
> + dst_path = rpath.buf;
> + dst_path_len = rpath.len;
> + }
[...]
> + /*
> + * Changes to submodules require special treatment.This loop writes a
Here and in few other places you are missing space after full stop.
+ * Changes to submodules require special treatment. This loop writes a
> + * temporary file to both the left and right directories to show the
> + * change in the recorded SHA1 for the submodule.
> + */
> + hashmap_iter_init(&submodules, &iter);
> + while ((entry = hashmap_iter_next(&iter))) {
> + if (*entry->left) {
> + add_path(&ldir, ldir_len, entry->path);
> + ensure_leading_directories(ldir.buf);
> + write_file(ldir.buf, "%s", entry->left);
> + }
> + if (*entry->right) {
> + add_path(&rdir, rdir_len, entry->path);
> + ensure_leading_directories(rdir.buf);
> + write_file(rdir.buf, "%s", entry->right);
> + }
> + }
> +
> + /*
> + * Symbolic links require special treatment.The standard "git diff"
Same here.
> + * shows only the link itself, not the contents of the link target.
> + * This loop replicates that behavior.
> + */
Best,
--
Jakub Narębski
^ permalink raw reply
* Re: [char-misc-next] mei: request async autosuspend at the end of enumeration
From: Jakub Narębski @ 2016-11-25 20:33 UTC (permalink / raw)
To: Jeff King, Winkler, Tomas
Cc: Matthieu Moy, git@vger.kernel.org,
Greg KH (gregkh@linuxfoundation.org), Usyskin, Alexander,
linux-kernel@vger.kernel.org
In-Reply-To: <20161125031425.gefijvssvygp6pl4@sigill.intra.peff.net>
W dniu 25.11.2016 o 04:14, Jeff King pisze:
> On Thu, Nov 24, 2016 at 10:37:14PM +0000, Winkler, Tomas wrote:
>
>>>>> Cc: <stable@vger.kernel.org> # 4.4+
>>>>
>>>> Looks like git send-email is not able to parse this address correctly
>>>> though this is suggested format by Documentation/stable_kernel_rules.txt.
>>>> Create wrong address If git parsers is used : 'stable@vger.kernel.org#4.4+'
[...]
> The patch just brings parity to the Mail::Address behavior and git's
> fallback parser, so that you don't end up with the broken
> stable@vger.kernel.org#4.4+ address. Instead, that content goes into the
> name part of the address.
>
> It sounds like you want the "# 4.4+" to be dropped entirely in the
> rfc822 header. It looks like send-email used to do that, but stopped in
> b1c8a11c8 (send-email: allow multiple emails using --cc, --to and --bcc,
> 2015-06-30).
>
> So perhaps there are further fixes required, but it's hard to know. The
> input isn't a valid rfc822 header, so it's not entirely clear what the
> output is supposed to be. I can buy either "drop it completely" or
> "stick it in the name field of the cc header" as reasonable.
Well, we could always convert it to email address comment, converting
for example the following trailer:
Cc: John Doe <john@example.com> # comment
to the following address:
John Doe <john@example.com> (comment)
Just FYI. Though I'm not sure how well this would work...
Best,
--
Jakub Narębski
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #05; Wed, 23)
From: Jakub Narębski @ 2016-11-25 19:40 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <xmqqk2btlr3x.fsf@gitster.mtv.corp.google.com>
W dniu 24.11.2016 o 00:21, Junio C Hamano pisze:
> * nd/rebase-forget (2016-10-28) 1 commit
> - rebase: add --forget to cleanup rebase, leave HEAD untouched
>
> "git rebase" learned "--forget" option, which allows a user to
> remove the metadata left by an earlier "git rebase" that was
> manually aborted without using "git rebase --abort".
>
> Waiting for a reroll.
It's always a good thing to stop requiring messing with .git insides.
> * jc/reset-unmerge (2016-10-24) 1 commit
> - reset: --unmerge
>
> After "git add" is run prematurely during a conflict resolution,
> "git diff" can no longer be used as a way to sanity check by
> looking at the combined diff. "git reset" learned a new
> "--unmerge" option to recover from this situation.
>
> Will discard.
> This may not be needed, given that update-index has a similar
> option.
OTOH update-index is considered plumbing, so having "git reset --unmerge"
might be good thing (note that we can re-checkout file merge).
> * jc/merge-drop-old-syntax (2015-04-29) 1 commit
> (merged to 'next' on 2016-10-11 at 8928c8b9b3)
> + merge: drop 'git merge <message> HEAD <commit>' syntax
>
> Stop supporting "git merge <message> HEAD <commit>" syntax that has
> been deprecated since October 2007, and issues a deprecation
> warning message since v2.5.0.
>
> Will cook in 'next'.
>
^ permalink raw reply
* [feature request] Make "commit --only" work with new files
From: Luis Ressel @ 2016-11-25 16:56 UTC (permalink / raw)
To: git
Hello,
currently "git commit --only <file>" only works if <file> is already
checked into the repo, but not with newly created and still untracked
files (builtin/commit.c:list_path() throws the error "error: pathspec
'<file>' did not match any file(s) known to git.")
I don't think this limitation is intented. I've had a look at the
relevant part of builtin/commit.c, but unfortunately it wasn't obvious
to me how to fix this.
Regards,
Luis Ressel
^ permalink raw reply
* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Jeff King @ 2016-11-25 17:47 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Junio C Hamano, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1611251841030.117539@virtualbox>
On Fri, Nov 25, 2016 at 06:41:23PM +0100, Johannes Schindelin wrote:
> > Ah, I didn't realize that was a requirement. If this is going to be part
> > of a release and real end-users are going to see it, that does make me
> > think the config option is the better path (than the presence of some
> > file), as it's our standard way of tweaking run-time behavior.
>
> So how do you easily switch back and forth between testing the old vs the
> new difftool via the test suite?
If it's for a specific tool, I'd consider teaching the test suite to run
the whole script twice: once with the flag set and once without.
That is sometimes more complicated, though, if the script creates many
sub-repos. An environment variable might be more natural. If you already
support flipping the default via config, you can probably do:
GIT_CONFIG_PARAMETERS="'difftool.usebuiltin=true'"
export GIT_CONFIG_PARAMETERS
-Peff
^ permalink raw reply
* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Johannes Schindelin @ 2016-11-25 17:41 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <20161125171940.rizbqyhsygdsoujr@sigill.intra.peff.net>
Hi Peff,
On Fri, 25 Nov 2016, Jeff King wrote:
> On Fri, Nov 25, 2016 at 12:05:00PM +0100, Johannes Schindelin wrote:
>
> > > I would have expected it to just be a build-time flag, like:
> > >
> > > make BUILTIN_DIFFTOOL=Yes test
> >
> > That works for Git developers.
> >
> > I want to let as many users as possible test the builtin difftool.
> > Hopefully a lot more users than there are Git developers.
> >
> > Which means that I need a feature flag in production code, not a build
> > time flag.
>
> Ah, I didn't realize that was a requirement. If this is going to be part
> of a release and real end-users are going to see it, that does make me
> think the config option is the better path (than the presence of some
> file), as it's our standard way of tweaking run-time behavior.
So how do you easily switch back and forth between testing the old vs the
new difftool via the test suite?
Ciao,
Dscho
^ permalink raw reply
* [PATCH v2] date-formats.txt: Typo fix
From: Luis Ressel @ 2016-11-25 17:36 UTC (permalink / raw)
To: git
Last time I checked, I was living in the UTC+01:00 time zone. UTC+02:00
would be Central European _Summer_ Time.
---
Documentation/date-formats.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/date-formats.txt b/Documentation/date-formats.txt
index 35e8da2..6926e0a 100644
--- a/Documentation/date-formats.txt
+++ b/Documentation/date-formats.txt
@@ -11,7 +11,7 @@ Git internal format::
It is `<unix timestamp> <time zone offset>`, where `<unix
timestamp>` is the number of seconds since the UNIX epoch.
`<time zone offset>` is a positive or negative offset from UTC.
- For example CET (which is 2 hours ahead UTC) is `+0200`.
+ For example CET (which is 1 hour ahead of UTC) is `+0100`.
RFC 2822::
The standard email format as described by RFC 2822, for example
--
2.10.2
^ permalink raw reply related
* [PATCH] date-formats.txt: Typo fix
From: Luis Ressel @ 2016-11-25 17:34 UTC (permalink / raw)
To: git
Last time I checked, I was living in the UTC+01:00 time zone. UTC+02:00
would be Central European _Summer_ Time.
---
Documentation/date-formats.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/date-formats.txt b/Documentation/date-formats.txt
index 35e8da2..9c3084e 100644
--- a/Documentation/date-formats.txt
+++ b/Documentation/date-formats.txt
@@ -11,7 +11,7 @@ Git internal format::
It is `<unix timestamp> <time zone offset>`, where `<unix
timestamp>` is the number of seconds since the UNIX epoch.
`<time zone offset>` is a positive or negative offset from UTC.
- For example CET (which is 2 hours ahead UTC) is `+0200`.
+ For example CET (which is 1 hours ahead of UTC) is `+0100`.
RFC 2822::
The standard email format as described by RFC 2822, for example
--
2.10.2
^ permalink raw reply related
* Re: [PATCH v2] merge-recursive.c: use string_list_sort instead of qsort
From: Jeff King @ 2016-11-25 17:15 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, René Scharfe, Junio C Hamano
In-Reply-To: <CACsJy8Atv9rkwmCcXgOqDb6pLP8RxQ7XnxMYt3=hN6KG4X79iA@mail.gmail.com>
On Fri, Nov 25, 2016 at 07:15:15PM +0700, Duy Nguyen wrote:
> > I guess I haven't used string_list_sort() in a while, but I was
> > surprised to find that it just feeds the strings to the comparator. That
> > makes sense for using a raw strcmp() as the comparator, but I wonder if
> > any callers would ever want to take the util field into account (e.g.,
> > to break ties).
> >
> > We don't seem to care here, though (which can be verified by reading the
> > code, but also because any mention of one->util would be a compilation
> > error after your patch). So I guess we can punt on it until the day that
> > some caller does need it.
>
> Some callers do need it, or at least fmt-merge-msg.c:add_people_info()
> does, maybe builtin/remote.c:show() and shortlog.c:shortlog_output()
> too. But I'll stop here and get back to my worktree stuff.
I started to work on this, figuring it would be a nice warm-up for the
day. But it actually is a little complicated, and I think not worth
doing. :)
The obvious backwards-compatible way to do it is to add a "cmp_item"
field to the string list. Sorting should use that if non-NULL, and
fallback to the string-oriented "cmp" otherwise.
And that does work when you want to sort via string_list_sort, like:
authors->cmp_item = cmp_string_list_util_as_integral;
string_list_sort(authors);
(the example is from fmt-merge-message.c). But the original use of
sorting in string-list was to keep a sorted list as you go with
string_list_insert(). And in that call we have _only_ the newly added
string, and the caller has not yet had an opportunity to set the util
field. So:
struct string_list list = STRING_LIST_INIT_DUP;
list.cmp_item = cmp_util_fields;
for (...)
string_list_insert(&list, foo[i])->util = bar[i];
is nonsense. It would always see a NULL util field during the
comparison.
Certainly "don't do that" is a possible answer. But it's just a bad
interface. It encourages a nonsensical use, and it makes a natural use
(sorting after the fact) more clunky by making the caller set a field in
the struct rather than pass a parameter. The correct interface is more
like:
string_list_sort_items(authors, cmp_string_list_util_as_integral);
but then we are not really saving much over the more generic:
QSORT(authors->items, authors->nr, cmp_string_list_util_as_integral);
So I'm inclined to leave it as-is.
-Peff
^ permalink raw reply
* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Jeff King @ 2016-11-25 17:19 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Junio C Hamano, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1611251201580.117539@virtualbox>
On Fri, Nov 25, 2016 at 12:05:00PM +0100, Johannes Schindelin wrote:
> > I would have expected it to just be a build-time flag, like:
> >
> > make BUILTIN_DIFFTOOL=Yes test
>
> That works for Git developers.
>
> I want to let as many users as possible test the builtin difftool.
> Hopefully a lot more users than there are Git developers.
>
> Which means that I need a feature flag in production code, not a build
> time flag.
Ah, I didn't realize that was a requirement. If this is going to be part
of a release and real end-users are going to see it, that does make me
think the config option is the better path (than the presence of some
file), as it's our standard way of tweaking run-time behavior.
The implementation can still remain slightly gross if it's eventually
going away.
> > I'm happy with pretty much anything under the reasoning of "this does not
> > matter much because it is going away soon".
>
> Yeah, well, I am more happy with anything along the lines of David's
> review, pointing out flaws in the current revision of the builtin difftool
> before it bites users ;-)
Sorry, I can't really help much there, not having much knowledge of
difftool.
-Peff
^ permalink raw reply
* Re: [PATCH 0/2] Fix segmentation fault with cherry-pick
From: Johannes Schindelin @ 2016-11-25 16:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Markus Klein
In-Reply-To: <cover.1480091758.git.johannes.schindelin@gmx.de>
Hi,
On Fri, 25 Nov 2016, Johannes Schindelin wrote:
> The culprit is actually not cherry-pick, but a special code path that
> expects refresh_cache_entry() not to return NULL. And the fix is to
> teach it to handle NULL there.
>
> This bug was brought to my attention by Markus Klein via
> https://github.com/git-for-windows/git/issues/952.
For the record, I looked at other callers of `refresh_cache_entry()`:
there is only `make_cache_entry()`, whose callers all handle NULL return
values except in resolve-undo.c. But that latter caller is okay because it
specifically does not allow refreshing (by passing 0 as options), so
refresh_cache_entry() cannot return NULL.
Ciao,
Dscho
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox