* [PATCH v2] run-command: report exec failure
From: Junio C Hamano @ 2018-12-12 18:36 UTC (permalink / raw)
To: git; +Cc: Jeff King, Johannes Schindelin
In-Reply-To: <xmqqsgz4jkgl.fsf@gitster-ct.c.googlers.com>
In 321fd823 ("run-command: mark path lookup errors with ENOENT",
2018-10-24), we rewrote the logic to execute a command by looking
in the directories on $PATH; as a side effect, a request to run a
command that is not found on $PATH is noticed even before a child
process is forked to execute it.
We however stopped to report an exec failure in such a case by
mistake. Add a logic to report the error unless silent-exec-failure
is requested, to match the original code.
Reported-by: John Passaro <john.a.passaro@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This time, tests look for the command name in the message, to
avoid getting affected by the differences the error is reported
by two codepaths (Windows codepath uses "spawn" while others say
"run"), which was pointed out by Dscho.
I am taking that https://travis-ci.org/git/git/jobs/466908193
that succeeded on Windows as a sign that this is now OK there.
run-command.c | 2 ++
t/t0061-run-command.sh | 9 ++++++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/run-command.c b/run-command.c
index d679cc267c..e2bc18a083 100644
--- a/run-command.c
+++ b/run-command.c
@@ -728,6 +728,8 @@ int start_command(struct child_process *cmd)
if (prepare_cmd(&argv, cmd) < 0) {
failed_errno = errno;
cmd->pid = -1;
+ if (!cmd->silent_exec_failure)
+ error_errno("cannot run %s", cmd->argv[0]);
goto end_of_spawn;
}
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index b9cfc03a53..8a484878ec 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -14,11 +14,13 @@ EOF
>empty
test_expect_success 'start_command reports ENOENT (slash)' '
- test-tool run-command start-command-ENOENT ./does-not-exist
+ test-tool run-command start-command-ENOENT ./does-not-exist 2>err &&
+ test_i18ngrep "\./does-not-exist" err
'
test_expect_success 'start_command reports ENOENT (no slash)' '
- test-tool run-command start-command-ENOENT does-not-exist
+ test-tool run-command start-command-ENOENT does-not-exist 2>err &&
+ test_i18ngrep "does-not-exist" err
'
test_expect_success 'run_command can run a command' '
@@ -34,7 +36,8 @@ test_expect_success 'run_command is restricted to PATH' '
write_script should-not-run <<-\EOF &&
echo yikes
EOF
- test_must_fail test-tool run-command run-command should-not-run
+ test_must_fail test-tool run-command run-command should-not-run 2>err &&
+ test_i18ngrep "should-not-run" err
'
test_expect_success !MINGW 'run_command can run a script without a #! line' '
--
2.20.0
^ permalink raw reply related
* Re: [PATCH] shortlog: pass the mailmap into the revision walker
From: Ævar Arnfjörð Bjarmason @ 2018-12-12 18:31 UTC (permalink / raw)
To: CB Bailey; +Cc: git
In-Reply-To: <20181212171052.13415-1-cb@hashpling.org>
On Wed, Dec 12 2018, CB Bailey wrote:
> From: CB Bailey <cbailey32@bloomberg.net>
>
> shortlog always respects the mailmap in its output. Pass the mailmap
> into the revision walker to allow the mailmap to be used with revision
> limiting options such as '--author'.
>
> This removes some apparently inconsistent behaviors when using
> '--author', such as not finding some or all commits for a given author
> which do appear under that author in an unrestricted invocation of
> shortlog or commits being summarized under a different author than the
> specified author.
>
> Signed-off-by: CB Bailey <cbailey32@bloomberg.net>
> ---
>
> Resending with omitted s-o-b.
>
> builtin/shortlog.c | 2 ++
> t/t4203-mailmap.sh | 28 ++++++++++++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/builtin/shortlog.c b/builtin/shortlog.c
> index 88f88e97b2..a6fb00ade8 100644
> --- a/builtin/shortlog.c
> +++ b/builtin/shortlog.c
> @@ -188,6 +188,8 @@ static void get_from_rev(struct rev_info *rev, struct shortlog *log)
> {
> struct commit *commit;
>
> + rev->mailmap = &log->mailmap;
> +
> if (prepare_revision_walk(rev))
> die(_("revision walk setup failed"));
> while ((commit = get_revision(rev)) != NULL)
> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
> index 43b1522ea2..9bee35b06c 100755
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -383,6 +383,34 @@ test_expect_success 'Shortlog output (complex mapping)' '
>
> '
>
> +test_expect_success 'Shortlog output (complex mapping, filtered)' '
> +
> + printf " 1\tA U Thor <author@example.com>\n" >expect &&
> +
> + git shortlog -es --author="A U Thor" HEAD >actual &&
> + test_cmp expect actual &&
> +
> + printf " 1\tCTO <cto@company.xx>\n" >expect &&
> +
> + git shortlog -es --author=CTO HEAD >actual &&
> + test_cmp expect actual &&
> +
> + printf " 2\tOther Author <other@author.xx>\n" >expect &&
> +
> + git shortlog -es --author="Other Author" HEAD >actual &&
> + test_cmp expect actual &&
> +
> + printf " 2\tSanta Claus <santa.claus@northpole.xx>\n" >expect &&
> +
> + git shortlog -es --author="Santa Claus" HEAD >actual &&
> + test_cmp expect actual &&
> +
> + printf " 1\tSome Dude <some@dude.xx>\n" >expect &&
> +
> + git shortlog -es --author="Some Dude" HEAD >actual &&
> + test_cmp expect actual
> +'
> +
> # git log with --pretty format which uses the name and email mailmap placemarkers
> cat >expect <<\EOF
> Author CTO <cto@coompany.xx> maps to CTO <cto@company.xx>
Makes sense. Not saying this is how it should be, just an equivalently
working patch that I came up with on top while poing at this:
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index a6fb00ade8..ad84d70d07 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -188,10 +188,9 @@ static void get_from_rev(struct rev_info *rev, struct shortlog *log)
{
struct commit *commit;
- rev->mailmap = &log->mailmap;
-
if (prepare_revision_walk(rev))
die(_("revision walk setup failed"));
+ rev->mailmap = &log->mailmap;
while ((commit = get_revision(rev)) != NULL)
shortlog_add_commit(log, commit);
}
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 9bee35b06c..74a269052d 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -384,17 +384,6 @@ test_expect_success 'Shortlog output (complex mapping)' '
'
test_expect_success 'Shortlog output (complex mapping, filtered)' '
-
- printf " 1\tA U Thor <author@example.com>\n" >expect &&
-
- git shortlog -es --author="A U Thor" HEAD >actual &&
- test_cmp expect actual &&
-
- printf " 1\tCTO <cto@company.xx>\n" >expect &&
-
- git shortlog -es --author=CTO HEAD >actual &&
- test_cmp expect actual &&
-
printf " 2\tOther Author <other@author.xx>\n" >expect &&
git shortlog -es --author="Other Author" HEAD >actual &&
I.e. we just need the assignment after prepare_revision_walk() and the
first 2x tests were things that passed before this change.
So that's not a "let's squash that on top" but "I was poking at this and
here's stuff that I fiddled with or surprised me slightly".
^ permalink raw reply
* RE: High locking contention during repack?
From: Iucha, Florin @ 2018-12-12 18:30 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Ævar Arnfjörð Bjarmason, Jeff King
In-Reply-To: <SN1PR12MB2384362EE5BA4980322931B795A70@SN1PR12MB2384.namprd12.prod.outlook.com>
It seems to be spending a lot of time in create_delta. I have added a breakpoint on calling create_delta and a few good minutes pass between the breakpoint being hit. The large loop in create_delta has practically no outgoing function calls (save for the realloc on line 476), so I still don't understand how dstat could report 0% user time. htop indicates the worker threads are using 100% CPU time.
florin
^ permalink raw reply
* [PATCH v2 2/2] t4256: mark support files as LF-only
From: Johannes Schindelin via GitGitGadget @ 2018-12-12 18:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <pull.98.v2.git.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The test t4256-am-format-flowed.sh requires carefully applying a
patch after ignoring padding whitespace. This breaks if the file
is munged to include CRLF line endings instead of LF.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
t/.gitattributes | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/.gitattributes b/t/.gitattributes
index e7acedabe1..df05434d32 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -16,6 +16,7 @@ t[0-9][0-9][0-9][0-9]/* -whitespace
/t4135/* eol=lf
/t4211/* eol=lf
/t4252/* eol=lf
+/t4256/1/* eol=lf
/t5100/* eol=lf
/t5515/* eol=lf
/t556x_common eol=lf
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 1/2] .gitattributes: ensure t/oid-info/* has eol=lf
From: Derrick Stolee via GitGitGadget @ 2018-12-12 18:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Derrick Stolee
In-Reply-To: <pull.98.v2.git.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
The new test_oid machinery in the test library requires reading
some information from t/oid-info/hash-info and t/oid-info/oid.
The shell logic that reads from these files is sensitive to CRLF
line endings, causing a problem when the test suite is run on a
Windows machine that converts LF to CRLF.
Exclude the files in this folder from this conversion.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
.gitattributes | 1 +
1 file changed, 1 insertion(+)
diff --git a/.gitattributes b/.gitattributes
index acf853e029..3738cea7eb 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -13,3 +13,4 @@
/Documentation/gitk.txt conflict-marker-size=32
/Documentation/user-manual.txt conflict-marker-size=32
/t/t????-*.sh conflict-marker-size=32
+/t/oid-info/* eol=lf
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 0/2] Add more eol=lf to .gitattributes
From: Derrick Stolee via GitGitGadget @ 2018-12-12 18:14 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <pull.98.git.gitgitgadget@gmail.com>
I noticed that our CI builds (see [1] for an example) were returning success
much faster than they did before Git v2.20.0. Turns out that there was a
test script failure involving the new test hash logic.
error: bug in the test script: bad hash algorithm
make[1]: *** [Makefile:56: t0000-basic.sh] Error 1
make[1]: *** Waiting for unfinished jobs....
This failure was due to an LF -> CRLF conversion in some data files in
t/oid-info/. Don't munge these files, and the tests can continue.
UPDATED IN V2: Unfortunately, I didn't check the full test suite after
getting beyond this point, and found another LF -> CRLF conversion problem
in t4256-am-format-flowed.sh due to example patches in t/t4256/1/. Add these
in a second commit. Thanks, dscho, for helping with the correct placement.
Thanks, -Stolee
[1] https://gvfs.visualstudio.com/ci/_build/results?buildId=4815
Derrick Stolee (1):
.gitattributes: ensure t/oid-info/* has eol=lf
Johannes Schindelin (1):
t4256: mark support files as LF-only
.gitattributes | 1 +
t/.gitattributes | 1 +
2 files changed, 2 insertions(+)
base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-98%2Fderrickstolee%2Ftest-oid-fix-windows-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-98/derrickstolee/test-oid-fix-windows-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/98
Range-diff vs v1:
1: 4a22502a31 = 1: 4a22502a31 .gitattributes: ensure t/oid-info/* has eol=lf
-: ---------- > 2: 4275b8a581 t4256: mark support files as LF-only
--
gitgitgadget
^ permalink raw reply
* RE: High locking contention during repack?
From: Iucha, Florin @ 2018-12-12 18:08 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org, Ævar Arnfjörð Bjarmason
In-Reply-To: <SN1PR12MB23849A5B991FB7D9213FB64595A70@SN1PR12MB2384.namprd12.prod.outlook.com>
(Apologize for top-posting but making Outlook quote properly is a bear)
Down to 1+4 threads:
--total-cpu-usage-- -dsk/total- -net/total- ---paging-- ---system--
usr sys idl wai stl| read writ| recv send| in out | int csw
0 13 87 0 0| 0 0 |2059B 15k| 0 0 |2200 1730
0 13 87 0 0| 0 0 | 585B 2754B| 0 0 |1792 1269
0 13 87 0 0| 0 0 | 178B 734B| 0 0 |1691 1157
0 13 87 0 0| 0 6554B| 225B 734B| 0 0 |1885 1238
0 13 87 0 0| 0 0 | 159B 736B| 0 0 |1958 1262
0 13 87 0 0| 0 0 | 310B 760B| 0 0 |1813 1175
(gdb) info threads
Id Target Id Frame
* 1 Thread 0x7f7b837dcb80 (LWP 11918) "git" 0x000055c7dded56eb in ll_find_deltas (processed=0x7ffce7da82a8, depth=<optimized out>, window=<optimized out>,
list_size=0, list=<optimized out>) at builtin/pack-objects.c:2516
2 Thread 0x7f782e16a700 (LWP 11929) "git" create_delta (index=0x55c807d2ff20, trg_buf=<optimized out>, trg_size=trg_size@entry=9677636,
delta_size=delta_size@entry=0x7f782e169e78, max_size=3585674) at diff-delta.c:381
3 Thread 0x7f6fe77fe700 (LWP 11948) "git" create_delta (index=0x7f6e35171010, trg_buf=<optimized out>, trg_size=trg_size@entry=163385853,
delta_size=delta_size@entry=0x7f6fe77fde78, max_size=2328926) at diff-delta.c:381
4 Thread 0x7f6fe67fc700 (LWP 11950) "git" create_delta (index=0x7f6e3f7f3010, trg_buf=<optimized out>, trg_size=trg_size@entry=74543780,
delta_size=delta_size@entry=0x7f6fe67fbe78, max_size=2582505) at diff-delta.c:381
5 Thread 0x7f6fe5ffb700 (LWP 11951) "git" create_delta (index=0x7f6f01d06010, trg_buf=<optimized out>, trg_size=trg_size@entry=28510185,
delta_size=delta_size@entry=0x7f6fe5ffae78, max_size=9146484) at diff-delta.c:381
(gdb) bt
#0 0x00007f7b82f909f3 in futex_wait_cancelable (private=<optimized out>, expected=0, futex_word=0x55c7de2ff348 <progress_cond+40>)
at ../sysdeps/unix/sysv/linux/futex-internal.h:88
#1 __pthread_cond_wait_common (abstime=0x0, mutex=0x55c7de2ff360 <progress_mutex>, cond=0x55c7de2ff320 <progress_cond>) at pthread_cond_wait.c:502
#2 __pthread_cond_wait (cond=cond@entry=0x55c7de2ff320 <progress_cond>, mutex=mutex@entry=0x55c7de2ff360 <progress_mutex>) at pthread_cond_wait.c:655
#3 0x000055c7dded56eb in ll_find_deltas (processed=0x7ffce7da82a8, depth=<optimized out>, window=<optimized out>, list_size=0, list=<optimized out>)
at builtin/pack-objects.c:2516
#4 prepare_pack (depth=<optimized out>, window=<optimized out>) at builtin/pack-objects.c:2673
#5 cmd_pack_objects (argc=<optimized out>, argv=<optimized out>, prefix=<optimized out>) at builtin/pack-objects.c:3491
#6 0x000055c7dde74711 in run_builtin (argv=<optimized out>, argc=<optimized out>, p=<optimized out>) at git.c:421
#7 handle_builtin (argc=<optimized out>, argv=<optimized out>) at git.c:647
#8 0x000055c7dde75725 in run_argv (argv=0x7ffce7daa7e0, argcp=0x7ffce7daa7ec) at git.c:701
#9 cmd_main (argc=<optimized out>, argv=<optimized out>) at git.c:798
#10 0x000055c7dde7437f in main (argc=6, argv=0x7ffce7daaa58) at common-main.c:45
(gdb) thread 2
[Switching to thread 2 (Thread 0x7f782e16a700 (LWP 11929))]
#0 create_delta (index=0x55c807d2ff20, trg_buf=<optimized out>, trg_size=trg_size@entry=9677636, delta_size=delta_size@entry=0x7f782e169e78, max_size=3585674)
at diff-delta.c:381
381 if (entry->val != val)
(gdb) bt
#0 create_delta (index=0x55c807d2ff20, trg_buf=<optimized out>, trg_size=trg_size@entry=9677636, delta_size=delta_size@entry=0x7f782e169e78, max_size=3585674)
at diff-delta.c:381
#1 0x000055c7dded0606 in try_delta (mem_usage=<synthetic pointer>, max_depth=50, src=0x7f702c256bc0, trg=0x7f702c256ca0) at builtin/pack-objects.c:2129
#2 find_deltas (list=<optimized out>, list_size=list_size@entry=0x55c7e467daf4, window=<optimized out>, depth=<optimized out>, processed=<optimized out>)
at builtin/pack-objects.c:2262
#3 0x000055c7dded0e29 in threaded_find_deltas (arg=0x55c7e467dae0) at builtin/pack-objects.c:2408
#4 0x00007f7b82f8a6db in start_thread (arg=0x7f782e16a700) at pthread_create.c:463
#5 0x00007f7b82cb388f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
(gdb) thread 2
[Switching to thread 2 (Thread 0x7f782e16a700 (LWP 11929))]
#0 create_delta (index=0x55c807d2ff20, trg_buf=<optimized out>, trg_size=trg_size@entry=9677636, delta_size=delta_size@entry=0x7f782e169e78, max_size=3585674)
at diff-delta.c:381
381 if (entry->val != val)
(gdb) bt
#0 create_delta (index=0x55c807d2ff20, trg_buf=<optimized out>, trg_size=trg_size@entry=9677636, delta_size=delta_size@entry=0x7f782e169e78, max_size=3585674)
at diff-delta.c:381
#1 0x000055c7dded0606 in try_delta (mem_usage=<synthetic pointer>, max_depth=50, src=0x7f702c256bc0, trg=0x7f702c256ca0) at builtin/pack-objects.c:2129
#2 find_deltas (list=<optimized out>, list_size=list_size@entry=0x55c7e467daf4, window=<optimized out>, depth=<optimized out>, processed=<optimized out>)
at builtin/pack-objects.c:2262
#3 0x000055c7dded0e29 in threaded_find_deltas (arg=0x55c7e467dae0) at builtin/pack-objects.c:2408
#4 0x00007f7b82f8a6db in start_thread (arg=0x7f782e16a700) at pthread_create.c:463
#5 0x00007f7b82cb388f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
florin
-----Original Message-----
From: Iucha, Florin
Sent: Wednesday, December 12, 2018 11:50 AM
To: 'Jeff King' <peff@peff.net>
Cc: git@vger.kernel.org
Subject: RE: High locking contention during repack?
Jeff,
Thank you for the advice, I will reduce the depth.
Running "git pack-objects --all --no-reuse-delta --delta-base-offset --stdout </dev/null >/dev/null", it got to 99% fairly quickly, now it has 5 threads running (using 99.9% CPU), but the "dstat 5" still shows lots of nothing, with some heavy system activity:
--total-cpu-usage-- -dsk/total- -net/total- ---paging-- ---system--
usr sys idl wai stl| read writ| recv send| in out | int csw
1 8 91 0 0| 166k 224k| 0 0 | 0 0 |6892 970
0 13 87 0 0| 0 0 | 333B 1758B| 0 0 |2285 1374
0 13 87 0 0| 0 0 | 309B 1190B| 0 0 |2233 1265
0 13 87 0 0| 0 0 | 282B 1053B| 0 0 |2174 1239
0 13 87 0 0| 0 0 | 278B 1251B| 0 0 |1930 1173
0 13 87 0 0| 0 0 | 274B 942B| 0 0 |1987 1139
0 13 87 0 0| 0 0 | 264B 1168B| 0 0 |1928 1205
0 13 87 0 0| 0 0 | 388B 2435B| 0 0 |2245 1280
0 13 87 0 0| 0 0 | 268B 1145B| 0 0 |2225 1221
0 13 87 0 0| 0 0 | 164B 732B| 0 0 |2607 1333
0 13 87 0 0| 0 0 | 156B 996B| 0 0 |2100 1270
0 13 87 0 0| 0 0 | 206B 1019B| 0 0 |2192 1296
0 13 87 0 0| 0 0 | 198B 824B| 0 0 |2019 1236
0 13 87 0 0| 0 0 | 245B 435B| 0 0 |1974 1195
0 13 87 0 0| 0 0 | 252B 855B| 0 0 |1852 1166
0 13 87 0 0| 0 0 | 230B 758B| 0 0 |2066 1299
0 13 87 0 0| 0 0 | 284B 925B| 0 0 |1860 1225
0 13 87 0 0| 0 0 | 289B 2682B| 0 0 |1796 1197
0 13 87 0 0| 0 0 | 939B 1263B| 0 0 |1913 1304
0 13 87 0 0| 0 0 |1212B 1366B| 0 0 |1915 1343
I will try running the command under the debugger and stop it when it gets to this point and poke around.
florin
^ permalink raw reply
* [PATCH] shortlog: pass the mailmap into the revision walker
From: CB Bailey @ 2018-12-12 17:10 UTC (permalink / raw)
To: git
In-Reply-To: <20181212164134.12922-1-cb@hashpling.org>
From: CB Bailey <cbailey32@bloomberg.net>
shortlog always respects the mailmap in its output. Pass the mailmap
into the revision walker to allow the mailmap to be used with revision
limiting options such as '--author'.
This removes some apparently inconsistent behaviors when using
'--author', such as not finding some or all commits for a given author
which do appear under that author in an unrestricted invocation of
shortlog or commits being summarized under a different author than the
specified author.
Signed-off-by: CB Bailey <cbailey32@bloomberg.net>
---
Resending with omitted s-o-b.
builtin/shortlog.c | 2 ++
t/t4203-mailmap.sh | 28 ++++++++++++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 88f88e97b2..a6fb00ade8 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -188,6 +188,8 @@ static void get_from_rev(struct rev_info *rev, struct shortlog *log)
{
struct commit *commit;
+ rev->mailmap = &log->mailmap;
+
if (prepare_revision_walk(rev))
die(_("revision walk setup failed"));
while ((commit = get_revision(rev)) != NULL)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 43b1522ea2..9bee35b06c 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -383,6 +383,34 @@ test_expect_success 'Shortlog output (complex mapping)' '
'
+test_expect_success 'Shortlog output (complex mapping, filtered)' '
+
+ printf " 1\tA U Thor <author@example.com>\n" >expect &&
+
+ git shortlog -es --author="A U Thor" HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 1\tCTO <cto@company.xx>\n" >expect &&
+
+ git shortlog -es --author=CTO HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 2\tOther Author <other@author.xx>\n" >expect &&
+
+ git shortlog -es --author="Other Author" HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 2\tSanta Claus <santa.claus@northpole.xx>\n" >expect &&
+
+ git shortlog -es --author="Santa Claus" HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 1\tSome Dude <some@dude.xx>\n" >expect &&
+
+ git shortlog -es --author="Some Dude" HEAD >actual &&
+ test_cmp expect actual
+'
+
# git log with --pretty format which uses the name and email mailmap placemarkers
cat >expect <<\EOF
Author CTO <cto@coompany.xx> maps to CTO <cto@company.xx>
--
2.17.0.rc0
^ permalink raw reply related
* Re: [PATCH 1/8] move worktree tests to t24*
From: Duy Nguyen @ 2018-12-12 17:07 UTC (permalink / raw)
To: Eric Sunshine
Cc: Thomas Gummerer, Git Mailing List, Junio C Hamano, Elijah Newren
In-Reply-To: <CAPig+cQ0pPiB01KOWsC7a3mHdJz6LtAJjtf8=MWF+34NFdVb1g@mail.gmail.com>
On Wed, Dec 12, 2018 at 2:27 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Tue, Dec 11, 2018 at 4:50 PM Thomas Gummerer <t.gummerer@gmail.com> wrote:
> > On 12/10, Duy Nguyen wrote:
> > > On Sun, Dec 9, 2018 at 9:04 PM Thomas Gummerer <t.gummerer@gmail.com> wrote:
> > > > Move the worktree tests to t24* to adhere to that guideline. We're
> > > > going to make use of the free'd up numbers in a subsequent commit.
> > >
> > > Heh.. I did the same thing (in my unsent switch-branch/restore-files
> > > series) and even used the same 24xx range :D You probably want to move
> > > t2028 and t2029 too (not sure if they have landed on 'master')
> >
> > [...] good to know someone
> > thought the same way. I started this work before t2028 and t2029
> > landed on master, so I failed to notice them.
>
> The thought of renumbering the test script came up as early as
> 2015-06-30. See the last bullet point of [1], for instance.
>
> [1]: https://public-inbox.org/git/1435640202-95945-1-git-send-email-sunshine@sunshineco.com/
Ah good. I thought I was just being lazy and picked a random range to add tests.
--
Duy
^ permalink raw reply
* Re: High locking contention during repack?
From: Ævar Arnfjörð Bjarmason @ 2018-12-12 16:54 UTC (permalink / raw)
To: Iucha, Florin; +Cc: Jeff King, Git Mailing List
In-Reply-To: <SN1PR12MB23849A5B991FB7D9213FB64595A70@SN1PR12MB2384.namprd12.prod.outlook.com>
On Wed, Dec 12, 2018 at 5:52 PM Iucha, Florin <Florin.Iucha@amd.com> wrote:
>
> Jeff,
>
> Thank you for the advice, I will reduce the depth.
>
> Running "git pack-objects --all --no-reuse-delta --delta-base-offset --stdout </dev/null >/dev/null", it got to 99% fairly quickly, now it has 5 threads running (using 99.9% CPU), but the "dstat 5" still shows lots of nothing, with some heavy system activity:
>
> --total-cpu-usage-- -dsk/total- -net/total- ---paging-- ---system--
> usr sys idl wai stl| read writ| recv send| in out | int csw
> 1 8 91 0 0| 166k 224k| 0 0 | 0 0 |6892 970
> 0 13 87 0 0| 0 0 | 333B 1758B| 0 0 |2285 1374
> 0 13 87 0 0| 0 0 | 309B 1190B| 0 0 |2233 1265
> 0 13 87 0 0| 0 0 | 282B 1053B| 0 0 |2174 1239
> 0 13 87 0 0| 0 0 | 278B 1251B| 0 0 |1930 1173
> 0 13 87 0 0| 0 0 | 274B 942B| 0 0 |1987 1139
> 0 13 87 0 0| 0 0 | 264B 1168B| 0 0 |1928 1205
> 0 13 87 0 0| 0 0 | 388B 2435B| 0 0 |2245 1280
> 0 13 87 0 0| 0 0 | 268B 1145B| 0 0 |2225 1221
> 0 13 87 0 0| 0 0 | 164B 732B| 0 0 |2607 1333
> 0 13 87 0 0| 0 0 | 156B 996B| 0 0 |2100 1270
> 0 13 87 0 0| 0 0 | 206B 1019B| 0 0 |2192 1296
> 0 13 87 0 0| 0 0 | 198B 824B| 0 0 |2019 1236
> 0 13 87 0 0| 0 0 | 245B 435B| 0 0 |1974 1195
> 0 13 87 0 0| 0 0 | 252B 855B| 0 0 |1852 1166
> 0 13 87 0 0| 0 0 | 230B 758B| 0 0 |2066 1299
> 0 13 87 0 0| 0 0 | 284B 925B| 0 0 |1860 1225
> 0 13 87 0 0| 0 0 | 289B 2682B| 0 0 |1796 1197
> 0 13 87 0 0| 0 0 | 939B 1263B| 0 0 |1913 1304
> 0 13 87 0 0| 0 0 |1212B 1366B| 0 0 |1915 1343
>
> I will try running the command under the debugger and stop it when it gets to this point and poke around.
FWIW compiling with gcc's gprof support is probably a better way to
figure out "what was my program doing all this time?".
^ permalink raw reply
* RE: High locking contention during repack?
From: Iucha, Florin @ 2018-12-12 16:49 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20181212112409.GB30673@sigill.intra.peff.net>
Jeff,
Thank you for the advice, I will reduce the depth.
Running "git pack-objects --all --no-reuse-delta --delta-base-offset --stdout </dev/null >/dev/null", it got to 99% fairly quickly, now it has 5 threads running (using 99.9% CPU), but the "dstat 5" still shows lots of nothing, with some heavy system activity:
--total-cpu-usage-- -dsk/total- -net/total- ---paging-- ---system--
usr sys idl wai stl| read writ| recv send| in out | int csw
1 8 91 0 0| 166k 224k| 0 0 | 0 0 |6892 970
0 13 87 0 0| 0 0 | 333B 1758B| 0 0 |2285 1374
0 13 87 0 0| 0 0 | 309B 1190B| 0 0 |2233 1265
0 13 87 0 0| 0 0 | 282B 1053B| 0 0 |2174 1239
0 13 87 0 0| 0 0 | 278B 1251B| 0 0 |1930 1173
0 13 87 0 0| 0 0 | 274B 942B| 0 0 |1987 1139
0 13 87 0 0| 0 0 | 264B 1168B| 0 0 |1928 1205
0 13 87 0 0| 0 0 | 388B 2435B| 0 0 |2245 1280
0 13 87 0 0| 0 0 | 268B 1145B| 0 0 |2225 1221
0 13 87 0 0| 0 0 | 164B 732B| 0 0 |2607 1333
0 13 87 0 0| 0 0 | 156B 996B| 0 0 |2100 1270
0 13 87 0 0| 0 0 | 206B 1019B| 0 0 |2192 1296
0 13 87 0 0| 0 0 | 198B 824B| 0 0 |2019 1236
0 13 87 0 0| 0 0 | 245B 435B| 0 0 |1974 1195
0 13 87 0 0| 0 0 | 252B 855B| 0 0 |1852 1166
0 13 87 0 0| 0 0 | 230B 758B| 0 0 |2066 1299
0 13 87 0 0| 0 0 | 284B 925B| 0 0 |1860 1225
0 13 87 0 0| 0 0 | 289B 2682B| 0 0 |1796 1197
0 13 87 0 0| 0 0 | 939B 1263B| 0 0 |1913 1304
0 13 87 0 0| 0 0 |1212B 1366B| 0 0 |1915 1343
I will try running the command under the debugger and stop it when it gets to this point and poke around.
florin
^ permalink raw reply
* [PATCH] shortlog: pass the mailmap into the revision walker
From: CB Bailey @ 2018-12-12 16:41 UTC (permalink / raw)
To: git
From: CB Bailey <cbailey32@bloomberg.net>
shortlog always respects the mailmap in its output. Pass the mailmap
into the revision walker to allow the mailmap to be used with revision
limiting options such as '--author'.
This removes some apparently inconsistent behaviors when using
'--author', such as not finding some or all commits for a given author
which do appear under that author in an unrestricted invocation of
shortlog or commits being summarized under a different author than the
specified author.
---
builtin/shortlog.c | 2 ++
t/t4203-mailmap.sh | 28 ++++++++++++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 88f88e97b2..a6fb00ade8 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -188,6 +188,8 @@ static void get_from_rev(struct rev_info *rev, struct shortlog *log)
{
struct commit *commit;
+ rev->mailmap = &log->mailmap;
+
if (prepare_revision_walk(rev))
die(_("revision walk setup failed"));
while ((commit = get_revision(rev)) != NULL)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 43b1522ea2..9bee35b06c 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -383,6 +383,34 @@ test_expect_success 'Shortlog output (complex mapping)' '
'
+test_expect_success 'Shortlog output (complex mapping, filtered)' '
+
+ printf " 1\tA U Thor <author@example.com>\n" >expect &&
+
+ git shortlog -es --author="A U Thor" HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 1\tCTO <cto@company.xx>\n" >expect &&
+
+ git shortlog -es --author=CTO HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 2\tOther Author <other@author.xx>\n" >expect &&
+
+ git shortlog -es --author="Other Author" HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 2\tSanta Claus <santa.claus@northpole.xx>\n" >expect &&
+
+ git shortlog -es --author="Santa Claus" HEAD >actual &&
+ test_cmp expect actual &&
+
+ printf " 1\tSome Dude <some@dude.xx>\n" >expect &&
+
+ git shortlog -es --author="Some Dude" HEAD >actual &&
+ test_cmp expect actual
+'
+
# git log with --pretty format which uses the name and email mailmap placemarkers
cat >expect <<\EOF
Author CTO <cto@coompany.xx> maps to CTO <cto@company.xx>
--
2.17.0.rc0
^ permalink raw reply related
* Re: [PATCH] run-command: report exec failure
From: John Passaro @ 2018-12-12 15:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes.Schindelin, git, Jeff King
In-Reply-To: <xmqqsgz4jkgl.fsf@gitster-ct.c.googlers.com>
Thank you for this incredibly quick fix.
I see the fix made it to pu as 6b206be3e5 ("run-command: report exec
failure" 2018-12-11). For what it's worth, it fixes the issue as far
as I'm concerned and I'm very glad to see the behavior is covered by
tests now.
As a procedural question: I'd like to reference this patch in one of
my own. Can I reference it as I typed it above? Or is there a chance
of the SHA1 changing before it goes into some sort of a main history?
John Passaro
(917) 678-8293
On Tue, Dec 11, 2018 at 7:50 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > This breaks on Windows (on Windows, the error message says "cannot spawn", see
>
> Thanks for a quick feedback. Let's update to look for the pathname
> of the command, as Peff suggested earlier.
^ permalink raw reply
* Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references
From: Tejun Heo @ 2018-12-12 14:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Junio C Hamano, Jeff King, kernel-team, Stefan Xenos
In-Reply-To: <xmqqo99rjjcu.fsf@gitster-ct.c.googlers.com>
Hello, Junio.
On Wed, Dec 12, 2018 at 04:26:57PM +0900, Junio C Hamano wrote:
> It also is not immediately obvious to me what your general strategy
> to maintain this reverse mapping is, when new ways and codepaths to
> cause new commits with "cherry-picked-from" trailer appear. Do we
> keep piling on new hooks? Or is the reverse mapping information
So, what I actually wanted was "this repo now has these new commits"
hook, but that didn't seem to be in line with other hooks as most were
bound to specific directly user-visible commands and actions.
> allowed to be a bit stale and be completed immediately before it
> gets used? A totally different approach could be to record list of
> commits, all commits behind which have been scanned for reverse
> mapping, in the tip of the notes history, perhaps in the commit log
> message (which is machine generated anyway). Then, before you need
> the up-to-date-to-the-last-second reverse mapping, you could run
>
> git rev-list --all --not $these_tips_recorded
Wouldn't it be more useful to have repo-updated-with-these-commits
hook instead rather than putting more logic on note handling?
> and scan the commits, just like you scan what you fetched. And when
> you update the reverse mapping notes tree, the commit to record that
> notes update can record the tip of the above traverseal.
>
> I also wonder how this topic and the topic Stefan Xenos has been
> working on (cf. <20181201005240.113210-1-sxenos@google.com>) can
> benefit from each other by cross pollination. Stefan's topic also
> needs to answer, given a commit, what other commits were created out
> of it by cherry-picking and then further amending the results, so
> that when the original commit itself gets amended, the cherry-picked
> ones that were created from the original can be easily identified
> and get updated in the same way as necessary.
Ah, I see, forward-propagating amends to cherry-picks.
> The motivating workflow your topic's cover letter gives us is to
> maintain the release branch that goes only forward, so in that
> context, how a commit on the release branch that was created by
> cherry-picking an original gets updated when the original commit
> gets amended would be different (I am assuming that you cannot
At least we don't do this with our trees and most trees that I've
worked with don't either as that would make it really confusing for
people to work together.
> affored to rebase the release branch to update a deeply buried
> commit that was an earlier cherry-pick), but I would imagine that
> both topics share the need for a good data structure to keep track
> of (1) the relationship between the original and copy of the
> cherry-pick and (2) the fact that the original of such a cherry-pick
> is now stale and the copy might want to get updated.
As long as we can keep the reverse rference notes consistent, wouldn't
amend propagation just consume them?
Thanks.
--
tejun
^ permalink raw reply
* Re: High locking contention during repack?
From: Ævar Arnfjörð Bjarmason @ 2018-12-12 14:04 UTC (permalink / raw)
To: Jeff King; +Cc: Iucha, Florin, git@vger.kernel.org
In-Reply-To: <20181212112409.GB30673@sigill.intra.peff.net>
On Wed, Dec 12 2018, Jeff King wrote:
> On Wed, Dec 12, 2018 at 03:01:47AM +0000, Iucha, Florin wrote:
>
>> I am running “git-repack -A -d -f -F --window=250 --depth=250” on a
>> Git repository converted using git-svn.
>
> Sort of tangential to your question, but:
>
> - Using "-F" implies "-f" already, so you don't need both. That said,
> you are probably wasting CPU to use "-F", unless you have adjusted
> zlib compression settings since the last pack. (Whereas using "-f"
> is useful, if you're willing to pay the CPU tradeoff).
>
> - Using --depth=250 does not actually decrease the packfile size very
> much, and results in a packfile which is more expensive for
> subsequent processes to use. Some experimentation showed that
> --depth=50 is a sweet spot, and that is the default for both normal
> "git gc" and "git gc --aggressive" these days.
>
> See 07e7dbf0db (gc: default aggressive depth to 50, 2016-08-11) for
> more discussion.
I wonder if the size is still too high. I'd rather take the 5-10%
speedup quoted in that commit than a slight decrease in disk space use.
The git.git repository now with the repack command in that commit
message, with --depth=$n:
573M /tmp/git-1
200M /tmp/git-2
118M /tmp/git-3
91M /tmp/git-4
82M /tmp/git-5
77M /tmp/git-6
74M /tmp/git-7
72M /tmp/git-8
71M /tmp/git-9
70M /tmp/git-10
67M /tmp/git-15
66M /tmp/git-20
65M /tmp/git-40
65M /tmp/git-35
65M /tmp/git-30
65M /tmp/git-25
64M /tmp/git-50
64M /tmp/git-45
Produced via:
parallel 'rm -rf /tmp/git-{}; cp -Rvp /tmp/git.git/ /tmp/git-{} && git -C /tmp/git-{} repack -adf --depth={} --window=250' ::: {1..10} 15 20 25 30 35 40 45 50
And then the log -S benchmarks:
s/iter log -S 1 log -S 50 log -S 45 log -S 35 log -S 30 log -S 25 log -S 20 log -S 15 log -S 10 log -S 5
log -S 1 12.6 -- -26% -27% -32% -36% -38% -38% -39% -40% -42%
log -S 50 9.28 36% -- -0% -7% -13% -15% -16% -18% -19% -21%
log -S 45 9.25 36% 0% -- -7% -12% -15% -16% -17% -19% -20%
log -S 35 8.62 46% 8% 7% -- -6% -9% -10% -11% -13% -14%
log -S 30 8.12 55% 14% 14% 6% -- -3% -4% -6% -8% -9%
log -S 25 7.86 60% 18% 18% 10% 3% -- -1% -3% -5% -6%
log -S 20 7.77 62% 19% 19% 11% 4% 1% -- -2% -3% -5%
log -S 15 7.64 65% 21% 21% 13% 6% 3% 2% -- -2% -4%
log -S 10 7.51 68% 24% 23% 15% 8% 5% 3% 2% -- -2%
log -S 5 7.37 71% 26% 25% 17% 10% 7% 5% 4% 2% --
So we're getting a 20% speedup in log -S by using --depth=5 instead of
--depth=50, neatly at the cost of 20% more disk space, but could also
get a 19% speedup for 8% (--depth=10) more disk space, etc.
Then in rev-list it makes less of a difference, narrowing this down a
bit:
s/iter rev-list 50 rev-list 25 rev-list 10 rev-list 5
rev-list 50 1.61 -- -1% -4% -5%
rev-list 25 1.60 1% -- -3% -4%
rev-list 10 1.55 4% 3% -- -1%
rev-list 5 1.54 5% 4% 1% --
It seems to me we should at least move this to --depth=25 by default. A
~15% speedup in log -S & other mass-blob lookups for 1.5% more disk
space seems like a good trade-off. As your commit notes RAM (or disk
space) is not commonly the bottleneck anymore.
>> The system is a 16 core / 32 thread Threadripper with 128GB of RAM and
>> NVMe storage. The repack starts strong, with 32 threads but it fairly
>> quickly gets to 99% done and the number of threads drops to 4 then 3
>> then 2. However, running “dstat 5” I see lots of “sys” time without
>> any IO time (the network traffic you see is caused by SSH).
>
> This sounds mostly normal and expected. The parallel part of a repack is
> the delta search, which is not infinitely parallelizable. Each worker
> thread is given a "chunk" of objects, and it uses a sliding window to
> search for delta pairs through that chunk. You don't want a chunk that
> approaches the window size, since at every chunk boundary you're missing
> delta possibilities.
>
> The default chunk size is about 1/nr_threads of the total list size
> (i.e., we portion out all the work). And then when a thread finishes, we
> take work from the thread with the most work remaining, and portion it
> out. However, at some point the chunks approach their minimum, and we
> stop dividing. So the number of threads will drop, eventually to 1, and
> you'll be waiting on it to finish that final chunk.
>
> So that's all working as planned. Having high sys load does seem a bit
> odd. Most of the effort should be going to reading the mmap'd data from
> disk, zlib-inflating it and computing a fingerprint, and then comparing
> the fingerprints. So that would mostly be user time.
>
>> Running a strace on the running git-repack process shows only these:
>> --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---
>> --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---
>> --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---
>> --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---
>> --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---
>> --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---
>>
>> Any idea on how to debug this? I have ran git-repack under gdb, but it seems to spin on builtin/repack.c line 409.
>
> The heavy lifting here is done by the pack-objects child process, not
> git-repack itself. Try running with GIT_TRACE=1 in the environment to
> see the exact invocation, but timing and debugging:
>
> git pack-objects --all --no-reuse-delta --delta-base-offset --stdout \
> </dev/null >/dev/null
>
> should produce interesting results.
>
> The SIGALRM loop you see above is likely just the progress meter
> triggering once per second (the actual worker threads are updating an
> int, and then at least once per second we'll show the int).
>
> -Peff
^ permalink raw reply
* Re: [PATCH v2 1/3] git clone <url> C:\cygwin\home\USER\repo' is working (again)
From: Johannes Schindelin @ 2018-12-12 13:57 UTC (permalink / raw)
To: Elijah Newren; +Cc: svnpenn, Torsten Bögershausen, Git Mailing List
In-Reply-To: <CABPp-BHqJ_Dqpi-N-GVu9difvh-SnD1SZ2-SqaG0ctu5fBX-Tw@mail.gmail.com>
Hi Elijah,
On Tue, 11 Dec 2018, Elijah Newren wrote:
> I'm worried based on other emails in this thread that there is a
> fundamental difference in frame of reference leading to a
> misunderstanding about rationale for naming, and worse that folks might
> not even realize where the misunderstanding is coming from, attributing
> it to different motives rather than different frames of reference. If
> that's true, I hope this email can begin the process of clearing up the
> differences of understanding. If I'm wrong, then I apologize for the
> noise; just ignore me.
I think you brought up quite a few good points (also in the part that I
did not quote).
The part I quoted brings up one particular aspect that I would like to
drive home a little more: the purpose of naming, and the historical
reality. ("hysterical raisins" comes to mind.)
In Git, we have an awful lot of references to MINGW, which is the name of
a project that tried to allow compiling software targeting pure Windows
(i.e. the Win32 API, without any POSIX compatibility layer) with the GNU C
compiler.
As many open source projects require more than just the GNU C compiler
(e.g. a Bash to run ./configure), there is also MSys, which is a minimal
fork of a then-current version of Cygwin, originally intended for the sole
purpose to support building MINGW software.
To make things more confusing, at some stage the mingw-w64 project was
started (not as a fork of MINGW, AFAIU), to address the notable lack of
64-bit support in MINGW, and later the MSYS2 project was started, based on
mingw-w64, to address the same issue with MSys (also not forking, but
instead starting from scratch).
Back in 2006, when I started to port Git to Windows, I made use of MINGW
and MSys (and I abused MSys by shipping their Bash with Git, which was
distinctly not intended a usage of their Bash). Hannes Sixt picked up when
I stopped having access to a fast Windows machine, and kept my naming:
compat/mingw.c.
Now, Philip Oakley, Jeff Hostetler and a few other developers spent quite
a bit of effort to make Git compile also with Visual C, and of course the
reused parts of compat/mingw.c (whose name now does not make too much
sense anymore, except in historical context).
Likewise, when I switched to MSYS2/mingw-w64 with the major version jump
to Git for Windows 2.x in 2015, I no longer use MINGW to compile
*anything*.
I hope that this illustrated a little bit how our names came about.
Of course, we could now spend some time to change the names to reflect
more the product and brand names involved. There does not seem to be any
really compelling reason to do so, though. And I'd rather spend my time
developing exciting features. But that's my preference for my time, so if
anybody comes along, making a strong case for renaming, in a well-crafted
patch series, who am I to say no to that.
Ciao,
Dscho
^ permalink raw reply
* Re: Difficulty with parsing colorized diff output
From: Jeff King @ 2018-12-12 13:52 UTC (permalink / raw)
To: George King; +Cc: Ævar Arnfjörð Bjarmason, Stefan Beller, git
In-Reply-To: <A97DD550-BE35-43BA-A181-708B7D065F3F@gmail.com>
On Tue, Dec 11, 2018 at 01:55:09PM -0500, George King wrote:
> I just noticed that while `wsErrorHighlight = none` fixes the problem
> of extra green codes for regular diff, it fails to have any effect
> during interactive `git add -p`.
This is due to the way add--interactive invokes diff. It uses the
plumbing commands (diff-tree, diff-files, etc), which do not respect
config which changes the output (since their purpose is to provide
stable machine-readable output).
But add--interactive does a slightly funny thing: it runs each diff
twice. Once to get the machine-readable version, and once to get a
colorized version which it shows to the user. When it does the latter,
it has to manually enable other options (e.g., passing --color as
appropriate, or passing along diff.algorithm).
So the matching behavior here would be for it to look as
wsErrorHighlight and pass it along as a command line option.
-Peff
^ permalink raw reply
* Re: [PATCH 1/1] .gitattributes: ensure t/oid-info/* has eol=lf
From: SZEDER Gábor @ 2018-12-12 13:39 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget; +Cc: git, Junio C Hamano, Derrick Stolee
In-Reply-To: <4a22502a318a65f144b3b6542cc5e711a1811c78.1544560544.git.gitgitgadget@gmail.com>
On Tue, Dec 11, 2018 at 12:35:46PM -0800, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <dstolee@microsoft.com>
>
> The new test_oid machinery in the test library requires reading
> some information from t/oid-info/hash-info and t/oid-info/oid.
> The shell logic that reads from these files is sensitive to CRLF
> line endings, causing a problem when the test suite is run on a
> Windows machine that converts LF to CRLF.
"What problem?" is what people will ask when they read this commit
message in the future.
Please include the relevant details in the commit message instead of
the cover letter.
> Exclude the files in this folder from this conversion.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
> .gitattributes | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/.gitattributes b/.gitattributes
> index acf853e029..3738cea7eb 100644
> --- a/.gitattributes
> +++ b/.gitattributes
> @@ -13,3 +13,4 @@
> /Documentation/gitk.txt conflict-marker-size=32
> /Documentation/user-manual.txt conflict-marker-size=32
> /t/t????-*.sh conflict-marker-size=32
> +/t/oid-info/* eol=lf
> --
> gitgitgadget
^ permalink raw reply
* Re: [PATCH v2 1/3] git clone <url> C:\cygwin\home\USER\repo' is working (again)
From: Johannes Schindelin @ 2018-12-12 13:33 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Steven Penny, git
In-Reply-To: <be24f331-5c8f-954d-e6f5-d5b09ee4e5f3@kdbg.org>
Hi Hannes,
On Wed, 12 Dec 2018, Johannes Sixt wrote:
> Am 12.12.18 um 01:42 schrieb Steven Penny:
> > On Tue, Dec 11, 2018 at 7:39 AM Johannes Schindelin wrote:
> > > > - pc-windows
> > > > - pc-win
> > > > - win
> > >
> > > I find all of those horrible.
> >
> > one windows triplet in use is "x86_64-pc-windows", used by Rust:
> >
> > https://forge.rust-lang.org/other-installation-methods.html
> >
> > which is how i came about my suggestions - again they arent great but they
> > arent
> > misleading as "Win32" is.
>
> As long as C:\Windows\System32 on my Windows computer contains only 64-Bit
> binaries, I consider the characters "3" and "2" next to each other in this
> order just noise and without any form of information. The important part of
> the name is "win".
FWIW I agree with you.
Thanks,
Dscho
^ permalink raw reply
* Re: [PATCH 1/8] move worktree tests to t24*
From: Eric Sunshine @ 2018-12-12 13:26 UTC (permalink / raw)
To: Thomas Gummerer
Cc: Nguyễn Thái Ngọc Duy, Git List, Junio C Hamano,
Elijah Newren
In-Reply-To: <20181211215019.GO4883@hank.intra.tgummerer.com>
On Tue, Dec 11, 2018 at 4:50 PM Thomas Gummerer <t.gummerer@gmail.com> wrote:
> On 12/10, Duy Nguyen wrote:
> > On Sun, Dec 9, 2018 at 9:04 PM Thomas Gummerer <t.gummerer@gmail.com> wrote:
> > > Move the worktree tests to t24* to adhere to that guideline. We're
> > > going to make use of the free'd up numbers in a subsequent commit.
> >
> > Heh.. I did the same thing (in my unsent switch-branch/restore-files
> > series) and even used the same 24xx range :D You probably want to move
> > t2028 and t2029 too (not sure if they have landed on 'master')
>
> [...] good to know someone
> thought the same way. I started this work before t2028 and t2029
> landed on master, so I failed to notice them.
The thought of renumbering the test script came up as early as
2015-06-30. See the last bullet point of [1], for instance.
[1]: https://public-inbox.org/git/1435640202-95945-1-git-send-email-sunshine@sunshineco.com/
^ permalink raw reply
* Re: Difficulty with parsing colorized diff output
From: Jeff King @ 2018-12-12 12:49 UTC (permalink / raw)
To: George King; +Cc: Ævar Arnfjörð Bjarmason, Stefan Beller, git
In-Reply-To: <EB1AF739-F97B-4905-9736-2A003722AD9A@gmail.com>
On Tue, Dec 11, 2018 at 11:41:18AM -0500, George King wrote:
> I first started playing around with terminal colors about 5 years ago,
> and I recall learning the hard way that Apple Terminal at least
> behaves very strangely when you have background colors cross line
> boundaries: background colors disappeared when I scrolled lines back
> into view. I filed a bug thinking it couldn't be right and Apple
> closed it as behaving according to compatibility expectations. I never
> figured out whether they had misunderstood my report or if old
> terminals were just that crazy. Instead I decided that the safe thing
> to do was reset after every line. Perhaps some git author reached the
> same conclusion.
Yes, that's exactly it. Certain codes do not do well as they cross
lines. It's been long enough that I don't remember the details, and a
quick grep of the archive found too many results for me to bother wading
through.
-Peff
^ permalink raw reply
* Re: [PATCH v2 1/3] git clone <url> C:\cygwin\home\USER\repo' is working (again)
From: Steven Penny @ 2018-12-12 12:40 UTC (permalink / raw)
To: j6t; +Cc: git
In-Reply-To: <be24f331-5c8f-954d-e6f5-d5b09ee4e5f3@kdbg.org>
On Wed, Dec 12, 2018 at 1:29 AM Johannes Sixt wrote:
> As long as C:\Windows\System32 on my Windows computer contains only
> 64-Bit binaries, I consider the characters "3" and "2" next to each
> other in this order just noise and without any form of information. The
> important part of the name is "win".
sorry friend, but thats a logical fallacy :(
http://yourlogicalfallacyis.com/no-true-scotsman
just because the name "System32" is still in use (wrongly, I might add) doesnt
mean that "Win32" should still be in use.
Each name is a separate argument. The "Win32" name has been changed by Microsoft
and shouldnt be used by Git if possible. Its an easy change and I could send
a pull request myself. However just because Microsoft hasnt changed "Sytem32"
doesnt mean that they wont or shouldnt - as you said its just as misleading. We
should fix ambiguities where we can, not embrace them.
^ permalink raw reply
* Re: High locking contention during repack?
From: Jeff King @ 2018-12-12 11:24 UTC (permalink / raw)
To: Iucha, Florin; +Cc: git@vger.kernel.org
In-Reply-To: <SN1PR12MB23840AFE62E41D908A40D1B095A70@SN1PR12MB2384.namprd12.prod.outlook.com>
On Wed, Dec 12, 2018 at 03:01:47AM +0000, Iucha, Florin wrote:
> I am running “git-repack -A -d -f -F --window=250 --depth=250” on a
> Git repository converted using git-svn.
Sort of tangential to your question, but:
- Using "-F" implies "-f" already, so you don't need both. That said,
you are probably wasting CPU to use "-F", unless you have adjusted
zlib compression settings since the last pack. (Whereas using "-f"
is useful, if you're willing to pay the CPU tradeoff).
- Using --depth=250 does not actually decrease the packfile size very
much, and results in a packfile which is more expensive for
subsequent processes to use. Some experimentation showed that
--depth=50 is a sweet spot, and that is the default for both normal
"git gc" and "git gc --aggressive" these days.
See 07e7dbf0db (gc: default aggressive depth to 50, 2016-08-11) for
more discussion.
> The system is a 16 core / 32 thread Threadripper with 128GB of RAM and
> NVMe storage. The repack starts strong, with 32 threads but it fairly
> quickly gets to 99% done and the number of threads drops to 4 then 3
> then 2. However, running “dstat 5” I see lots of “sys” time without
> any IO time (the network traffic you see is caused by SSH).
This sounds mostly normal and expected. The parallel part of a repack is
the delta search, which is not infinitely parallelizable. Each worker
thread is given a "chunk" of objects, and it uses a sliding window to
search for delta pairs through that chunk. You don't want a chunk that
approaches the window size, since at every chunk boundary you're missing
delta possibilities.
The default chunk size is about 1/nr_threads of the total list size
(i.e., we portion out all the work). And then when a thread finishes, we
take work from the thread with the most work remaining, and portion it
out. However, at some point the chunks approach their minimum, and we
stop dividing. So the number of threads will drop, eventually to 1, and
you'll be waiting on it to finish that final chunk.
So that's all working as planned. Having high sys load does seem a bit
odd. Most of the effort should be going to reading the mmap'd data from
disk, zlib-inflating it and computing a fingerprint, and then comparing
the fingerprints. So that would mostly be user time.
> Running a strace on the running git-repack process shows only these:
> --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---
> --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---
> --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---
> --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---
> --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---
> --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} ---
>
> Any idea on how to debug this? I have ran git-repack under gdb, but it seems to spin on builtin/repack.c line 409.
The heavy lifting here is done by the pack-objects child process, not
git-repack itself. Try running with GIT_TRACE=1 in the environment to
see the exact invocation, but timing and debugging:
git pack-objects --all --no-reuse-delta --delta-base-offset --stdout \
</dev/null >/dev/null
should produce interesting results.
The SIGALRM loop you see above is likely just the progress meter
triggering once per second (the actual worker threads are updating an
int, and then at least once per second we'll show the int).
-Peff
^ permalink raw reply
* Re: Minor(?) usability issue with branch.<name>.pushRemote
From: Sergey Organov @ 2018-12-12 11:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqh8fjhy8k.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com> writes:
>
>> So, finally, it's 'branch.linux.pushremote' that is the "offender".
>>
>> Looks like both 'git status' and 'git branch -vv' should somehow learn
>> about 'branch.<name>.pushremote' feature so that their
>> output/suggestions make more sense?
>
> Doesn't "ahead of X by N" mean "you forked from X and built N commits
> on top", not "you have N commits that is not in X which is where you
> would push to"?
Sure, but the problem is that 'git status' gives exact suggestion:
(use "git push" to publish your local commits)
that is somewhat misleading in this particular case ('git push' is now a
no-op), and then 'git branch -vv', while tells me relationship to
"upstream", keeps silence about non-matching push destination.
To add even more complexity to the case, notice that the first time
after I committed locally, the invocation of 'git push' (exactly as
suggested by 'git status') did successfully push those two commits, so
the suggestion is exactly right before the commits are pushed, yet
is rather misleading afterwards.
--
Sergey
^ permalink raw reply
* Re: [PATCH v3 1/4] pack-protocol.txt: accept error packets in any context
From: Jeff King @ 2018-12-12 11:02 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git, gitster, masayasuzuki
In-Reply-To: <df7d3659ae5f11d163f1e992f3b9403be709ddb7.1544572142.git.steadmon@google.com>
On Tue, Dec 11, 2018 at 04:25:15PM -0800, Josh Steadmon wrote:
> From: Masaya Suzuki <masayasuzuki@google.com>
>
> In the Git pack protocol definition, an error packet may appear only in
> a certain context. However, servers can face a runtime error (e.g. I/O
> error) at an arbitrary timing. This patch changes the protocol to allow
> an error packet to be sent instead of any packet.
>
> Following this protocol spec change, the error packet handling code is
> moved to pkt-line.c.
This is a change in the spec with an accompanying change in the code,
which raises the question: what do other implementations do with this
change (both older Git, and implementations like JGit, libgit2, etc)?
I think the answer for older Git is "hang up unceremoniously", which is
probably OK given the semantics of the change. And I'd suspect most
other implementations would do the same. I just wonder if anybody tested
it with other implementations.
> +An error packet is a special pkt-line that contains an error string.
> +
> +----
> + error-line = PKT-LINE("ERR" SP explanation-text)
> +----
> +
> +Throughout the protocol, where `PKT-LINE(...)` is expected, an error packet MAY
> +be sent. Once this packet is sent by a client or a server, the data transfer
> +process defined in this protocol is terminated.
The packfile data is typically packetized, too, and contains arbitrary
data (that could have "ERR" in it). It looks like we don't specifically
say PKT-LINE() in that part of the protocol spec, though, so I think
this is OK.
Likewise, in the implementation:
> diff --git a/pkt-line.c b/pkt-line.c
> index 04d10bbd03..ce9e42d10e 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -346,6 +346,10 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
> return PACKET_READ_EOF;
> }
>
> + if (starts_with(buffer, "ERR ")) {
> + die(_("remote error: %s"), buffer + 4);
> + }
> +
> if ((options & PACKET_READ_CHOMP_NEWLINE) &&
> len && buffer[len-1] == '\n')
> len--;
This ERR handling has been moved to a very low level. What happens if
we're passing arbitrary data via the packet_read() code? Could we
erroneously trigger an error if a packfile happens to have the bytes
"ERR " at a packet boundary?
For packfiles via upload-pack, I _think_ we're OK, because we only
packetize it when a sideband is in use. In which case this would never
match, because we'd have "\1" in the first byte slot.
But are there are other cases we need to worry about? Just
brainstorming, I can think of:
1. We also pass packetized packfiles between git-remote-https and
the stateless-rpc mode of fetch-pack/send-pack. And I don't think
we use sidebands there.
2. The packet code is used for long-lived clean/smudge filters these
days, which also pass arbitrary data.
So I think it's probably not a good idea to unconditionally have callers
of packet_read_with_status() handle this. We'd need a flag like
PACKET_READ_RESPECT_ERR, and to trigger it from the appropriate callers.
-Peff
^ 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