Commit Graph

22502 Commits

Author SHA1 Message Date
Junio C Hamano
712d970c01 Merge branch 'jk/apply-patch-mode-check-fix' into maint-2.46
Test fix.

* jk/apply-patch-mode-check-fix:
  t4129: fix racy index when calling chmod after git-add
  apply: canonicalize modes read from patches
2024-09-12 11:02:15 -07:00
Junio C Hamano
5072ad8260 Merge branch 'xx/diff-tree-remerge-diff-fix' into maint-2.46
"git rev-list ... | git diff-tree -p --remerge-diff --stdin" should
behave more or less like "git log -p --remerge-diff" but instead it
crashed, forgetting to prepare a temporary object store needed.

* xx/diff-tree-remerge-diff-fix:
  diff-tree: fix crash when used with --remerge-diff
2024-08-26 11:10:25 -07:00
Junio C Hamano
164cffa35c Merge branch 'rs/t-example-simplify' into maint-2.46
Unit test simplification.

* rs/t-example-simplify:
  t-example-decorate: remove test messages
2024-08-26 11:10:24 -07:00
Junio C Hamano
c93649f98a Merge branch 'jc/safe-directory' into maint-2.46
Follow-up on 2.45.1 regression fix.

* jc/safe-directory:
  safe.directory: setting safe.directory="." allows the "current" directory
  safe.directory: normalize the configured path
  safe.directory: normalize the checked path
  safe.directory: preliminary clean-up
2024-08-26 11:10:24 -07:00
Junio C Hamano
24a64ea0eb Merge branch 'kl/test-fixes' into maint-2.46
A flakey test and incorrect calls to strtoX() functions have been
fixed.

* kl/test-fixes:
  t6421: fix test to work when repo dir contains d0
  set errno=0 before strtoX calls
2024-08-26 11:10:21 -07:00
Junio C Hamano
710ef8a945 Merge branch 'jc/reflog-expire-lookup-commit-fix' into maint-2.46
"git reflog expire" failed to honor annotated tags when computing
reachable commits.

* jc/reflog-expire-lookup-commit-fix:
  Revert "reflog expire: don't use lookup_commit_reference_gently()"
2024-08-26 11:10:21 -07:00
Junio C Hamano
528a762ca6 Merge branch 'jc/leakfix-mailmap' into maint-2.46
Leakfix.

* jc/leakfix-mailmap:
  mailmap: plug memory leak in read_mailmap_blob()
2024-08-26 11:10:20 -07:00
Junio C Hamano
88639e5d4c Merge branch 'jc/leakfix-hashfile' into maint-2.46
Leakfix.

* jc/leakfix-hashfile:
  csum-file: introduce discard_hashfile()
2024-08-26 11:10:19 -07:00
Junio C Hamano
a5e4f53baf Merge branch 'jc/jl-git-no-advice-fix' into maint-2.46
Remove leftover debugging cruft from a test script.

* jc/jl-git-no-advice-fix:
  t0018: remove leftover debugging cruft
2024-08-26 11:10:19 -07:00
Junio C Hamano
5613c83f30 Merge branch 'tb/config-fixed-value-with-valueless-true' into maint-2.46
"git config --value=foo --fixed-value section.key newvalue" barfed
when the existing value in the configuration file used the
valueless true syntax, which has been corrected.

* tb/config-fixed-value-with-valueless-true:
  config.c: avoid segfault with --fixed-value and valueless config
2024-08-26 11:10:18 -07:00
Junio C Hamano
a991ffff92 Merge branch 'ps/ls-remote-out-of-repo-fix' into maint-2.46
A recent update broke "git ls-remote" used outside a repository,
which has been corrected.

* ps/ls-remote-out-of-repo-fix:
  builtin/ls-remote: fall back to SHA1 outside of a repo
2024-08-26 11:10:18 -07:00
Junio C Hamano
d07bb0cd2a Merge branch 'ps/p4-tests-updates' into maint-2.46
Perforce tests have been updated.
cf. <na5mwletzpnacietbc7pzqcgb622mvrwgrkjgjosysz3gvjcso@gzxxi7d7icr7>

* ps/p4-tests-updates:
  t98xx: mark Perforce tests as memory-leak free
  ci: update Perforce version to r23.2
  t98xx: fix Perforce tests with p4d r23 and newer
2024-08-16 12:50:56 -07:00
Junio C Hamano
e6698fbfa9 Merge branch 'ks/unit-test-comment-typofix' into maint-2.46
Typofix.

* ks/unit-test-comment-typofix:
  unit-tests/test-lib: fix typo in check_pointer_eq() description
2024-08-16 12:50:56 -07:00
Junio C Hamano
c09721cb63 Merge branch 'dd/notes-empty-no-edit-by-default' into maint-2.46
"git notes add -m '' --allow-empty" and friends that take prepared
data to create notes should not invoke an editor, but it started
doing so since Git 2.42, which has been corrected.

