Commit Graph

18816 Commits

Author SHA1 Message Date
Eric Sunshine
fbd992b61b chainlint.sed: improve ?!SEMI?! placement accuracy
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-13 14:15:29 -08:00
Eric Sunshine
db8c7a1cc0 chainlint.sed: improve ?!AMP?! placement accuracy
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-13 14:15:29 -08:00
Eric Sunshine
f30c1d5eb1 t/Makefile: optimize chainlint self-test
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-13 14:15:28 -08:00
Eric Sunshine
0cca54c706 t/chainlint/one-liner: avoid overly intimate chainlint.sed knowledge
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-13 14:15:28 -08:00
Eric Sunshine
1ad0780a77 t/chainlint/*.test: generalize self-test commentary
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-13 14:15:28 -08:00
Eric Sunshine
5459bc1bbb t/chainlint/*.test: fix invalid test cases due to mixing quote types
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-13 14:15:28 -08:00
Eric Sunshine
925d7304d2 t/chainlint/*.test: don't use invalid shell syntax
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-13 14:15:28 -08:00
Junio C Hamano
3e1dbfa135 Merge branch 'en/rebase-x-fix'
"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-10 14:35:16 -08:00
Junio C Hamano
bb47eee9df Merge branch 'em/missing-pager'
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-10 14:35:15 -08:00
Junio C Hamano
670703e9d6 Merge branch 'mp/absorb-submodule-git-dir-upon-deinit'
"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-10 14:35:15 -08:00
Junio C Hamano
b8148376a2 Merge branch 'hn/create-reflog-simplify'
A small simplification of API.

* hn/create-reflog-simplify:
  refs: drop force_create argument of create_reflog API
2021-12-10 14:35:13 -08:00
Junio C Hamano
97991dfab7 Merge branch 'jk/t7006-sigpipe-tests-fix'
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-10 14:35:13 -08:00
Junio C Hamano
f0850875fd Merge branch 'vd/sparse-reset'
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-10 14:35:12 -08:00
Junio C Hamano
7b11728a7b Merge branch 'ab/checkout-branch-info-leakfix'
Leakfix.

* ab/checkout-branch-info-leakfix:
  checkout: fix "branch info" memory leaks
2021-12-10 14:35:09 -08:00
Junio C Hamano
b0b5337876 Merge branch 'jk/t5319-midx-corruption-test-deflake'
Test fix.

* jk/t5319-midx-corruption-test-deflake:
  t5319: corrupt more bytes of the midx checksum
2021-12-10 14:35:08 -08:00
Junio C Hamano
03194a1afa Merge branch 'tw/var-default-branch'
"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-10 14:35:07 -08:00
Junio C Hamano
1c39c822a9 Merge branch 'jk/strbuf-addftime-seconds-since-epoch'
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-10 14:35:07 -08:00
Junio C Hamano
b5e7f5e5b1 Merge branch 'if/redact-packfile-uri'
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-10 14:35:04 -08:00
Junio C Hamano
6d1e149ac0 Merge branch 'gc/remote-with-fewer-static-global-variables'
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-10 14:35:02 -08:00
Junio C Hamano
5396d7b298 Merge branch 'vd/sparse-sparsity-fix-on-read'
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-12-10 14:35:01 -08:00
Elijah Newren
cc9dcdee61 sequencer: avoid adding exec commands for non-commit creating commands
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-29 22:53:26 -08:00
Junio C Hamano
f9ba6acaa9 Merge branch 'mc/clean-smudge-with-llp64'
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-29 15:41:51 -08:00
Junio C Hamano
49767c3d9f Merge branch 'tb/plug-pack-bitmap-leaks'
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-29 15:41:49 -08:00
Junio C Hamano
7c2abf1a83 Merge branch 'tp/send-email-completion'
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-29 15:41:49 -08:00
Junio C Hamano
5126145ba8 Merge branch 'jc/fix-ref-sorting-parse'
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-29 15:41:47 -08:00
Junio C Hamano
44ac8fd1b4 Merge branch 'so/stash-staged'
"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-29 15:41:47 -08:00
Junio C Hamano
96f6623ada Merge branch 'ab/refs-errno-cleanup'
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-29 15:41:45 -08:00
Junio C Hamano
dea96aae4d Merge branch 'ow/stash-count-in-status-porcelain-output'
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-29 15:41:44 -08:00
Junio C Hamano
96eca029bf Merge branch 'jk/loosen-urlmatch'
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-29 15:41:44 -08:00
Victoria Dye
4d1cfc1351 reset: make --mixed sparse-aware
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-29 12:51:26 -08:00
Victoria Dye
20ec2d034c reset: make sparse-aware (except --mixed)
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-29 12:51:26 -08:00
Victoria Dye
291d77eb3e reset: expand test coverage for sparse checkouts
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-29 12:51:26 -08:00
Victoria Dye
7ca4fc8819 sparse-index: update do_read_index to ensure correct sparsity
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-24 16:32:39 -08:00
Victoria Dye
336d82e472 test-read-cache.c: prepare_repo_settings after config init
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-24 16:32:38 -08:00
Enzo Matsumiya
f917f57f40 pager: fix crash when pager program doesn't exist
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-24 16:14:10 -08:00
Junio C Hamano
eef0a8e7c1 Merge branch 'ds/add-rm-with-sparse-index' into maint
Regression fix for 2.34

* ds/add-rm-with-sparse-index:
  dir: revert "dir: select directories correctly"
2021-11-23 14:48:11 -08:00
Junio C Hamano
ad03180c5c Merge branch 'ev/pull-already-up-to-date-is-noop' into maint
"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-23 14:48:04 -08:00
Junio C Hamano
a650ff5aec Merge branch 'hm/paint-hits-in-log-grep' into maint
"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-23 14:48:00 -08:00
Junio C Hamano
1bf2673685 Merge branch 'ds/add-rm-with-sparse-index'
Regression fix for 2.34

* ds/add-rm-with-sparse-index:
  dir: revert "dir: select directories correctly"
2021-11-22 18:40:10 -08:00
Jeff King
5263e22cba t7006: simplify exit-code checks for sigpipe tests
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-22 15:43:44 -08:00
Jeff King
f7991f01f2 t7006: clean up SIGPIPE handling in trace2 tests
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-22 15:43:44 -08:00
Jeff King
96bfb2d8ce run-command: unify signal and regular logic for wait_or_whine()
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-22 15:43:18 -08:00
Derrick Stolee
33c5d6c845 dir: revert "dir: select directories correctly"
This reverts commit f6526728f9.

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-22 14:53:23 -08:00
Han-Wen Nienhuys
7b089120d9 refs: drop force_create argument of create_reflog API
There is only one caller, builtin/checkout.c, and it hardcodes
force_create=1.

This argument was introduced in abd0cd3a30 (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-22 11:01:25 -08:00
Junio C Hamano
0f2140f105 Merge branch 'ev/pull-already-up-to-date-is-noop'
"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-21 21:57:04 -08:00
Junio C Hamano
ccd3258b4d Merge branch 'hm/paint-hits-in-log-grep'
"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-21 21:57:03 -08:00
Mugdha Pattnaik
0adc8ba6ae submodule: absorb git dir instead of dying on deinit
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-19 09:19:54 -08:00
Junio C Hamano
e7f3925bed Revert "grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data"
This reverts commit ae39ba431a, 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>
2021-11-19 09:10:27 -08:00
Glen Choo
e083ef5d54 t5516: add test case for pushing remote refspecs
"git push remote-name" (that is, with no refspec given on the command
line) should push the refspecs in remote.remote-name.push. There is no
test case that checks this behavior in detached HEAD, so add one.

Signed-off-by: Glen Choo <chooglen@google.com>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-18 22:31:18 -08:00
Erwin Villejo
ea1954af77 pull: should be noop when already-up-to-date
The already-up-to-date pull bug was fixed for --ff-only but it did not
include the case where --ff or --ff-only are not specified. This updates
the --ff-only fix to include the case where --ff or --ff-only are not
specified in command line flags or config.

Signed-off-by: Erwin Villejo <erwin.villejo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-18 14:38:53 -08:00