Commit Graph

17099 Commits

Author SHA1 Message Date
Nipunn Koorapati
1c6833c800 t/perf/fsmonitor: add benchmark for dirty status
This benchmark covers the git status time for a heavily
dirty directory - benchmarking fsmonitor's refresh

When running to compare our perl vs rs-git-fsmonitor - we see that
the perl script incurs significant overhead - further motivation
to provide a faster implementation within git.

7519.7: status (dirty) (fsmonitor=query-watchman) 10.05(7.78+1.56)
7519.20: status (dirty) (fsmonitor=rs-git-fsmonitor) 6.72(4.37+1.64)
7519.33: status (dirty) (fsmonitor=disabled) 5.62(4.24+2.03)

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-26 16:39:34 -07:00
Nipunn Koorapati
a948864ae7 t/perf/fsmonitor: perf comparison of multiple fsmonitor integrations
Allows for simple perf comparison of different integrations. I ran it
to compare our perl script w/ rs-git-fsmonitor and found 20-30ms of
overhead on every command.

Output looks like this (extra newlines added for readability)

Test                                                        this tree
---------------------------------------------------------------------------
7519.4: status (fsmonitor=query-watchman)                   0.42(0.37+0.05)
7519.5: status -uno (fsmonitor=query-watchman)              0.19(0.12+0.07)
7519.6: status -uall (fsmonitor=query-watchman)             1.36(0.73+0.62)
7519.7: diff (fsmonitor=query-watchman)                     0.14(0.09+0.05)
7519.8: diff -- 0_files (fsmonitor=query-watchman)          0.14(0.11+0.03)
7519.9: diff -- 10_files (fsmonitor=query-watchman)         0.14(0.10+0.04)
7519.10: diff -- 100_files (fsmonitor=query-watchman)       0.14(0.09+0.05)
7519.11: diff -- 1000_files (fsmonitor=query-watchman)      0.14(0.08+0.06)
7519.12: diff -- 10000_files (fsmonitor=query-watchman)     0.14(0.09+0.05)
7519.13: add (fsmonitor=query-watchman)                     2.04(1.32+0.66)

7519.16: status (fsmonitor=rs-git-fsmonitor)                0.39(0.32+0.08)
7519.17: status -uno (fsmonitor=rs-git-fsmonitor)           0.17(0.11+0.06)
7519.18: status -uall (fsmonitor=rs-git-fsmonitor)          1.33(0.71+0.61)
7519.19: diff (fsmonitor=rs-git-fsmonitor)                  0.11(0.07+0.04)
7519.20: diff -- 0_files (fsmonitor=rs-git-fsmonitor)       0.11(0.09+0.03)
7519.21: diff -- 10_files (fsmonitor=rs-git-fsmonitor)      0.11(0.09+0.03)
7519.22: diff -- 100_files (fsmonitor=rs-git-fsmonitor)     0.11(0.07+0.04)
7519.23: diff -- 1000_files (fsmonitor=rs-git-fsmonitor)    0.11(0.06+0.06)
7519.24: diff -- 10000_files (fsmonitor=rs-git-fsmonitor)   0.11(0.06+0.06)
7519.25: add (fsmonitor=rs-git-fsmonitor)                   2.03(1.28+0.69)

7519.28: status (fsmonitor=disabled)                        0.77(0.59+0.99)
7519.29: status -uno (fsmonitor=disabled)                   0.42(0.33+0.85)
7519.30: status -uall (fsmonitor=disabled)                  1.59(1.02+1.34)
7519.31: diff (fsmonitor=disabled)                          0.35(0.30+0.81)
7519.32: diff -- 0_files (fsmonitor=disabled)               0.11(0.08+0.04)
7519.33: diff -- 10_files (fsmonitor=disabled)              0.11(0.07+0.04)
7519.34: diff -- 100_files (fsmonitor=disabled)             0.11(0.08+0.03)
7519.35: diff -- 1000_files (fsmonitor=disabled)            0.11(0.10+0.02)
7519.36: diff -- 10000_files (fsmonitor=disabled)           0.12(0.07+0.06)
7519.37: add (fsmonitor=disabled)                           2.24(1.48+1.44)

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-26 16:39:34 -07:00
Nipunn Koorapati
6cba4234a5 t/perf/fsmonitor: initialize test with git reset
Previously, the git add of the previous suiterun would
pollute the numbers in the second run