* dd/notes-empty-no-edit-by-default:
  notes: do not trigger editor when adding an empty note
2024-08-16 12:50:55 -07:00
Junio C Hamano
72a50fa03b Merge branch 'pw/add-patch-with-suppress-blank-empty' into maint-2.46
"git add -p" by users with diff.suppressBlankEmpty set to true
failed to parse the patch that represents an unmodified empty line
with an empty line (not a line with a single space on it), which
has been corrected.

* pw/add-patch-with-suppress-blank-empty:
  add-patch: use normalize_marker() when recounting edited hunk
  add-patch: handle splitting hunks with diff.suppressBlankEmpty
2024-08-16 12:50:54 -07:00
Junio C Hamano
bb250b5378 Merge branch 'jc/checkout-no-op-switch-errors' into maint-2.46
"git checkout --ours" (no other arguments) complained that the
option is incompatible with branch switching, which is technically
correct, but found confusing by some users.  It now says that the
user needs to give pathspec to specify what paths to checkout.

* jc/checkout-no-op-switch-errors:
  checkout: special case error messages during noop switching
2024-08-16 12:50:51 -07:00
Jeff King
49e5cc5b26 t4129: fix racy index when calling chmod after git-add
This patch fixes a racy test failure in t4129.

The deletion test added by e95d515141 (apply: canonicalize modes read
from patches, 2024-08-05) wants to make sure that git-apply does not
complain about a non-canonical mode in the patch, even if that mode does
not match the working tree file. So it does this:

	echo content >non-canon &&
	git add non-canon &&
	chmod 666 non-canon &&

This is wrong, because running chmod will update the ctime on the file,
making it stat-dirty and causing git-apply to refuse to apply the patch.
But this only happens sometimes, since it depends on the timestamps
crossing a second boundary (but it triggers pretty quickly when run with
--stress).

We can fix this by doing the chmod before updating the index. The order
isn't important here, as the mode will be canonicalized to 100644 in the
index anyway (in fact, the chmod is not even that important in the first
place, since git-apply will only look at the index; I only added it as
an extra confirmation that git-apply would not be confused by it).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-15 09:41:11 -07:00
Xing Xin
a77554ea09 diff-tree: fix crash when used with --remerge-diff
When using "git-diff-tree" to get the tree diff for merge commits with
the diff format set to `remerge`, a bug is triggered as shown below:

  $ git diff-tree -r --remerge-diff 363337e6eb
  363337e6eb
  BUG: log-tree.c:1006: did a remerge diff without remerge_objdir?!?

This bug is reported by `log-tree.c:do_remerge_diff`, where a bug check
added in commit 7b90ab467a (log: clean unneeded objects during log
--remerge-diff, 2022-02-02) detects the absence of `remerge_objdir` when
attempting to clean up temporary objects generated during the remerge
process.

After some further digging, I find that the remerge-related diff options
were introduced in db757e8b8d (show, log: provide a --remerge-diff
capability, 2022-02-02), which also affect the setup of `rev_info` for
"git-diff-tree", but were not accounted for in the original
implementation (inferred from the commit message).

Elijah Newren, the author of the remerge diff feature, notes that other
callers of `log-tree.c:log_tree_commit` (the only caller of
`log-tree.c:do_remerge_diff`) also exist, but:

  `builtin/am.c`: manually sets all flags; remerge_diff is not among them
  `sequencer.c`: manually sets all flags; remerge_diff is not among them

so `builtin/diff-tree.c` really is the only caller that was overlooked
when remerge-diff functionality was added.

This commit resolves the crash by adding `remerge_objdir` setup logic to
`builtin/diff-tree.c`, mirroring `builtin/log.c:cmd_log_walk_no_free`.
It also includes the necessary cleanup for `remerge_objdir`.

Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Xing Xin <xingxin.xx@bytedance.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-09 08:07:44 -07:00
Kyle Lippincott
ec60bb9fc4 t6421: fix test to work when repo dir contains d0
The `grep` statement in this test looks for `d0.*<string>`, attempting
to filter to only show lines that had tabular output where the 2nd
column had `d0` and the final column had a substring of
[`git -c `]`fetch.negotiationAlgorithm`. These lines also have
`child_start` in the 4th column, but this isn't part of the condition.

A subsequent line will have `d1` in the 2nd column, `start` in the 4th
column, and `/path/to/git/git -c fetch.negotiationAlgorihm` in the final
column. If `/path/to/git/git` contains the substring `d0`, then this
line is included by `grep` as well as the desired line, leading to an
effective doubling of the number of lines, and test failures.

Tighten the grep expression to require `d0` to be surrounded by spaces,
and to have the `child_start` label.

