Commit Graph

21251 Commits

Author SHA1 Message Date
Junio C Hamano
8cc32c6b37 Merge branch 'tb/mark-more-tests-as-leak-free'
Tests that are known to pass with LSan are now marked as such.

* tb/mark-more-tests-as-leak-free:
  leak tests: mark t5583-push-branches.sh as leak-free
  leak tests: mark t3321-notes-stripspace.sh as leak-free
  leak tests: mark a handful of tests as leak-free
2023-09-05 14:38:56 -07:00
Junio C Hamano
3b4e395cb3 Merge branch 'ob/format-patch-description-file'
"git format-patch" learns a way to feed cover letter description,
that (1) can be used on detached HEAD where there is no branch
description available, and (2) also can override the branch
description if there is one.

* ob/format-patch-description-file:
  format-patch: add --description-file option
2023-09-01 11:26:28 -07:00
Junio C Hamano
f137bd4358 Merge branch 'jk/diff-result-code-cleanup'
"git diff --no-such-option" and other corner cases around the exit
status of the "diff" command has been corrected.

* jk/diff-result-code-cleanup:
  diff: drop useless "status" parameter from diff_result_code()
  diff: drop useless return values in git-diff helpers
  diff: drop useless return from run_diff_{files,index} functions
  diff: die when failing to read index in git-diff builtin
  diff: show usage for unknown builtin_diff_files() options
  diff-files: avoid negative exit value
  diff: spell DIFF_INDEX_CACHED out when calling run_diff_index()
2023-09-01 11:26:28 -07:00
Junio C Hamano
967bfc5894 Merge branch 'ch/t6300-verify-commit-test-cleanup'
Test clean-up.

* ch/t6300-verify-commit-test-cleanup:
  t/t6300: drop magic filtering
  t/lib-gpg: forcibly run a trustdb update
2023-08-31 14:31:42 -07:00
Junio C Hamano
cc48906c3b Merge branch 'ts/unpacklimit-config-fix'
transfer.unpackLimit ought to be used as a fallback, but overrode
fetch.unpackLimit and receive.unpackLimit instead.

* ts/unpacklimit-config-fix:
  transfer.unpackLimit: fetch/receive.unpackLimit takes precedence
2023-08-30 13:50:41 -07:00
Junio C Hamano
74a2e88700 Merge branch 'jc/diff-exit-code-with-w-fixes'
"git diff -w --exit-code" with various options did not work
correctly, which is being addressed.

* jc/diff-exit-code-with-w-fixes:
  diff: the -w option breaks --exit-code for --raw and other output modes
  t4040: remove test that succeeded for a wrong reason
  diff: teach "--stat -w --exit-code" to notice differences
  diff: mode-only change should be noticed by "--patch -w --exit-code"
  diff: move the fallback "--exit-code" code down
2023-08-30 13:50:41 -07:00
Junio C Hamano
5ba560ba76 Merge branch 'tb/commit-graph-verify-fix'
The commit-graph verification code that detects mixture of zero and
non-zero generation numbers has been updated.

* tb/commit-graph-verify-fix:
  commit-graph: avoid repeated mixed generation number warnings
  t/t5318-commit-graph.sh: test generation zero transitions during fsck
  commit-graph: verify swapped zero/non-zero generation cases
  commit-graph: introduce `commit_graph_generation_from_graph()`
2023-08-30 13:50:40 -07:00
Junio C Hamano
19cb1fc37b Merge branch 'ds/scalar-updates'
Scalar updates.

* ds/scalar-updates:
  scalar reconfigure: help users remove buggy repos
  setup: add discover_git_directory_reason()
  scalar: add --[no-]src option
2023-08-29 13:51:44 -07:00
Junio C Hamano
354356feff Merge branch 'sl/sparse-check-attr'
Teach "git check-attr" work better with sparse-index.

* sl/sparse-check-attr:
  check-attr: integrate with sparse-index
  attr.c: read attributes in a sparse directory
  t1092: add tests for 'git check-attr'