Before:
Test                                                          this tree
-----------------------------------------------------------------------------
7519.4: status (fsmonitor=fsmonitor-watchman)                 0.40(0.36+0.04)
7519.5: status -uno (fsmonitor=fsmonitor-watchman)            0.19(0.12+0.07)
7519.6: status -uall (fsmonitor=fsmonitor-watchman)           1.36(0.74+0.61)
7519.7: diff (fsmonitor=fsmonitor-watchman)                   0.14(0.10+0.04)
7519.8: diff -- 0_files (fsmonitor=fsmonitor-watchman)        0.14(0.10+0.04)
7519.9: diff -- 10_files (fsmonitor=fsmonitor-watchman)       0.14(0.09+0.05)
7519.10: diff -- 100_files (fsmonitor=fsmonitor-watchman)     0.14(0.10+0.04)
7519.11: diff -- 1000_files (fsmonitor=fsmonitor-watchman)    0.14(0.08+0.06)
7519.12: diff -- 10000_files (fsmonitor=fsmonitor-watchman)   0.14(0.10+0.04)
7519.13: add (fsmonitor=fsmonitor-watchman)                   2.03(1.28+0.69)
7519.16: status (fsmonitor=disabled)                          0.64(0.49+0.90)
7519.17: status -uno (fsmonitor=disabled)                     1.15(0.92+1.00)
7519.18: status -uall (fsmonitor=disabled)                    2.32(1.46+1.55)
7519.19: diff (fsmonitor=disabled)                            1.44(1.12+1.76)
7519.20: diff -- 0_files (fsmonitor=disabled)                 0.11(0.07+0.05)
7519.21: diff -- 10_files (fsmonitor=disabled)                0.11(0.06+0.05)
7519.22: diff -- 100_files (fsmonitor=disabled)               0.11(0.08+0.03)
7519.23: diff -- 1000_files (fsmonitor=disabled)              0.11(0.08+0.04)
7519.24: diff -- 10000_files (fsmonitor=disabled)             0.12(0.06+0.07)
7519.25: add (fsmonitor=disabled)                             2.25(1.47+1.47)

After:
Test                                                          this tree
-----------------------------------------------------------------------------
7519.4: status (fsmonitor=fsmonitor-watchman)                 0.41(0.33+0.09)
7519.5: status -uno (fsmonitor=fsmonitor-watchman)            0.20(0.14+0.07)
7519.6: status -uall (fsmonitor=fsmonitor-watchman)           1.37(0.78+0.58)
7519.7: diff (fsmonitor=fsmonitor-watchman)                   0.14(0.10+0.04)
7519.8: diff -- 0_files (fsmonitor=fsmonitor-watchman)        0.14(0.08+0.06)
7519.9: diff -- 10_files (fsmonitor=fsmonitor-watchman)       0.14(0.09+0.05)
7519.10: diff -- 100_files (fsmonitor=fsmonitor-watchman)     0.14(0.10+0.05)
7519.11: diff -- 1000_files (fsmonitor=fsmonitor-watchman)    0.14(0.11+0.04)
7519.12: diff -- 10000_files (fsmonitor=fsmonitor-watchman)   0.14(0.09+0.05)
7519.13: add (fsmonitor=fsmonitor-watchman)                   2.04(1.27+0.71)
7519.16: status (fsmonitor=disabled)                          0.78(0.59+0.99)
7519.17: status -uno (fsmonitor=disabled)                     0.43(0.32+0.88)
7519.18: status -uall (fsmonitor=disabled)                    1.58(0.96+1.38)
7519.19: diff (fsmonitor=disabled)                            0.36(0.31+0.79)
7519.20: diff -- 0_files (fsmonitor=disabled)                 0.11(0.08+0.03)
7519.21: diff -- 10_files (fsmonitor=disabled)                0.11(0.07+0.04)
7519.22: diff -- 100_files (fsmonitor=disabled)               0.11(0.08+0.04)
7519.23: diff -- 1000_files (fsmonitor=disabled)              0.11(0.07+0.05)
7519.24: diff -- 10000_files (fsmonitor=disabled)             0.12(0.08+0.05)
7519.25: add (fsmonitor=disabled)                             2.25(1.48+1.47)

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-26 16:39:34 -07:00
Nipunn Koorapati
a05b71ab91 t/perf/fsmonitor: factor setup for fsmonitor into function
This prepares for it being called multiple times when
testing different hooks

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-26 16:39:34 -07:00
Nipunn Koorapati
78ff8b3236 t/perf/fsmonitor: silence initial git commit
It is extremely verbose, printing >10K non-useful lines

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-26 16:39:34 -07:00
Nipunn Koorapati
dd79c16746 t/perf/fsmonitor: shorten DESC to basename
The full name is lengthy and makes it hard to read
Before:
7519.3: status (fsmonitor=/home/nipunn/src/server/.git/hooks/rs-git-fsmonitor) 0.02(0.01+0.00)

After
7519.3: status (fsmonitor=rs-git-fsmonitor) 0.03(0.02+0.00)

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-26 16:39:34 -07:00
Nipunn Koorapati
3d53ebcd10 t/perf/fsmonitor: factor description out for readability
There was much duplication here. Prepares for making
changes to the description.

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-26 16:39:34 -07:00
Nipunn Koorapati
33226af42b t/perf/fsmonitor: improve error message if typoing hook name
Previously - it would silently run the perf suite w/o using
fsmonitor - fsmonitor errors are not hard failures.
Now it errors loudly.

GIT_PERF_7519_FSMONITOR="$HOME/rs-git-fsmonitorr"
./p7519-fsmonitor.sh -i -v