Signed-off-by: Kyle Lippincott <spectral@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-05 10:59:21 -07:00
Kyle Lippincott
b928d57ca9 set errno=0 before strtoX calls
To detect conversion failure after calls to functions like `strtod`, one
can check `errno == ERANGE`. These functions are not guaranteed to set
`errno` to `0` on successful conversion, however. Manual manipulation of
`errno` can likely be avoided by checking that the output pointer
differs from the input pointer, but that's not how other locations, such
as parse.c:139, handle this issue; they set errno to 0 prior to
executing the function.

For every place I could find a strtoX function with an ERANGE check
following it, set `errno = 0;` prior to executing the conversion
function.

Signed-off-by: Kyle Lippincott <spectral@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-05 10:59:20 -07:00
Jeff King
e95d515141 apply: canonicalize modes read from patches
Git stores only canonical modes for blobs. So for a regular file, we
care about only "100644" or "100755" (depending only on the executable
bit), but never modes where the group or other permissions are more
exotic. So never "100664", "100700", etc. When a file in the working
tree has such a mode, we quietly turn it into one of the two canonical
modes, and that's what is stored both in the index and in tree objects.

However, we don't canonicalize modes we read from incoming patches in
git-apply. These may appear in a few lines:

  - "old mode" / "new mode" lines for mode changes

  - "new file mode" lines for newly created files

  - "deleted file mode" for removing files

For "new mode" and for "new file mode", this is harmless. The patch is
asking the result to have a certain mode, but:

  - when we add an index entry (for --index or --cached), it is
    canonicalized as we create the entry, via create_ce_mode().

  - for a working tree file, try_create_file() passes either 0777 or
    0666 to open(), so what you get depends only on your umask, not any
    other bits (aside from the executable bit) in the original mode.

However, for "old mode" and "deleted file mode", there is a minor
annoyance. We compare the patch's expected preimage mode with the
current state. But that current state is always going to be a canonical
mode itself:

  - updating an index entry via --cached will have the canonical mode in
    the index

  - for updating a working tree file, check_preimage() runs the mode
    through ce_mode_from_stat(), which does the usual canonicalization

So if the patch feeds a non-canonical mode, it's impossible for it to
match, and we will always complain with something like:

  file has type 100644, expected 100664

Since this is just a warning, the operation proceeds, but it's
confusing and annoying.

These cases should be pretty rare in practice. Git would never produce a
patch with non-canonical modes itself (since it doesn't store them).
And while we do accept patches from other programs, all of those lines
were invented by Git. So you'd need a program trying to be Git
compatible, but not handling canonicalization the same way. Reportedly
"quilt" is such a program.

We should canonicalize the modes as we read them so that the user never
sees the useless warning.

A few notes on the tests:

  - I've covered instances of all lines for completeness, even though
    the "new mode" / "new file mode" ones behave OK currently.

  - the tests apply patches to both the index and working tree, and
    check the result of both. Again, we know that all of these paths
    canonicalize anyway, but it's giving us extra coverage (although we
    are even less likely to have such a bug now since we canonicalize up
    front).

  - the test patches are missing "index" lines, which is also something
    Git would never produce. But they don't matter for the test, they do
    match the case from quilt we saw in the wild, and they avoid some
    sha1/sha256 complexity.

Reported-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-05 08:50:43 -07:00
Patrick Steinhardt
9e89dcb66a builtin/ls-remote: fall back to SHA1 outside of a repo
In c8aed5e8da (repository: stop setting SHA1 as the default object hash,
2024-05-07), we have stopped setting the default hash algorithm for
`the_repository`. Consequently, code that relies on `the_hash_algo` will
now crash when it hasn't explicitly been initialized, which may be the
case when running outside of a Git repository.

It was reported that git-ls-remote(1) may crash in such a way when using
a remote helper that advertises refspecs. This is because the refspec
announced by the helper will get parsed during capability negotiation.
At that point we haven't yet figured out what object format the remote
uses though, so when run outside of a repository then we will fail.

The course of action is somewhat dubious in the first place. Ideally, we
should only parse object IDs once we have asked the remote helper for
the object format. And if the helper didn't announce the "object-format"
capability, then we should always assume SHA256. But instead, we used to
take either SHA1 if there was no repository, or we used the hash of the
local repository, which is wrong.

Arguably though, crashing hard may not be in the best interest of our
users, either. So while the old behaviour was buggy, let's restore it
for now as a short-term fix. We should eventually revisit, potentially
by deferring the point in time when we parse the refspec until after we
have figured out the remote's object hash.

Reported-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-02 08:24:05 -07:00
Junio C Hamano
7c7516b8db t0018: remove leftover debugging cruft
The actual file is copied out to /tmp, presumably so that the tester
can inspect it after the test is done, which may have been a useful
debugging aid.

