summaryrefslogtreecommitdiff
path: root/t
AgeCommit message (Collapse)AuthorFilesLines
2020-06-08Merge branch 'tb/commit-graph-no-check-oids'Libravatar Junio C Hamano1-9/+16
Clean-up the commit-graph codepath. * tb/commit-graph-no-check-oids: commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flag t5318: reorder test below 'graph_read_expect' commit-graph.c: simplify 'fill_oids_from_commits' builtin/commit-graph.c: dereference tags in builtin builtin/commit-graph.c: extract 'read_one_commit()' commit-graph.c: peel refs in 'add_ref_to_set' commit-graph.c: show progress of finding reachable commits commit-graph.c: extract 'refs_cb_data'
2020-06-08Merge branch 'cb/t4210-illseq-auto-detect'Libravatar Junio C Hamano3-52/+125
As FreeBSD is not the only platform whose regexp library reports a REG_ILLSEQ error when fed invalid UTF-8, add logic to detect that automatically and skip the affected tests. * cb/t4210-illseq-auto-detect: t4210: detect REG_ILLSEQ dynamically and skip affected tests t/helper: teach test-regex to report pattern errors (like REG_ILLSEQ)
2020-06-08Merge branch 'ds/line-log-on-bloom'Libravatar Junio C Hamano1-0/+68
"git log -L..." now takes advantage of the "which paths are touched by this commit?" info stored in the commit-graph system. * ds/line-log-on-bloom: line-log: integrate with changed-path Bloom filters line-log: try to use generation number-based topo-ordering line-log: more responsive, incremental 'git log -L' t4211-line-log: add tests for parent oids line-log: remove unused fields from 'struct line_log_data'
2020-06-02Merge branch 'en/fast-import-looser-date'Libravatar Junio C Hamano1-0/+28
Some repositories in the wild have commits that record nonsense committer timezone (e.g. rails.git); "git fast-import" learned an option to pass these nonsense timestamps intact to allow recreating existing repositories as-is. * en/fast-import-looser-date: fast-import: add new --date-format=raw-permissive format
2020-06-02Merge branch 'la/diff-relative-config'Libravatar Junio C Hamano2-3/+92
The commands in the "diff" family learned to honor "diff.relative" configuration variable. * la/diff-relative-config: diff: add config option relative
2020-06-02Merge branch 'rs/checkout-b-track-error'Libravatar Junio C Hamano2-0/+34
The error message from "git checkout -b foo -t bar baz" was confusing. * rs/checkout-b-track-error: checkout: improve error messages for -b with extra argument checkout: add tests for -b and --track
2020-06-02Merge branch 'cb/t5608-cleanup'Libravatar Junio C Hamano1-6/+5
Test fixup. * cb/t5608-cleanup: t5608: avoid say() and use "skip_all" instead for consistency
2020-05-31Merge branch 'cb/test-use-ere-for-alternation'Libravatar Junio C Hamano2-2/+2
Portability fix for tests added recently. * cb/test-use-ere-for-alternation: t: avoid alternation (not POSIX) in grep's BRE
2020-05-31fast-import: add new --date-format=raw-permissive formatLibravatar Elijah Newren1-0/+28
There are multiple repositories in the wild with random, invalid timezones. Most notably is a commit from rails.git with a timezone of "+051800"[1]. A few searches will find other repos with that same invalid timezone as well. Further, Peff reports that GitHub relaxed their fsck checks in August 2011 to accept any timezone value[2], and there have been multiple reports to filter-repo about fast-import crashing while trying to import their existing repositories since they had timezone values such as "-7349423" and "-43455309"[3]. The existing check on timezone values inside fast-import may prove useful for people who are crafting fast-import input by hand or with a new script. For them, the check may help them avoid accidentally recording invalid dates. (Note that this check is rather simplistic and there are still several forms of invalid dates that fast-import does not check for: dates in the future, timezone values with minutes that are not divisible by 15, and timezone values with minutes that are 60 or greater.) While this simple check may have some value for those users, other users or tools will want to import existing repositories as-is. Provide a --date-format=raw-permissive format that will not error out on these otherwise invalid timezones so that such existing repositories can be imported. [1] https://github.com/rails/rails/commit/4cf94979c9f4d6683c9338d694d5eb3106a4e734 [2] https://lore.kernel.org/git/20200521195513.GA1542632@coredump.intra.peff.net/ [3] https://github.com/newren/git-filter-repo/issues/88 Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-29t: avoid alternation (not POSIX) in grep's BRELibravatar Carlo Marcelo Arenas Belón2-2/+2
f1e3df3169 (t: increase test coverage of signature verification output, 2020-03-04) adds GPG dependent tests to t4202 and t6200 that were found problematic with at least OpenBSD 6.7. Using an escaped '|' for alternations works only in some implementations of grep (e.g. GNU and busybox). It is not part of POSIX[1] and not supported by some BSD, macOS, and possibly other POSIX compatible implementations. Use `grep -E`, and write it using extended regular expression. [1] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_03 Helped-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-24Merge branch 'dd/t5703-grep-a-fix'Libravatar Junio C Hamano1-3/+6
Update an unconditional use of "grep -a" with a perl script in a test. * dd/t5703-grep-a-fix: t5703: replace "grep -a" usage by perl
2020-05-24Merge branch 'gs/commit-graph-path-filter'Libravatar Junio C Hamano1-1/+1
Test fix. * gs/commit-graph-path-filter: t4216: avoid unnecessary subshell in test_bloom_filters_not_used
2020-05-24Merge branch 'dl/merge-autostash'Libravatar Junio C Hamano1-1/+1
Test fix. * dl/merge-autostash: t5520: avoid alternation in grep's BRE (not POSIX)
2020-05-24Merge branch 'jt/avoid-prefetch-when-able-in-diff'Libravatar Junio C Hamano1-2/+2
Test-coverage enhancement. * jt/avoid-prefetch-when-able-in-diff: t4067: make rename detection test output raw diff
2020-05-24Merge branch 'gp/hppa-stack-test-fix'Libravatar Junio C Hamano1-2/+10
Platform dependent tweak to a test for HP-PA. * gp/hppa-stack-test-fix: tests: skip small-stack tests on hppa architecture
2020-05-24diff: add config option relativeLibravatar Laurent Arnoud2-3/+92
The `diff.relative` boolean option set to `true` shows only changes in the current directory/value specified by the `path` argument of the `relative` option and shows pathnames relative to the aforementioned directory. Teach `--no-relative` to override earlier `--relative` Add for git-format-patch(1) options documentation `--relative` and `--no-relative` Signed-off-by: Laurent Arnoud <laurent@spkdev.net> Acked-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-24t5608: avoid say() and use "skip_all" instead for consistencyLibravatar Carlo Marcelo Arenas Belón1-6/+5
Printing a message directly to stdout could affect TAP processing and is not really needed, as there is a standard way to skip all tests that could be used instead, while printing an equivalent message. While at it; update the message to better reflect that since a85efb5985 (t5608-clone-2gb.sh: turn GIT_TEST_CLONE_2GB into a bool, 2019-11-22), the enabling variable should be a recognized boolean (ex: true, false, 1, 0) and get rid of the prerequisite that used to guard all the tests, since "skip_all" is just much faster and idempotent. Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-24checkout: improve error messages for -b with extra argumentLibravatar René Scharfe2-2/+2
When we try to create a branch "foo" based on "origin/master" and give git commit -b an extra unsupported argument "bar", it confusingly reports: $ git checkout -b foo origin/master bar fatal: 'bar' is not a commit and a branch 'foo' cannot be created from it $ git checkout --track -b foo origin/master bar fatal: 'bar' is not a commit and a branch 'foo' cannot be created from it That's wrong, because it very well understands that "origin/master" is supposed to be the start point for the new branch and not "bar". Check if we got a commit and show more fitting messages in that case instead: $ git checkout -b foo origin/master bar fatal: Cannot update paths and switch to branch 'foo' at the same time. $ git checkout --track -b foo origin/master bar fatal: '--track' cannot be used with updating paths Original-patch-by: Jeff King <peff@peff.net> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-24checkout: add tests for -b and --trackLibravatar René Scharfe2-0/+34
Test git checkout -b with and without --track and demonstrate unexpected error messages when it's given an extra (i.e. unsupported) path argument. In both cases it reports: $ git checkout -b foo origin/master bar fatal: 'bar' is not a commit and a branch 'foo' cannot be created from it The problem is that the start point we gave for the new branch is "origin/master" and "bar" is just some extra argument -- it could even be a valid commit, which would make the message even more confusing. We have more fitting error messages in git commit, but get confused; use the text of the rights ones in the tests. Reported-by: Dana Dahlstrom <dahlstrom@google.com> Original-test-by: Jeff King <peff@peff.net> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-20t5520: avoid alternation in grep's BRE (not POSIX)Libravatar Carlo Marcelo Arenas Belón1-1/+1
Instead of using a BRE, that broke tests 30-32, 37-39, 42 at least with OpenBSD 6.7; use a simpler ERE. Fixes: d9f15d37f1 (pull: pass --autostash to merge, 2020-04-07) Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-20t4216: avoid unnecessary subshell in test_bloom_filters_not_usedLibravatar Carlo Marcelo Arenas Belón1-1/+1
Seems to trigger a bug in at least OpenBSD's 6.7 sh where it is interpreted as a history lookup and therefore fails 125-126, 128, 130. Remove the subshell and get a space between ! and grep, so tests pass successfully. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-20Merge branch 'jc/fix-tap-output-under-bash'Libravatar Junio C Hamano1-15/+1
A recent attempt to make the test output nicer to view on CI systems broke TAP output under bash. The effort has been reverted to be re-attempted in the next cycle. * jc/fix-tap-output-under-bash: Revert "tests: when run in Bash, annotate test failures with file name/line number" Revert "ci: add a problem matcher for GitHub Actions" Revert "t/test_lib: avoid naked bash arrays in file_lineno"
2020-05-20Merge branch 'en/merge-rename-rename-worktree-fix'Libravatar Junio C Hamano1-0/+55
When a binary file gets modified and renamed on both sides of history to different locations, both files would be written to the working tree but both would have the contents from "ours". This has been corrected so that the path from each side gets their original content. * en/merge-rename-rename-worktree-fix: merge-recursive: fix rename/rename(1to2) for working tree with a binary
2020-05-20Merge branch 'dd/t1509-i18n-fix'Libravatar Junio C Hamano1-2/+2
A few tests were not i18n clean. * dd/t1509-i18n-fix: t1509: correct i18n test
2020-05-19t4067: make rename detection test output raw diffLibravatar Jonathan Tan1-2/+2
95acf11a3d ("diff: restrict when prefetching occurs", 2020-04-07) taught diff to prefetch blobs in a more limited set of situations. These limited situations include when the output format requires blob data, and when inexact rename detection is needed. There is an existing test case that tests inexact rename detection, but it also uses an output format that requires blob data, resulting in the inexact-rename-detection-only code not being tested. Update this test to use the raw output format, which does not require blob data. Thanks to Derrick Stolee for noticing this lapse in code coverage and for doing the preliminary analysis [1]. [1] https://lore.kernel.org/git/853759d3-97c3-241f-98e1-990883cd204e@gmail.com/ Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-19t5703: replace "grep -a" usage by perlLibravatar Đoàn Trần Công Danh1-3/+6
On some platforms likes HP-UX, grep(1) doesn't understand "-a". Let's switch to perl. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-18t4210: detect REG_ILLSEQ dynamically and skip affected testsLibravatar Carlo Marcelo Arenas Belón2-24/+59
7187c7bbb8 (t4210: skip i18n tests that don't work on FreeBSD, 2019-11-27) adds a REG_ILLSEQ prerequisite, and to do that copies the common branch in test-lib and expands it to include it in a special case for FreeBSD. Instead; test for it using a previously added extension to test-tool and use that, together with a function that identifies when regcomp/regexec will be called with broken patterns to avoid any test that would otherwise rely on undefined behaviour. The description of the first test which wasn't accurate has been corrected, and the test rearranged for clarity, including a helper function that avoids overly long lines. Only the affected engines will have their tests suppressed, also including "fixed" if the PCRE optimization that uses LIBPCRE2 since b65abcafc7 (grep: use PCRE v2 for optimized fixed-string search, 2019-07-01) is not available. Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-18t/helper: teach test-regex to report pattern errors (like REG_ILLSEQ)Libravatar Carlo Marcelo Arenas Belón1-28/+66
7187c7bbb8 (t4210: skip i18n tests that don't work on FreeBSD, 2019-11-27) adds a REG_ILLSEQ prerequisite to avoid failures from the tests added in 4e2443b181 (log tests: test regex backends in "--encode=<enc>" tests, 2019-06-28), but hardcodes it to be only enabled in FreeBSD. Instead of hardcoding the affected platform, teach the test-regex helper, how to validate a pattern and report back, so it can be used to detect the same issue in other affected systems (like DragonFlyBSD or macOS). While at it, refactor the tool so it can report back the source of the errors it founds, and can be invoked also in a --silent mode, when needed, for backward compatibility. A missing flag has been added and the code reformatted, as well as updates to the way the parameters are handled, for consistency. To minimize changes, it is assumed the regcomp error is of the right type since we control the only caller, and is also assumed to affect both basic and extended syntax (only basic is tested, but both behave the same in all three affected platforms since they use the same function). Based-on-patch-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-18commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flagLibravatar Taylor Blau1-4/+11
Since 7c5c9b9c57 (commit-graph: error out on invalid commit oids in 'write --stdin-commits', 2019-08-05), the commit-graph builtin dies on receiving non-commit OIDs as input to '--stdin-commits'. This behavior can be cumbersome to work around in, say, the case of piping 'git for-each-ref' to 'git commit-graph write --stdin-commits' if the caller does not want to cull out non-commits themselves. In this situation, it would be ideal if 'git commit-graph write' wrote the graph containing the inputs that did pertain to commits, and silently ignored the remainder of the input. Some options have been proposed to the effect of '--[no-]check-oids' which would allow callers to have the commit-graph builtin do just that. After some discussion, it is difficult to imagine a caller who wouldn't want to pass '--no-check-oids', suggesting that we should get rid of the behavior of complaining about non-commit inputs altogether. If callers do wish to retain this behavior, they can easily work around this change by doing the following: git for-each-ref --format='%(objectname) %(objecttype) %(*objecttype)' | awk ' !/commit/ { print "not-a-commit:"$1 } /commit/ { print $1 } ' | git commit-graph write --stdin-commits To make it so that valid OIDs that refer to non-existent objects are indeed an error after loosening the error handling, perform an extra lookup to make sure that object indeed exists before sending it to the commit-graph internals. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-18t5318: reorder test below 'graph_read_expect'Libravatar Taylor Blau1-9/+9
In the subsequent commit, we will introduce a dependency on 'graph_read_expect' from t5318.7. Preemptively move it below 'graph_read_expect()'s definition so that the test can call it. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-18tests: skip small-stack tests on hppa architectureLibravatar Greg Price1-2/+10
On hppa these tests crash because the allocated stack space is too small, even after it was doubled in b9a190789 (and the data size doubled to match) to make it work on powerpc. For this arch just skip these tests, which is enough to make the whole suite pass. Fixes: https://bugs.debian.org/757402 Based-on-patch-by: John David Anglin <dave.anglin@bell.net> Signed-off-by: Greg Price <gnprice@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-15Revert "tests: when run in Bash, annotate test failures with file name/line ↵Libravatar Junio C Hamano1-13/+1
number" This reverts commit 662f9cf1548cf069cb819e9e95f224657015fcf9, to fix the TAP output broken for bash.
2020-05-15Revert "t/test_lib: avoid naked bash arrays in file_lineno"Libravatar Junio C Hamano1-10/+8
This reverts commit 303775a25f0b4ac5d6ad2e96eb4404c24209cad8; instead of trying to salvage the tap-breaking change, let's revert the whole thing for now.
2020-05-14Merge branch 'es/trace-log-progress'Libravatar Junio C Hamano1-0/+26
Teach codepaths that show progress meter to also use the start_progress() and the stop_progress() calls as a "region" to be traced. * es/trace-log-progress: trace2: log progress time and throughput
2020-05-14Merge branch 'jt/t5500-unflake'Libravatar Junio C Hamano1-6/+6
Test fix for a topic already in 'master' and meant for 'maint'. * jt/t5500-unflake: t5500: count objects through stderr, not trace
2020-05-14Merge branch 'sn/midx-repack-with-config'Libravatar Junio C Hamano1-0/+27
"git multi-pack-index repack" has been taught to honor some repack.* configuration variables. * sn/midx-repack-with-config: multi-pack-index: respect repack.packKeptObjects=false midx: teach "git multi-pack-index repack" honor "git repack" configurations
2020-05-14Merge branch 'ds/bloom-cleanup'Libravatar Junio C Hamano2-4/+4
Code cleanup and typofixes * ds/bloom-cleanup: completion: offer '--(no-)patch' among 'git log' options bloom: use num_changes not nr for limit detection bloom: de-duplicate directory entries Documentation: changed-path Bloom filters use byte words bloom: parse commit before computing filters test-bloom: fix usage typo bloom: fix whitespace around tab length
2020-05-14Merge branch 'rs/fsck-duplicate-names-in-trees'Libravatar Junio C Hamano1-0/+16
"git fsck" ensures that the paths recorded in tree objects are sorted and without duplicates, but it failed to notice a case where a blob is followed by entries that sort before a tree with the same name. This has been corrected. * rs/fsck-duplicate-names-in-trees: fsck: report non-consecutive duplicate names in trees
2020-05-14Merge branch 'ao/p4-d-f-conflict-recover'Libravatar Junio C Hamano1-0/+70
"git p4" learned to recover from a (broken) state where a directory and a file are recorded at the same path in the Perforce repository the same way as their clients do. * ao/p4-d-f-conflict-recover: git-p4: recover from inconsistent perforce history
2020-05-14Merge branch 'js/rebase-autosquash-double-fixup-fix'Libravatar Junio C Hamano1-0/+16
"rebase -i" segfaulted when rearranging a sequence that has a fix-up that applies another fix-up (which may or may not be a fix-up of yet another step). * js/rebase-autosquash-double-fixup-fix: rebase --autosquash: fix a potential segfault
2020-05-14Merge branch 'cw/bisect-replay-with-dos'Libravatar Junio C Hamano1-0/+7
"git bisect replay" had trouble with input files when they used CRLF line ending, which has been corrected. * cw/bisect-replay-with-dos: bisect: allow CRLF line endings in "git bisect replay" input
2020-05-14Merge branch 'es/bugreport-with-hooks'Libravatar Junio C Hamano1-0/+15
"git bugreport" learned to report enabled hooks in the repository. * es/bugreport-with-hooks: bugreport: collect list of populated hooks
2020-05-14merge-recursive: fix rename/rename(1to2) for working tree with a binaryLibravatar Elijah Newren1-0/+55
With a rename/rename(1to2) conflict, we attempt to do a three-way merge of the file contents, so that the correct contents can be placed in the working tree at both paths. If the file is a binary, however, no content merging is possible and we should just use the original version of the file at each of the paths. Reported-by: Chunlin Zhang <zhangchunlin@gmail.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-13Merge branch 'cc/upload-pack-v2-fetch-fix'Libravatar Junio C Hamano1-4/+3
Serving a "git fetch" client over "git://" and "ssh://" protocols using the on-wire protocol version 2 was buggy on the server end when the client needs to make a follow-up request to e.g. auto-follow tags. * cc/upload-pack-v2-fetch-fix: upload-pack: clear filter_options for each v2 fetch command
2020-05-13Merge branch 'dd/bloom-sparse-fix'Libravatar Junio C Hamano3-3/+3
Code clean-up. * dd/bloom-sparse-fix: bloom: fix `make sparse` warning
2020-05-13Merge branch 'tb/bitmap-walk-with-tree-zero-filter'Libravatar Junio C Hamano2-0/+31
The object walk with object filter "--filter=tree:0" can now take advantage of the pack bitmap when available. * tb/bitmap-walk-with-tree-zero-filter: pack-bitmap: pass object filter to fill-in traversal pack-bitmap.c: support 'tree:0' filtering pack-bitmap.c: make object filtering functions generic list-objects-filter: treat NULL filter_options as "disabled"
2020-05-13t1509: correct i18n testLibravatar Đoàn Trần Công Danh1-2/+2
git-init(1)'s messages is subjected to i18n. They should be tested by test_i18n* family. Fix them. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-12trace2: log progress time and throughputLibravatar Emily Shaffer1-0/+26
Rather than teaching only one operation, like 'git fetch', how to write down throughput to traces, we can learn about a wide range of user operations that may seem slow by adding tooling to the progress library itself. Operations which display progress are likely to be slow-running and the kind of thing we want to monitor for performance anyways. By showing object counts and data transfer size, we should be able to make some derived measurements to ensure operations are scaling the way we expect. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-11bloom: use num_changes not nr for limit detectionLibravatar Derrick Stolee1-1/+1
As diff_tree_oid() computes a diff, it will terminate early if the total number of changed paths is strictly larger than max_changes. This includes the directories that changed, not just the file paths. However, only the file paths are reflected in the resulting diff queue's "nr" value. Use the "num_changes" from diffopt to check if the diff terminated early. This is incredibly important, as it can result in incorrect filters! For example, the first commit in the Linux kernel repo reports only 471 changes, but since these are nested inside several directories they expand to 513 "real" changes, and in fact the total list of changes is not reported. Thus, the computed filter for this commit is incorrect. Demonstrate the subtle difference by using one fewer file change in the 'get bloom filter for commit with 513 changes' test. Before, this edited 513 files inside "bigDir" which hit this inequality. However, dropping the file count by one demonstrates how the previous inequality was incorrect but the new one is correct. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-11line-log: more responsive, incremental 'git log -L'Libravatar SZEDER Gábor1-1/+1
The current line-level log implementation performs a preprocessing step in prepare_revision_walk(), during which the line_log_filter() function filters and rewrites history to keep only commits modifying the given line range. This preprocessing affects both responsiveness and correctness: - Git doesn't produce any output during this preprocessing step. Checking whether a commit modified the given line range is somewhat expensive, so depending on the size of the given revision range this preprocessing can result in a significant delay before the first commit is shown. - Limiting the number of displayed commits (e.g. 'git log -3 -L...') doesn't limit the amount of work during preprocessing, because that limit is applied during history traversal. Alas, by that point this expensive preprocessing step has already churned through the whole revision range to find all commits modifying the revision range, even though only a few of them need to be shown. - It rewrites parents, with no way to turn it off. Without the user explicitly requesting parent rewriting any parent object ID shown should be that of the immediate parent, just like in case of a pathspec-limited history traversal without parent rewriting. However, after that preprocessing step rewrote history, the subsequent "regular" history traversal (i.e. get_revision() in a loop) only sees commits modifying the given line range. Consequently, it can only show the object ID of the last ancestor that modified the given line range (which might happen to be the immediate parent, but many-many times it isn't). This patch addresses both the correctness and, at least for the common case, the responsiveness issues by integrating line-level log filtering into the regular revision walking machinery: - Make process_ranges_arbitrary_commit(), the static function in 'line-log.c' deciding whether a commit modifies the given line range, public by removing the static keyword and adding the 'line_log_' prefix, so it can be called from other parts of the revision walking machinery. - If the user didn't explicitly ask for parent rewriting (which, I believe, is the most common case): - Call this now-public function during regular history traversal, namely from get_commit_action() to ignore any commits not modifying the given line range. Note that while this check is relatively expensive, it must be performed before other, much cheaper conditions, because the tracked line range must be adjusted even when the commit will end up being ignored by other conditions. - Skip the line_log_filter() call, i.e. the expensive preprocessing step, in prepare_revision_walk(), because, thanks to the above points, the revision walking machinery is now able to filter out commits not modifying the given line range while traversing history. This way the regular history traversal sees the unmodified history, and is therefore able to print the object ids of the immediate parents of the listed commits. The eliminated preprocessing step can greatly reduce the delay before the first commit is shown, see the numbers below. - However, if the user did explicitly ask for parent rewriting via '--parents' or a similar option, then stick with the current implementation for now, i.e. perform that expensive filtering and history rewriting in the preprocessing step just like we did before, leaving the initial delay as long as it was. I tried to integrate line-level log filtering with parent rewriting into the regular history traversal, but, unfortunately, several subtleties resisted... :) Maybe someday we'll figure out how to do that, but until then at least the simple and common (i.e. without parent rewriting) 'git log -L:func:file' commands can benefit from the reduced delay. This change makes the failing 'parent oids without parent rewriting' test in 't4211-line-log.sh' succeed. The reduced delay is most noticable when there's a commit modifying the line range near the tip of a large-ish revision range: # no parent rewriting requested, no commit-graph present $ time git --no-pager log -L:read_alternate_refs:sha1-file.c -1 v2.23.0 Before: real 0m9.570s user 0m9.494s sys 0m0.076s After: real 0m0.718s user 0m0.674s sys 0m0.044s A significant part of the remaining delay is spent reading and parsing commit objects in limit_list(). With the help of the commit-graph we can eliminate most of that reading and parsing overhead, so here are the timing results of the same command as above, but this time using the commit-graph: Before: real 0m8.874s user 0m8.816s sys 0m0.057s After: real 0m0.107s user 0m0.091s sys 0m0.013s The next patch will further reduce the remaining delay. To be clear: this patch doesn't actually optimize the line-level log, but merely moves most of the work from the preprocessing step to the history traversal, so the commits modifying the line range can be shown as soon as they are processed, and the traversal can be terminated as soon as the given number of commits are shown. Consequently, listing the full history of a line range, potentially all the way to the root commit, will take the same time as before (but at least the user might start reading the output earlier). Furthermore, if the most recent commit modifying the line range is far away from the starting revision, then that initial delay will still be significant. Additional testing by Derrick Stolee: In the Linux kernel repository, the MAINTAINERS file was changed ~3,500 times across the ~915,000 commits. In addition to that edit frequency, the file itself is quite large (~18,700 lines). This means that a significant portion of the computation is taken up by computing the patch-diff of the file. This patch improves the real time it takes to output the first result quite a bit: Command: git log -L 100,200:MAINTAINERS -n 1 >/dev/null Before: 3.88 s After: 0.71 s If we drop the "-n 1" in the command, then there is no change in end-to-end process time. This is because the command still needs to walk the entire commit history, which negates the point of this patch. This is expected. As a note for future reference, the ~4.3 seconds in the old code spends ~2.6 seconds computing the patch-diffs, and the rest of the time is spent walking commits and computing diffs for which paths changed at each commit. The changed-path Bloom filters could improve the end-to-end computation time (i.e. no "-n 1" in the command). Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>