fatal: cannot run /home/nipunn/rs-git-fsmonitorr:
No such file or directory
not ok 2 - setup for fsmonitor

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-26 16:39:33 -07:00
Nipunn Koorapati
0288b9322d t/perf/fsmonitor: move watchman setup to one-time-repo-setup
It is only required to be set up once. This prepares for
testing multiple hooks in one invocation.

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-26 16:39:33 -07:00
Nipunn Koorapati
bb7cc7e754 t/perf/fsmonitor: separate one time repo initialization
In preparation for testing multiple fsmonitor hooks

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-26 16:39:33 -07:00
Nipunn Koorapati
2bfa953e5d p7519-fsmonitor: add a git add benchmark
Test                                                                     v2.29.0-rc1       this tree
-----------------------------------------------------------------------------------------------------------------
7519.2: status (fsmonitor=.git/hooks/fsmonitor-watchman)                 1.48(0.79+0.67)   1.48(0.79+0.67) +0.0%
7519.3: status -uno (fsmonitor=.git/hooks/fsmonitor-watchman)            0.16(0.11+0.05)   0.17(0.13+0.04) +6.3%
7519.4: status -uall (fsmonitor=.git/hooks/fsmonitor-watchman)           1.36(0.77+0.58)   1.37(0.72+0.63) +0.7%
7519.5: diff (fsmonitor=.git/hooks/fsmonitor-watchman)                   0.84(0.21+0.63)   0.14(0.11+0.03) -83.3%
7519.6: diff -- 0_files (fsmonitor=.git/hooks/fsmonitor-watchman)        0.12(0.07+0.05)   0.13(0.09+0.04) +8.3%
7519.7: diff -- 10_files (fsmonitor=.git/hooks/fsmonitor-watchman)       0.12(0.09+0.04)   0.13(0.07+0.06) +8.3%
7519.8: diff -- 100_files (fsmonitor=.git/hooks/fsmonitor-watchman)      0.12(0.08+0.05)   0.12(0.08+0.05) +0.0%
7519.9: diff -- 1000_files (fsmonitor=.git/hooks/fsmonitor-watchman)     0.12(0.08+0.05)   0.13(0.09+0.04) +8.3%
7519.10: diff -- 10000_files (fsmonitor=.git/hooks/fsmonitor-watchman)   0.14(0.08+0.06)   0.13(0.07+0.06) -7.1%
7519.11: add (fsmonitor=.git/hooks/fsmonitor-watchman)                   2.75(1.41+1.27)   2.03(1.26+0.70) -26.2%
7519.13: status (fsmonitor=)                                             1.38(1.03+1.04)   1.37(1.04+1.04) -0.7%
7519.14: status -uno (fsmonitor=)                                        1.11(0.83+0.98)   1.10(0.89+0.90) -0.9%
7519.15: status -uall (fsmonitor=)                                       2.30(1.57+1.42)   2.31(1.49+1.50) +0.4%
7519.16: diff (fsmonitor=)                                               1.43(1.13+1.76)   1.46(1.19+1.72) +2.1%
7519.17: diff -- 0_files (fsmonitor=)                                    0.10(0.08+0.04)   0.11(0.08+0.04) +10.0%
7519.18: diff -- 10_files (fsmonitor=)                                   0.10(0.07+0.05)   0.11(0.08+0.04) +10.0%
7519.19: diff -- 100_files (fsmonitor=)                                  0.10(0.07+0.04)   0.11(0.07+0.05) +10.0%
7519.20: diff -- 1000_files (fsmonitor=)                                 0.10(0.08+0.03)   0.11(0.08+0.04) +10.0%
7519.21: diff -- 10000_files (fsmonitor=)                                0.11(0.08+0.05)   0.12(0.07+0.06) +9.1%
7519.22: add (fsmonitor=)                                                2.26(1.46+1.49)   2.27(1.42+1.55) +0.4%

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-20 12:52:23 -07:00
Nipunn Koorapati
471b115745 p7519-fsmonitor: refactor to avoid code duplication
Much of the benchmark code is redundant. This is
easier to understand and edit.

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-20 12:52:23 -07:00
Nipunn Koorapati
ed5a24573d perf lint: add make test-lint to perf tests
Perf tests have not been linted for some time.
They've grown some seq instead of test_seq. This
runs the existing lints on the perf tests as well.

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-20 12:52:23 -07:00
Nipunn Koorapati
89afd5f5ad t/perf: add fsmonitor perf test for git diff
Results for the git-diff fsmonitor optimization
in patch in the parent-rev (using a 400k file repo to test)

As you can see here - git diff with fsmonitor running is
significantly better with this patch series (80% faster on my
workload)!

GIT_PERF_LARGE_REPO=~/src/server ./run v2.29.0-rc1 . -- p7519-fsmonitor.sh