But in the final shape of the test suite, such a code should not
exist.  We cannot even assume that we are allowed to write into /tmp
(our TMPDIR may not even be pointing at it) or read from it for that
matter.

Noticed-by: Randall S. Becker <rsbecker@nexbridge.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-01 11:52:11 -07:00
Taylor Blau
615d2de3b4 config.c: avoid segfault with --fixed-value and valueless config
When using `--fixed-value` with a key whose value is left empty (implied
as being "true"), 'git config' may crash when invoked like either of:

    $ git config set --file=config --value=value --fixed-value \
        section.key pattern
    $ git config --file=config --fixed-value section.key value pattern

The original bugreport[1] bisects to 00bbdde141 (builtin/config:
introduce "set" subcommand, 2024-05-06), which is a red-herring, since
the original bugreport uses the new 'git config set' invocation.

The behavior likely bisects back to c90702a1f6 (config: plumb
--fixed-value into config API, 2020-11-25), which introduces the new
--fixed-value option in the first place.

Looking at the relevant frame from a failed process's coredump, the
crash appears in config.c::matches() like so:

    (gdb) up
    #1  0x000055b3e8b06022 in matches (key=0x55b3ea894360 "section.key", value=0x0,
        store=0x7ffe99076eb0) at config.c:2884
    2884			return !strcmp(store->fixed_value, value);

where we are trying to compare the `--fixed-value` argument to `value`,
which is NULL.

Avoid attempting to match `--fixed-value` for configuration keys with no
explicit value. A future patch could consider the empty value to mean
"true", "yes", "on", etc. when invoked with `--type=bool`, but let's
punt on that for now in the name of avoiding the segfault.

[1]: https://lore.kernel.org/git/CANrWfmTek1xErBLrnoyhHN+gWU+rw14y6SQ+abZyzGoaBjmiKA@mail.gmail.com/

Reported-by: Han Jiang <jhcarl0814@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-01 10:48:15 -07:00
Patrick Steinhardt
63ee933383 t98xx: mark Perforce tests as memory-leak free
All the Perforce tests are free of memory leaks. This went unnoticed
because most folks do not have p4 and p4d installed on their computers.
Consequently, given that the prerequisites for running those tests
aren't fulfilled, `TEST_PASSES_SANITIZE_LEAK=check` won't notice that
those tests are indeed memory leak free.

Mark those tests accordingly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-31 10:05:18 -07:00
Patrick Steinhardt
49f4fd901a t98xx: fix Perforce tests with p4d r23 and newer
Some of the tests in t98xx modify the Perforce depot in ways that the
tool wouldn't normally allow. This is done to test behaviour of git-p4
in certain edge cases that we have observed in the wild, but which
should in theory not be possible.

Naturally, modifying the depot on disk directly is quite intimate with
the tool and thus prone to breakage when Perforce updates the way that
data is stored. And indeed, those tests are broken nowadays with r23 of
Perforce. While a file revision was previously stored as a plain file
"depot/file,v", it is now stored in a directory "depot/file,d" with
compression.

Adapt those tests to handle both old- and new-style depot layouts.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-31 10:05:18 -07:00
René Scharfe
098be29f5b t-example-decorate: remove test messages
The test_msg() calls only repeat information already present in test
descriptions and check definitions, which are shown automatically if
the checks fail.  Remove the redundant messages to simplify the tests
and their output.  Here it is with all of them failing before:

 # check "ret == NULL" failed at t/unit-tests/t-example-decorate.c:18
 # when adding a brand-new object, NULL should be returned
 # check "ret == NULL" failed at t/unit-tests/t-example-decorate.c:21
 # when adding a brand-new object, NULL should be returned
 not ok 1 - Add 2 objects, one with a non-NULL decoration and one with a NULL decoration.
 # check "ret == &vars->decoration_a" failed at t/unit-tests/t-example-decorate.c:29
 # when readding an already existing object, existing decoration should be returned
 # check "ret == NULL" failed at t/unit-tests/t-example-decorate.c:32
 # when readding an already existing object, existing decoration should be returned
 not ok 2 - When re-adding an already existing object, the old decoration is returned.
 # check "ret == NULL" failed at t/unit-tests/t-example-decorate.c:40
 # lookup should return added declaration
 # check "ret == &vars->decoration_b" failed at t/unit-tests/t-example-decorate.c:43
 # lookup should return added declaration
 # check "ret == NULL" failed at t/unit-tests/t-example-decorate.c:46
 # lookup for unknown object should return NULL
 not ok 3 - Lookup returns the added declarations, or NULL if the object was never added.
 # check "objects_noticed == 2" failed at t/unit-tests/t-example-decorate.c:58
 #    left: 1
 #   right: 2
 # should have 2 objects
 not ok 4 - The user can also loop through all entries.
 1..4