2023-08-29 13:51:43 -07:00
Taylor Blau
5fafe8c95f leak tests: mark t5583-push-branches.sh as leak-free
When t5583-push-branches.sh was originally introduced via 425b4d7f47
(push: introduce '--branches' option, 2023-05-06), it was not leak-free.
In fact, the test did not even run correctly until 022fbb655d (t5583:
fix shebang line, 2023-05-12), but after applying that patch, we see a
failure at t5583.8:

    ==2529087==ERROR: LeakSanitizer: detected memory leaks

    Direct leak of 384 byte(s) in 1 object(s) allocated from:
        #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98
        #1 0x55e07606cbf9 in xrealloc wrapper.c:140
        #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42
        #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917
        #4 0x55e075fe9cce in add_missing_tags remote.c:1518
        #5 0x55e075fea1e4 in match_push_refs remote.c:1665
        #6 0x55e076050a8e in transport_push transport.c:1378
        #7 0x55e075e2eb74 in push_with_options builtin/push.c:401
        #8 0x55e075e2edb0 in do_push builtin/push.c:458
        #9 0x55e075e2ff7a in cmd_push builtin/push.c:702
        #10 0x55e075d8aaf0 in run_builtin git.c:452
        #11 0x55e075d8af08 in handle_builtin git.c:706
        #12 0x55e075d8b12c in run_argv git.c:770
        #13 0x55e075d8b6a0 in cmd_main git.c:905
        #14 0x55e075e81f07 in main common-main.c:60
        #15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
        #16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360
        #17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453)

    SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s).

This leak was addressed independently via 68b51172e3 (commit-reach: fix
memory leak in get_reachable_subset(), 2023-06-03), which makes t5583
leak-free.

But t5583 was not in the tree when 68b51172e3 was written, and the two
only met after the latter was merged back in via 693bde461c (Merge
branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20).

At that point, t5583 was leak-free. Let's mark it as such accordingly.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-29 09:41:56 -07:00
Taylor Blau
bac3ccc290 leak tests: mark t3321-notes-stripspace.sh as leak-free
This test was leak-free when t3321 was originally introduced, but never
marked as such:

    $ rev="$(git log --format='%H' --reverse -1 HEAD^ -- t/t3321-notes-stripspace.sh)"
    $ git checkout $rev

    $ make SANITIZE=leak
    [...]

    $ make -C t GIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_OPTS=--immediate t3321-notes-stripspace.sh
    [...]
    # passed all 27 test(s)
    1..27
    # faking up non-zero exit with --invert-exit-code
    make: *** [Makefile:66: t3321-notes-stripspace.sh] Error 1
    make: Leaving directory '/home/ttaylorr/src/git/t'

Mark this test as leak-free accordingly.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-29 09:41:56 -07:00
Taylor Blau
20debfb210 leak tests: mark a handful of tests as leak-free
In the topic merged via 5a4f8381b6 (Merge branch
'ab/mark-leak-free-tests', 2021-10-25), a handful of tests in the suite
were marked as leak-free.

Since then, a handful of tests have become leak-free due to changes like

  - 861c56f6f9 (branch: fix a leak in setup_tracking, 2023-06-11), and
  - 866b43e644 (do_read_index(): always mark index as initialized unless
    erroring out, 2023-06-29)

, but weren't updated at the time to mark themselves as such. This leads
to test "failures" when running:

    $ make SANITIZE=leak
    $ make -C t \
        GIT_TEST_PASSING_SANITIZE_LEAK=check \
        GIT_TEST_SANITIZE_LEAK_LOG=true \
        GIT_TEST_OPTS=-vi test

This patch closes those gaps by exporting TEST_PASSES_SANITIZE_LEAK=true
before sourcing t/test-lib.sh on most remaining leak-free tests.

There are a couple of other tests which are similarly leak-free, but not
included in the list of tests touched by this patch. The remaining tests
will be addressed in the subsequent two patches.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-29 09:41:56 -07:00
Junio C Hamano
e839608295 Merge branch 'mh/credential-libsecret-attrs'
The way authentication related data other than passwords (e.g.
oath token and password expiration data) are stored in libsecret
keyrings has been rethought.

* mh/credential-libsecret-attrs:
  credential/libsecret: store new attributes
2023-08-28 09:51:16 -07:00
Derrick Stolee
4527db8ff8 scalar: add --[no-]src option
Some users have strong aversions to Scalar's opinion that the repository
should be in a 'src' directory, even though this creates a clean slate
for placing build artifacts in adjacent directories.

The new --no-src option allows users to opt out of the default behavior.

While adding options, make sure the usage output by 'scalar clone -h'
reports the same as the SYNOPSIS line in Documentation/scalar.txt.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-28 09:16:06 -07:00
Junio C Hamano
23013a49c8 Merge branch 'ob/t9001-indent-fix'
Test style fix.

* ob/t9001-indent-fix:
  t9001: fix indentation in test_no_confirm()
2023-08-25 10:37:37 -07:00
Junio C Hamano
6d159f5757 Merge branch 'rs/parse-options-negation-help'
"git cmd -h" learned to signal which options can be negated by
listing such options like "--[no-]opt".