Test                                                                     v2.29.0-rc1       this tree
-----------------------------------------------------------------------------------------------------------------
7519.2: status (fsmonitor=.git/hooks/fsmonitor-watchman)                 1.46(0.82+0.64)   1.47(0.83+0.62) +0.7%
7519.3: status -uno (fsmonitor=.git/hooks/fsmonitor-watchman)            0.16(0.12+0.04)   0.17(0.12+0.05) +6.3%
7519.4: status -uall (fsmonitor=.git/hooks/fsmonitor-watchman)           1.36(0.73+0.62)   1.37(0.76+0.60) +0.7%
7519.5: diff (fsmonitor=.git/hooks/fsmonitor-watchman)                   0.85(0.22+0.63)   0.14(0.10+0.05) -83.5%
7519.6: diff -- 0_files (fsmonitor=.git/hooks/fsmonitor-watchman)        0.12(0.08+0.05)   0.13(0.11+0.02) +8.3%
7519.7: diff -- 10_files (fsmonitor=.git/hooks/fsmonitor-watchman)       0.12(0.08+0.04)   0.13(0.09+0.04) +8.3%
7519.8: diff -- 100_files (fsmonitor=.git/hooks/fsmonitor-watchman)      0.12(0.07+0.05)   0.13(0.07+0.06) +8.3%
7519.9: diff -- 1000_files (fsmonitor=.git/hooks/fsmonitor-watchman)     0.12(0.09+0.04)   0.13(0.08+0.05) +8.3%
7519.10: diff -- 10000_files (fsmonitor=.git/hooks/fsmonitor-watchman)   0.14(0.09+0.05)   0.13(0.10+0.03) -7.1%
7519.12: status (fsmonitor=)                                             1.67(0.93+1.49)   1.67(0.99+1.42) +0.0%
7519.13: status -uno (fsmonitor=)                                        0.37(0.30+0.82)   0.37(0.33+0.79) +0.0%
7519.14: status -uall (fsmonitor=)                                       1.58(0.97+1.35)   1.57(0.86+1.45) -0.6%
7519.15: diff (fsmonitor=)                                               0.34(0.28+0.83)   0.34(0.27+0.83) +0.0%
7519.16: diff -- 0_files (fsmonitor=)                                    0.09(0.06+0.04)   0.09(0.08+0.02) +0.0%
7519.17: diff -- 10_files (fsmonitor=)                                   0.09(0.07+0.03)   0.09(0.06+0.05) +0.0%
7519.18: diff -- 100_files (fsmonitor=)                                  0.09(0.06+0.04)   0.09(0.06+0.04) +0.0%
7519.19: diff -- 1000_files (fsmonitor=)                                 0.09(0.06+0.04)   0.09(0.05+0.05) +0.0%
7519.20: diff -- 10000_files (fsmonitor=)                                0.10(0.08+0.04)   0.10(0.06+0.05) +0.0%

I also added a benchmark for a tiny git diff workload w/ a pathspec.
I see an approximately .02 second overhead added w/ and w/o fsmonitor

From looking at these results, I suspected that refresh_fsmonitor
is already happening during git diff - independent of this patch
series' optimization. Confirmed that suspicion by breaking on
refresh_fsmonitor.

(gdb) bt  [simplified]
0  refresh_fsmonitor  at fsmonitor.c:176
1  ie_match_stat  at read-cache.c:375
2  match_stat_with_submodule at diff-lib.c:237
4  builtin_diff_files  at builtin/diff.c:260
5  cmd_diff  at builtin/diff.c:541
6  run_builtin  at git.c:450
7  handle_builtin  at git.c:700
8  run_argv  at git.c:767
9  cmd_main  at git.c:898
10 main  at common-main.c:52

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-20 12:52:22 -07:00
Nipunn Koorapati
5851462e8d t/perf/p7519-fsmonitor.sh: warm cache on first git status
The first git status would be inflated due to warming of
filesystem cache. This makes the results comparable.

Before
Test                                                             this tree
--------------------------------------------------------------------------------
7519.2: status (fsmonitor=.git/hooks/fsmonitor-watchman)         2.52(1.59+1.56)
7519.3: status -uno (fsmonitor=.git/hooks/fsmonitor-watchman)    0.18(0.12+0.06)
7519.4: status -uall (fsmonitor=.git/hooks/fsmonitor-watchman)   1.36(0.73+0.62)
7519.7: status (fsmonitor=)                                      0.69(0.52+0.90)
7519.8: status -uno (fsmonitor=)                                 0.37(0.28+0.81)
7519.9: status -uall (fsmonitor=)                                1.53(0.93+1.32)

After
Test                                                             this tree
--------------------------------------------------------------------------------
7519.2: status (fsmonitor=.git/hooks/fsmonitor-watchman)         0.39(0.33+0.06)
7519.3: status -uno (fsmonitor=.git/hooks/fsmonitor-watchman)    0.17(0.13+0.05)
7519.4: status -uall (fsmonitor=.git/hooks/fsmonitor-watchman)   1.34(0.77+0.56)
7519.7: status (fsmonitor=)                                      0.70(0.53+0.90)
7519.8: status -uno (fsmonitor=)                                 0.37(0.32+0.78)
7519.9: status -uall (fsmonitor=)                                1.55(1.01+1.25)

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-20 12:52:22 -07:00
Nipunn Koorapati
dc69d47d21 t/perf/README: elaborate on output format
Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-20 12:52:22 -07:00
Junio C Hamano
62564ba4e5 Merge branch 'js/default-branch-name-part-3'
Test preparation for the switch of default branch name continues.

* js/default-branch-name-part-3:
  tests: avoid using the branch name `main`
  t1415: avoid using `main` as ref name
2020-10-08 21:53:26 -07:00
Junio C Hamano
c7ac8c0a7c Merge branch 'jk/index-pack-hotfixes'
Hotfix and clean-up for the jt/threaded-index-pack topic that has
graduated to v2.29-rc0.

* jk/index-pack-hotfixes:
  index-pack: make get_base_data() comment clearer
  index-pack: drop type_cas mutex
  index-pack: restore "resolving deltas" progress meter
2020-10-08 21:53:26 -07:00
Junio C Hamano
f491ce954b Merge branch 'hx/push-atomic-with-cert'
Hotfix to a recently added test script.

* hx/push-atomic-with-cert:
  t5534: split stdout and stderr redirection