... and here with the patch applied:

 # check "ret == NULL" failed at t/unit-tests/t-example-decorate.c:18
 # check "ret == NULL" failed at t/unit-tests/t-example-decorate.c:20
 not ok 1 - Add 2 objects, one with a non-NULL decoration and one with a NULL decoration.
 # check "ret == &vars->decoration_a" failed at t/unit-tests/t-example-decorate.c:27
 # check "ret == NULL" failed at t/unit-tests/t-example-decorate.c:29
 not ok 2 - When re-adding an already existing object, the old decoration is returned.
 # check "ret == NULL" failed at t/unit-tests/t-example-decorate.c:36
 # check "ret == &vars->decoration_b" failed at t/unit-tests/t-example-decorate.c:38
 # check "ret == NULL" failed at t/unit-tests/t-example-decorate.c:40
 not ok 3 - Lookup returns the added declarations, or NULL if the object was never added.
 # check "objects_noticed == 2" failed at t/unit-tests/t-example-decorate.c:51
 #    left: 1
 #   right: 2
 not ok 4 - The user can also loop through all entries.
 1..4

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-30 13:31:46 -07:00
Junio C Hamano
ee0be850b0 safe.directory: setting safe.directory="." allows the "current" directory
When "git daemon" enters a repository, it chdir's to the requested
repository and then uses "." (the curent directory) to consult the
"is this repository considered safe?" when it is not owned by the
same owner as the process.

Make sure this access will be allowed by setting safe.directory to
".", as that was once advertised on the list as a valid workaround
to the overly tight safe.directory settings introduced by 2.45.1
(cf. <834862fd-b579-438a-b9b3-5246bf27ce8a@gmail.com>).

Also add simlar test to show what happens in the same setting if the
safe.directory is set to "*" instead of "."; in short, "." is a bit
tighter (as it is custom designed for git-daemon situation) than
"anything goes" settings given by "*".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-30 11:47:59 -07:00
Junio C Hamano
dc0edbb01c safe.directory: normalize the configured path
The pathname of a repository comes from getcwd() and it could be a
path aliased via symbolic links, e.g., the real directory may be
/home/u/repository but a symbolic link /home/u/repo may point at it,
and the clone request may come as "git clone file:///home/u/repo/"

A request to check if /home/u/repository is safe would be rejected
if the safe.directory configuration allows /home/u/repo/ but not its
alias /home/u/repository/.  Normalize the paths configured for the
safe.directory configuration variable before comparing them with the
path being checked.

Two and a half things to note, compared to the previous step to
normalize the actual path of the suspected repository, are:

 - A configured safe.directory may be coming from .gitignore in the
   home directory that may be shared across machines.  The path
   meant to match with an entry may not necessarily exist on all of
   such machines, so not being able to convert them to real path on
   this machine is *not* a condition that is worthy of warning.
   Hence, we ignore a path that cannot be converted to a real path.

 - A configured safe.directory is essentially a random string that
   user throws at us, written completely unrelated to the directory
   the current process happens to be in.  Hence it makes little
   sense to give a non-absolute path.  Hence we ignore any
   non-absolute paths, except for ".".

 - The safe.directory set to "." was once advertised on the list as
   a valid workaround for the regression caused by the overly tight
   safe.directory check introduced in 2.45.1; we treat it to mean
   "if we are at the top level of a repository, it is OK".
   (cf. <834862fd-b579-438a-b9b3-5246bf27ce8a@gmail.com>).

Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-30 11:47:59 -07:00
Junio C Hamano
7f547c99a6 safe.directory: normalize the checked path
The pathname of a repository comes from getcwd() and it could be a
path aliased via symbolic links, e.g., the real directory may be
/home/u/repository but a symbolic link /home/u/repo may point at it,
and the clone request may come as "git clone file:///home/u/repo/".

A request to check if /home/u/repo is safe would be rejected if the
safe.directory configuration allows /home/u/repository/ but not its
alias /home/u/repo/.  Normalize the path being checked before
comparing with safe.directory value(s).

Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-30 11:47:59 -07:00
David Disseldorp
8b426c84f3 notes: do not trigger editor when adding an empty note
With "git notes add -C $blob", the given blob contents are to be made
into a note without involving an editor.  But when "--allow-empty" is
given, the editor is invoked, which can cause problems for
non-interactive callers[1].

This behaviour started with 90bc19b3ae (notes.c: introduce
'--separator=<paragraph-break>' option, 2023-05-27), which changed
editor invocation logic to check for a zero length note_data buffer.

Restore the original behaviour of "git note" that takes the contents
given via the "-m", "-C", "-F" options without invoking an editor, by
checking for any prior parameter callbacks, indicated by a non-zero
note_data.msg_nr.  Remove the now-unneeded note_data.given flag.

Add a test for this regression by checking whether GIT_EDITOR is
invoked alongside "git notes add -C $empty_blob --allow-empty"