* rs/parse-options-negation-help:
  parse-options: simplify usage_padding()
  parse-options: no --[no-]no-...
  parse-options: factor out usage_indent() and usage_padding()
  parse-options: show negatability of options in short help
  t1502: test option negation
  t1502: move optionspec help output to a file
  t1502, docs: disallow --no-help
  subtree: disallow --no-{help,quiet,debug,branch,message}
2023-08-25 10:37:37 -07:00
Junio C Hamano
c7b6a6c0be Merge branch 'ds/maintenance-schedule-fuzz'
Hourly and other schedule of "git maintenance" jobs are randomly
distributed now.

* ds/maintenance-schedule-fuzz:
  maintenance: update schedule before config
  maintenance: fix systemd schedule overlaps
  maintenance: use random minute in systemd scheduler
  maintenance: swap method locations
  maintenance: use random minute in cron scheduler
  maintenance: use random minute in Windows scheduler
  maintenance: use random minute in launchctl scheduler
  maintenance: add get_random_minute()
2023-08-24 09:32:34 -07:00
Junio C Hamano
004a383091 Merge branch 'ob/test-lib-rebase-fake-editor-updates'
Test updates.

* ob/test-lib-rebase-fake-editor-updates:
  t/lib-rebase: improve documentation of set_fake_editor()
  t/lib-rebase: set_fake_editor(): handle FAKE_LINES more consistently
  t/lib-rebase: set_fake_editor(): fix recognition of reset's short command
2023-08-24 09:32:34 -07:00
Junio C Hamano
aaf0a421e2 Merge branch 'mp/rebase-label-length-limit'
Overly long label names used in the sequencer machinery are now
chopped to fit under filesystem limitation.

* mp/rebase-label-length-limit:
  rebase: allow overriding the maximal length of the generated labels
  sequencer: truncate labels to accommodate loose refs
2023-08-24 09:32:33 -07:00
Junio C Hamano
f5f23a430f Merge branch 'rj/branch-in-use-error-message'
A message written in olden time prevented a branch from getting
checked out saying it is already checked out elsewhere, but these
days, we treat a branch that is being bisected or rebased just like
a branch that is checked out and protect it.  Rephrase the message
to say that the branch is in use.

* rj/branch-in-use-error-message:
  branch: error message checking out a branch in use
  branch: error message deleting a branch in use
2023-08-24 09:32:33 -07:00
Christian Hesse
d0fc552bfc t/t6300: drop magic filtering
Now that we ran a trustdb check forcibly, it no longer pollutes the
output, and filtering is no longer required.

Signed-off-by: Christian Hesse <mail@eworm.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-23 09:13:17 -07:00
Junio C Hamano
f3d33f8cfe transfer.unpackLimit: fetch/receive.unpackLimit takes precedence
The transfer.unpackLimit configuration variable is documented to be
used only as a fallback value when the more operation-specific
fetch.unpackLimit and receive.unpackLimit variables are not set, but
the implementation had the precedence reversed.  Apparently this was
broken since the transfer.unpackLimit was introduced in e28714c5
(Consolidate {receive,fetch}.unpackLimit, 2007-01-24).

Often when documentation and code have diverged for so long, we
prefer to change the documentation instead, to avoid disrupting
users.  But doing so would make these weirdly unlike most other
"specific overrides general" config options. And the fact that the
bug has existed for so long without anyone noticing implies to me
that nobody really tries to mix and match them much.

Signed-off-by: Taylor Santiago <taylorsantiago@google.com>
[jc: rewrote the log message, added tests, covered receive-pack as well]
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-22 18:30:49 -07:00
Christian Hesse
031fff289a t/lib-gpg: forcibly run a trustdb update
We want to compare output later, so randomly popping up 'gpg: checking
the trustdb' breaks the tests. Run the trustdb update forcibly.

Signed-off-by: Christian Hesse <mail@eworm.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-22 09:29:06 -07:00
Junio C Hamano
a64f8b2595 diff: the -w option breaks --exit-code for --raw and other output modes
The output from "--raw", "--name-status", and "--name-only" modes in
"git diff" does depend on and does not reflect how certain different
contents are considered equal, unlike "--patch" and "--stat" output
modes do, when used with options like "-w" (another way of thinking
about it is that it is not like we recompute the hash of the blob
after removing all whitespaces to show "git diff --raw -w" output).

