summaryrefslogtreecommitdiff
path: root/t
AgeCommit message (Collapse)AuthorFilesLines
2021-12-13chainlint.sed: drop unnecessary distinction between ?!AMP?! and ?!SEMI?!Libravatar Eric Sunshine5-25/+24
>From inception, when chainlint.sed encountered a line using semicolon to separate commands rather than `&&`, it would insert a ?!SEMI?! annotation at the beginning of the line rather ?!AMP?! even though the &&-chain is also broken by the semicolon. Given a line such as: ?!SEMI?! cmd1; cmd2 && the ?!SEMI?! annotation makes it easier to see what the problem is than if the output had been: ?!AMP?! cmd1; cmd2 && which might confuse the test author into thinking that the linter is broken (since the line clearly ends with `&&`). However, now that the ?!AMP?! an ?!SEMI?! annotations are inserted at the point of breakage rather than at the beginning of the line, and taking into account that both represent a broken &&-chain, there is little reason to distinguish between the two. Using ?!AMP?! alone is sufficient to point the test author at the problem. For instance, in: cmd1; ?!AMP?! cmd2 && cmd3 it is clear that the &&-chain is broken between `cmd1` and `cmd2`. Likewise, in: cmd1 && cmd2 ?!AMP?! cmd3 it is clear that the &&-chain is broken between `cmd2` and `cmd3`. Finally, in: cmd1; ?!AMP?! cmd2 ?!AMP?! cmd3 it is clear that the &&-chain is broken between each command. Hence, there is no longer a good reason to make a distinction between a broken &&-chain due to a semicolon and a broken chain due to a missing `&&` at end-of-line. Therefore, drop the ?!SEMI?! annotation and use ?!AMP?! exclusively. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-13chainlint.sed: tolerate harmless ";" at end of last line in blockLibravatar Eric Sunshine2-7/+8
chainlint.sed flags ";" when used as a command terminator since it breaks the &&-chain, thus can allow failures to go undetected. However, when a command terminated by ";" is the last command in the body of a compound statement, such as `command-2` in: if test $# -gt 1 then command-1 && command-2; fi then the ";" is harmless and the exit code from `command-2` is passed through untouched and becomes the exit code of the compound statement, as if the ";" was not present. Therefore, tolerate a trailing ";" in this position rather than complaining about broken &&-chain. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-13chainlint.sed: improve ?!SEMI?! placement accuracyLibravatar Eric Sunshine5-18/+18
When chainlint.sed detects commands separated by a semicolon rather than by `&&`, it places a ?!SEMI?! annotation at the beginning of the line. However, this is an unusual location for programmers accustomed to error messages (from compilers, for instance) indicating the exact point of the problem. Therefore, relocate the ?!SEMI?! annotation to the location of the semicolon in order to better direct the programmer's attention to the source of the problem. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-13chainlint.sed: improve ?!AMP?! placement accuracyLibravatar Eric Sunshine23-38/+38
When chainlint.sed detects a broken &&-chain, it places an ?!AMP?! annotation at the beginning of the line. However, this is an unusual location for programmers accustomed to error messages (from compilers, for instance) indicating the exact point of the problem. Therefore, relocate the ?!AMP?! annotation to the end of the line in order to better direct the programmer's attention to the source of the problem. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-13t/Makefile: optimize chainlint self-testLibravatar Eric Sunshine1-6/+4
Rather than running `chainlint` and `diff` once per self-test -- which may become expensive as more tests are added -- instead run `chainlint` a single time over all tests bodies collectively and compare the result to the collective "expected" output. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-13t/chainlint/one-liner: avoid overly intimate chainlint.sed knowledgeLibravatar Eric Sunshine2-2/+2
The purpose of chainlint.sed is to detect &&-chain breakage only within subshells (one level deep); it doesn't bother checking for top-level &&-chain breakage since the &&-chain checker built into t/test-lib.sh should detect broken &&-chains outside of subshells by making them magically exit with code 117. Unfortunately, one of the chainlint.sed self-tests has overly intimate knowledge of this particular division of responsibilities and only cares about what chainlint.sed itself will produce, while ignoring the fact that a more all-encompassing linter would complain about a broken &&-chain outside the subshell. This makes it difficult to re-use the test with a more capable chainlint implementation should one ever be developed. Therefore, adjust the test and its "expected" output to avoid being specific to the tunnel-vision of this one implementation. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-13t/chainlint/*.test: generalize self-test commentaryLibravatar Eric Sunshine6-9/+6
The purpose of chainlint.sed is to detect &&-chain breakage only within subshells (one level deep); it doesn't bother checking for top-level &&-chain breakage since the &&-chain checker built into t/test-lib.sh should detect broken &&-chains outside of subshells by making them magically exit with code 117. However, this division of labor may not always be the case if a more capable chainlint implementation is ever developed. Beyond that, due to being sed-based and due to its use of heuristics, chainlint.sed has several limitations (such as being unable to detect &&-chain breakage in subshells more than one level deep since it only manually emulates recursion into a subshell). Some of the comments in the chainlint self-tests unnecessarily reflect the limitations of chainlint.sed even though those limitations are not what is being tested. Therefore, simplify and generalize the comments to explain only what is being tested, thus ensuring that they won't become outdated if a more capable chainlint is ever developed. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-13t/chainlint/*.test: fix invalid test cases due to mixing quote typesLibravatar Eric Sunshine20-70/+38
The chainlint self-test code snippets are supposed to represent the body of a test_expect_success() or test_expect_failure(), yet the contents of a few tests would have caused the shell to report syntax errors had they been real test bodies due to the mix of single- and double-quotes. Although chainlint.sed, with its simplistic heuristics, is blind to this problem, a future more robust chainlint implementation might not have such a limitation. Therefore, stop mixing quote types haphazardly in those tests and unify quoting throughout. While at it, drop chunks of tests which merely repeat what is already tested elsewhere but with alternative quotes. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-13t/chainlint/*.test: don't use invalid shell syntaxLibravatar Eric Sunshine3-4/+6
The chainlint self-test code snippets are supposed to represent the body of a test_expect_success() or test_expect_failure(), yet the contents of these tests would have caused the shell to report syntax errors had they been real test bodies. Although chainlint.sed, with its simplistic heuristics, is blind to these syntactic problems, a future more robust chainlint implementation might not have such a limitation, so make these snippets syntactically valid. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-10Merge branch 'en/rebase-x-fix'Libravatar Junio C Hamano1-1/+6
"git rebase -x" added an unnecessary 'exec' instructions before 'noop', which has been corrected. * en/rebase-x-fix: sequencer: avoid adding exec commands for non-commit creating commands
2021-12-10Merge branch 'em/missing-pager'Libravatar Junio C Hamano1-0/+5
When a non-existent program is given as the pager, we tried to reuse an uninitialized child_process structure and crashed, which has been fixed. * em/missing-pager: pager: fix crash when pager program doesn't exist
2021-12-10Merge branch 'mp/absorb-submodule-git-dir-upon-deinit'Libravatar Junio C Hamano1-6/+5
"git submodule deinit" for a submodule whose .git metadata directory is embedded in its working tree refused to work, until the submodule gets converted to use the "absorbed" form where the metadata directory is stored in superproject, and a gitfile at the top-level of the working tree of the submodule points at it. The command is taught to convert such submodules to the absorbed form as needed. * mp/absorb-submodule-git-dir-upon-deinit: submodule: absorb git dir instead of dying on deinit
2021-12-10Merge branch 'hn/create-reflog-simplify'Libravatar Junio C Hamano3-4/+3
A small simplification of API. * hn/create-reflog-simplify: refs: drop force_create argument of create_reflog API
2021-12-10Merge branch 'jk/t7006-sigpipe-tests-fix'Libravatar Junio C Hamano1-40/+14
The function to cull a child process and determine the exit status had two separate code paths for normal callers and callers in a signal handler, and the latter did not yield correct value when the child has caught a signal. The handling of the exit status has been unified for these two code paths. An existing test with flakiness has also been corrected. * jk/t7006-sigpipe-tests-fix: t7006: simplify exit-code checks for sigpipe tests t7006: clean up SIGPIPE handling in trace2 tests run-command: unify signal and regular logic for wait_or_whine()
2021-12-10Merge branch 'vd/sparse-reset'Libravatar Junio C Hamano3-15/+159
Various operating modes of "git reset" have been made to work better with the sparse index. * vd/sparse-reset: unpack-trees: improve performance of next_cache_entry reset: make --mixed sparse-aware reset: make sparse-aware (except --mixed) reset: integrate with sparse index reset: expand test coverage for sparse checkouts sparse-index: update command for expand/collapse test reset: preserve skip-worktree bit in mixed reset reset: rename is_missing to !is_in_reset_tree
2021-12-10Merge branch 'ab/checkout-branch-info-leakfix'Libravatar Junio C Hamano35-0/+43
Leakfix. * ab/checkout-branch-info-leakfix: checkout: fix "branch info" memory leaks
2021-12-10Merge branch 'jk/t5319-midx-corruption-test-deflake'Libravatar Junio C Hamano1-2/+4
Test fix. * jk/t5319-midx-corruption-test-deflake: t5319: corrupt more bytes of the midx checksum
2021-12-10Merge branch 'tw/var-default-branch'Libravatar Junio C Hamano1-0/+20
"git var GIT_DEFAULT_BRANCH" is a way to see what name is used for the newly created branch if "git init" is run. * tw/var-default-branch: var: add GIT_DEFAULT_BRANCH variable
2021-12-10Merge branch 'jk/strbuf-addftime-seconds-since-epoch'Libravatar Junio C Hamano1-0/+4
The "--date=format:<strftime>" gained a workaround for the lack of system support for a non-local timezone to handle "%s" placeholder. * jk/strbuf-addftime-seconds-since-epoch: strbuf_addftime(): handle "%s" manually
2021-12-10Merge branch 'if/redact-packfile-uri'Libravatar Junio C Hamano1-0/+51
Redact the path part of packfile URI that appears in the trace output. * if/redact-packfile-uri: http-fetch: redact url on die() message fetch-pack: redact packfile urls in traces
2021-12-10Merge branch 'gc/remote-with-fewer-static-global-variables'Libravatar Junio C Hamano1-0/+9
Code clean-up to eventually allow information on remotes defined for an arbitrary repository to be read. * gc/remote-with-fewer-static-global-variables: remote: die if branch is not found in repository remote: remove the_repository->remote_state from static methods remote: use remote_state parameter internally remote: move static variables into per-repository struct t5516: add test case for pushing remote refspecs
2021-12-10Merge branch 'vd/sparse-sparsity-fix-on-read'Libravatar Junio C Hamano2-2/+34
Ensure that the sparseness of the in-core index matches the index.sparse configuration specified by the repository immediately after the on-disk index file is read. * vd/sparse-sparsity-fix-on-read: sparse-index: update do_read_index to ensure correct sparsity sparse-index: add ensure_correct_sparsity function sparse-index: avoid unnecessary cache tree clearing test-read-cache.c: prepare_repo_settings after config init
2021-11-29sequencer: avoid adding exec commands for non-commit creating commandsLibravatar Elijah Newren1-1/+6
The `--exec <cmd>` is documented as Append "exec <cmd>" after each line creating a commit in the final history. ... If --autosquash is used, "exec" lines will not be appended for the intermediate commits, and will only appear at the end of each squash/fixup series. Unfortunately, it would also add exec commands after non-pick operations, such as 'no-op', which could be seen for example with git rebase -i --exec true HEAD todo_list_add_exec_commands() intent was to insert exec commands after each logical pick, while trying to consider a chains of fixup and squash commits to be part of the pick before it. So it would keep an 'insert' boolean tracking if it had seen a pick or merge, but not write the exec command until it saw the next non-fixup/squash command. Since that would make it miss the final exec command, it had some code that would check whether it still needed to insert one at the end, but instead of a simple if (insert) it had a if (insert || <condition that is always true>) That's buggy; as per the docs, we should only add exec commands for lines that create commits, i.e. only if insert is true. Fix the conditional. There was one testcase in the testsuite that we tweak for this change; it was introduced in 54fd3243da ("rebase -i: reread the todo list if `exec` touched it", 2017-04-26), and was merely testing that after an exec had fired that the todo list would be re-read. The test at the time would have worked given any revision at all, though it would only work with 'HEAD' as a side-effect of this bug. Since we're fixing this bug, choose something other than 'HEAD' for that test. Finally, add a testcase that verifies when we have no commits to pick, that we get no exec lines in the generated todo list. Reported-by: Nikita Bobko <nikitabobko@gmail.com> Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-29Merge branch 'mc/clean-smudge-with-llp64'Libravatar Junio C Hamano3-4/+47
The clean/smudge conversion code path has been prepared to better work on platforms where ulong is narrower than size_t. * mc/clean-smudge-with-llp64: clean/smudge: allow clean filters to process extremely large files odb: guard against data loss checking out a huge file git-compat-util: introduce more size_t helpers odb: teach read_blob_entry to use size_t t1051: introduce a smudge filter test for extremely large files test-lib: add prerequisite for 64-bit platforms test-tool genzeros: generate large amounts of data more efficiently test-genzeros: allow more than 2G zeros in Windows
2021-11-29Merge branch 'tb/plug-pack-bitmap-leaks'Libravatar Junio C Hamano1-1/+2
Leakfix. * tb/plug-pack-bitmap-leaks: pack-bitmap.c: more aggressively free in free_bitmap_index() pack-bitmap.c: don't leak type-level bitmaps midx.c: write MIDX filenames to strbuf builtin/multi-pack-index.c: don't leak concatenated options builtin/repack.c: avoid leaking child arguments builtin/pack-objects.c: don't leak memory via arguments t/helper/test-read-midx.c: free MIDX within read_midx_file() midx.c: don't leak MIDX from verify_midx_file midx.c: clean up chunkfile after reading the MIDX
2021-11-29Merge branch 'tp/send-email-completion'Libravatar Junio C Hamano1-0/+3
The command line complation for "git send-email" options have been tweaked to make it easier to keep it in sync with the command itself. * tp/send-email-completion: send-email docs: add format-patch options send-email: programmatically generate bash completions
2021-11-29Merge branch 'jc/fix-ref-sorting-parse'Libravatar Junio C Hamano2-1/+37
Things like "git -c branch.sort=bogus branch new HEAD", i.e. the operation modes of the "git branch" command that do not need the sort key information, no longer errors out by seeing a bogus sort key. * jc/fix-ref-sorting-parse: for-each-ref: delay parsing of --sort=<atom> options
2021-11-29Merge branch 'so/stash-staged'Libravatar Junio C Hamano1-0/+11
"git stash" learned the "--staged" option to stash away what has been added to the index (and nothing else). * so/stash-staged: stash: get rid of unused argument in stash_staged() stash: implement '--staged' option for 'push' and 'save'
2021-11-29Merge branch 'ab/refs-errno-cleanup'Libravatar Junio C Hamano3-1/+89
The "remainder" of hn/refs-errno-cleanup topic. * ab/refs-errno-cleanup: (21 commits) refs API: post-migration API renaming [2/2] refs API: post-migration API renaming [1/2] refs API: don't expose "errno" in run_transaction_hook() refs API: make expand_ref() & repo_dwim_log() not set errno refs API: make resolve_ref_unsafe() not set errno refs API: make refs_ref_exists() not set errno refs API: make refs_resolve_refdup() not set errno refs tests: ignore ignore errno in test-ref-store helper refs API: ignore errno in worktree.c's find_shared_symref() refs API: ignore errno in worktree.c's add_head_info() refs API: make files_copy_or_rename_ref() et al not set errno refs API: make loose_fill_ref_dir() not set errno refs API: make resolve_gitlink_ref() not set errno refs API: remove refs_read_ref_full() wrapper refs/files: remove "name exist?" check in lock_ref_oid_basic() reflog tests: add --updateref tests refs API: make refs_rename_ref_available() static refs API: make parse_loose_ref_contents() not set errno refs API: make refs_read_raw_ref() not set errno refs API: add a version of refs_resolve_ref_unsafe() with "errno" ...
2021-11-29Merge branch 'ow/stash-count-in-status-porcelain-output'Libravatar Junio C Hamano1-0/+15
Allow "git status --porcelain=v2" to show the number of stash entries with --show-stash like the normal output does. * ow/stash-count-in-status-porcelain-output: status: print stash info with --porcelain=v2 --show-stash status: count stash entries in separate function
2021-11-29Merge branch 'jk/loosen-urlmatch'Libravatar Junio C Hamano1-1/+1
Treat "_" as any other URL-valid characters in an URL when matching the per-URL configuration variable names. * jk/loosen-urlmatch: urlmatch: add underscore to URL_HOST_CHARS
2021-11-29reset: make --mixed sparse-awareLibravatar Victoria Dye1-0/+17
Remove the `ensure_full_index` guard on `read_from_tree` and update `git reset --mixed` to ensure it can use sparse directory index entries wherever possible. Sparse directory entries are reset using `diff_tree_oid`, which requires `change` and `add_remove` functions to process the internal contents of the sparse directory. The `recursive` diff option handles cases in which `reset --mixed` must diff/merge files that are nested multiple levels deep in a sparse directory. The use of pathspecs with `git reset --mixed` introduces scenarios in which internal contents of sparse directories may be matched by the pathspec. In order to reset *all* files in the repo that may match the pathspec, the following conditions on the pathspec require index expansion before performing the reset: * "magic" pathspecs * wildcard pathspecs that do not match only in-cone files or entire sparse directories * literal pathspecs matching something outside the sparse checkout definition Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-29reset: make sparse-aware (except --mixed)Libravatar Victoria Dye1-2/+13
Remove `ensure_full_index` guard on `prime_cache_tree` and update `prime_cache_tree_rec` to correctly reconstruct sparse directory entries in the cache tree. While processing a tree's entries, `prime_cache_tree_rec` must determine whether a directory entry is sparse or not by searching for it in the index (*without* expanding the index). If a matching sparse directory index entry is found, no subtrees are added to the cache tree entry and the entry count is set to 1 (representing the sparse directory itself). Otherwise, the tree is assumed to not be sparse and its subtrees are recursively added to the cache tree. Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-29reset: expand test coverage for sparse checkoutsLibravatar Victoria Dye2-0/+101
Add new tests for `--merge` and `--keep` modes, as well as mixed reset with pathspecs. New performance test cases exercise various execution paths for `reset`. Co-authored-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-24sparse-index: update do_read_index to ensure correct sparsityLibravatar Victoria Dye1-0/+31
Unless `command_requires_full_index` forces index expansion, ensure in-core index sparsity matches config settings on read by calling `ensure_correct_sparsity`. This makes the behavior of the in-core index more consistent between different methods of updating sparsity: manually changing the `index.sparse` config setting vs. executing `git sparse-checkout --[no-]sparse-index init` Although index sparsity is normally updated with `git sparse-checkout init`, ensuring correct sparsity after a manual `index.sparse` change has some practical benefits: 1. It allows for command-by-command sparsity toggling with `-c index.sparse=<true|false>`, e.g. when troubleshooting issues with the sparse index. 2. It prevents users from experiencing abnormal slowness after setting `index.sparse` to `true` due to use of a full index in all commands until the on-disk index is updated. Helped-by: Junio C Hamano <gitster@pobox.com> Co-authored-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-24test-read-cache.c: prepare_repo_settings after config initLibravatar Victoria Dye1-2/+3
Move `prepare_repo_settings` after the git directory has been set up in `test-read-cache.c`. The git directory settings must be initialized to properly assign repo settings using the worktree-level git config. Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-24pager: fix crash when pager program doesn't existLibravatar Enzo Matsumiya1-0/+5
When prepare_cmd() fails for, e.g., pager process setup, child_process_clear() frees the memory in pager_process.args, but .argv was pointed to pager_process.args.v earlier in start_command(), so it's now a dangling pointer. setup_pager() is then called a second time, from cmd_log_init_finish() in this case, and any further operations using its .argv, e.g. strvec_*, will use the dangling pointer and eventually crash. According to trivial tests, setup_pager() is not called twice if the first call is successful. This patch makes sure that pager_process is properly initialized on setup_pager(). Drop CHILD_PROCESS_INIT from its declaration since it's no longer really necessary. Add a test to catch possible regressions. Reproducer: $ git config pager.show INVALID_PAGER $ git show $VALID_COMMIT error: cannot run INVALID_PAGER: No such file or directory [1] 3619 segmentation fault (core dumped) git show $VALID_COMMIT Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-23Merge branch 'ds/add-rm-with-sparse-index' into maintLibravatar Junio C Hamano1-0/+17
Regression fix for 2.34 * ds/add-rm-with-sparse-index: dir: revert "dir: select directories correctly"
2021-11-23Merge branch 'ev/pull-already-up-to-date-is-noop' into maintLibravatar Junio C Hamano1-0/+6
"git pull" with any strategy when the other side is behind us should succeed as it is a no-op, but doesn't. * ev/pull-already-up-to-date-is-noop: pull: should be noop when already-up-to-date
2021-11-23Merge branch 'hm/paint-hits-in-log-grep' into maintLibravatar Junio C Hamano1-48/+0
"git grep" looking in a blob that has non-UTF8 payload was completely broken when linked with versions of PCREv2 library older than 10.34 in the latest release. * hm/paint-hits-in-log-grep: Revert "grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data"
2021-11-22Merge branch 'ds/add-rm-with-sparse-index'Libravatar Junio C Hamano1-0/+17
Regression fix for 2.34 * ds/add-rm-with-sparse-index: dir: revert "dir: select directories correctly"
2021-11-22t7006: simplify exit-code checks for sigpipe testsLibravatar Jeff King1-17/+5
Some tests in t7006 check for a SIGPIPE result by recording $? and comparing it with test_match_signal. Before the previous commit, the command was on the left-hand side of a pipe, and so we had to do some subshell trickery to extract it. But now that this is no longer the case, we can do things much more simply: just run the command directly, using braces to avoid wrecking the &&-chain, and then record $?. We could almost use test_expect_code here, but it doesn't know about test_match_signal. Likewise, for tests which expect success (i.e., not SIGPIPE), we can just put them in the &&-chain as usual. That even lets us get rid of the !MINGW check, since the expectation is the same on both sides. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-22t7006: clean up SIGPIPE handling in trace2 testsLibravatar Jeff King1-28/+14
Comit c24b7f6736 (pager: test for exit code with and without SIGPIPE, 2021-02-02) introduced some tests that don't reliably generate SIGPIPE where we expect it (i.e., when our pager doesn't read all of the output from git-log). There are two problems that somewhat cancel each other out. First is that the output of git-log isn't very large (only around 800 bytes). So even if the pager doesn't read all of our output, it's racy whether or not we'll actually get a SIGPIPE (we won't if we write all of the output into the pipe buffer before the pager exits). But we wrap git-log with test_terminal, which is supposed to propagate the exit status of git-log. However, it doesn't always do so; test_terminal will copy to stdout any lines that it got from our fake pager, and it pipes to an empty command. So most of the time we are seeing a SIGPIPE from test_terminal itself (though this is likewise racy). Let's try to make this more robust in two ways: 1. We'll put a commit with a huge message at the tip of history. Since this is over a megabyte, it should fill the OS pipe buffer completely, causing git-log to keep trying to write even after the pager has exited. 2. We'll redirect the output of test_terminal to /dev/null. That means it can never get SIGPIPE itself, and will always be giving us the exit code from git-log. These two changes reveal that one of the tests was looking for the wrong behavior. If we try to start a pager that does not exist (according to execve()), then the error propagates from start_command() back to the pager code as an error, and we avoid redirecting git-log's stdout to the broken pager entirely. Instead, it goes straight to the original stdout (test_terminal's pty in this case), and we do not see a SIGPIPE at all. So the test "git attempts to page to nonexisting pager command, gets SIGPIPE" is checking the wrong outcome; it should be looking for a successful exit (and was only confused by test_terminal's SIGPIPE). There's a related test, "git discards nonexisting pager without SIGPIPE", which sets the pager to a shell command which will read all input and _then_ run a non-existing command. But that doesn't trigger the same execve() behavior. We really do run the shell there and redirect git-log's stdout to it. And the fact that the shell then exits 127 is not interesting. It is not different at that point than the earlier test to check for "exit 1". So we can drop that test entirely. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-22run-command: unify signal and regular logic for wait_or_whine()Libravatar Jeff King1-1/+1
Since 507d7804c0 (pager: don't use unsafe functions in signal handlers, 2015-09-04), we have a separate code path in wait_or_whine() for the case that we're in a signal handler. But that code path misses some of the cases handled by the main logic. This was improved in be8fc53e36 (pager: properly log pager exit code when signalled, 2021-02-02), but that covered only case: actually returning the correct error code. But there are some other cases: - if waitpid() returns failure, we wouldn't notice and would look at uninitialized garbage in the status variable; it's not clear if it's possible to trigger this or not - if the process exited by signal, then we would still report "-1" rather than the correct signal code This latter case even had a test added in be8fc53e36, but it doesn't work reliably. It sets the pager command to: >pager-used; test-tool sigchain The latter command will die by signal, but because there are multiple commands, there will be a shell in between. And it's the shell whose waitpid() call will see the signal death, and it will then exit with code 143, which is what Git will see. To make matters even more confusing, some shells (such as bash) will realize that there's nothing for the shell to do after test-tool finishes, and will turn it into an exec. So the test was only checking what it thought when /bin/sh points to a shell like bash (we're relying on the shell used internally by Git to spawn sub-commands here, so even running the test under bash would not be enough). This patch adjusts the tests to explicitly call "exec" in the pager command, which produces a consistent outcome regardless of shell. Note that without the code change in this patch it _should_ fail reliably, but doesn't. That test, like its siblings, tries to trigger SIGPIPE in the git-log process writing to the pager, but only do so racily. That will be fixed in a follow-on patch. For the code change here, we have two options: - we can teach the in_signal code to handle WIFSIGNALED() - we can stop returning early when in_signal is set, and instead annotate individual calls that we need to skip in this case The former is a simpler patch, but means we're essentially duplicating all of the logic. So instead I went with the latter. The result is a bigger patch, and we do run the risk of new code being added but forgetting to handle in_signal. But in the long run it seems more maintainable. I've skipped any non-trivial calls for the in_signal case, like calling error(). We'll also skip the call to clear_child_for_cleanup(), as we were before. This is arguably the wrong thing to do, since we wouldn't want to try to clean it up again. But: - we can't call it as-is, because it calls free(), which we must avoid in a signal handler (we'd have to pass in_signal so it can skip the free() call) - we'll only go through the list of children to clean once, since our cleanup_children_on_signal() handler pops itself after running (and then re-raises, so eventually we'd just exit). So this cleanup only matters if a process is on the cleanup list _and_ it has a separate handler to clean itself up. Which is questionable in the first place (and AFAIK we do not do). - double-cleanup isn't actually that bad anyway. waitpid() will just return an error, which we won't even report because of in_signal. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-22dir: revert "dir: select directories correctly"Libravatar Derrick Stolee1-0/+17
This reverts commit f6526728f950cacfd5b5e42bcc65f2c47f3da654. The change in f652672 (dir: select directories correctly, 2021-09-24) caused a regression in directory-based matches with non-cone-mode patterns, especially for .gitignore patterns. A test is included to prevent this regression in the future. The commit ed495847 (dir: fix pattern matching on dirs, 2021-09-24) was reverted in 5ceb663 (dir: fix directory-matching bug, 2021-11-02) for similar reasons. Neither commit changed tests, and tests added later in the series continue to pass when these commits are reverted. Reported-by: Danial Alihosseini <danial.alihosseini@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-22refs: drop force_create argument of create_reflog APILibravatar Han-Wen Nienhuys3-4/+3
There is only one caller, builtin/checkout.c, and it hardcodes force_create=1. This argument was introduced in abd0cd3a301 (refs: new public ref function: safe_create_reflog, 2015-07-21), which promised to immediately use it in a follow-on commit, but that never happened. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-21Merge branch 'ev/pull-already-up-to-date-is-noop'Libravatar Junio C Hamano1-0/+6
"git pull" with any strategy when the other side is behind us should succeed as it is a no-op, but doesn't. * ev/pull-already-up-to-date-is-noop: pull: should be noop when already-up-to-date
2021-11-21Merge branch 'hm/paint-hits-in-log-grep'Libravatar Junio C Hamano1-48/+0
"git grep" looking in a blob that has non-UTF8 payload was completely broken when linked with certain versions of PCREv2 library in the latest release. * hm/paint-hits-in-log-grep: Revert "grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data"
2021-11-19submodule: absorb git dir instead of dying on deinitLibravatar Mugdha Pattnaik1-6/+5
Currently, running 'git submodule deinit' on repos where the submodule's '.git' is a directory, aborts with a message that is not exactly user friendly. Let's change this to instead warn the user that the .git/ directory has been absorbed into the superproject. The rest of the deinit function can operate as it already does with new-style submodules. In one test, we used to require "git submodule deinit" to fail even with the "--force" option when the submodule's .git/ directory is not absorbed. Adjust it to expect the operation to pass. Suggested-by: Atharva Raykar <raykar.ath@gmail.com> Signed-off-by: Mugdha Pattnaik <mugdhapattnaik@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-19Revert "grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data"Libravatar Junio C Hamano1-48/+0
This reverts commit ae39ba431ab861548eb60b4bd2e1d8b8813db76f, as it breaks "grep" when looking for a string in non UTF-8 haystack, when linked with certain versions of PCREv2 library. Signed-off-by: Junio C Hamano <gitster@pobox.com>