[1] https://github.com/ddiss/icyci/issues/12

Signed-off-by: David Disseldorp <ddiss@suse.de>
[jc: enhanced the test with -m/-F options]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-29 15:31:30 -07:00
Kousik Sanagavarapu
6e71d6ac7c unit-tests/test-lib: fix typo in check_pointer_eq() description
The comment surrounding check_pointer_eq() should explain about what
this function does instead of explaining check_int().  Correct this.

Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-29 14:23:14 -07:00
Junio C Hamano
c3d034df16 csum-file: introduce discard_hashfile()
The hashfile API is used to write out a "hashfile", which has a
final checksum (typically SHA-1) at the end.  An in-core hashfile
structure has up to two file descriptors and a few buffers that can
only be freed by calling a helper function that is private to the
csum-file implementation.

The usual flow of a user of the API is to first open a file
descriptor for writing, obtain a hashfile associated with that write
file descriptor by calling either hashfd() or hashfd_check(), call
hashwrite() number of times to write data to the file, and then call
finalize_hashfile(), which appends th checksum to the end of the
file, closes file descriptors and releases associated buffers.

But what if a caller finds some error after calling hashfd() to
start the process and/or hashwrite() to send some data to the file,
and wants to abort the operation?  The underlying file descriptor is
often managed by the tempfile API, so aborting will clean the file
out of the filesystem, but the resources associated with the in-core
hashfile structure is lost.

Introduce discard_hashfile() API function to allow them to release
the resources held by a hashfile structure the callers want to
dispose of, and use that in read-cache.c:do_write_index(), which is
a central place that writes the index file.

Mark t2107 as leak-free, as this leak in "update-index --cacheinfo"
test that deliberately makes it fail is now plugged.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-26 09:04:02 -07:00
Junio C Hamano
d98d9c77e5 mailmap: plug memory leak in read_mailmap_blob()
When a named object to read mailmap from is not a blob, the code
correctly errors out, but it forgot to free the object data before
doing so.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-26 08:00:09 -07:00
Junio C Hamano
ec9d46588e Merge branch 'ds/midx-write-repack-fix'
Repacking a repository with multi-pack index started making stupid
pack selections in Git 2.45, which has been corrected.

* ds/midx-write-repack-fix:
  midx-write: revert use of --stdin-packs
  t5319: add failing test case for repack/expire
2024-07-23 16:54:33 -07:00
Phillip Wood
39bdd84eaf add-patch: handle splitting hunks with diff.suppressBlankEmpty
When "add -p" parses diffs, it looks for context lines starting with a
single space. But when diff.suppressBlankEmpty is in effect, an empty
context line will omit the space, giving us a true empty line. This
confuses the parser, which is unable to split based on such a line.

It's tempting to say that we should just make sure that we generate a
diff without that option.  However, although we do not parse hunks that
the user has manually edited with parse_diff() we do allow the user
to split such hunks. As POSIX calls the decision of whether to print the
space here "implementation-defined" we need to handle edited hunks where
empty context lines omit the space.

So let's handle both cases: a context line either starts with a space or
consists of a totally empty line by normalizing the first character to a
space when we parse them. Normalizing the first character rather than
changing the code to check for a space or newline will hopefully future
proof against introducing similar bugs if the code is changed.

Reported-by: Ilya Tumaykin <itumaykin@gmail.com>
Helped-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>
2024-07-20 16:29:14 -07:00
Derrick Stolee
8fb6d11fad midx-write: revert use of --stdin-packs
This reverts b7d6f23a17 (midx-write.c: use `--stdin-packs` when
repacking, 2024-04-01) and then marks the test created in the previous
change as passing.

The fundamental issue with the reverted change is that the focus on
pack-files separates the object selection from how the multi-pack-index
selects a single pack-file for an object ID with multiple copies among
the tracked pack-files.

The change was made with the intention of improving delta compression in
the resulting pack-file, but that can be resolved with the existing
object list mechanism. There are other potential pitfalls of doing an
object walk at this time if the repository is a blobless partial clone,
and that will require additional testing on top of the one that changes
here.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
Acked-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-19 07:19:01 -07:00
Derrick Stolee
738fab524c t5319: add failing test case for repack/expire
Git 2.45.0 included the change b7d6f23a17 (midx-write.c: use
`--stdin-packs` when repacking, 2024-04-01) which caused the 'git
multi-pack-index repack' command to use 'git pack-objects --stdin-packs'
instead of listing the objects to repack. While this change was
motivated by efficient cross-process communication and the ability to
improve delta compression, it breaks a fundamental function of the
'incremental-repack' task that is enabled by default in Scalar clones or
Git repositories that run 'git maintenance start'.