But the fact that "--raw" and friends ignore "-w" is not a good
excuse for "diff --raw -w --exit-code" to also ignore the request to
report the differences with its exit status.  When run without "-w",
"git diff --exit-code --raw" does report with its exit status the
differences as requested, and we should do the same when run with
"-w", too.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-21 18:56:03 -07:00
Taylor Blau
db6044d762 commit-graph: avoid repeated mixed generation number warnings
When validating that a commit-graph has either all zero, or all non-zero
generation numbers, we emit a warning on both the rising and falling
edge of transitioning between the two.

So if we are unfortunate enough to see a commit-graph which has a
repeating sequence of zero, then non-zero generation numbers, we'll
generate many warnings that contain more or less the same information.

Avoid this by keeping track of a single example for a commit with zero-
and non-zero generation, and emit a single warning at the end of
verification if both are non-NULL.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-21 16:16:23 -07:00
Taylor Blau
ce7629a315 t/t5318-commit-graph.sh: test generation zero transitions during fsck
The second test called "detect incorrect generation number" asserts that
we correctly warn during an fsck when we see a non-zero generation
number after seeing a zero beforehand.

The other transition (going from non-zero to zero) was previously
untested. Test both directions, and rename the existing test to make
clear which direction it is exercising.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-21 16:07:34 -07:00
Taylor Blau
868c991155 commit-graph: introduce commit_graph_generation_from_graph()
In 2ee11f7261 (commit-graph: return generation from memory, 2023-03-20),
the `commit_graph_generation()` function stopped returning zeros when
asked to locate the generation number of a given commit.

This was done at the time to prepare for a later change which set
generation values in memory, meaning that we could no longer rely on
`graph_pos` alone to tell us whether or not to trust the generation
number returned by this function.

In 2ee11f7261, it was noted that this change only impacted very old
commit-graphs, which were written with all commits having generation
number 0. Indeed, zero is not a valid generation number, so we should
never expect to see that value outside of the aforementioned case.

The test fallout in 2ee11f7261 indicated that we were no longer able to
fsck a specific old case of commit-graph corruption, where we see a
non-zero generation number after having seen a generation number of 0
earlier.

Introduce a variant of `commit_graph_generation()` which behaves like
that function did prior to 2ee11f7261, known as
`commit_graph_generation_from_graph()`. Then use this function in the
context of `verify_one_commit_graph()`, where we only want to trust the
values from the graph.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-21 16:07:33 -07:00
Jeff King
5ad6e2b495 diff: show usage for unknown builtin_diff_files() options
The git-diff command has many modes (comparing worktree to index, index
to HEAD, individual blobs, etc). As a result, it dispatches to many
helper functions and cannot completely parse its options until we're in
those helper functions.

Most of them, when seeing an unknown option, exit immediately by calling
usage(). But builtin_diff_files(), which is the default if no revision
or blob arguments are given, instead prints an error() and returns -1.

One obvious shortcoming here is that the user doesn't get to see the
usual usage message. But there's a much more important bug: the -1
return is fed to diff_result_code(), which is not ready to handle it.
By default, it passes the code along as an exit code. We try to avoid
negative exit codes because they get converted to unsigned values, but
it should at least consistently show up as non-zero (i.e., a failure).

But much worse is that when --exit-code is in effect, diff_result_code()
will _ignore_ the status passed in by the caller, and instead only
report on whether the diff found changes. It didn't, of course, because
we never ran the diff, and the program unexpectedly exits with success!

We can fix this bug by just calling usage(), like the other helpers do.
Another option would of course be to teach diff_result_code() to handle
this value. But as we'll see in the next few patches, it can be cleaned
up even further. Let's just fix this bug directly to start with.

Reported-by: Romain Chossart <romainchossart@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-21 15:33:24 -07:00
Oswald Buddenhagen
67f4b36e33 format-patch: add --description-file option
This patch makes it possible to directly feed a branch description to
derive the cover letter from. The use case is formatting dynamically
created temporary commits which are not referenced anywhere.

The most obvious alternative would be creating a temporary branch and
setting a description on it, but that doesn't seem particularly elegant.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-21 15:03:47 -07:00
Junio C Hamano
5626558e63 t4040: remove test that succeeded for a wrong reason
"diff-tree -b --exit-code" without "--patch" exits with 0 status,
not because it finds that the two input files are equivalent while
ignoring whitespaces, but because the implied "--raw" mode always
exits with 0 when whitespace tweaking options like "-b" and "-w"
are given, which is a long-standing bug.