2020-10-08 21:53:25 -07:00
Johannes Schindelin
538228ed23 tests: avoid using the branch name main
In the near future, we want to change Git's default branch name to
`main`. In preparation for that, stop using it as a branch name in the
test suite. Replace that branch name by `topic`, the same name we used
to rename variations of `master` in b6211b89eb (tests: avoid variations
of the `master` branch name, 2020-09-26).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-08 10:40:18 -07:00
Johannes Schindelin
a15ad5d1bc t1415: avoid using main as ref name
In preparation for a patch series that will change the fall-back for
`init.defaultBranch` to `main`, let's not use `main` as ref name in this
test script.

Otherwise, the `git for-each-ref ... | grep main` which wants to catch
those refs would also unexpectedly catch `refs/heads/main`.

Since the refs in question are worktree-local ones (i.e. each worktree
has their own, just like `HEAD`), and since the test case already uses a
secondary worktree called "second", let's use the name "first" for those
refs instead.

While at it, adjust the test titles that talk about a "repo" when they
meant a "worktree" instead.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-08 10:40:16 -07:00
Jeff King
cea69151a4 index-pack: restore "resolving deltas" progress meter
Commit f08cbf60fe (index-pack: make quantum of work smaller, 2020-09-08)
refactored the main loop in threaded_second_pass(), but also deleted the
call to display_progress() at the top of the loop. This means that users
typically see no progress at all during the delta resolution phase (and
for large repositories, Git appears to hang).

This looks like an accident that was unrelated to the intended change of
that commit, since we continue to update nr_resolved_deltas in
resolve_delta(). Let's restore the call to get that progress back.

We'll also add a test that confirms we generate the expected progress.
This isn't perfect, as it wouldn't catch a bug where progress was
delayed to the end. That was probably possible to trigger when receiving
a thin pack, because we'd eventually call display_progress() from
fix_unresolved_deltas(), but only once after doing all the work.
However, since our test case generates a complete pack, it reliably
demonstrates this particular bug and its fix. And we can't do better
without making the test racy.

Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-07 11:50:09 -07:00
Đoàn Trần Công Danh
2cd6e1d552 t5534: split stdout and stderr redirection
On atomic pushing failure with GnuPG, we expect a very specific output
in stdout due to `--porcelain` switch.

On such failure, we also write down some helpful hint into stderr
in order to help user understand what happens and how to continue from
those failures.

On a lot of system, those hint (in stderr) will be flushed first,
then those messages in stdout will be flushed. In such systems, the
current test code is fine as is.

However, we don't have such guarantee, (at least) there're some real
systems that writes those stream interleaved. On such systems, we may
see the stderr stream written in the middle of stdout stream.

Let's split those stream redirection. By splitting those stream,
the output stream will contain exactly what we want to compare,
thus, saving us a "sed" invocation.

While we're at it, change the `test_i18ncmp` to `test_cmp` because we
will never translate those messages (because of `--porcelain`).

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-06 12:14:14 -07:00
Junio C Hamano
5f8c70a148 Merge branch 'jk/format-auto-base-when-able'
"git format-patch" learns to take "whenAble" as a possible value
for the format.useAutoBase configuration variable to become no-op
when the  automatically computed base does not make sense.

* jk/format-auto-base-when-able:
  format-patch: teach format.useAutoBase "whenAble" option
2020-10-05 14:01:55 -07:00
Junio C Hamano
7da656f1e0 Merge branch 'jk/diff-cc-oidfind-fix'
"log -c --find-object=X" did not work well to find a merge that
involves a change to an object X from only one parent.

* jk/diff-cc-oidfind-fix:
  combine-diff: handle --find-object in multitree code path
2020-10-05 14:01:55 -07:00
Junio C Hamano
8e3ec76a20 Merge branch 'jk/refspecs-negative'
"git fetch" and "git push" support negative refspecs.

* jk/refspecs-negative:
  refspec: add support for negative refspecs
2020-10-05 14:01:54 -07:00
Junio C Hamano
f6b06b4590 Merge branch 'rs/archive-add-file'
"git archive" learns the "--add-file" option to include untracked
files into a snapshot from a tree-ish.

* rs/archive-add-file:
  Makefile: use git-archive --add-file
  archive: add --add-file
  archive: read short blobs in archive.c::write_archive_entry()
2020-10-05 14:01:53 -07:00
Junio C Hamano
e68f0a4e57 Merge branch 'jt/keep-partial-clone-filter-upon-lazy-fetch'
The lazy fetching done internally to make missing objects available
in a partial clone incorrectly made permanent damage to the partial
clone filter in the repository, which has been corrected.

* jt/keep-partial-clone-filter-upon-lazy-fetch:
  fetch: do not override partial clone filter
  promisor-remote: remove unused variable
2020-10-05 14:01:53 -07:00
Junio C Hamano
300cd14ee9 Merge branch 'td/submodule-update-quiet'
"git submodule update --quiet" did not squelch underlying "rebase"
and "pull" commands.

* td/submodule-update-quiet:
  submodule update: silence underlying merge/rebase with "--quiet"
2020-10-05 14:01:53 -07:00
Junio C Hamano
19dd352d03 Merge branch 'jk/unused'
Code cleanup.

* jk/unused:
  dir.c: drop unused "untracked" from treat_path_fast()
  sequencer: handle ignore_footer when parsing trailers
  test-advise: check argument count with argc instead of argv
  sparse-checkout: fill in some options boilerplate
  sequencer: drop repository argument from run_git_commit()
  push: drop unused repo argument to do_push()
  assert PARSE_OPT_NONEG in parse-options callbacks
  env--helper: write to opt->value in parseopt helper
  drop unused argc parameters
  convert: drop unused crlf_action from check_global_conv_flags_eol()
