summaryrefslogtreecommitdiff
path: root/t
AgeCommit message (Collapse)AuthorFilesLines
2021-06-29test-lib.sh: set COLUMNS=80 for --verbose repeatabilityLibravatar Ævar Arnfjörð Bjarmason1-2/+3
Some tests will fail under --verbose because while we've unset COLUMNS since b1d645b58ac (tests: unset COLUMNS inherited from environment, 2012-03-27), we also look for the columns with an ioctl(.., TIOCGWINSZ, ...) on some platforms. By setting COLUMNS again we preempt the TIOCGWINSZ lookup in pager.c's term_columns(), it'll take COLUMNS over TIOCGWINSZ, This fixes t0500-progress-display.sh., which broke because of a combination of the this issue and the progress output reacting to the column width since 545dc345ebd (progress: break too long progress bar lines, 2019-04-12). The t5324-split-commit-graph.sh fails in a similar manner due to progress output, see [1] for details. The issue is not specific to progress.c, the diff code also checks COLUMNS and some of its tests can be made to fail in a similar manner[2], anything that invokes a pager is potentially affected. See ea77e675e56 (Make "git help" react to window size correctly, 2005-12-18) and ad6c3739a33 (pager: find out the terminal width before spawning the pager, 2012-02-12) for how the TIOCGWINSZ code ended up in pager.c 1. http://lore.kernel.org/git/20210624051253.GG6312@szeder.dev 2. https://lore.kernel.org/git/20210627074419.GH6312@szeder.dev/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-06Merge branch 'rs/parallel-checkout-test-fix'Libravatar Junio C Hamano1-1/+1
Test fix. * rs/parallel-checkout-test-fix: parallel-checkout: avoid dash local bug in tests
2021-06-06parallel-checkout: avoid dash local bug in testsLibravatar René Scharfe1-1/+1
Dash bug https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097 lets the shell erroneously perform field splitting on the expansion of a command substitution during declaration of a local variable. It causes the parallel-checkout tests to fail e.g. when running them with /bin/dash on MacOS 11.4, where they error out like this: ./t2080-parallel-checkout-basics.sh: 33: local: 0: bad variable name That's because the output of wc -l contains leading spaces and the returned number of lines is treated as another variable to declare, i.e. as in "local workers= 0". Work around it by enclosing the command substitution in quotes. Helped-by: Matheus Tavares Bernardino <matheus.bernardino@usp.br> Helped-by: SZEDER Gábor <szeder.dev@gmail.com> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-27Merge branch 'mt/t2080-cp-symlink-fix'Libravatar Junio C Hamano1-1/+1
Test portability fix. * mt/t2080-cp-symlink-fix: t2080: fix cp invocation to copy symlinks instead of following them
2021-05-27Merge branch 'ab/send-email-inline-hooks-path'Libravatar Junio C Hamano1-3/+4
Code simplification. * ab/send-email-inline-hooks-path: send-email: move "hooks_path" invocation to git-send-email.perl send-email: don't needlessly abs_path() the core.hooksPath
2021-05-27Merge branch 'ds/t1092-fix-flake-from-progress'Libravatar Junio C Hamano1-3/+3
Workaround flaky tests introduced recently. * ds/t1092-fix-flake-from-progress: t1092: revert the "-1" hack for emulating "no progress meter" t1092: use GIT_PROGRESS_DELAY for consistent results
2021-05-27t2080: fix cp invocation to copy symlinks instead of following themLibravatar Matheus Tavares1-1/+1
t2080 makes a few copies of a test repository and later performs a branch switch on each one of the copies to verify that parallel checkout and sequential checkout produce the same results. However, the repository is copied with `cp -R` which, on some systems, defaults to following symlinks on the directory hierarchy and copying their target files instead of copying the symlinks themselves. AIX is one example of system where this happens. Because the symlinks are not preserved, the copied repositories have paths that do not match what is in the index, causing git to abort the checkout operation that we want to test. This makes the test fail on these systems. Fix this by copying the repository with the POSIX flag '-P', which forces cp to copy the symlinks instead of following them. Note that we already use this flag for other cp invocations in our test suite (see t7001). With this change, t2080 now passes on AIX. Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-27send-email: don't needlessly abs_path() the core.hooksPathLibravatar Ævar Arnfjörð Bjarmason1-3/+4
In c8243933c74 (git-send-email: Respect core.hooksPath setting, 2021-03-23) we started supporting core.hooksPath in "send-email". It's been reported that on Windows[1] doing this by calling abs_path() results in different canonicalizations of the absolute path. This wasn't an issue in c8243933c74 itself, but was revealed by my ea7811b37e0 (git-send-email: improve --validate error output, 2021-04-06) when we started emitting the path to the hook, which was previously only internal to git-send-email.perl. The just-landed 53753a37d09 (t9001-send-email.sh: fix expected absolute paths on Windows, 2021-05-24) narrowly fixed this issue, but I believe we can do better here. We should not be relying on whatever changes Perl's abs_path() makes to the path "rev-parse --git-path hooks" hands to us. Let's instead trust it, and hand it to Perl's system() in git-send-email.perl. It will handle either a relative or absolute path. So let's revert most of 53753a37d09 and just have "hooks_path" return what we get from "rev-parse" directly without modification. This has the added benefit of making the error message friendlier in the common case, we'll no longer print an absolute path for repository-local hook errors. 1. http://lore.kernel.org/git/bb30fe2b-cd75-4782-24a6-08bb002a0367@kdbg.org Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-26t1092: revert the "-1" hack for emulating "no progress meter"Libravatar Junio C Hamano1-3/+3
This looked like a good idea, but it seems to break tests on 32-bit builds rather badly. Revert to just use "100 thousands must be big enough" for now. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-25Merge branch 'mt/init-template-userpath-fix'Libravatar Junio C Hamano1-8/+20
Regression fix. * mt/init-template-userpath-fix: init: fix bug regarding ~/ expansion in init.templateDir
2021-05-25Merge branch 'jt/send-email-validate-errors-fix'Libravatar Junio C Hamano1-4/+3
Fix a test breakage. * jt/send-email-validate-errors-fix: t9001-send-email.sh: fix expected absolute paths on Windows
2021-05-25Merge branch 'ab/send-email-validate-errors-fix'Libravatar Junio C Hamano1-2/+21
* ab/send-email-validate-errors-fix: send-email: fix missing error message regression
2021-05-25t1092: use GIT_PROGRESS_DELAY for consistent resultsLibravatar Derrick Stolee1-3/+3
The t1092-sparse-checkout-compatibility.sh tests compare the stdout and stderr for several Git commands across both full checkouts, sparse checkouts with a full index, and sparse checkouts with a sparse index. Since these are direct comparisons, sometimes a progress indicator can flush at unpredictable points, especially on slower machines. This causes the tests to be flaky. One standard way to avoid this is to add GIT_PROGRESS_DELAY=0 to the Git commands that are run, as this will force every progress indicator created with start_progress_delay() to be created immediately. However, there are some progress indicators that are created in the case of a full index that are not created with a sparse index. Moreover, their values may be different as those indexes have a different number of entries. Instead, use GIT_PROGRESS_DELAY=-1 (which will turn into UINT_MAX) to ensure that any reasonable machine running these tests would never display delayed progress indicators. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-25init: fix bug regarding ~/ expansion in init.templateDirLibravatar Matheus Tavares1-8/+20
We used to read the init.templateDir setting at builtin/init-db.c using a git_config() callback that, in turn, called git_config_pathname(). To simplify the config reading logic at this file and plug a memory leak, this was replaced by a direct call to git_config_get_value() at e4de4502e6 ("init: remove git_init_db_config() while fixing leaks", 2021-03-14). However, this function doesn't provide path expanding semantics, like git_config_pathname() does, so paths with '~/' and '~user/' are treated literally. This makes 'git init' fail to handle init.templateDir paths using these constructs: $ git config init.templateDir '~/templates_dir' $ git init 'warning: templates not found in ~/templates_dir' Replace the git_config_get_value() call by git_config_get_pathname(), which does the '~/' and '~user/' expansions. Also add a regression test. Note that unlike git_config_get_value(), the config cache does not own the memory for the path returned by git_config_get_pathname(), so we must free() it. Reported on IRC by rkta. Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-25send-email: fix missing error message regressionLibravatar Ævar Arnfjörð Bjarmason1-2/+21
Fix a regression with the "the editor exited uncleanly, aborting everything" error message going missing after my d21616c0394 (git-send-email: refactor duplicate $? checks into a function, 2021-04-06). I introduced a $msg variable, but did not actually use it. This caused us to miss the optional error message supplied by the "do_edit" codepath. Fix that, and add tests to check that this works. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-25t9001-send-email.sh: fix expected absolute paths on WindowsLibravatar Johannes Sixt1-4/+3
Git for Windows is a native Windows program that works with native absolute paths in the drive letter style C:\dir. The auxiliary infrastructure is based on MSYS2, which uses POSIX style /C/dir. When we test for output of absolute paths produced by git.exe, we usally have to expect C:\dir style paths. To produce such expected paths, we have to use $(pwd) in the test scripts; the alternative, $PWD, produces a POSIX style path. ($PWD is a shell variable, and the shell is bash, an MSYS2 program, and operates in the POSIX realm.) There are two recently added tests that were written to expect C:\dir paths. The output that is tested is produced by `git send-email`, but behind the scenes, this is a Perl script, which also works in the POSIX realm and produces /C/dir style output. In the first test case that is changed here, replace $(pwd) by $PWD so that the expected path is constructed using /C/dir style. The second test case sets core.hooksPath to an absolute path. Since the test script talks to native git.exe, it is supposed to place a C:/dir style path into the configuration; therefore, keep $(pwd). When this configuration value is consumed by the Perl script, it is transformed to /C/dir style by the MSYS2 layer and echoed back in this form in the error message. Hence, do use $PWD for the expected value. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-22Merge branch 'dl/stash-show-untracked-fixup'Libravatar Junio C Hamano1-0/+2
Another brown paper bag inconsistency fix for a new feature introduced during this cycle. * dl/stash-show-untracked-fixup: stash show: use stash.showIncludeUntracked even when diff options given
2021-05-22Merge branch 'wm/rev-parse-path-format-wo-arg'Libravatar Junio C Hamano1-0/+4
The "rev-parse" command did not diagnose the lack of argument to "--path-format" option, which was introduced in v2.31 era, which has been corrected. * wm/rev-parse-path-format-wo-arg: rev-parse: fix segfault with missing --path-format argument
2021-05-22stash show: use stash.showIncludeUntracked even when diff options givenLibravatar Denton Liu1-0/+2
If options pertaining to how the diff is displayed is provided to `git stash show`, the command will ignore the stash.showIncludeUntracked configuration variable, defaulting to not showing any untracked files. This is unintuitive behaviour since the format of the diff output and whether or not to display untracked files are orthogonal. Use stash.showIncludeUntracked even when diff options are given. Of course, this is still overridable via the command-line options. Update the documentation to explicitly say which configuration variables will be overridden when a diff options are given. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-21Merge branch 'tz/c-locale-output-is-no-more'Libravatar Junio C Hamano1-1/+1
Test update. * tz/c-locale-output-is-no-more: t7500: remove non-existant C_LOCALE_OUTPUT prereq
2021-05-21Merge branch 'cs/http-use-basic-after-failed-negotiate'Libravatar Junio C Hamano1-0/+41
Regression fix for a change made during this cycle. * cs/http-use-basic-after-failed-negotiate: Revert "remote-curl: fall back to basic auth if Negotiate fails" t5551: test http interaction with credential helpers
2021-05-20Merge branch 'jk/test-chainlint-softer'Libravatar Junio C Hamano4-3/+21
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
2021-05-20Merge branch 'zh/ref-filter-push-remote-fix'Libravatar Junio C Hamano1-0/+19
The handling of "%(push)" formatting element of "for-each-ref" and friends was broken when the same codepath started handling "%(push:<what>)", which has been corrected. * zh/ref-filter-push-remote-fix: ref-filter: fix read invalid union member bug
2021-05-20Merge branch 'en/dir-traversal'Libravatar Junio C Hamano6-91/+176
"git clean" and "git ls-files -i" had confusion around working on or showing ignored paths inside an ignored directory, which has been corrected. * en/dir-traversal: dir: introduce readdir_skip_dot_and_dotdot() helper dir: update stale description of treat_directory() dir: traverse into untracked directories if they may have ignored subfiles dir: avoid unnecessary traversal into ignored directory t3001, t7300: add testcase showcasing missed directory traversal t7300: add testcase showing unnecessary traversal into ignored directory ls-files: error out on -i unless -o or -c are specified dir: report number of visited directories and paths with trace2 dir: convert trace calls to trace2 equivalents
2021-05-19Revert "remote-curl: fall back to basic auth if Negotiate fails"Libravatar Jeff King1-1/+1
This reverts commit 1b0d9545bb85912a16b367229d414f55d140d3be. That commit does fix the situation it intended to (avoiding Negotiate even when the credentials were provided in the URL), but it creates a more serious regression: we now never hit the conditional for "we had a username and password, tried them, but the server still gave us a 401". That has two bad effects: 1. we never call credential_reject(), and thus a bogus credential stored by a helper will live on forever 2. we never return HTTP_NOAUTH, so the error message the user gets is "The requested URL returned error: 401", instead of "Authentication failed". Doing this correctly seems non-trivial, as we don't know whether the Negotiate auth was a problem. Since this is a regression in the upcoming v2.23.0 release (for which we're in -rc0), let's revert for now and work on a fix separately. (Note that this isn't a pure revert; the previous commit added a test showing the regression, so we can now flip it to expect_success). Reported-by: Ben Humphreys <behumphreys@atlassian.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-19t5551: test http interaction with credential helpersLibravatar Jeff King1-0/+41
We test authentication with http, and we independently test that credential helpers work, but we don't have any tests that cover the two features working together. Let's add two: 1. Make sure that a successful request asks the helper to save the credential. This works as expected. 2. Make sure that a failed request asks the helper to forget the credential. This is marked as expect_failure, as it was recently regressed by 1b0d9545bb (remote-curl: fall back to basic auth if Negotiate fails, 2021-03-22). The symptom here is that the second request should prompt the user, but doesn't. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-18t7500: remove non-existant C_LOCALE_OUTPUT prereqLibravatar Todd Zullinger1-1/+1
The C_LOCALE_OUTPUT prerequisite was removed in b1e079807b (tests: remove last uses of C_LOCALE_OUTPUT, 2021-02-11), where Ævar noted: I'm not leaving the prerequisite itself in place for in-flight changes as there currently are none that introduce new tests that rely on it, and because C_LOCALE_OUTPUT is currently a noop on the master branch we likely won't have any new submissions that use it. One more use of C_LOCALE_OUTPUT did creep in with 3d1bda6b5b (t7500: add tests for --fixup=[amend|reword] options, 2021-03-15). This causes a number of the tests to be skipped by default: ok 35 # SKIP --fixup=reword: incompatible with --all (missing C_LOCALE_OUTPUT) ok 36 # SKIP --fixup=reword: incompatible with --include (missing C_LOCALE_OUTPUT) ok 37 # SKIP --fixup=reword: incompatible with --only (missing C_LOCALE_OUTPUT) ok 38 # SKIP --fixup=reword: incompatible with --interactive (missing C_LOCALE_OUTPUT) ok 39 # SKIP --fixup=reword: incompatible with --patch (missing C_LOCALE_OUTPUT) Remove the C_LOCALE_OUTPUT prerequisite from these tests so they are not skipped. Signed-off-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-17rev-parse: fix segfault with missing --path-format argumentLibravatar Wolfgang Müller1-0/+4
Calling "git rev-parse --path-format" without an argument segfaults instead of giving an error message. Commit fac60b8925 (rev-parse: add option for absolute or relative path formatting, 2020-12-13) added the argument parsing code but forgot to handle NULL. Returning an error makes sense here because there is no default value we could use. Add a test case to verify. Signed-off-by: Wolfgang Müller <wolf@oriole.systems> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-16Merge branch 'dl/stash-show-untracked-fixup'Libravatar Junio C Hamano1-1/+16
The code to handle options recently added to "git stash show" around untracked part of the stash segfaulted when these options were used on a stash entry that does not record untracked part. * dl/stash-show-untracked-fixup: stash show: fix segfault with --{include,only}-untracked t3905: correct test title
2021-05-16Merge branch 'wc/packed-ref-removal-cleanup'Libravatar Junio C Hamano1-0/+9
When "git update-ref -d" removes a ref that is packed, it left empty directories under $GIT_DIR/refs/ for * wc/packed-ref-removal-cleanup: refs: cleanup directories when deleting packed ref
2021-05-16Merge branch 'dd/mailinfo-quoted-cr'Libravatar Junio C Hamano7-0/+165
"git mailinfo" (hence "git am") learned the "--quoted-cr" option to control how lines ending with CRLF wrapped in base64 or qp are handled. * dd/mailinfo-quoted-cr: am: learn to process quoted lines that ends with CRLF mailinfo: allow stripping quoted CR without warning mailinfo: allow squelching quoted CRLF warning mailinfo: warn if CRLF found in decoded base64/QP email mailinfo: stop parsing options manually mailinfo: load default metainfo_charset lazily
2021-05-16Merge branch 'mt/parallel-checkout-part-3'Libravatar Junio C Hamano7-24/+660
The final part of "parallel checkout". * mt/parallel-checkout-part-3: ci: run test round with parallel-checkout enabled parallel-checkout: add tests related to .gitattributes t0028: extract encoding helpers to lib-encoding.sh parallel-checkout: add tests related to path collisions parallel-checkout: add tests for basic operations checkout-index: add parallel checkout support builtin/checkout.c: complete parallel checkout support make_transient_cache_entry(): optionally alloc from mem_pool
2021-05-16Merge branch 'jt/push-negotiation'Libravatar Junio C Hamano3-1/+125
"git push" learns to discover common ancestor with the receiving end over protocol v2. * jt/push-negotiation: send-pack: support push negotiation fetch: teach independent negotiation (no packfile) fetch-pack: refactor command and capability write fetch-pack: refactor add_haves() fetch-pack: refactor process_acks()
2021-05-14Merge branch 'ow/no-dryrun-in-add-i'Libravatar Junio C Hamano1-0/+4
"git add -i --dry-run" does not dry-run, which was surprising. The combination of options has taught to error out. * ow/no-dryrun-in-add-i: add: die if both --dry-run and --interactive are given
2021-05-14Merge branch 'jk/p4-locate-branch-point-optim'Libravatar Junio C Hamano1-0/+2
"git p4" learned to find branch points more efficiently. * jk/p4-locate-branch-point-optim: git-p4: speed up search for branch parent git-p4: ensure complex branches are cloned correctly
2021-05-14Merge branch 'ba/object-info'Libravatar Junio C Hamano1-0/+26
Over-the-wire protocol learns a new request type to ask for object sizes given a list of object names. * ba/object-info: object-info: support for retrieving object info
2021-05-14Merge branch 'pw/word-diff-zero-width-matches'Libravatar Junio C Hamano1-0/+5
The word-diff mode has been taught to work better with a word regexp that can match an empty string. * pw/word-diff-zero-width-matches: word diff: handle zero length matches
2021-05-13t: avoid sed-based chain-linting in some expensive casesLibravatar Jeff King4-3/+21
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>
2021-05-13stash show: fix segfault with --{include,only}-untrackedLibravatar Denton Liu1-0/+15
When `git stash show --include-untracked` or `git stash show --only-untracked` is run on a stash that doesn't include an untracked entry, a segfault occurs. This happens because we do not check whether the untracked entry is actually present and just attempt to blindly dereference it. Ensure that the untracked entry is present before actually attempting to dereference it. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-13t3905: correct test titleLibravatar Denton Liu1-1/+1
We reference the non-existent option `git stash show --show-untracked` when we really meant `--only-untracked`. Correct the test title accordingly. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-13dir: traverse into untracked directories if they may have ignored subfilesLibravatar Elijah Newren2-2/+2
A directory that is untracked does not imply that all files under it should be categorized as untracked; in particular, if the caller is interested in ignored files, many files or directories underneath the untracked directory may be ignored. We previously partially handled this right with DIR_SHOW_IGNORED_TOO, but missed DIR_SHOW_IGNORED. It was not obvious, though, because the logic for untracked and excluded files had been fused together making it harder to reason about. The previous commit split that logic out, making it easier to notice that DIR_SHOW_IGNORED was missing. Add it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-13dir: avoid unnecessary traversal into ignored directoryLibravatar Elijah Newren1-1/+1
The show_other_directories case in treat_directory() tried to handle both excludes and untracked files with the same logic, and mishandled both the excludes and the untracked files in the process, in different ways. Split that logic apart, and then focus on the logic for the excludes; a subsequent commit will address the logic for untracked files. For show_other_directories, an excluded directory means that every path underneath that directory will also be excluded. Given that the calling code requested to just show directories when everything under a directory had the same state (that's what the "DIR_SHOW_OTHER_DIRECTORIES" flag means), we generally do not need to traverse into such directories and can just immediately mark them as ignored (i.e. as path_excluded). The only reason we cannot just immediately return path_excluded is the DIR_HIDE_EMPTY_DIRECTORIES flag and the possibility that the ignored directory is an empty directory. The code previously treated DIR_SHOW_IGNORED_TOO in most cases as an exception as well, which was wrong. It can sometimes reduce the number of cases where we need to recurse (namely if DIR_SHOW_IGNORED_TOO_MODE_MATCHING is also set), but should not be able to increase the number of cases where we need to recurse. Fix the logic accordingly. Some sidenotes about possible confusion with dir.c: * "ignored" often refers to an untracked ignore", i.e. a file which is not tracked which matches one of the ignore/exclusion rules. But you can also have a "tracked ignore", a tracked file that happens to match one of the ignore/exclusion rules and which dir.c has to worry about since "git ls-files -c -i" is supposed to list them. * The dir code often uses "ignored" and "excluded" interchangeably, which you need to keep in mind while reading the code. * "exclude" is used multiple ways in the code: * As noted above, "exclude" is often a synonym for "ignored". * The logic for parsing .gitignore files was re-used in .git/info/sparse-checkout, except there it is used to mark paths that the user wants to *keep*. This was mostly addressed by commit 65edd96aec ("treewide: rename 'exclude' methods to 'pattern'", 2019-09-03), but every once in a while you'll find a comment about "exclude" referring to these patterns that might in fact be in use by the sparse-checkout machinery for inclusion rules. * The word "EXCLUDE" is also used for pathspec negation, as in (pathspec->items[3].magic & PATHSPEC_EXCLUDE) Thus if a user had a .gitignore file containing *~ *.log !settings.log And then ran git add -- 'settings.*' ':^settings.log' Then :^settings.log is a pathspec negation making settings.log not be requested to be added even though all other settings.* files are being added. Also, !settings.log in the gitignore file is a negative exclude pattern meaning that settings.log is normally a file we want to track even though all other *.log files are ignored. Sometimes it feels like dir.c needs its own glossary with its many definitions, including the multiply-defined terms. Reported-by: Jason Gore <Jason.Gore@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-13t3001, t7300: add testcase showcasing missed directory traversalLibravatar Elijah Newren2-0/+24
In the last commit, we added a testcase showing that the directory traversal machinery sometimes traverses into directories unnecessarily. Here we show that there are cases where it does the opposite: it does not traverse into directories, despite those directories having important files that need to be flagged. Add a testcase showing that `git ls-files -o -i --directory` can omit some of the files it should be listing, and another showing that `git clean -fX` can fail to clean out some of the expected files. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-13t7300: add testcase showing unnecessary traversal into ignored directoryLibravatar Elijah Newren1-0/+23
The PNPM package manager is apparently creating deeply nested (but ignored) directory structures; traversing them is costly performance-wise, unnecessary, and in some cases is even throwing warnings/errors because the paths are too long to handle on various platforms. Add a testcase that checks for such unnecessary directory traversal. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-13ls-files: error out on -i unless -o or -c are specifiedLibravatar Elijah Newren2-3/+3
ls-files --ignored can be used together with either --others or --cached. After being perplexed for a bit and digging in to the code, I assumed that ls-files -i was just broken and not printing anything and I had a nice patch ready to submit when I finally realized that -i can be used with --cached to find tracked ignores. While that was a mistake on my part, and a careful reading of the documentation could have made this more clear, I suspect this is an error others are likely to make as well. In fact, of two uses in our testsuite, I believe one of the two did make this error. In t1306.13, there are NO tracked files, and all the excludes built up and used in that test and in previous tests thus have to be about untracked files. However, since they were looking for an empty result, the mistake went unnoticed as their erroneous command also just happened to give an empty answer. -i will most the time be used with -o, which would suggest we could just make -i imply -o in the absence of either a -o or -c, but that would be a backward incompatible break. Instead, let's just flag -i without either a -o or -c as an error, and update the two relevant testcases to specify their intent. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-13dir: report number of visited directories and paths with trace2Libravatar Elijah Newren1-1/+2
Provide more statistics in trace2 output that include the number of directories and total paths visited by the directory traversal logic. Subsequent patches will take advantage of this to ensure we do not unnecessarily traverse into ignored directories. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-13dir: convert trace calls to trace2 equivalentsLibravatar Elijah Newren2-88/+125
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-12ref-filter: fix read invalid union member bugLibravatar ZheNing Hu1-0/+19
used_atom.u is an union, and it has different members depending on what atom the auxiliary data the union part of the "struct used_atom" wants to record. At most only one of the members can be valid at any one time. Since the code checks u.remote_ref without even making sure if the atom is "push" or "push:" (which are only two cases that u.remote_ref.push becomes valid), but u.remote_ref shares the same storage for other members of the union, the check was reading from an invalid member, which was the bug. Modify the condition here to check whether the atom name equals to "push" or starts with "push:", to avoid reading the value of invalid member of the union. Signed-off-by: ZheNing Hu <adlternative@gmail.com> [jc: further test fixes] Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-11Merge branch 'zh/trailer-cmd'Libravatar Junio C Hamano1-0/+84
The way the command line specified by the trailer.<token>.command configuration variable receives the end-user supplied value was both error prone and misleading. An alternative to achieve the same goal in a safer and more intuitive way has been added, as the trailer.<token>.cmd configuration variable, to replace it. * zh/trailer-cmd: trailer: add new .cmd config option docs: correct descript of trailer.<token>.command
2021-05-11Merge branch 'jk/symlinked-dotgitx-cleanup'Libravatar Junio C Hamano3-41/+151
Various test and documentation updates about .gitsomething paths that are symlinks. * jk/symlinked-dotgitx-cleanup: docs: document symlink restrictions for dot-files fsck: warn about symlinked dotfiles we'll open with O_NOFOLLOW t0060: test ntfs/hfs-obscured dotfiles t7450: test .gitmodules symlink matching against obscured names t7450: test verify_path() handling of gitmodules t7415: rename to expand scope fsck_tree(): wrap some long lines fsck_tree(): fix shadowed variable t7415: remove out-dated comment about translation