summaryrefslogtreecommitdiffstats
path: root/t/README (follow)
Commit message (Collapse)AuthorAgeFilesLines
* test-lib: unconditionally enable leak checkingPatrick Steinhardt2024-11-211-21/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Over the last two releases we have plugged a couple hundred of memory leaks exposed by the Git test suite. With the preceding commits we have finally fixed the last leak exposed by our test suite, which means that we are now basically leak free wherever we have branch coverage. From hereon, the Git test suite should ideally stay free of memory leaks. Most importantly, any test suite that is being added should automatically be subject to the leak checker, and if that test does not pass it is a strong signal that the added code introduced new memory leaks and should not be accepted without further changes. Drop the infrastructure around TEST_PASSES_SANITIZE_LEAK to reflect this new requirement. Like this, all test suites will be subject to the leak checker by default. This is being intentionally strict, but we still have an escape hatch: the SANITIZE_LEAK prerequisite. There is one known case in t5601 where the leak sanitizer itself is buggy, so adding this prereq in such cases is acceptable. Another acceptable situation is when a newly added test uncovers preexisting memory leaks: when fixing that memory leak would be sufficiently complicated it is fine to annotate and document the leak accordingly. But in any case, the burden is now on the patch author to explain why exactly they have to add the SANITIZE_LEAK prerequisite. The TEST_PASSES_SANITIZE_LEAK annotations will be dropped in the next patch. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t/README: add missing value for GIT_TEST_DEFAULT_REF_FORMATBence Ferdinandy2024-10-111-2/+3
| | | | | | | | The documentation only lists "files" as a possible value, but "reftable" is also valid. Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t/test-lib: allow skipping leak checks for passing testsPatrick Steinhardt2024-09-051-0/+3
| | | | | | | | | | | | | | | | | | | | | | | With `GIT_TEST_PASSING_SANITIZE_LEAK=check`, one can double check whether a memory leak fix caused some test suites to become leak free. This is done by running all tests with the leak checker enabled. If a test suite does not declare `TEST_PASSES_SANITIZE_LEAK=true` but still finishes successfully with the leak checker enabled, then this indicates that the test is leak free and thus missing the annotation. It is somewhat slow to execute though because it runs all of our test suites with the leak sanitizer enabled. It is also pointless in most cases, because the only test suites that need to be checked are those which _aren't_ yet marked with `TEST_PASSES_SANITIZE_LEAK=true`. Introduce a new value "check-failing". When set, we behave the same as if "check" was passed, except that we only check those tests which do not have `TEST_PASSES_SANITIZE_LEAK=true` set. This is significantly faster than running all test suites but still fulfills the usecase of finding newly-leak-free test suites. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'tb/incremental-midx-part-1'Junio C Hamano2024-08-191-3/+3
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Incremental updates of multi-pack index files. * tb/incremental-midx-part-1: midx: implement support for writing incremental MIDX chains t/t5313-pack-bounds-checks.sh: prepare for sub-directories t: retire 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP' midx: implement verification support for incremental MIDXs midx: support reading incremental MIDX chains midx: teach `midx_fanout_add_midx_fanout()` about incremental MIDXs midx: teach `midx_preferred_pack()` about incremental MIDXs midx: teach `midx_contains_pack()` about incremental MIDXs midx: remove unused `midx_locate_pack()` midx: teach `fill_midx_entry()` about incremental MIDXs midx: teach `nth_midxed_offset()` about incremental MIDXs midx: teach `bsearch_midx()` about incremental MIDXs midx: introduce `bsearch_one_midx()` midx: teach `nth_bitmapped_pack()` about incremental MIDXs midx: teach `nth_midxed_object_oid()` about incremental MIDXs midx: teach `prepare_midx_pack()` about incremental MIDXs midx: teach `nth_midxed_pack_int_id()` about incremental MIDXs midx: add new fields for incremental MIDX chains Documentation: describe incremental MIDX format
| * midx: implement support for writing incremental MIDX chainsTaylor Blau2024-08-061-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that the rest of the MIDX subsystem and relevant callers have been updated to learn about how to read and process incremental MIDX chains, let's finally update the implementation in `write_midx_internal()` to be able to write incremental MIDX chains. This new feature is available behind the `--incremental` option for the `multi-pack-index` builtin, like so: $ git multi-pack-index write --incremental The implementation for doing so is relatively straightforward, and boils down to a handful of different kinds of changes implemented in this patch: - The `compute_sorted_entries()` function is taught to reject objects which appear in any existing MIDX layer. - Functions like `write_midx_revindex()` are adjusted to write pack_order values which are offset by the number of objects in the base MIDX layer. - The end of `write_midx_internal()` is adjusted to move non-incremental MIDX files when necessary (i.e. when creating an incremental chain with an existing non-incremental MIDX in the repository). There are a handful of other changes that are introduced, like new functions to clear incremental MIDX files that are unrelated to the current chain (using the same "keep_hash" mechanism as in the non-incremental case). The tests explicitly exercising the new incremental MIDX feature are relatively limited for two reasons: 1. Most of the "interesting" behavior is already thoroughly covered in t5319-multi-pack-index.sh, which handles the core logic of reading objects through a MIDX. The new tests in t5334-incremental-multi-pack-index.sh are mostly focused on creating and destroying incremental MIDXs, as well as stitching their results together across layers. 2. A new GIT_TEST environment variable is added called "GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL", which modifies the entire test suite to write incremental MIDXs after repacking when combined with the "GIT_TEST_MULTI_PACK_INDEX" variable. This exercises the long tail of other interesting behavior that is defined implicitly throughout the rest of the CI suite. It is likewise added to the linux-TEST-vars job. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * t: retire 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP'Taylor Blau2024-08-061-4/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Two years ago, commit ff1e653c8e2 (midx: respect 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP', 2021-08-31) introduced a new environment variable which caused the test suite to write MIDX bitmaps after any 'git repack' invocation. At the time, this was done to help flush out any bugs with MIDX bitmaps that weren't explicitly covered in the t5326-multi-pack-bitmap.sh script. Two years later, that flag has served us well and is no longer providing meaningful coverage, as the script in t5326 has matured substantially and covers many more interesting cases than it did back when ff1e653c8e2 was originally written. Remove the 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP' environment variable as it is no longer serving a useful purpose. More importantly, removing this variable clears the way for us to introduce a new one to help similarly flush out bugs related to incremental MIDX chains. Because these incremental MIDX chains are (for now) incompatible with MIDX bitmaps, we cannot have both. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jk/test-body-in-here-doc'Junio C Hamano2024-07-171-0/+8
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The test framework learned to take the test body not as a single string but as a here-document. * jk/test-body-in-here-doc: t/.gitattributes: ignore whitespace in chainlint expect files t: convert some here-doc test bodies test-lib: allow test snippets as here-docs chainlint.pl: add tests for test body in heredoc chainlint.pl: recognize test bodies defined via heredoc chainlint.pl: check line numbers in expected output chainlint.pl: force CRLF conversion when opening input files chainlint.pl: do not spawn more threads than we have scripts chainlint.pl: only start threads if jobs > 1 chainlint.pl: add test_expect_success call to test snippets
| * | test-lib: allow test snippets as here-docsJeff King2024-07-101-0/+8
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Most test snippets are wrapped in single quotes, like: test_expect_success 'some description' ' do_something ' This sometimes makes the snippets awkward to write, because you can't easily use single quotes within them. We sometimes work around this with $SQ, or by loosening regexes to use "." instead of a literal quote, or by using double quotes when we'd prefer to use single-quotes (and just adding extra backslash-escapes to avoid interpolation). This commit adds another option: feeding the snippet via the function's stdin. This doesn't conflict with anything the snippet would want to do, because we always redirect its stdin from /dev/null anyway (which we'll continue to do). A few notes on the implementation: - it would be nice to push this down into test_run_, but we can't, as test_expect_success and test_expect_failure want to see the actual script content to report it for verbose-mode. A helper function limits the amount of duplication in those callers here. - The helper function is a little awkward to call, as you feed it the name of the variable you want to set. The more natural thing in shell would be command substitution like: body=$(body_or_stdin "$2") but that loses trailing whitespace. There are tricks around this, like: body=$(body_or_stdin "$2"; printf .) body=${body%.} but we'd prefer to keep such tricks in the helper, not in each caller. - I implemented the helper using a sequence of "read" calls. Together with "-r" and unsetting the IFS, this preserves incoming whitespace. An alternative is to use "cat" (which then requires the gross "." trick above). But this saves us a process, which is probably a good thing. The "read" builtin does use more read() syscalls than necessary (one per byte), but that is almost certainly a win over a separate process. Both are probably slower than passing a single-quoted string, but the difference is lost in the noise for a script that I converted as an experiment. - I handle test_expect_success and test_expect_failure here. If we like this style, we could easily extend it to other spots (e.g., lazy_prereq bodies) on top of this patch. - even though we are using "local", we have to be careful about our variable names. Within test_expect_success, any variable we declare with local will be seen as local by the test snippets themselves (so it wouldn't persist between tests like normal variables would). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* / test-lib: GIT_TEST_SANITIZE_LEAK_LOG enabled by defaultRubén Justo2024-07-111-25/+1
|/ | | | | | | | | | | | | | | | | | | | | | | | As we currently describe in t/README, it can happen that: Some tests run "git" (or "test-tool" etc.) without properly checking the exit code, or git will invoke itself and fail to ferry the abort() exit code to the original caller. Therefore, GIT_TEST_SANITIZE_LEAK_LOG=true is needed to be set to capture all memory leaks triggered by our tests. It seems unnecessary to force users to remember this option, as forgetting it could lead to missed memory leaks. We could solve the problem by making it "true" by default, but that might suggest we think "false" makes sense, which isn't the case. Therefore, the best approach is to remove the option entirely while maintaining the capability to detect memory leaks in blind spots of our tests. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'pb/test-scripts-are-build-targets'Junio C Hamano2024-04-031-0/+7
|\ | | | | | | | | | | | | | | The t/README file now gives a hint on running individual tests in the "t/" directory with "make t<num>-*.sh t<num>-*.sh". * pb/test-scripts-are-build-targets: t/README: mention test files are make targets
| * t/README: mention test files are make targetsPhilippe Blain2024-03-251-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since 23fc63bf8f (make tests ignorable with "make -i", 2005-11-08), each test file defines a target in the test Makefile, such that one can invoke: make *checkout* to run all tests with 'checkout' in their filename. This is useful to run a subset of tests when you have a good idea of what part of the code is touched by the changes your are testing. Document that in t/README to help new (or more seasoned) contributors that might not be aware. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ps/t7800-variable-interpolation-fix'Junio C Hamano2024-04-011-0/+20
|\ \ | |/ |/| | | | | | | | | | | | | | | | | Fix the way recently added tests interpolate variables defined outside them, and document the best practice to help future developers. * ps/t7800-variable-interpolation-fix: t/README: document how to loop around test cases t7800: use single quotes for test bodies t7800: improve test descriptions with empty arguments
| * t/README: document how to loop around test casesPatrick Steinhardt2024-03-221-0/+20
| | | | | | | | | | | | | | | | | | | | | | | | In some cases it makes sense to loop around test cases so that we can execute the same test with slightly different arguments. There are some gotchas around quoting here though that are easy to miss and that may lead to easy-to-miss errors and portability issues. Document the proper way to do this in "t/README". Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * Merge branch 'js/update-urls-in-doc-and-comment' into maint-2.43Junio C Hamano2024-02-091-2/+2
| |\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stale URLs have been updated to their current counterparts (or archive.org) and HTTP links are replaced with working HTTPS links. * js/update-urls-in-doc-and-comment: doc: refer to internet archive doc: update links for andre-simon.de doc: switch links to https doc: update links to current pages
* | | t: introduce GIT_TEST_DEFAULT_REF_FORMAT envvarPatrick Steinhardt2024-01-021-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduce a new GIT_TEST_DEFAULT_REF_FORMAT environment variable that lets developers run the test suite with a different default ref format without impacting the ref format used by non-test Git invocations. This is modeled after GIT_TEST_DEFAULT_OBJECT_FORMAT, which does the same thing for the repository's object format. Adapt the setup of the `REFFILES` test prerequisite to be conditionally set based on the default ref format. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'js/update-urls-in-doc-and-comment'Junio C Hamano2023-12-181-2/+2
|\ \ \ | |/ / |/| / | |/ | | | | | | | | | | | | | | Stale URLs have been updated to their current counterparts (or archive.org) and HTTP links are replaced with working HTTPS links. * js/update-urls-in-doc-and-comment: doc: refer to internet archive doc: update links for andre-simon.de doc: switch links to https doc: update links to current pages
| * doc: switch links to httpsJosh Soref2023-11-261-2/+2
| | | | | | | | | | | | | | | | These sites offer https versions of their content. Using the https versions provides some protection for users. Signed-off-by: Josh Soref <jsoref@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | t/README: fix multi-prerequisite exampleŠtěpán Němec2023-10-051-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | With the broken quoting the test wouldn't even parse correctly, but there's also the '==' instead of POSIX '=' (of the shells I tested, busybox ash, bash and ksh (93 and OpenBSD) accept '==', dash and zsh do not), and 'print 2' from Python 2 days. (I assume the test failing due to 3 != 4 is intentional or immaterial.) Fixes: 93a572461386 ("test-lib: Add support for multiple test prerequisites") Signed-off-by: Štěpán Němec <stepnem@smrk.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | doc: fix some typos, grammar and wording issuesŠtěpán Němec2023-10-051-16/+15
|/ | | | | Signed-off-by: Štěpán Němec <stepnem@smrk.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'bc/more-git-var'Junio C Hamano2023-07-051-0/+6
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add more "git var" for toolsmiths to learn various locations Git is configured with either via the configuration or hardcoded defaults. * bc/more-git-var: var: add config file locations var: add attributes files locations attr: expose and rename accessor functions var: adjust memory allocation for strings var: format variable structure with C99 initializers var: add support for listing the shell t: add a function to check executable bit var: mark unused parameters in git_var callbacks
| * t: add a function to check executable bitbrian m. carlson2023-06-271-0/+6
| | | | | | | | | | | | | | | | | | | | | | In line with our other helper functions for paths, let's add a function to check whether a path is executable, and if not, print a suitable error message. Document this function, and note that it must only be used under the POSIXPERM prerequisite, since it doesn't otherwise work on Windows. Signed-off-by: brian m. carlson <bk2204@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | pack-bitmap.c: use commit boundary during bitmap traversalTaylor Blau2023-05-081-0/+4
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When reachability bitmap coverage exists in a repository, Git will use a different (and hopefully faster) traversal to compute revision walks. Consider a set of positive and negative tips (which we'll refer to with their standard bitmap parlance by "wants", and "haves"). In order to figure out what objects exist between the tips, the existing traversal in `prepare_bitmap_walk()` does something like: 1. Consider if we can even compute the set of objects with bitmaps, and fall back to the usual traversal if we cannot. For example, pathspec limiting traversals can't be computed using bitmaps (since they don't know which objects are at which paths). The same is true of certain kinds of non-trivial object filters. 2. If we can compute the traversal with bitmaps, partition the (dereferenced) tips into two object lists, "haves", and "wants", based on whether or not the objects have the UNINTERESTING flag, respectively. 3. Fall back to the ordinary object traversal if either (a) there are more than zero haves, none of which are in the bitmapped pack or MIDX, or (b) there are no wants. 4. Construct a reachability bitmap for the "haves" side by walking from the revision tips down to any existing bitmaps, OR-ing in any bitmaps as they are found. 5. Then do the same for the "wants" side, stopping at any objects that appear in the "haves" bitmap. 6. Filter the results if any object filter (that can be easily computed with bitmaps alone) was given, and then return back to the caller. When there is good bitmap coverage relative to the traversal tips, this walk is often significantly faster than an ordinary object traversal because it can visit far fewer objects. But in certain cases, it can be significantly *slower* than the usual object traversal. Why? Because we need to compute complete bitmaps on either side of the walk. If either one (or both) of the sides require walking many (or all!) objects before they get to an existing bitmap, the extra bitmap machinery is mostly or all overhead. One of the benefits, however, is that even if the walk is slower, bitmap traversals are guaranteed to provide an *exact* answer. Unlike the traditional object traversal algorithm, which can over-count the results by not opening trees for older commits, the bitmap walk builds an exact reachability bitmap for either side, meaning the results are never over-counted. But producing non-exact results is OK for our traversal here (both in the bitmap case and not), as long as the results are over-counted, not under. Relaxing the bitmap traversal to allow it to produce over-counted results gives us the opportunity to make some significant improvements. Instead of the above, the new algorithm only has to walk from the *boundary* down to the nearest bitmap, instead of from each of the UNINTERESTING tips. The boundary-based approach still has degenerate cases, but we'll show in a moment that it is often a significant improvement. The new algorithm works as follows: 1. Build a (partial) bitmap of the haves side by first OR-ing any bitmap(s) that already exist for UNINTERESTING commits between the haves and the boundary. 2. For each commit along the boundary, add it as a fill-in traversal tip (where the traversal terminates once an existing bitmap is found), and perform fill-in traversal. 3. Build up a complete bitmap of the wants side as usual, stopping any time we intersect the (partial) haves side. 4. Return the results. And is more-or-less equivalent to using the *old* algorithm with this invocation: $ git rev-list --objects --use-bitmap-index $WANTS --not \ $(git rev-list --objects --boundary $WANTS --not $HAVES | perl -lne 'print $1 if /^-(.*)/') The new result performs significantly better in many cases, particularly when the distance from the boundary commit(s) to an existing bitmap is shorter than the distance from (all of) the have tips to the nearest bitmapped commit. Note that when using the old bitmap traversal algorithm, the results can be *slower* than without bitmaps! Under the new algorithm, the result is computed faster with bitmaps than without (at the cost of over-counting the true number of objects in a similar fashion as the non-bitmap traversal): # (Computing the number of tagged objects not on any branches # without bitmaps). $ time git rev-list --count --objects --tags --not --branches 20 real 0m1.388s user 0m1.092s sys 0m0.296s # (Computing the same query using the old bitmap traversal). $ time git rev-list --count --objects --tags --not --branches --use-bitmap-index 19 real 0m22.709s user 0m21.628s sys 0m1.076s # (this commit) $ time git.compile rev-list --count --objects --tags --not --branches --use-bitmap-index 19 real 0m1.518s user 0m1.234s sys 0m0.284s The new algorithm is still slower than not using bitmaps at all, but it is nearly a 15-fold improvement over the existing traversal. In a more realistic setting (using my local copy of git.git), I can observe a similar (if more modest) speed-up: $ argv="--count --objects --branches --not --tags" hyperfine \ -n 'no bitmaps' "git.compile rev-list $argv" \ -n 'existing traversal' "git.compile rev-list --use-bitmap-index $argv" \ -n 'boundary traversal' "git.compile -c pack.useBitmapBoundaryTraversal=true rev-list --use-bitmap-index $argv" Benchmark 1: no bitmaps Time (mean ± σ): 124.6 ms ± 2.1 ms [User: 103.7 ms, System: 20.8 ms] Range (min … max): 122.6 ms … 133.1 ms 22 runs Benchmark 2: existing traversal Time (mean ± σ): 368.6 ms ± 3.0 ms [User: 325.3 ms, System: 43.1 ms] Range (min … max): 365.1 ms … 374.8 ms 10 runs Benchmark 3: boundary traversal Time (mean ± σ): 167.6 ms ± 0.9 ms [User: 139.5 ms, System: 27.9 ms] Range (min … max): 166.1 ms … 169.2 ms 17 runs Summary 'no bitmaps' ran 1.34 ± 0.02 times faster than 'boundary traversal' 2.96 ± 0.05 times faster than 'existing traversal' Here, the new algorithm is also still slower than not using bitmaps, but represents a more than 2-fold improvement over the existing traversal in a more modest example. Since this algorithm was originally written (nearly a year and a half ago, at the time of writing), the bitmap lookup table shipped, making the new algorithm's result more competitive. A few other future directions for improving bitmap traversal times beyond not using bitmaps at all: - Decrease the cost to decompress and OR together many bitmaps together (particularly when enumerating the uninteresting side of the walk). Here we could explore more efficient bitmap storage techniques, like Roaring+Run and/or use SIMD instructions to speed up ORing them together. - Store pseudo-merge bitmaps, which could allow us to OR together fewer "summary" bitmaps (which would also help with the above). Helped-by: Jeff King <peff@peff.net> Helped-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t: invert `GIT_TEST_WRITE_REV_INDEX`Taylor Blau2023-04-131-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Back in e8c58f894b (t: support GIT_TEST_WRITE_REV_INDEX, 2021-01-25), we added a test knob to conditionally enable writing a ".rev" file when indexing a pack. At the time, this was used to ensure that the test suite worked even when ".rev" files were written, which served as a stress-test for the on-disk reverse index implementation. Now that reading from on-disk ".rev" files is enabled by default, the test knob `GIT_TEST_WRITE_REV_INDEX` no longer has any meaning. We could get rid of the option entirely, but there would be no convenient way to test Git when ".rev" files *aren't* in place. Instead of getting rid of the option, invert its meaning to instead disable writing ".rev" files, thereby running the test suite in a mode where the reverse index is generated from scratch. This ensures that, when GIT_TEST_NO_WRITE_REV_INDEX is set to some spelling of "true", we are still running and exercising Git's behavior when forced to generate reverse indexes from scratch. Do so by setting it in the linux-TEST-vars CI run to ensure that we are maintaining good coverage of this now-legacy code. Signed-off-by: Taylor Blau <me@ttaylorr.com> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* add: remove "add.interactive.useBuiltin" & Perl "git add--interactive"Ævar Arnfjörð Bjarmason2023-02-071-4/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since [1] first released with Git v2.37.0 the built-in version of "add -i" has been the default. That built-in implementation was added in [2], first released with Git v2.25.0. At this point enough time has passed to allow for finding any remaining bugs in this new implementation, so let's remove the fallback code. As with similar migrations for "stash"[3] and "rebase"[4] we're keeping a mention of "add.interactive.useBuiltin" in the documentation, but adding a warning() to notify any outstanding users that the built-in is now the default. As with [5] and [6] we should follow-up in the future and eventually remove that warning. 1. 0527ccb1b55 (add -i: default to the built-in implementation, 2021-11-30) 2. f83dff60a78 (Start to implement a built-in version of `git add --interactive`, 2019-11-13) 3. 8a2cd3f5123 (stash: remove the stash.useBuiltin setting, 2020-03-03) 4. d03ebd411c6 (rebase: remove the rebase.useBuiltin setting, 2019-03-18) 5. deeaf5ee077 (stash: remove documentation for `stash.useBuiltin`, 2022-01-27) 6. 9bcde4d5314 (rebase: remove transitory rebase.useBuiltin setting & env, 2021-03-23) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* test-lib: retire "lint harder" optimization hackEric Sunshine2022-09-011-5/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | `test_run_` in test-lib.sh "lints" the body of a test by sending it down a `sed chainlint.sed | grep` pipeline; this happens once for each test run by a test script. Although this pipeline may seem relatively cheap in isolation, it can become expensive when invoked 26800+ times by `make test`, once for each test run, despite the existence of only 16500+ test definitions across all tests scripts. This difference in the number of tests defined in the scripts (16500+) and the number of tests actually run by `make test` (26800+) is explained by the fact that some test scripts run a very large number of small tests, all driven by a series of functions/loops which fill in the test bodies. This means that certain test definitions are being linted repeatedly (tens or hundreds of times) unnecessarily. To avoid such unnecessary work, 2d86a96220 (t: avoid sed-based chain-linting in some expensive cases, 2021-05-13) added an optimization hack which allows individual scripts to manually suppress the unnecessary repeated linting of the same test definition. However, unlike chainlint.sed which checks a test body as the test is run, chainlint.pl checks each test definition just once, no matter how many times the test is run, thus the sort of optimization hack introduced by 2d86a96220 is no longer needed and can be retired. Therefore, revert 2d86a96220. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* test-lib: have the "check" mode for SANITIZE=leak consider leak logsÆvar Arnfjörð Bjarmason2022-07-281-0/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As noted in previous on-list discussions[1] we have various tests that will falsely report being leak-free because we're missing the relevant exit code from LSAN as summarized below. We should fix those issues, but in the meantime and as an additional sanity check we can and should consider our own ASAN logs before reporting that a test is leak-free. Before this compiling with SANITIZE=leak and running: ./t6407-merge-binary.sh Will exit successfully, now we'll get an error and an informative message on: GIT_TEST_SANITIZE_LEAK_LOG=true ./t6407-merge-binary.sh Even better, as noted in the updated t/README we'll now error out when combined with the "check" mode: GIT_TEST_PASSING_SANITIZE_LEAK=check \ GIT_TEST_SANITIZE_LEAK_LOG=true \ ./t4058-diff-duplicates.sh Why do we miss these leaks? Because: * We have leaks inside "test_expect_failure" blocks, which by design will not distinguish a "normal" failure from an abort() or segfault. See [1] for a discussion of it shortcomings. * We have "git" invocations outside of "test_expect_success", e.g. setup code in the main body of the test, or in test helper functions that don't use &&-chaining. * Our tests will otherwise catch segfaults and abort(), but if we invoke a command that invokes another command it needs to ferry the exit code up to us. Notably a command that e.g. might invoke "git pack-objects" might itself exit with status 128 if that "pack-objects" segfaults or abort()'s. If the test invoking the parent command(s) is using "test_must_fail" we'll consider it an expected "ok" failure. * run-command.c doesn't (but probably should) ferry up such exit codes, so for e.g. "git push" tests where we expect a failure and an underlying "git" command fails we won't ferry up the segfault or abort exit code. * We have gitweb.perl and some other perl code ignoring return values from close(), i.e. ignoring exit codes from "git rev-parse" et al. * We have in-tree shellscripts like "git-merge-one-file.sh" invoking git commands, they'll usually return their own exit codes on "git" failure, rather then ferrying up segfault or abort() exit code. E.g. these invocations in git-merge-one-file.sh leak, but aren't reflected in the "git merge" exit code: src1=$(git unpack-file $2) src2=$(git unpack-file $3) That case would be easily "fixed" by adding a line like this after each assignment: test $? -ne 0 && exit $? But we'd then in e.g. "t6407-merge-binary.sh" run into write_tree_trivial() in "builtin/merge.c" calling die() instead of ferrying up the relevant exit code. Let's remove "TEST_PASSES_SANITIZE_LEAK=true" from tests we were falsely marking as leak-free. In the case of t6407-merge-binary.sh it was marked as leak-free in 9081a421a6d (checkout: fix "branch info" memory leaks, 2021-11-16). I'd previously removed other bad "TEST_PASSES_SANITIZE_LEAK=true" opt-ins in the series merged in ea05fd5fbf7 (Merge branch 'ab/keep-git-exit-codes-in-tests', 2022-03-16). The case of t1060-object-corruption.sh is more subtle, and will be discussed in a subsequent commit. 1. https://lore.kernel.org/git/cover-0.7-00000000000-20220318T002951Z-avarab@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* test-lib: add a GIT_TEST_PASSING_SANITIZE_LEAK=check modeÆvar Arnfjörð Bjarmason2022-07-281-0/+17
| | | | | | | | | | | | | | | | | | | | | Add a new "GIT_TEST_PASSING_SANITIZE_LEAK=check" mode to the test-lib.sh. As noted in the updated "t/README" this compliments the existing "GIT_TEST_PASSING_SANITIZE_LEAK=true" mode added in 956d2e4639b (tests: add a test mode for SANITIZE=leak, run it in CI, 2021-09-23). Rather than document this all in one (even more) dense paragraph split up the discussion of how it combines with --immediate into its own paragraph following the discussion of "GIT_TEST_SANITIZE_LEAK_LOG=true". Before the removal of "test_external" in a preceding commit we would have had to special-case t9700-perl-git.sh and t0202-gettext-perl.sh. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* test-lib: simplify by removing test_externalÆvar Arnfjörð Bjarmason2022-07-281-26/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove the "test_external" function added in [1]. This arguably makes the output of t9700-perl-git.sh and friends worse. But as we'll argue below the trade-off is worth it, since "chaining" to another TAP emitter in test-lib.sh is more trouble than it's worth. The new output of t9700-perl-git.sh is now: $ ./t9700-perl-git.sh ok 1 - set up test repository ok 2 - use t9700/test.pl to test Git.pm # passed all 2 test(s) 1..2 Whereas before this change it would be: $ ./t9700-perl-git.sh ok 1 - set up test repository # run 1: Perl API (perl /home/avar/g/git/t/t9700/test.pl) ok 2 - use Git; [... omitting tests 3..46 from t/t9700/test.pl ...] ok 47 - unquote escape sequences 1..47 # test_external test Perl API was ok # test_external_without_stderr test no stderr: Perl API was ok At the time of its addition supporting "test_external" was easy, but when test-lib.sh itself started to emit TAP in [2] we needed to make everything surrounding the emission of the plan consider "test_external". I added that support in [2] so that we could run: prove ./t9700-perl-git.sh :: -v But since then in [3] the door has been closed on combining $HARNESS_ACTIVE and -v, we'll now just die: $ prove ./t9700-perl-git.sh :: -v Bailout called. Further testing stopped: verbose mode forbidden under TAP harness; try --verbose-log FAILED--Further testing stopped: verbose mode forbidden under TAP harness; try --verbose-log So the only use of this has been that *if* we had failure in one of these tests we could e.g. in CI see which test failed based on the test number. Now we'll need to look at the full verbose logs to get that same information. I think this trade-off is acceptable given the reduction in complexity, and it brings these tests in line with other similar tests, e.g. the reftable tests added in [4] will be condensed down to just one test, which invokes the C helper: $ ./t0032-reftable-unittest.sh ok 1 - unittests # passed all 1 test(s) 1..1 It would still be nice to have that ":: -v" form work again, it never *really* worked, but even though we've had edge cases test output screwing up the TAP it mostly worked between d998bd4ab67 and [3], so we may have been overzealous in forbidding it outright. I have local patches which I'm planning to submit sooner than later that get us to that goal, and in a way that isn't buggy. In the meantime getting rid of this special case makes hacking on this area of test-lib.sh easier, as we'll do in subsequent commits. The switch from "perl" to "$PERL_PATH" here is because "perl" is defined as a shell function in the test suite, see a5bf824f3b4 (t: prevent '-x' tracing from interfering with test helpers' stderr, 2018-02-25). On e.g. the OSX CI the "command perl"... will be part of the emitted stderr. 1. fb32c410087 (t/test-lib.sh: add test_external and test_external_without_stderr, 2008-06-19) 2. d998bd4ab67 (test-lib: Make the test_external_* functions TAP-aware, 2010-06-24) 3. 614fe015212 (test-lib: bail out when "-v" used under "prove", 2016-10-22) 4. ef8a6c62687 (reftable: utility functions, 2021-10-07) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* test-lib: add a SANITIZE=leak logging modeÆvar Arnfjörð Bjarmason2022-07-281-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add the ability to run the test suite under a new "GIT_TEST_SANITIZE_LEAK_LOG=true" mode, when true we'll log the leaks we find an a new "test-results/<test-name>.leak" directory. That new path is consistent with the existing "test-results/<test-name>.<type>" results, except that those are all files, not directories. We also set "log_exe_name=1" to include the name of the executable in the filename. This gives us files like "trace.git.<pid>" instead of the default of "trace.<pid>". I.e. we'll be able to distinguish "git" leaks from "test-tool", "git-daemon" etc. We then set "dedup_token_length" to non-zero ("0" is the default) to succinctly log a token we can de-duplicate these stacktraces on. The string is simply a one-line stack-trace with only function names up to N frames, which we limit at "9999" as a shorthand for "infinite" (there appears to be no way to say "no limit"). With these combined we can now easily get e.g. the top 10 leaks in the test suite grouped by full stacktrace: grep -o -P -h '(?<=DEDUP_TOKEN: ).*' test-results/*.leak/trace.git.* | sort | uniq -c | sort -nr | head -n 10 Or add "grep -E -o '[^-]+'" to that to group by functions instead of stack traces: grep -o -P -h '(?<=DEDUP_TOKEN: ).*' test-results/*.leak/trace.git.* | grep -E -o '[^-]+' | sort | uniq -c | sort -nr | head -n 20 This new mode requires git to be compiled with SANITIZE=leak, rather than explaining that in the documentation let's make it self-documenting by bailing out if the user asks for this without git having been compiled with SANITIZE=leak, as we do with GIT_TEST_PASSING_SANITIZE_LEAK=true. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t/README: reword the "GIT_TEST_PASSING_SANITIZE_LEAK" descriptionÆvar Arnfjörð Bjarmason2022-07-281-6/+4
| | | | | | | | | | | | | | Reword the documentation added in 956d2e4639b (tests: add a test mode for SANITIZE=leak, run it in CI, 2021-09-23) for brevity. The comment added in the same commit was also misleading: We skip certain tests if SANITIZE=leak and GIT_TEST_PASSING_SANITIZE_LEAK=true, not if we're compiled with SANITIZE=leak. Let's just remove the comment, the control flow here is obvious enough that the code can speak for itself. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* ls-files: update test styleLi Linchao2022-07-061-0/+55
| | | | | | | | | | | | Update test style in t/t30[*].sh for uniformity, that's to keep test title the same line with helper function itself, and fix some indentions. Add a new section "recommended style" in t/README to encourage people to use more modern style in test. Signed-off-by: Li Linchao <lilinchao@oschina.cn> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'js/use-builtin-add-i'Junio C Hamano2022-05-311-1/+1
|\ | | | | | | | | | | | | | | | | | | "git add -i" was rewritten in C some time ago and has been in testing; the reimplementation is now exposed to general public by default. * js/use-builtin-add-i: add -i: default to the built-in implementation t2016: require the PERL prereq only when necessary
| * add -i: default to the built-in implementationJohannes Schindelin2021-12-011-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In 9a5315edfdf (Merge branch 'js/patch-mode-in-others-in-c', 2020-02-05), Git acquired a built-in implementation of `git add`'s interactive mode that could be turned on via the config option `add.interactive.useBuiltin`. The first official Git version to support this knob was v2.26.0. In 2df2d81ddd0 (add -i: use the built-in version when feature.experimental is set, 2020-09-08), this built-in implementation was also enabled via `feature.experimental`. The first version with this change was v2.29.0. More than a year (and very few bug reports) later, it is time to declare the built-in implementation mature and to turn it on by default. We specifically leave the `add.interactive.useBuiltin` configuration in place, to give users an "escape hatch" in the unexpected case should they encounter a previously undetected bug in that implementation. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | fsmonitor: config settings are repository-specificJeff Hostetler2022-03-261-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move fsmonitor config settings to a new and opaque `struct fsmonitor_settings` structure. Add a lazily-loaded pointer to this into `struct repo_settings` Create an `enum fsmonitor_mode` type in `struct fsmonitor_settings` to represent the state of fsmonitor. This lets us represent which, if any, fsmonitor provider (hook or IPC) is enabled. Create `fsm_settings__get_*()` getters to lazily look up fsmonitor- related config settings. Get rid of the `core_fsmonitor` global variable. Move the code to lookup the existing `core.fsmonitor` config value into the fsmonitor settings. Create a hook pathname variable in `struct fsmonitor-settings` and only set it when in hook mode. Extend the definition of `core.fsmonitor` to be either a boolean or a hook pathname. When true, the builtin FSMonitor is used. When false or unset, no FSMonitor (neither builtin nor hook) is used. The existing `core_fsmonitor` global variable was used to store the pathname to the fsmonitor hook *and* it was used as a boolean to see if fsmonitor was enabled. This dual usage and global visibility leads to confusion when we add the IPC-based provider. So lets hide the details in fsmonitor-settings.c and let it decide which provider to use in the case of multiple settings. This avoids cluttering up repo-settings.c with these private details. A future commit in builtin-fsmonitor series will add the ability to disqualify worktrees for various reasons, such as being mounted from a remote volume, where fsmonitor should not be started. Having the config settings hidden in fsmonitor-settings.c allows such worktree restrictions to override the config values used. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | t/README: fix typoMarc Strapetz2022-01-051-1/+1
| | | | | | | | | | Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | test-lib: introduce required prereq for test runsFabian Stelzer2021-11-211-0/+6
|/ | | | | | | | | | | | | In certain environments or for specific test scenarios we might expect a specific prerequisite check to succeed. Therefore we would like to abort running our tests if this is not the case. To remedy this we add the environment variable GIT_TEST_REQUIRE_PREREQ which can be set to a space separated list of prereqs. If one of these prereq tests fail then the whole test run will abort. Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'jt/no-abuse-alternate-odb-for-submodules'Junio C Hamano2021-10-261-5/+2
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | Follow through the work to use the repo interface to access submodule objects in-process, instead of abusing the alternate object database interface. * jt/no-abuse-alternate-odb-for-submodules: submodule: trace adding submodule ODB as alternate submodule: pass repo to check_has_commit() object-file: only register submodule ODB if needed merge-{ort,recursive}: remove add_submodule_odb() refs: peeling non-the_repository iterators is BUG refs: teach arbitrary repo support to iterators refs: plumb repo into ref stores
| * submodule: trace adding submodule ODB as alternateJonathan Tan2021-10-091-5/+2
| | | | | | | | | | | | | | | | | | | | Submodule ODBs are never added as alternates during the execution of the test suite, but there may be a rare interaction that the test suite does not have coverage of. Add a trace message when this happens, so that users who trace their commands can notice such occurrences. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/sanitize-leak-ci'Junio C Hamano2021-10-111-0/+7
|\ \ | |/ |/| | | | | | | | | | | CI learns to run the leak sanitizer builds. * ab/sanitize-leak-ci: tests: add a test mode for SANITIZE=leak, run it in CI Makefile: add SANITIZE=leak flag to GIT-BUILD-OPTIONS
| * tests: add a test mode for SANITIZE=leak, run it in CIÆvar Arnfjörð Bjarmason2021-09-231-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While git can be compiled with SANITIZE=leak, we have not run regression tests under that mode. Memory leaks have only been fixed as one-offs without structured regression testing. This change adds CI testing for it. We'll now build and small set of whitelisted t00*.sh tests under Linux with a new job called "linux-leaks". The CI target uses a new GIT_TEST_PASSING_SANITIZE_LEAK=true test mode. When running in that mode, we'll assert that we were compiled with SANITIZE=leak. We'll then skip all tests, except those that we've opted-in by setting "TEST_PASSES_SANITIZE_LEAK=true". A test setting "TEST_PASSES_SANITIZE_LEAK=true" setting can in turn make use of the "SANITIZE_LEAK" prerequisite, should they wish to selectively skip tests even under "GIT_TEST_PASSING_SANITIZE_LEAK=true". In the preceding commit we started doing this in "t0004-unwritable.sh" under SANITIZE=leak, now it'll combine nicely with "GIT_TEST_PASSING_SANITIZE_LEAK=true". This is how tests that don't set "TEST_PASSES_SANITIZE_LEAK=true" will be skipped under GIT_TEST_PASSING_SANITIZE_LEAK=true: $ GIT_TEST_PASSING_SANITIZE_LEAK=true ./t0001-init.sh 1..0 # SKIP skip all tests in t0001 under SANITIZE=leak, TEST_PASSES_SANITIZE_LEAK not set The intent is to add more TEST_PASSES_SANITIZE_LEAK=true annotations as follow-up change, but let's start small to begin with. In ci/run-build-and-tests.sh we make use of the default "*" case to run "make test" without any GIT_TEST_* modes. SANITIZE=leak is known to fail in combination with GIT_TEST_SPLIT_INDEX=true in t0016-oidmap.sh, and we're likely to have other such failures in various GIT_TEST_* modes. Let's focus on getting the base tests passing, we can expand coverage to GIT_TEST_* modes later. It would also be possible to implement a more lightweight version of this by only relying on setting "LSAN_OPTIONS". See <YS9OT/pn5rRK9cGB@coredump.intra.peff.net>[1] and <YS9ZIDpANfsh7N+S@coredump.intra.peff.net>[2] for a discussion of that. I've opted for this approach of adding a GIT_TEST_* mode instead because it's consistent with how we handle other special test modes. Being able to add a "!SANITIZE_LEAK" prerequisite and calling "test_done" early if it isn't satisfied also means that we can more incrementally add regression tests without being forced to fix widespread and hard-to-fix leaks at the same time. We have tests that do simple checking of some tool we're interested in, but later on in the script might be stressing trace2, or common sources of leaks like "git log" in combination with the tool (e.g. the commit-graph tests). To be clear having a prerequisite could also be accomplished by using "LSAN_OPTIONS" directly. On the topic of "LSAN_OPTIONS": It would be nice to have a mode to aggregate all failures in our various scripts, see [2] for a start at doing that which sets "log_path" in "LSAN_OPTIONS". I've punted on that for now, it can be added later. As of writing this we've got major regressions between master..seen, i.e. the t000*.sh tests and more fixed since 31f9acf9ce2 (Merge branch 'ah/plugleaks', 2021-08-04) have regressed recently. See the discussion at <87czsv2idy.fsf@evledraar.gmail.com>[3] about the lack of this sort of test mode, and 0e5bba53af (add UNLEAK annotation for reducing leak false positives, 2017-09-08) for the initial addition of SANITIZE=leak. See also 09595ab381 (Merge branch 'jk/leak-checkers', 2017-09-19), 7782066f67 (Merge branch 'jk/apache-lsan', 2019-05-19) and the recent 936e58851a (Merge branch 'ah/plugleaks', 2021-05-07) for some of the past history of "one-off" SANITIZE=leak (and more) fixes. As noted in [5] we can't support this on OSX yet until Clang 14 is released, at that point we'll probably want to resurrect that "osx-leaks" job. 1. https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer 2. https://lore.kernel.org/git/YS9OT%2Fpn5rRK9cGB@coredump.intra.peff.net/ 3. https://lore.kernel.org/git/87czsv2idy.fsf@evledraar.gmail.com/ 4. https://lore.kernel.org/git/YS9ZIDpANfsh7N+S@coredump.intra.peff.net/ 5. https://lore.kernel.org/git/20210916035603.76369-1-carenas@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jt/grep-wo-submodule-odb-as-alternate'Junio C Hamano2021-09-211-0/+10
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The code to make "git grep" recurse into submodules has been updated to migrate away from the "add submodule's object store as an alternate object store" mechanism (which is suboptimal). * jt/grep-wo-submodule-odb-as-alternate: t7814: show lack of alternate ODB-adding submodule-config: pass repo upon blob config read grep: add repository to OID grep sources grep: allocate subrepos on heap grep: read submodule entry with explicit repo grep: typesafe versions of grep_source_init grep: use submodule-ODB-as-alternate lazy-addition submodule: lazily add submodule ODBs as alternates
| * | submodule: lazily add submodule ODBs as alternatesJonathan Tan2021-09-081-0/+10
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Teach Git to add submodule ODBs as alternates to the object store of the_repository only upon the first access of an object not in the_repository, and not when add_submodule_odb() is called. This provides a means of gradually migrating from accessing a submodule's object through alternates to accessing a submodule's object by explicitly passing its repository object. Any Git command can declare that it might access submodule objects by calling add_submodule_odb() (as they do now), but the submodule ODBs themselves will not be added until needed, so individual commands and/or combinations of arguments can be migrated one by one. [The advantage of explicit repository-object passing is code clarity (it is clear which repository an object read is from), performance (there is no need to linearly search through all submodule ODBs whenever an object is accessed from any repository, whether superproject or submodule), and the possibility of future features like partial clone submodules (which right now is not possible because if an object is missing, we do not know which repository to lazy-fetch into).] This commit also introduces an environment variable that a test may set to make the actual registration of alternates fatal, in order to demonstrate that its codepaths do not need this registration. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Reviewed-by: Emily Shaffer <emilyshaffer@google.com> Reviewed-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'tb/multi-pack-bitmaps'Junio C Hamano2021-09-211-0/+4
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The reachability bitmap file used to be generated only for a single pack, but now we've learned to generate bitmaps for history that span across multiple packfiles. * tb/multi-pack-bitmaps: (29 commits) pack-bitmap: drop bitmap_index argument from try_partial_reuse() pack-bitmap: drop repository argument from prepare_midx_bitmap_git() p5326: perf tests for MIDX bitmaps p5310: extract full and partial bitmap tests midx: respect 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP' t7700: update to work with MIDX bitmap test knob t5319: don't write MIDX bitmaps in t5319 t5310: disable GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP t0410: disable GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP t5326: test multi-pack bitmap behavior t/helper/test-read-midx.c: add --checksum mode t5310: move some tests to lib-bitmap.sh pack-bitmap: write multi-pack bitmaps pack-bitmap: read multi-pack bitmaps pack-bitmap.c: avoid redundant calls to try_partial_reuse pack-bitmap.c: introduce 'bitmap_is_preferred_refname()' pack-bitmap.c: introduce 'nth_bitmap_object_oid()' pack-bitmap.c: introduce 'bitmap_num_objects()' midx: avoid opening multiple MIDXs when writing midx: close linked MIDXs, avoid leaking memory ...
| * | midx: respect 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP'Taylor Blau2021-09-011-0/+4
| |/ | | | | | | | | | | | | | | | | Introduce a new 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP' environment variable to also write a multi-pack bitmap when 'GIT_TEST_MULTI_PACK_INDEX' is set. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | test-lib-functions: keep user's debugger config files and TERM in 'debug'Philippe Blain2021-09-071-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The 'debug' function in test-lib-functions.sh is used to invoke a debugger at a specific line in a test. It inherits the value of HOME and TERM set by 'test-lib.sh': HOME="$TRASH_DIRECTORY" and TERM=dumb. Changing the value of HOME means that any customization configured in a developers' debugger configuration file (like $HOME/.gdbinit or $HOME/.lldbinit) are not available in the debugger invoked by 'test_pause'. Changing the value of TERM to 'dumb' means that colored output is disabled in the debugger. To make the debugging experience with 'debug' more pleasant, leverage the variable USER_HOME, added in the previous commit, to copy a developer's ~/.gdbinit and ~/.lldbinit to the test HOME. We do not set HOME to USER_HOME as in 'test_pause' to avoid user configuration in $USER_HOME/.gitconfig from interfering with the command being debugged. Also, add a flag to launch the debugger with the original value of TERM, and add the same warning as for 'test_pause'. Helped-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Acked-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | test-lib-functions: optionally keep HOME, TERM and SHELL in 'test_pause'Philippe Blain2021-09-071-2/+3
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The 'test_pause' function, which is designed to help interactive debugging and exploration of tests, currently inherits the value of HOME and TERM set by 'test-lib.sh': HOME="$TRASH_DIRECTORY" and TERM=dumb. It also invokes the shell defined by TEST_SHELL_PATH, which defaults to /bin/sh (through SHELL_PATH). Changing the value of HOME means that any customization configured in a developers' shell startup files and any Git aliases defined in their global Git configuration file are not available in the shell invoked by 'test_pause'. Changing the value of TERM to 'dumb' means that colored output is disabled for all commands in that shell. Using /bin/sh as the shell invoked by 'test_pause' is not ideal since some platforms (i.e. Debian and derivatives) use Dash as /bin/sh, and this shell is usually compiled without readline support, which makes for a poor interactive command line experience. To make the interactive command line experience in the shell invoked by 'test_pause' more pleasant, save the values of HOME and TERM in USER_HOME and USER_TERM before changing them in test-lib.sh, and add options to 'test_pause' to optionally use these variables to invoke the shell. Also add an option to invoke SHELL instead of TEST_SHELL_PATH, so that developer's interactive shell is used. We use options instead of changing the behaviour unconditionally since these three variables can slightly change command behaviour. Moreover, using the original HOME means commands could overwrite files in a user's home directory. Be explicit about these caveats in the new 'Usage' section in test-lib-functions.sh. Finally, add '[options]' to the test_pause synopsys in t/README, and mention that the full list of helper functions and their options can be found in test-lib-functions.sh. Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Acked-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'hn/prep-tests-for-reftable'Junio C Hamano2021-07-141-0/+6
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Preliminary clean-up of tests before the main reftable changes hits the codebase. * hn/prep-tests-for-reftable: (22 commits) t1415: set REFFILES for test specific to storage format t4202: mark bogus head hash test with REFFILES t7003: check reflog existence only for REFFILES t7900: stop checking for loose refs t1404: mark tests that muck with .git directly as REFFILES. t2017: mark --orphan/logAllRefUpdates=false test as REFFILES t1414: mark corruption test with REFFILES t1407: require REFFILES for for_each_reflog test test-lib: provide test prereq REFFILES t5304: use "reflog expire --all" to clear the reflog t5304: restyle: trim empty lines, drop ':' before > t7003: use rev-parse rather than FS inspection t5000: inspect HEAD using git-rev-parse t5000: reformat indentation to the latest fashion t1301: fix typo in error message t1413: use tar to save and restore entire .git directory t1401-symbolic-ref: avoid direct filesystem access t1401: use tar to snapshot and restore repo state t5601: read HEAD using rev-parse t9300: check ref existence using test-helper rather than a file system check ...
| * test-lib: provide test prereq REFFILESHan-Wen Nienhuys2021-06-021-0/+6
| | | | | | | | | | | | | | | | | | | | REFFILES can be used to mark tests that are specific to the packed/loose ref storage format and its limitations. Marking such tests is a preparation for introducing the reftable storage backend. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jk/test-chainlint-softer'Junio C Hamano2021-05-201-0/+5
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | The "chainlint" feature in the test framework is a handy way to catch common mistakes in writing new tests, but tends to get expensive. An knob to selectively disable it has been introduced to help running tests that the developer has not modified. * jk/test-chainlint-softer: t: avoid sed-based chain-linting in some expensive cases
| * | t: avoid sed-based chain-linting in some expensive casesJeff King2021-05-131-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 878f988350 (t/test-lib: teach --chain-lint to detect broken &&-chains in subshells, 2018-07-11) introduced additional chain-lint tests which add an extra "sed" pipeline to each test we run. This has a measurable impact on runtime. Here are timings with and without a new environment variable (added by this patch) that lets you disable just the additional sed-based chain-lint tests: Benchmark #1: GIT_TEST_CHAIN_LINT_HARDER=1 make test Time (mean ± σ): 64.202 s ± 1.030 s [User: 622.469 s, System: 301.402 s] Range (min … max): 61.571 s … 65.662 s 10 runs Benchmark #2: GIT_TEST_CHAIN_LINT_HARDER=0 make test Time (mean ± σ): 57.591 s ± 0.333 s [User: 529.368 s, System: 270.618 s] Range (min … max): 57.143 s … 58.309 s 10 runs Summary 'GIT_TEST_CHAIN_LINT_HARDER=0 make test' ran 1.11 ± 0.02 times faster than 'GIT_TEST_CHAIN_LINT_HARDER=1 make test' Of course those extra lint checks are doing something useful, so paying a few extra seconds (at least on Linux) isn't so bad (though note the CPU time; we're bounded in our parallel run here by the slowest test, so it really is ~120s of CPU improvement). But we can observe that there are some test scripts where they produce a much stronger effect, and provide less value. In t0027 and t3070 we run a very large number of small tests, all driven by a series of functions/loops which are filling in the test bodies. There we get much less bang for our buck in terms of bug-finding versus CPU cost. This patch introduces a mechanism for controlling when those extra lint checks are run, at two levels: - a user can ask to disable or to force-enable the checks by setting GIT_TEST_CHAIN_LINT_HARDER - if the user hasn't specified a preference, individual scripts can disable the checks by setting GIT_TEST_CHAIN_LINT_HARDER_DEFAULT; scripts which don't set that get the current behavior of enabling them. In addition, this patch flips the default for t0027 and t3070's mass-generated sections to disable the extra checks. Here are the timing results for t0027: Benchmark #1: GIT_TEST_CHAIN_LINT_HARDER=1 ./t0027-auto-crlf.sh Time (mean ± σ): 17.078 s ± 0.848 s [User: 14.878 s, System: 7.075 s] Range (min … max): 15.952 s … 18.421 s 10 runs Benchmark #2: GIT_TEST_CHAIN_LINT_HARDER=0 ./t0027-auto-crlf.sh Time (mean ± σ): 9.063 s ± 0.759 s [User: 7.890 s, System: 3.362 s] Range (min … max): 7.747 s … 10.619 s 10 runs Benchmark #3: ./t0027-auto-crlf.sh Time (mean ± σ): 9.186 s ± 0.881 s [User: 7.957 s, System: 3.427 s] Range (min … max): 7.796 s … 10.498 s 10 runs Summary 'GIT_TEST_CHAIN_LINT_HARDER=0 ./t0027-auto-crlf.sh' ran 1.01 ± 0.13 times faster than './t0027-auto-crlf.sh' 1.88 ± 0.18 times faster than 'GIT_TEST_CHAIN_LINT_HARDER=1 ./t0027-auto-crlf.sh' We can see that disabling the checks for the whole script buys us an almost 2x speedup. But the new default behavior, disabling them only for the mass-generated part, gets us most of that speedup (but still leaves the checks on for further manual tests people might write). As a side note, I'd caution about comparing runtimes and CPU seconds between this timing and the earlier "make test" one. In "make test", we're running a lot of scripts in parallel, so the CPU is throttling down (and thus a CPU second saved here would count for more during a parallel run; the same work takes more CPU seconds there). We get similar results for t3070: Benchmark #1: GIT_TEST_CHAIN_LINT_HARDER=1 ./t3070-wildmatch.sh Time (mean ± σ): 20.054 s ± 3.967 s [User: 16.003 s, System: 8.286 s] Range (min … max): 11.891 s … 23.671 s 10 runs Benchmark #2: GIT_TEST_CHAIN_LINT_HARDER=0 ./t3070-wildmatch.sh Time (mean ± σ): 12.399 s ± 2.256 s [User: 7.542 s, System: 5.342 s] Range (min … max): 9.606 s … 15.727 s 10 runs Benchmark #3: ./t3070-wildmatch.sh Time (mean ± σ): 10.726 s ± 3.476 s [User: 6.790 s, System: 4.365 s] Range (min … max): 5.444 s … 15.376 s 10 runs Summary './t3070-wildmatch.sh' ran 1.16 ± 0.43 times faster than 'GIT_TEST_CHAIN_LINT_HARDER=0 ./t3070-wildmatch.sh' 1.87 ± 0.71 times faster than 'GIT_TEST_CHAIN_LINT_HARDER=1 ./t3070-wildmatch.sh' Again, we get almost a 2x speedup disabling these. In this case, there are no tests not covered by the script's "default to disable" behavior, so the second two benchmarks should be the same (and while they do differ, you can see the variance is quite high but they're within one standard deviation). So it seems like for these two scripts, at least, disabling the extra checks is a reasonable tradeoff. Sadly, the overall runtime of "make test" on my system doesn't get much faster. But that's because we're mostly limited by the cost of the single biggest test. Here are the top-5 tests by wall-clock time from a parallel run, before my patch: 57.9192368984222 t9001-send-email.sh 45.6329638957977 t0027-auto-crlf.sh 32.5278220176697 t3070-wildmatch.sh 22.2701289653778 t7610-mergetool.sh 20.8635759353638 t1701-racy-split-index.sh And after: 57.1476998329163 t9001-send-email.sh 33.776211977005 t0027-auto-crlf.sh 21.3116669654846 t7610-mergetool.sh 20.7748689651489 t1701-racy-split-index.sh 19.6957249641418 t7112-reset-submodule.sh We dropped 12s from t0027, and t3070 dropped off our list entirely at around 16s. In both cases we're bound by t9001, but its slowness is due to the actual tests, so we'll have to deal with it in a different way. But this reduces overall CPU, and means that dealing with t9001 (by improving the speed of send-email or splitting it apart) will let us reduce our overall runtime even on multi-core machines. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>