2020-10-05 14:01:52 -07:00
Junio C Hamano
58138d3f26 Merge branch 'js/default-branch-name-part-2'
Update the tests to drop word 'master' from them.

* js/default-branch-name-part-2:
  t9902: avoid using the branch name `master`
  tests: avoid variations of the `master` branch name
  t3200: avoid variations of the `master` branch name
  fast-export: avoid using unnecessary language in a code comment
  t/test-terminal: avoid non-inclusive language
2020-10-05 14:01:50 -07:00
Junio C Hamano
c01b041ef0 Merge branch 'ds/in-merge-bases-many-optim-bug'
in_merge_bases_many(), a way to see if a commit is reachable from
any commit in a set of commits, was totally broken when the
commit-graph feature was in use, which has been corrected.

* ds/in-merge-bases-many-optim-bug:
  commit-reach: fix in_merge_bases_many bug
2020-10-05 14:01:50 -07:00
Junio C Hamano
2fa8aacc72 Merge branch 'jk/shortlog-group-by-trailer'
"git shortlog" has been taught to group commits by the contents of
the trailer lines, like "Reviewed-by:", "Coauthored-by:", etc.

* jk/shortlog-group-by-trailer:
  shortlog: allow multiple groups to be specified
  shortlog: parse trailer idents
  shortlog: rename parse_stdin_ident()
  shortlog: de-duplicate trailer values
  shortlog: match commit trailers with --group
  trailer: add interface for iterating over commit trailers
  shortlog: add grouping option
  shortlog: change "author" variables to "ident"
2020-10-04 12:49:14 -07:00
Junio C Hamano
03a01824a4 Merge branch 'cc/bisect-start-fix'
"git bisect start X Y", when X and Y are not valid committish
object names, should take X and Y as pathspec, but didn't.

* cc/bisect-start-fix:
  bisect: don't use invalid oid as rev when starting
2020-10-04 12:49:08 -07:00
Junio C Hamano
230ff3e997 Merge branch 'jc/blame-ignore-fix'
"git blame --ignore-rev/--ignore-revs-file" failed to validate
their input are valid revision, and failed to take into account
that the user may want to give an annotated tag instead of a
commit, which has been corrected.

* jc/blame-ignore-fix:
  blame: validate and peel the object names on the ignore list
  t8013: minimum preparatory clean-up
2020-10-04 12:49:07 -07:00
Derrick Stolee
8791bf1841 commit-reach: fix in_merge_bases_many bug
Way back in f9b8908b (commit.c: use generation numbers for
in_merge_bases(), 2018-05-01), a heuristic was used to short-circuit
the in_merge_bases() walk. This works just fine as long as the
caller is checking only two commits, but when there are multiple,
there is a possibility that this heuristic is _very wrong_.

Some code moves since then has changed this method to
repo_in_merge_bases_many() inside commit-reach.c. The heuristic
computes the minimum generation number of the "reference" list, then
compares this number to the generation number of the "commit".

In a recent topic, a test was added that used in_merge_bases_many()
to test if a commit was reachable from a number of commits pulled
from a reflog. However, this highlighted the problem: if any of the
reference commits have a smaller generation number than the given
commit, then the walk is skipped _even if there exist some with
higher generation number_.

This heuristic is wrong! It must check the MAXIMUM generation number
of the reference commits, not the MINIMUM.

This highlights a testing gap. t6600-test-reach.sh covers many
methods in commit-reach.c, including in_merge_bases() and
get_merge_bases_many(), but since these methods either restrict to
two input commits or actually look for the full list of merge bases,
they don't check this heuristic!

Add a possible input to "test-tool reach" that tests
in_merge_bases_many() and add tests to t6600-test-reach.sh that
cover this heuristic. This includes cases for the reference commits
having generation above and below the generation of the input commit,
but also having maximum generation below the generation of the input
commit.

The fix itself is to swap min_generation with a max_generation in
repo_in_merge_bases_many().

Reported-by: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com>
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-02 10:26:31 -07:00
Jacob Keller
7efba5fa39 format-patch: teach format.useAutoBase "whenAble" option
The format.useAutoBase configuration option exists to allow users to
enable '--base=auto' for format-patch by default.

This can sometimes lead to poor workflow, due to unexpected failures
when attempting to format an ancient patch:

    $ git format-patch -1 <an old commit>
    fatal: base commit shouldn't be in revision list

This can be very confusing, as it is not necessarily immediately obvious
that the user requested a --base (since this was in the configuration,
not on the command line).

We do want --base=auto to fail when it cannot provide a suitable base,
as it would be equally confusing if a formatted patch did not include
the base information when it was requested.

Teach format.useAutoBase a new mode, "whenAble". This mode will cause
format-patch to attempt to include a base commit when it can. However,
if no valid base commit can be found, then format-patch will continue
formatting the patch without a base commit.

In order to avoid making yet another branch name unusable with --base,
do not teach --base=whenAble or --base=whenable.

Instead, refactor the base_commit option to use a callback, and rely on
the global configuration variable auto_base.

This does mean that a user cannot request this optional base commit
generation from the command line. However, this is likely not too
valuable. If the user requests base information manually, they will be
immediately informed of the failure to acquire a suitable base commit.
This allows the user to make an informed choice about whether to
continue the format.

