summaryrefslogtreecommitdiff
path: root/t
AgeCommit message (Collapse)AuthorFilesLines
2021-07-07oidtree: a crit-bit tree for odb_loose_cacheLibravatar Eric Wong4-0/+100
This saves 8K per `struct object_directory', meaning it saves around 800MB in my case involving 100K alternates (half or more of those alternates are unlikely to hold loose objects). This is implemented in two parts: a generic, allocation-free `cbtree' and the `oidtree' wrapper on top of it. The latter provides allocation using alloc_state as a memory pool to improve locality and reduce free(3) overhead. Unlike oid-array, the crit-bit tree does not require sorting. Performance is bound by the key length, for oidtree that is fixed at sizeof(struct object_id). There's no need to have 256 oidtrees to mitigate the O(n log n) overhead like we did with oid-array. Being a prefix trie, it is natively suited for expanding short object IDs via prefix-limited iteration in `find_short_object_filename'. On my busy workstation, p4205 performance seems to be roughly unchanged (+/-8%). Startup with 100K total alternates with no loose objects seems around 10-20% faster on a hot cache. (800MB in memory savings means more memory for the kernel FS cache). The generic cbtree implementation does impose some extra overhead for oidtree in that it uses memcmp(3) on "struct object_id" so it wastes cycles comparing 12 extra bytes on SHA-1 repositories. I've not yet explored reducing this overhead, but I expect there are many places in our code base where we'd want to investigate this. More information on crit-bit trees: https://cr.yp.to/critbit.html Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-14Merge branch 'ab/test-lib-updates'Libravatar Junio C Hamano14-119/+89
Test clean-up. * ab/test-lib-updates: test-lib: split up and deprecate test_create_repo() test-lib: do not show advice about init.defaultBranch under --verbose test-lib: reformat argument list in test_create_repo() submodule tests: use symbolic-ref --short to discover branch name test-lib functions: add --printf option to test_commit describe tests: convert setup to use test_commit test-lib functions: add an --annotated option to "test_commit" test-lib-functions: document test_commit --no-tag test-lib-functions: reword "test_commit --append" docs test-lib tests: remove dead GIT_TEST_FRAMEWORK_SELFTEST variable test-lib: bring $remove_trash out of retirement
2021-06-14Merge branch 'dd/honor-users-tar-in-tests'Libravatar Junio C Hamano2-4/+4
Test portability fix. * dd/honor-users-tar-in-tests: t: use configured TAR instead of tar
2021-06-14Merge branch 'so/log-m-implies-p'Libravatar Junio C Hamano5-1/+163
The "-m" option in "git log -m" that does not specify which format, if any, of diff is desired did not have any visible effect; it now implies some form of diff (by default "--patch") is produced. * so/log-m-implies-p: diff-merges: let "-m" imply "-p" diff-merges: rename "combined_imply_patch" to "merges_imply_patch" stash list: stop passing "-m" to "git log" git-svn: stop passing "-m" to "git rev-list" diff-merges: move specific diff-index "-m" handling to diff-index t4013: test "git diff-index -m" t4013: test "git diff-tree -m" t4013: test "git log -m --stat" t4013: test "git log -m --raw" t4013: test that "-m" alone has no effect in "git log"
2021-06-14Merge branch 'en/ort-perf-batch-11'Libravatar Junio C Hamano3-20/+792
Optimize out repeated rename detection in a sequence of mergy operations. * en/ort-perf-batch-11: merge-ort, diffcore-rename: employ cached renames when possible merge-ort: handle interactions of caching and rename/rename(1to1) cases merge-ort: add helper functions for using cached renames merge-ort: preserve cached renames for the appropriate side merge-ort: avoid accidental API mis-use merge-ort: add code to check for whether cached renames can be reused merge-ort: populate caches of rename detection results merge-ort: add data structures for in-memory caching of rename detection t6429: testcases for remembering renames fast-rebase: write conflict state to working tree, index, and HEAD fast-rebase: change assert() to BUG() Documentation/technical: describe remembering renames optimization t6423: rename file within directory that other side renamed
2021-06-14Merge branch 'jk/clone-clean-upon-transport-error'Libravatar Junio C Hamano1-0/+7
Recent "git clone" left a temporary directory behind when the transport layer returned an failure. * jk/clone-clean-upon-transport-error: clone: clean up directory after transport_fetch_refs() failure
2021-06-14Merge branch 'ga/send-email-sendmail-cmd'Libravatar Junio C Hamano1-0/+31
"git send-email" learned the "--sendmail-cmd" command line option and the "sendemail.sendmailCmd" configuration variable, which is a more sensible approach than the current way of repurposing the "smtp-server" that is meant to name the server to instead name the command to talk to the server. * ga/send-email-sendmail-cmd: git-send-email: add option to specify sendmail command
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-22t: use configured TAR instead of tarLibravatar Đoàn Trần Công Danh2-4/+4
Despite that tar is available everywhere, it's not required by POSIX. In our build system, users are allowed to specify which tar to be used in Makefile knobs. Furthermore, GNU tar (gtar) is prefered when autotools is being used. In our testsuite, 7 out of 9 tar-required-tests use "$TAR", the other two use "tar". Let's change the remaining two tests to "$TAR". Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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-21diff-merges: let "-m" imply "-p"Libravatar Sergey Organov1-2/+2
Fix long standing inconsistency between -c/--cc that do imply -p on one side, and -m that did not imply -p on the other side. Change corresponding test accordingly, as "log -m" output should now match one from "log -m -p", rather than from just "log". Change documentation accordingly. NOTES: After this patch git log -m produces diffs without need to provide -p as well, that improves both consistency and usability. It gets even more useful if one sets "log.diffMerges" configuration variable to "first-parent" to force -m produce usual diff with respect to first parent only. This patch, however, does not change behavior when specific diff format is explicitly provided on the command-line, so that commands like git log -m --raw git log -m --stat are not affected, nor does it change commands where specific diff format is active by default, such as: git diff-tree -m It's also worth to be noticed that exact historical semantics of -m is still provided by --diff-merges=separate. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-21stash list: stop passing "-m" to "git log"Libravatar Sergey Organov1-1/+1
Passing "-m" in "git log --first-parent -m" is not needed as --first-parent implies --diff-merges=first-parent anyway. OTOH, it will stop being harmless once we let "-m" imply "-p". While we are at it, fix corresponding test description in t3903-stash to match what it actually tests. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-21t4013: test "git diff-index -m"Libravatar Sergey Organov1-0/+13
-m in "git diff-index" means "match missing", that differs from its meaning in "git diff". Let's check it in diff-index. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-21t4013: test "git diff-tree -m"Libravatar Sergey Organov2-0/+12
We want to ensure we don't affect plumbing commands with our changes of "-m" semantics, so add corresponding test. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-21t4013: test "git log -m --stat"Libravatar Sergey Organov2-0/+67
This is to ensure we won't break different diff formats when we start to imply "-p" by "-m". Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-21t4013: test "git log -m --raw"Libravatar Sergey Organov2-0/+62
This is to ensure we won't break different diff formats when we start to imply "-p" by "-m". Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-21t4013: test that "-m" alone has no effect in "git log"Libravatar Sergey Organov1-0/+8
This is to notice current behavior that we are going to change when we start to imply "-p" by "-m". Signed-off-by: Sergey Organov <sorganov@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-ort, diffcore-rename: employ cached renames when possibleLibravatar Elijah Newren1-20/+28
When there are many renames between the old base of a series of commits and the new base, the way sequencer.c, merge-recursive.c, and diffcore-rename.c have traditionally split the work resulted in redetecting the same renames with each and every commit being transplanted. To address this, the last several commits have been creating a cache of rename detection results, determining when it was safe to use such a cache in subsequent merge operations, adding helper functions, and so on. See the previous half dozen commit messages for additional discussion of this optimization, particularly the message a few commits ago entitled "add code to check for whether cached renames can be reused". This commit finally ties all of that work together, modifying the merge algorithm to make use of these cached renames. For the testcases mentioned in commit 557ac0350d ("merge-ort: begin performance work; instrument with trace2_region_* calls", 2020-10-28), this change improves the performance as follows: Before After no-renames: 5.665 s ± 0.129 s 5.622 s ± 0.059 s mega-renames: 11.435 s ± 0.158 s 10.127 s ± 0.073 s just-one-mega: 494.2 ms ± 6.1 ms 500.3 ms ± 3.8 ms That's a fairly small improvement, but mostly because the previous optimizations were so effective for these particular testcases; this optimization only kicks in when the others don't. If we undid the basename-guided rename detection and skip-irrelevant-renames optimizations, then we'd see that this series by itself improved performance as follows: Before Basename Series After Just This Series no-renames: 13.815 s ± 0.062 s 5.697 s ± 0.080 s mega-renames: 1799.937 s ± 0.493 s 205.709 s ± 0.457 s Since this optimization kicks in to help accelerate cases where the previous optimizations do not apply, this last comparison shows that this cached-renames optimization has the potential to help signficantly in cases that don't meet the requirements for the other optimizations to be effective. The changes made in this optimization also lay some important groundwork for a future optimization around having collect_merge_info() avoid recursing into subtrees in more cases. However, for this optimization to be effective, merge_switch_to_result() should only be called when the rebase or cherry-pick operation has either completed or hit a case where the user needs to resolve a conflict or edit the result. If it is called after every commit, as sequencer.c does, then the working tree and index are needlessly updated with every commit and the cached metadata is tossed, defeating this optimization. Refactoring sequencer.c to only call merge_switch_to_result() at the end of the operation is a bigger undertaking, and the practical benefits of this optimization will not be realized until that work is performed. Since `test-tool fast-rebase` only updates at the end of the operation, it was used to obtain the timings above. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-20t6429: testcases for remembering renamesLibravatar Elijah Newren1-0/+692
We will soon be adding an optimization that caches (in memory only, never written to disk) upstream renames during a sequence of merges such as occurs during a cherry-pick or rebase operation. Add several tests meant to stress such an implementation to ensure it does the right thing, and include a test whose outcome we will later change due to this optimization as well. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-20fast-rebase: write conflict state to working tree, index, and HEADLibravatar Elijah Newren1-19/+32
Previously, when fast-rebase hit a conflict, it simply aborted and left HEAD, the index, and the working tree where they were before the operation started. While fast-rebase does not support restarting from a conflicted state, write the conflicted state out anyway as it gives us a way to see what the conflicts are and write tests that check for them. This will be important in the upcoming commits, because sequencer.c is only superficially integrated with merge-ort.c; in particular, it calls merge_switch_to_result() after EACH merge instead of only calling it at the end of all the sequence of merges (or when a conflict is hit). This not only causes needless updates to the working copy and index, but also causes all intermediate data to be freed and tossed, preventing caching information from one merge to the next. However, integrating sequencer.c more deeply with merge-ort.c is a big task, and making this small extension to fast-rebase.c provides us with a simple way to test the edge and corner cases that we want to make sure continue working. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-20fast-rebase: change assert() to BUG()Libravatar Elijah Newren1-1/+2
assert() can succinctly document expectations for the code, and do so in a way that may be useful to future folks trying to refactor the code and change basic assumptions; it allows them to more quickly find some places where their violations of previous assumptions trips things up. Unfortunately, assert() can surround a function call with important side-effects, which is a huge mistake since some users will compile with assertions disabled. I've had to debug such mistakes before in other codebases, so I should know better. Luckily, this was only in test code, but it's still very embarrassing. Change an assert() to an if (...) BUG (...). Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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-19clone: clean up directory after transport_fetch_refs() failureLibravatar Jeff King1-0/+7
git-clone started respecting errors from the transport subsystem in aab179d937 (builtin/clone.c: don't ignore transport_fetch_refs() errors, 2020-12-03). However, that commit didn't handle the cleanup of the filesystem quite right. The cleanup of the directory that cmd_clone() creates is done by an atexit() handler, which we control with a flag. It starts as JUNK_LEAVE_NONE ("clean up everything"), then progresses to JUNK_LEAVE_REPO when we know we have a valid repo but not working tree, and then finally JUNK_LEAVE_ALL when we have a successful checkout. Most errors cause us to die(), which then triggers the handler to do the right thing based on how far into cmd_clone() we got. But the checks added by aab179d937 instead set the "err" variable and then jump to a new "cleanup" label, which then returns our non-zero status. However, the code after the cleanup label includes setting the flag to JUNK_LEAVE_ALL, and so we accidentally leave the repository and working tree in place. One obvious option to fix this is to reorder the end of the function to set the flag first, before cleanup code, and put the label between them. But we can observe another small bug: the error return from transport_fetch_refs() is generally "-1", and we propagate that to the return value of cmd_clone(), which ultimately becomes the exit code of the process. And we try to avoid transmitting negative values via exit codes (only the low 8 bits are passed along as an unsigned value, though in practice for "-1" this at least retains the property that it's non-zero). Instead, let's just die(). That makes us consistent with rest of the code in the function. It does add a new "fatal:" line to the output, but I'd argue that's a good thing: - in the rare case that the transport code didn't say anything, now the user gets _some_ error message - even if the transport code said something like "error: ssh died of signal 9", it's nice to also say "fatal" to indicate that we considered that to be a show-stopper. Triggering this in the test suite turns out to be surprisingly difficult. Almost every error we'd encounter, including ones deep inside the transport code, cause us to just die() right there! However, one way is to put a fake wrapper around git-upload-pack that sends the complete packfile but exits with a failure code. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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-17git-send-email: add option to specify sendmail commandLibravatar Gregory Anders1-0/+31
The sendemail.smtpServer configuration option and --smtp-server command line option both support using a sendmail-like program to send emails by specifying an absolute file path. However, this is not ideal for the following reasons: 1. It overloads the meaning of smtpServer (now a program is being used for the server?) 2. It doesn't allow for non-absolute paths, arguments, or arbitrary scripting Requiring an absolute path is bad for portability, as the same program may be in different locations on different systems. If a user wishes to pass arguments to their program, they have to use the smtpServerOption option, which is cumbersome (as it must be repeated for each option) and doesn't adhere to normal git conventions. Introduce a new configuration option sendemail.sendmailCmd as well as a command line option --sendmail-cmd that can be used to specify a command (with or without arguments) or shell expression to run to send email. The name of this option is consistent with --to-cmd and --cc-cmd. This invocation honors the user's $PATH so that absolute paths are not necessary. Arbitrary shell expressions are also supported, allowing users to do basic scripting. Give this option a higher precedence over --smtp-server and sendemail.smtpServer, as the new interface is more flexible. For backward compatibility, continue to support absolute paths in --smtp-server and sendemail.smtpServer. Signed-off-by: Gregory Anders <greg@gpanders.com> 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