We are about to fix it so that "--raw" and friends report the
differences with the exit status (even though they ignore the
whitespace tweaking options when producing their output), which
will make this test, which succeeded for a wrong reason, start
failing.  Remove it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-18 17:01:12 -07:00
Junio C Hamano
e8efd86369 diff: teach "--stat -w --exit-code" to notice differences
When options like "-w" is used while "--exit-code" option is in
effect, instead of the usual "do we have any filepair whose preimage
and postimage have different <mode,object>?" check, we need to compare
the contents of the blobs, taking into account that certain changes
are considered no-op.

With the previous step, we taught "--patch" codepath to set the
.found_changes bit correctly, even for a change that only affects
the mode and not object.  The "--stat" codepath, however, did not
set the .found_changes bit at all.  This lead to

    $ git diff --stat -w --exit-code

for a change that does have an output to exit with status 0.

Set the bit by inspecting the list of paths the diffstat output is
given for (a mode-only change will still appear as a "0-line added
0-line deleted" change) to fix it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-18 17:01:11 -07:00
Junio C Hamano
c9a3e724cf diff: mode-only change should be noticed by "--patch -w --exit-code"
The codepath to notice the content-level changes, taking certain
no-op changes like "ignore whitespace" into account, forgot that
a mode-only change is still a change.  This resulted in

    $ git diff --patch --exit-code -w

to exit with status 0 even when there is such a mode-only change,
breaking both "--patch" and "--quiet" output formats.

Teach the builtin_diff() codepath that creation and deletion as well
as mode changes are all interesting changes.

Note that the test specifically checks removal of an empty file,
because if there is anything in the preimage (i.e. the removed file
is not empty), the removal would still trigger textual patch output
and the codepath for that does update .found_changes bit to report
that it found an interesting change.  We need to make sure that the
.found_changes bit is set even without triggering textual patch
output.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-18 17:01:11 -07:00
Junio C Hamano
f9fe84b5a2 Merge branch 'pw/diff-no-index-from-named-pipes'
Test updates.

* pw/diff-no-index-from-named-pipes:
  t4053: avoid writing to unopened pipe
  t4053: avoid race when killing background processes
2023-08-15 10:19:47 -07:00
Junio C Hamano
8e12aaa7ce Merge branch 'st/mv-lstat-fix'
Correct use of lstat() that assumed a failing call would not
clobber the statbuf.

* st/mv-lstat-fix:
  mv: handle lstat() failure correctly
2023-08-15 10:19:47 -07:00
Junio C Hamano
cecd6a5ffc Merge branch 'jc/send-email-pre-process-fix'
Test fix.

* jc/send-email-pre-process-fix:
  t9001: remove excessive GIT_SEND_EMAIL_NOTTY=1
2023-08-15 10:19:47 -07:00
Junio C Hamano
fc6bba66bc Merge branch 'js/allow-t4000-to-be-indented-with-spaces'
File attribute update.

* js/allow-t4000-to-be-indented-with-spaces:
  t0040: declare non-tab indentation to be okay in this script
2023-08-14 13:26:41 -07:00
Junio C Hamano
fc71d024ad Merge branch 'jk/send-email-with-new-readline'
Adjust to newer Term::ReadLine to prevent it from breaking
the interactive prompt code in send-email.

* jk/send-email-with-new-readline:
  send-email: avoid creating more than one Term::ReadLine object
  send-email: drop FakeTerm hack
2023-08-14 13:26:41 -07:00
Oswald Buddenhagen
b46d806ea5 t9001: fix indentation in test_no_confirm()
The continuations of the compound command were indented as if they were
continuations of the embedded pipe, which was misleading.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-13 16:32:28 -07:00
Jeff King
e5cb1e3f09 t4053: avoid writing to unopened pipe
This fixes an occasional hang I see when running t4053 with
--verbose-log using dash.

Commit 1e3f26542a (diff --no-index: support reading from named pipes,
2023-07-05) added a test that "diff --no-index" will complain when
comparing a named pipe and a directory. The minimum we need to test this
is to mkfifo the pipe, and then run "git diff --no-index pipe some_dir".
But the test does one thing more: it spawns a background shell process
that opens the pipe for writing, like this:

        {
                (>pipe) &
        } &&

This extra writer _could_ be useful if Git misbehaves and tries to open
the pipe for reading. Without the writer, Git would block indefinitely
and the test would never end. But since we do not have such a bug, Git
does not open the pipe and it is the writing process which will block
indefinitely, since there are no readers. The test addresses this by
running "kill $!" in a test_when_finished block. Since the writer should
be blocking forever, this kill command will reliably find it waiting.

However, this seems to be somewhat racy, in that the writing process
sometimes hangs around even after the "kill". In a normal run of the
test script without options, this doesn't have any effect; the
main test script completes anyway. But with --verbose-log, we spawn a
"tee" process that reads the script output, and it won't end until all
descriptors pointing to its input pipe are closed. And the background
process that is hanging around still has its stderr, etc, pointed into
that pipe.

You can reproduce the situation like this:

  cd t
  ./t4053-diff-no-index.sh --verbose-log --stress

Let that run for a few minutes, and then you'll find that some of the
runs have hung. For example, at 11:53, I ran:

  $ ps xk start o pid,start,command | grep tee | head
   713459 11:48:06 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-9.out
   713527 11:48:06 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-15.out
   719434 11:48:07 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-1.out
   728117 11:48:08 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-5.out
   738738 11:48:09 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-31.out
   739457 11:48:09 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-27.out
   744432 11:48:10 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-21.out
   744471 11:48:10 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-29.out
   761961 11:48:12 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-0.out
   812299 11:48:19 tee -a /home/peff/compile/git/t/test-results/t4053-diff-no-index.stress-8.out

All of these have been hung for several minutes. We can investigate one
and see that it's waiting to get EOF on its input:

  $ strace -p 713459
  strace: Process 713459 attached
  read(0,
  ^C

Who else has that descriptor open?

  $ lsof -a -p 713459 -d 0 +E
  COMMAND    PID USER   FD   TYPE DEVICE SIZE/OFF    NODE NAME
  tee     713459 peff    0r  FIFO   0,13      0t0 3943636 pipe 719203,sh,5w 719203,sh,7w 719203,sh,12w 719203,sh,13w
  sh      719203 peff    5w  FIFO   0,13      0t0 3943636 pipe 713459,tee,0r 719203,sh,7w 719203,sh,12w 719203,sh,13w
  sh      719203 peff    7w  FIFO   0,13      0t0 3943636 pipe 713459,tee,0r 719203,sh,5w 719203,sh,12w 719203,sh,13w
  sh      719203 peff   12w  FIFO   0,13      0t0 3943636 pipe 713459,tee,0r 719203,sh,5w 719203,sh,7w 719203,sh,13w
  sh      719203 peff   13w  FIFO   0,13      0t0 3943636 pipe 713459,tee,0r 719203,sh,5w 719203,sh,7w 719203,sh,12w

It's a shell, presumably a subshell spawned by the main script. Though
it may seem odd, having the same descriptor open several times is not
unreasonable (they're all basically the original stdout/stderr of the
script that has been copied). And they should all close when the process
exits. So what's it doing? Curiously, it will exit as soon as we strace
it:

  $ strace -s 64 -p 719203
  strace: Process 719203 attached
  openat(AT_FDCWD, "pipe", O_WRONLY|O_CREAT|O_TRUNC, 0666) = -1 ENOENT (No such file or directory)
  write(2, "./t4053-diff-no-index.sh: 7: eval: ", 35) = 35
  write(2, "cannot create pipe: Directory nonexistent", 41) = 41
  write(2, "\n", 1)                       = 1
  exit_group(2)                           = ?
  +++ exited with 2 +++

I think what happens is this:

  - it is blocking in the openat() call for the pipe, as we expect (so
    this is definitely the backgrounded subshell mentioned above)

  - strace sends signals (probably STOP/CONT); those cause the kernel to
    stop blocking, but libc will restart the system call automatically

  - by this time, the "pipe" fifo is gone, so we'll actually try to
    create a regular file. But of course the surrounding directory is
    gone, too! So we get ENOENT, and then exit as normal.

So the blocking is something we expect to happen. But what we didn't
expect is for the process to still exist at all! It should have been
killed earlier when the parent process called "kill", but it wasn't. And
we can't catch the race at this point, because it happened much earlier.

One can guess, though, that there is some race with the shell setting up
the signal handling in the backgrounded subshell, and possibly blocking
or ignoring signals at the time that the "kill" is received.  Curiously,
the race does not seem to happen if I use "bash" instead of "dash", so
presumably bash's setup here is more atomic.

One fix might be to try killing the subshell more aggressively, either
using SIGKILL, or looping on kill/wait. But that seems complex and
likely to introduce new problems/races. Instead, we can observe that the
writer is not needed at all. Git will notice the pipe via stat() before
it is ever opened. So we can simply drop the writer subshell entirely.

If we ever changed Git to open the path and fstat() it, this would
result in the test hanging. But we're not likely to do that. After all,
we have to stat() paths to see if they are openable at all (e.g., it
could be a directory), so this seems like a low risk. And anybody who
does make such a change will immediately see the issue, as Git would
hang consistently.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-13 16:30:36 -07:00
Shuqi Liang
f9815878c1 check-attr: integrate with sparse-index
Set the requires-full-index to false for "check-attr".

Add a test to ensure that the index is not expanded whether the files
are outside or inside the sparse-checkout cone when the sparse index is
enabled.

The `p2000` tests demonstrate a ~63% execution time reduction for
'git check-attr' using a sparse index.

Test                                            before  after
-----------------------------------------------------------------------
2000.106: git check-attr -a f2/f4/a (full-v3)    0.05   0.05 +0.0%
2000.107: git check-attr -a f2/f4/a (full-v4)    0.05   0.05 +0.0%
2000.108: git check-attr -a f2/f4/a (sparse-v3)  0.04   0.02 -50.0%
2000.109: git check-attr -a f2/f4/a (sparse-v4)  0.04   0.01 -75.0%

Helped-by: Victoria Dye <vdye@github.com>
Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-11 09:44:52 -07:00
Shuqi Liang
4723ae1007 attr.c: read attributes in a sparse directory
Before this patch, git check-attr was unable to read the attributes from
a .gitattributes file within a sparse directory. The original comment
was operating under the assumption that users are only interested in
files or directories inside the cones. Therefore, in the original code,
in the case of a cone-mode sparse-checkout, we didn't load the
.gitattributes file.

However, this behavior can lead to missing attributes for files inside
sparse directories, causing inconsistencies in file handling.

To resolve this, revise 'git check-attr' to allow attribute reading for
files in sparse directories from the corresponding .gitattributes files:

1.Utilize path_in_cone_mode_sparse_checkout() and index_name_pos_sparse
to check if a path falls within a sparse directory.

2.If path is inside a sparse directory, employ the value of
index_name_pos_sparse() to find the sparse directory containing path and
path relative to sparse directory. Proceed to read attributes from the
tree OID of the sparse directory using read_attr_from_blob().

3.If path is not inside a sparse directory,ensure that attributes are
fetched from the index blob with read_blob_data_from_index().

Change the test 'check-attr with pathspec outside sparse definition' to
'test_expect_success' to reflect that the attributes inside a sparse
directory can now be read. Ensure that the sparse index case works
correctly for git check-attr to illustrate the successful handling of
attributes within sparse directories.

Helped-by: Victoria Dye <vdye@github.com>
Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-11 09:44:51 -07:00
Shuqi Liang
fd4faf7a5d t1092: add tests for 'git check-attr'
Add tests for `git check-attr`, make sure attribute file does get read
from index when path is either inside or outside of sparse-checkout
definition.

Add a test named 'diff --check with pathspec outside sparse definition'.
It starts by disabling the trailing whitespace and space-before-tab
checks using the core. whitespace configuration option. Then, it
specifically re-enables the trailing whitespace check for a file located
in a sparse directory by adding a whitespace=trailing-space rule to the
.gitattributes file within that directory. Next, create and populate the
folder1 directory, and then add the .gitattributes file to the index.
Edit the contents of folder1/a, add it to the index, and proceed to
"re-sparsify" 'folder1/' with 'git sparse-checkout reapply'. Finally,
use 'git diff --check --cached' to compare the 'index vs. HEAD',
ensuring the correct application of the attribute rules even when the
file's path is outside the sparse-checkout definition.

Mark the two tests 'check-attr with pathspec outside sparse definition'
and 'diff --check with pathspec outside sparse definition' as
'test_expect_failure' to reflect an existing issue where the attributes
inside a sparse directory are ignored. Ensure that the 'check-attr'
command fails to read the required attributes to demonstrate this
expected failure.

Helped-by: Victoria Dye <vdye@github.com>
Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-11 09:44:51 -07:00
Derrick Stolee
69ecfcacfd maintenance: update schedule before config
When running 'git maintenance start', the current pattern is to
configure global config settings to enable maintenance on the current
repository and set 'maintenance.auto' to false and _then_ to set up the
schedule with the system scheduler.

This has a problematic error condition: if the scheduler fails to
initialize, the repository still will not use automatic maintenance due
to the 'maintenance.auto' setting.

Fix this gap by swapping the order of operations. If Git fails to
initialize maintenance, then the config changes should never happen.

Reported-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-10 14:04:17 -07:00
Derrick Stolee
daa787010c maintenance: use random minute in systemd scheduler
The get_random_minute() method was created to allow maintenance
schedules to be fixed to a random minute of the hour. This randomness is
only intended to spread out the load from a number of clients, but each
client should have an hour between each maintenance cycle.

Add this random minute to the systemd integration.

This integration is more complicated than similar changes for other
schedulers because of a neat trick that systemd allows: templating.

The previous implementation generated two template files with names
of the form 'git-maintenance@.(timer|service)'. The '.timer' or
'.service' indicates that this is a template that is picked up when we
later specify '...@<schedule>.timer' or '...@<schedule>.service'. The
'<schedule>' string is then used to insert into the template both the
'OnCalendar' schedule setting and the '--schedule' parameter of the
'git maintenance run' command.

In order to set these schedules to a given minute, we can no longer use
the 'hourly', 'daily', or 'weekly' strings for '<schedule>' and instead
need to abandon the template model for the .timer files. We can still
use templates for the .service files. For this reason, we split these
writes into two methods.

Modify the template with a custom schedule in the 'OnCalendar' setting.
This schedule has some interesting differences from cron-like patterns,
but is relatively easy to figure out from context. The one that might be
confusing is that '*-*-*' is a date-based pattern, but this must be
omitted when using 'Mon' to signal that we care about the day of the
week. Monday is used since that matches the day used for the 'weekly'
schedule used previously.

Now that the timer files are not templates, we might want to abandon the
'@' symbol in the file names. However, this would cause users with
existing schedules to get two competing schedules due to different
names. The work to remove the old schedule name is one thing that we can
avoid by keeping the '@' symbol in our unit names. Since we are locked
into this name, it makes sense that we keep the template model for the
.service files.

The rest of the change involves making sure we are writing these .timer
and .service files before initializing the schedule with 'systemctl' and
deleting the files when we are done. Some changes are also made to share
the random minute along with a single computation of the execution path
of the current Git executable.

In addition, older Git versions may have written a
'git-maintenance@.timer' template file. Be sure to remove this when
successfully enabling maintenance (or disabling maintenance).

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-10 14:04:16 -07:00
Johannes Schindelin
ac300bda10 rebase: allow overriding the maximal length of the generated labels
With this change, users can override the compiled-in default for the
maximal length of the label names generated by `git rebase
--rebase-merges`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Mark Ruvald Pedersen <mped@demant.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-10 10:12:31 -07:00
Oswald Buddenhagen
20a0bd45fa t/lib-rebase: improve documentation of set_fake_editor()
Firstly, make it reflect better what actually happens. Not omitting some
possibilities makes it easier to fully exploit them, and not
contradicting the implementation makes it easier to grok and thus modify
the code.

Secondly, improve the overall structure, putting more general info
further up.

Thirdly, document `merge`, `fakesha`, and `break`, which were previously
omitted entirely.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-10 09:18:07 -07:00
Phillip Wood
231e86c10c t4053: avoid race when killing background processes
The test 'diff --no-index reads from pipes' starts a couple of
background processes that write to the pipes that are passed to "diff
--no-index". If the test passes then we expect these processes to exit
as all their output will have been read. However if the test fails
then we want to make sure they do not hang about on the users machine
and the test remembers they should be killed by calling

      test_when_finished  "! kill $!"

after each background process is created. Unfortunately there is a
race where test_when_finished may run before the background process
exits even when all its output has been read resulting in the kill
command succeeding which causes the test to fail. Fix this by ignoring
the exit status of the kill command. If the diff is successful we
could instead wait for the background process to exit and check their
status but that feels like it is testing the platform's printf
implementation rather than git's code.

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-10 09:16:27 -07:00
Junio C Hamano
e8c53ff912 Merge branch 'pw/rebase-skip-commit-message-fix'
"git rebase -i" with a series of squash/fixup, when one of the
steps stopped in conflicts and ended up getting skipped, did not
handle the accumulated commit log messages, which has been
corrected.

* pw/rebase-skip-commit-message-fix:
  rebase --skip: fix commit message clean up when skipping squash
2023-08-09 16:18:16 -07:00
Junio C Hamano
cf07e53bae Merge branch 'bc/ident-dot-is-no-longer-crud-letter'
Exclude "." from the set of characters to be removed from the
beginning and the end of the human-readable name.

* bc/ident-dot-is-no-longer-crud-letter:
  ident: don't consider '.' a crud
2023-08-09 16:18:15 -07:00
Oswald Buddenhagen
b3dcd24b8a t9001: remove excessive GIT_SEND_EMAIL_NOTTY=1
This was added by 3ece9bf0f9 (send-email: clear the $message_id after
validation, 2023-05-17) for no apparent reason, as this is required only
in cases when git's stdin is (must be) redirected, which isn't the case
here.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-09 12:44:07 -07:00