Add tests to cover the new mode of operation for --base.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-01 15:22:10 -07:00
Theodore Dubois
3ad0401e9e submodule update: silence underlying merge/rebase with "--quiet"
Commands such as

    $ git pull --rebase --recurse-submodules --quiet

produce non-quiet output from the merge or rebase.  Pass the --quiet
option down when invoking "rebase" and "merge".

Also fix the parsing of git submodule update -v.

When e84c3cf3 (git-submodule.sh: accept verbose flag in cmd_update
to be non-quiet, 2018-08-14) taught "git submodule update" to take
"--quiet", it apparently did not know how ${GIT_QUIET:+--quiet}
works, and reviewers seem to have missed that setting the variable
to "0", rather than unsetting it, still results in "--quiet" being
passed to underlying commands.

Signed-off-by: Theodore Dubois <tbodt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-01 08:50:24 -07:00
Jacob Keller
c0192df630 refspec: add support for negative refspecs
Both fetch and push support pattern refspecs which allow fetching or
pushing references that match a specific pattern. Because these patterns
are globs, they have somewhat limited ability to express more complex
situations.

For example, suppose you wish to fetch all branches from a remote except
for a specific one. To allow this, you must setup a set of refspecs
which match only the branches you want. Because refspecs are either
explicit name matches, or simple globs, many patterns cannot be
expressed.

Add support for a new type of refspec, referred to as "negative"
refspecs. These are prefixed with a '^' and mean "exclude any ref
matching this refspec". They can only have one "side" which always
refers to the source. During a fetch, this refers to the name of the ref
on the remote. During a push, this refers to the name of the ref on the
local side.