The 'incremental-repack' task performs a two-step process of the
'expire' and 'repack' subcommands of the 'git multi-pack-index' builtin.
The 'expire' command removes any pack-files listed in the
multi-pack-index but without any referenced objects. The 'repack' task
then finds a batch of pack-files to repack and sends their objects to
'git pack-objects'. Both the pack-files chosen for the batch and the
objects chosen to repack are based on the ones that the multi-pack-index
references. Objects that appear in a pack-file but have a duplicate copy
in a newer pack-file are not considered in this case. Since the
multi-pack-index references only the newest copy of an object, this
allows the next 'incremental-repack' task to remove the pack-files in
the next 'expire' task. This delay is intentional due to how Windows
handles may block deletion of files with open read handles.

However, the mentioned commit changed this behavior to divorce the set
of objects referenced by the multi-pack-index and instead use a set of
"included" and "excluded" pack-files in the 'git pack-objects' builtin.
When a pack-file is selected as "included", only the objects it contains
but are not in any "excluded" pack-files are considered for repacking.
This has led to client repositories failing to remove old pack-files as
they still have some referenced objects. This grows over time until the
point that Git is trying to repack the same pack-files over and over.

For now, create a test case that demonstrates the expected behavior, but
also fails in its final line. The setup here it attempting to recreate a
typical situation for a repository that uses a blobless partial clone.
There would be a large initial pack-file from the clone that is never
selected in the 'repack' batch. There are other pack-files that have a
combination of new objects from incremental fetches and possibly blobs
that are not connected to those incremental fetches; these blobs could
be filled in from commands like 'git checkout' or 'git blame'. The
pack-files also have some overlap on purpose so test-1 has some
duplicates in test-2 and test-2 has some duplicates in test-3.

At the end of the test, the test-2 pack-file still exists though it
should have been expired. This test will pass when reverting the
offending commit.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-18 14:53:27 -07:00
Junio C Hamano
1aac20a4b0 Merge branch 'jk/am-retry'
Test fix as a follow-up to an already graduated topic.

* jk/am-retry:
  t4153: stop redirecting input from /dev/zero
2024-07-18 08:30:27 -07:00
Junio C Hamano
76e018b9a1 Merge branch 'js/var-git-shell-path'
"git var GIT_SHELL_PATH" should report the path to the shell used
to spawn external commands, but it didn't do so on Windows, which
has been corrected.

* js/var-git-shell-path:
  var(win32): do report the GIT_SHELL_PATH that is actually used
  run-command: declare the `git_shell_path()` function globally
  run-command(win32): resolve the path to the Unix shell early
  mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, too
  win32: override `fspathcmp()` with a directory separator-aware version
  strvec: declare the `strvec_push_nodup()` function globally
  run-command: refactor getting the Unix shell path into its own function
2024-07-17 10:47:27 -07:00
Junio C Hamano
e13feda98f Merge branch 'kn/push-empty-fix'
"git push '' HEAD:there" used to hit a BUG(); it has been corrected
to die with "fatal: bad repository ''".

* kn/push-empty-fix:
  builtin/push: call set_refspecs after validating remote
2024-07-17 10:47:26 -07:00
Junio C Hamano
b19a8c00c6 Merge branch 'jk/test-body-in-here-doc'
The test framework learned to take the test body not as a single
string but as a here-document.

* jk/test-body-in-here-doc:
  t/.gitattributes: ignore whitespace in chainlint expect files
  t: convert some here-doc test bodies
  test-lib: allow test snippets as here-docs
  chainlint.pl: add tests for test body in heredoc
  chainlint.pl: recognize test bodies defined via heredoc
  chainlint.pl: check line numbers in expected output
  chainlint.pl: force CRLF conversion when opening input files
  chainlint.pl: do not spawn more threads than we have scripts
  chainlint.pl: only start threads if jobs > 1
  chainlint.pl: add test_expect_success call to test snippets
2024-07-17 10:47:25 -07:00
Junio C Hamano
6da44da936 Merge branch 'rj/test-sanitize-leak-log-fix'
Tests that use GIT_TEST_SANITIZE_LEAK_LOG feature got their exit
status inverted, which has been corrected.

* rj/test-sanitize-leak-log-fix:
  test-lib: GIT_TEST_SANITIZE_LEAK_LOG enabled by default
  test-lib: fix GIT_TEST_SANITIZE_LEAK_LOG
2024-07-17 10:47:24 -07:00
Jeff King
2a959ec21a t4153: stop redirecting input from /dev/zero
Commit 852a171018 (am: let command-line options override saved options,
2015-08-04) redirected a few "git am" invocations from /dev/zero, even
though it did not expect "am" to read the input. This was necessary at
the time because those tests used test_terminal, and as described in
18d8c26930 (test_terminal: redirect child process' stdin to a pty,
2015-08-04):

  Note that due to the way the code is structured, the child's stdin
  pseudo-tty will be closed when we finish reading from our stdin. This
  means that in the common case, where our stdin is attached to /dev/null,
  the child's stdin pseudo-tty will be closed immediately. Some operations
  like isatty(), which git-am uses, require the file descriptor to be
  open, and hence if the success of the command depends on such functions,
  test_terminal's stdin should be redirected to a source with large amount
  of data to ensure that the child's stdin is not closed, e.g.

              test_terminal git am --3way </dev/zero