With negative refspecs, users can express more complex patterns. For
example:

 git fetch origin refs/heads/*:refs/remotes/origin/* ^refs/heads/dontwant

will fetch all branches on origin into remotes/origin, but will exclude
fetching the branch named dontwant.

Refspecs today are commutative, meaning that order doesn't expressly
matter. Rather than forcing an implied order, negative refspecs will
always be applied last. That is, in order to match, a ref must match at
least one positive refspec, and match none of the negative refspecs.
This is similar to how negative pathspecs work.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-30 14:52:00 -07:00
Jeff King
957876f17d combine-diff: handle --find-object in multitree code path
When doing combined diffs, we have two possible code paths:

  - a slower one which independently diffs against each parent, applies
    any filters, and then intersects the resulting paths

  - a faster one which walks all trees simultaneously

When the diff options specify that we must do certain filters, like
pickaxe, then we always use the slow path, since the pickaxe code only
knows how to handle filepairs, not the n-parent entries generated for
combined diffs.

But there are two problems with the slow path:

 1. It's slow. Running:

      git rev-list HEAD | git diff-tree --stdin -r -c

    in git.git takes ~3s on my machine. But adding "--find-object" to
    that increases it to ~6s, even though find-object itself should
    incur only a few extra oid comparisons. On linux.git, it's even
    worse: 35s versus 215s.

 2. It doesn't catch all cases where a particular path is interesting.
    Consider a merge with parent blobs X and Y for a particular path,
    and end result Z. That should be interesting according to "-c",
    because the result doesn't match either parent. And it should be
    interesting even with "--find-object=X", because "X" went away in
    the merge.

    But because we perform each pairwise diff independently, this
    confuses the intersection code. The change from X to Z is still
    interesting according to --find-object. But in the other parent we
    went from Y to Z, so the diff appears empty! That causes the
    intersection code to think that parent didn't change the path, and
    thus it's not interesting for "-c".

This patch fixes both by implementing --find-object for the multitree
code. It's a bit unfortunate that we have to duplicate some logic from
diffcore-pickaxe, but this is the best we can do for now. In an ideal
world, all of the diffcore code would stop thinking about filepairs and
start thinking about n-parent sets, and we could use the multitree walk
with all of it.

Until then, there are some leftover warts:

  - other pickaxe operations, like -S or -G, still suffer from both
    problems. These would be hard to adapt because they rely on having
    a diff_filespec() for each path to look at content. And we'd need to
    define what an n-way "change" means in each case (probably easy for
    "-S", which can compare counts, but not so clear for -G, which is
    about grepping diffs).

  - other options besides --find-object may cause us to use the slow
    pairwise path, in which case we'll go back to producing a different
    (wrong) answer for the X/Y/Z case above.

We may be able to hack around these, but I think the ultimate solution
will be a larger rewrite of the diffcore code. For now, this patch
improves one specific case but leaves the rest.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-30 13:35:24 -07:00
Jeff King
26e28fe7bb test-advise: check argument count with argc instead of argv
We complain if "test-tool advise" is not given an argument, but we
quietly ignore any additional arguments it receives. Let's instead check
that we got the expected number. As a bonus, this silences
-Wunused-parameter, which notes that we don't ever look at argc.

While we're here, we can also fix the indentation in the conditional.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-30 12:53:48 -07:00
Jeff King
e885a84f1b drop unused argc parameters
Many functions take an argv/argc pair, but never actually look at argc.
This makes it useless at best (we use the NULL sentinel in argv to find
the end of the array), and misleading at worst (what happens if the argc
count does not match the argv NULL?).

In each of these instances, the argv NULL does match the argc count, so
there are no bugs here. But let's tighten the interfaces to make it
harder to get wrong (and to reduce some -Wunused-parameter complaints).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-30 12:53:47 -07:00
Junio C Hamano
299deeac8a Merge branch 'ah/pull'
Earlier we taught "git pull" to warn when the user does not say the
histories need to be merged, rebased or accepts only fast-
forwarding, but the warning triggered for those who have set the
pull.ff configuration variable.

* ah/pull:
  pull: don't warn if pull.ff has been set
2020-09-29 14:01:22 -07:00
Junio C Hamano
ac4089da7b Merge branch 'tg/range-diff-same-file-fix'
"git range-diff" showed incorrect diffstat, which has been
corrected.

* tg/range-diff-same-file-fix:
  diff: fix modified lines stats with --stat and --numstat
2020-09-29 14:01:21 -07:00
Junio C Hamano
71a9b82dd4 Merge branch 'jc/t1506-rev-parse-leaves-range-endpoint-unpeeled'
Test update.

* jc/t1506-rev-parse-leaves-range-endpoint-unpeeled:
  t1506: rev-parse A..B and A...B
2020-09-29 14:01:21 -07:00
Junio C Hamano
b28919c7bc Merge branch 'bc/clone-with-git-default-hash-fix'
"git clone" that clones from SHA-1 repository, while
GIT_DEFAULT_HASH set to use SHA-256 already, resulted in an
unusable repository that half-claims to be SHA-256 repository
with SHA-1 objects and refs.  This has been corrected.

* bc/clone-with-git-default-hash-fix:
  builtin/clone: avoid failure with GIT_DEFAULT_HASH
2020-09-29 14:01:20 -07:00
Junio C Hamano
288ed98bf7 Merge branch 'tb/bloom-improvements'
"git commit-graph write" learned to limit the number of bloom
filters that are computed from scratch with the --max-new-filters
option.

* tb/bloom-improvements:
  commit-graph: introduce 'commitGraph.maxNewFilters'
  builtin/commit-graph.c: introduce '--max-new-filters=<n>'
  commit-graph: rename 'split_commit_graph_opts'
  bloom: encode out-of-bounds filters as non-empty
  bloom/diff: properly short-circuit on max_changes
  bloom: use provided 'struct bloom_filter_settings'
  bloom: split 'get_bloom_filter()' in two
  commit-graph.c: store maximum changed paths
  commit-graph: respect 'commitGraph.readChangedPaths'
  t/helper/test-read-graph.c: prepare repo settings
  commit-graph: pass a 'struct repository *' in more places
  t4216: use an '&&'-chain
  commit-graph: introduce 'get_bloom_filter_settings()'
2020-09-29 14:01:20 -07:00
Jonathan Tan
23547c4051 fetch: do not override partial clone filter
When a fetch with the --filter argument is made, the configured default
filter is set even if one already exists. This change was made in
5e46139376 ("builtin/fetch: remove unique promisor remote limitation",
2019-06-25) - in particular, changing from:

 * If this is the FIRST partial-fetch request, we enable partial
 * on this repo and remember the given filter-spec as the default
 * for subsequent fetches to this remote.

to:

 * If this is a partial-fetch request, we enable partial on
 * this repo if not already enabled and remember the given
 * filter-spec as the default for subsequent fetches to this
 * remote.

(The given filter-spec is "remembered" even if there is already an
existing one.)

This is problematic whenever a lazy fetch is made, because lazy fetches
are made using "git fetch --filter=blob:none", but this will also happen
if the user invokes "git fetch --filter=<filter>" manually. Therefore,
restore the behavior prior to 5e46139376, which writes a filter-spec
only if the current fetch request is the first partial-fetch one (for
that remote).

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-28 16:11:59 -07:00
Jeff King
63d24fa0b0 shortlog: allow multiple groups to be specified
Now that shortlog supports reading from trailers, it can be useful to
combine counts from multiple trailers, or between trailers and authors.
This can be done manually by post-processing the output from multiple
runs, but it's non-trivial to make sure that each name/commit pair is
counted only once.

This patch teaches shortlog to accept multiple --group options on the
command line, and pull data from all of them. That makes it possible to
run:

  git shortlog -ns --group=author --group=trailer:co-authored-by

to get a shortlog that counts authors and co-authors equally.

The implementation is mostly straightforward. The "group" enum becomes a
bitfield, and the trailer key becomes a list. I didn't bother
implementing the multi-group semantics for reading from stdin. It would
be possible to do, but the existing matching code makes it awkward, and
I doubt anybody cares.

The duplicate suppression we used for trailers now covers authors and
committers as well (though in non-trailer single-group mode we can skip
the hash insertion and lookup, since we only see one value per commit).

There is one subtlety: we now care about the case when no group bit is
set (in which case we default to showing the author). The caller in
builtin/log.c needs to be adapted to ask explicitly for authors, rather
than relying on shortlog_init(). It would be possible with some
gymnastics to make this keep working as-is, but it's not worth it for a
single caller.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27 12:21:05 -07:00
Jeff King
56d5dde752 shortlog: parse trailer idents
Trailers don't necessarily contain name/email identity values, so
shortlog has so far treated them as opaque strings. However, since many
trailers do contain identities, it's useful to treat them as such when
they can be parsed. That lets "-e" work as usual, as well as mailmap.

When they can't be parsed, we'll continue with the old behavior of
treating them as a single string (there's no new test for that here,
since the existing tests cover a trailer like this).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27 12:21:05 -07:00