But we later dropped the use of test_terminal in 53ce2e3f0a (am: add
explicit "--retry" option, 2024-06-06). That commit dropped one of the
redirections from /dev/zero but not the other.

In theory the remaining one should not cause any problems, but it turns
out that at least one platform (NonStop) does not have /dev/zero at all.
We never noticed before because it also did not pass the TTY prereq,
meaning these tests were not run at all there until 53ce2e3f0a.

So let's drop the useless /dev/zero mention. There are others in the
test suite, but they are run only for tests marked with EXPENSIVE (so
not typically by default).

Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-17 08:31:27 -07:00
Junio C Hamano
5133ead528 Revert "reflog expire: don't use lookup_commit_reference_gently()"
During Git 2.35 timeframe, daf1d828 (reflog expire: don't use
lookup_commit_reference_gently(), 2021-12-22) replaced a call to
lookup_commit_reference_gently() with a call to lookup_commit().

What it failed to consider was that our refs do not necessarily
point at commits (most notably, we have annotated and signed tags),
and more importantly that lookup_commit() does not dereference a tag
to return a commit; instead it returns NULL when a tag is given.

Since the commit returned is used as a starting point for the
reachability check, this ejected the commits that are reachable only
by an annotated tag out of the set of reachable commits, breaking
the computation to correctly implement the "--expire-unreachable"
option.  We also started giving an error message that the API
function expected to be fed a commit object.

This problem hasn't been reported or noticed for a long time,
probably because the "refs/tags/" hierarchy by default is not
covered by reflogs, as nobody usually moves tags.

Revert the change to correctly find the commit pointed at by the ref
to restore the previous behaviour, but do so only in a more modern
codebase, as we had significant code churn since then and it is not
grave enough to worry about for older maintenance tracks.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-16 14:15:35 -07:00
Junio C Hamano
fe5ba894ec Merge branch 'bc/http-proactive-auth'
The http transport can now be told to send request with
authentication material without first getting a 401 response.

* bc/http-proactive-auth:
  http: allow authenticating proactively
2024-07-16 11:18:57 -07:00
Junio C Hamano
5d71940dda Merge branch 'ds/advice-sparse-index-expansion'
A new warning message is issued when a command has to expand a
sparse index to handle working tree cruft that are outside of the
sparse checkout.

* ds/advice-sparse-index-expansion:
  advice: warn when sparse index expands
2024-07-16 11:18:56 -07:00
Junio C Hamano
f4c6a0e275 Merge branch 'cb/send-email-sanitize-trailer-addresses'
Address-looking strings found on the trailer are now placed on the
Cc: list after running through sanitize_address by "git send-email".

* cb/send-email-sanitize-trailer-addresses:
  git-send-email: use sanitized address when reading mbox body
2024-07-16 11:18:56 -07:00
Junio C Hamano
ffc8f1142c Merge branch 'en/ort-inner-merge-error-fix'
The "ort" merge backend saw one bugfix for a crash that happens
when inner merge gets killed, and assorted code clean-ups.

* en/ort-inner-merge-error-fix:
  merge-ort: fix missing early return
  merge-ort: convert more error() cases to path_msg()
  merge-ort: upon merge abort, only show messages causing the abort
  merge-ort: loosen commented requirements
  merge-ort: clearer propagation of failure-to-function from merge_submodule
  merge-ort: fix type of local 'clean' var in handle_content_merge ()
  merge-ort: maintain expected invariant for priv member
  merge-ort: extract handling of priv member into reusable function
2024-07-16 11:18:55 -07:00
Junio C Hamano
9118e46e81 Merge branch 'cp/unit-test-reftable-record'
A test in reftable library has been rewritten using the unit test
framework.

* cp/unit-test-reftable-record:
  t-reftable-record: add tests for reftable_log_record_compare_key()
  t-reftable-record: add tests for reftable_ref_record_compare_name()
  t-reftable-record: add index tests for reftable_record_is_deletion()
  t-reftable-record: add obj tests for reftable_record_is_deletion()
  t-reftable-record: add log tests for reftable_record_is_deletion()
  t-reftable-record: add ref tests for reftable_record_is_deletion()
  t-reftable-record: add comparison tests for obj records
  t-reftable-record: add comparison tests for index records
  t-reftable-record: add comparison tests for ref records
  t-reftable-record: add reftable_record_cmp() tests for log records
  t: move reftable/record_test.c to the unit testing framework
2024-07-15 10:11:44 -07:00