fetch: free "raw" string when shrinking refspec

The "--prefetch" option to git-fetch modifies the default refspec,
including eliminating some entries entirely. When we drop an entry we
free the strings in the refspec_item, but we forgot to free the matching
string in the "raw" array of the refspec struct. There's no behavioral
bug here (since we correctly shrink the raw array, too), but we're
leaking the allocated string.

Let's add in the leak-fix, and while we're at it drop "const" from
the type of the raw string array. These strings are always allocated by
refspec_append(), etc, and this makes the memory ownership more clear.

This is all a bit more intimate with the refspec code than I'd like, and
I suspect it would be better if each refspec_item held on to its own raw
string, we had a single array, and we could use refspec_item_clear() to
clean up everything. But that's a non-trivial refactoring, since
refspec_item structs can be held outside of a "struct refspec", without
having a matching raw string at all. So let's leave that for now and
just fix the leak in the most immediate way.

This lets us mark t5582 as leak-free.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

transport-helper: fix strbuf leak in push_refs_with_push()

We loop over the refs to push, building up a strbuf with the set of
"push" directives to send to the remote helper. But if the atomic-push
flag is set and we hit a rejected ref, we'll bail from the function
early. We clean up most things, but forgot to release the strbuf.

Fixing this lets us mark t5541 as leak-free.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

send-pack: free cas options before exit

The send-pack --force-with-lease option populates a push_cas_option
struct with allocated strings. Exiting without cleaning this up will
cause leak-checkers to complain.

We can fix this by calling clear_cas_option(), after making it publicly
available. Previously it was used only for resetting the list when we
saw --no-force-with-lease.

The git-push command has the same "leak", though in this case it won't
trigger a leak-checker since it stores the push_cas_option struct as a
global rather than on the stack (and is thus reachable even after main()
exits). I've added cleanup for it here anyway, though, as
future-proofing.

The leak is triggered by t5541 (it tests --force-with-lease over http,
which requires a separate send-pack process under the hood), but we
can't mark it as leak-free yet.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit: avoid leaking already-saved buffer

When we parse a commit via repo_parse_commit_internal(), if
save_commit_buffer is set we'll stuff the buffer of the object contents
into a cache, overwriting any previous value.

This can result in a leak of that previously cached value, though it's
rare in practice. If we have a value in the cache it would have come
from a previous parse, and during that parse we'd set the object.parsed
flag, causing any subsequent parse attempts to exit without doing any
work.

But it's possible to "unparse" a commit, which we do when registering a
commit graft. And since shallow fetches are implemented using grafts,
the leak is triggered in practice by t5539.

There are a number of possible ways to address this:

  1. the unparsing function could clear the cached commit buffer, too. I
     think this would work for the case I found, but I'm not sure if
     there are other ways to end up in the same state (an unparsed
     commit with an entry in the commit buffer cache).

  2. when we parse, we could check the buffer cache and prefer it to
     reading the contents from the object database. In theory the
     contents of a particular sha1 are immutable, but the code in
     question is violating the immutability with grafts. So this
     approach makes me a bit nervous, although I think it would work in
     practice (the grafts are applied to what we parse, but we still
     retain the original contents).

  3. We could realize the cache is already populated and discard its
     contents before overwriting. It's possible some other code could be
     holding on to a pointer to the old cache entry (and we'd introduce
     a use-after-free), but I think the risk of that is relatively low.

  4. The reverse of (3): when the cache is populated, don't bother
     saving our new copy. This is perhaps a little weird, since we'll
     have just populated the commit struct based on a different buffer.
     But the two buffers should be the same, even in the presence of
     grafts (as in (2) above).

I went with option 4. It addresses the leak directly and doesn't carry
any risk of breaking other assumptions. And it's the same technique used
by parse_object_buffer() for this situation, though I'm not sure when it
would even come up there. The extra safety has been there since
bd1e17e245 (Make "parse_object()" also fill in commit message buffer
data., 2005-05-25).

This lets us mark t5539 as leak-free.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fetch-pack, send-pack: clean up shallow oid array

When we call get_remote_heads() for protocol v0, that may populate the
"shallow" oid_array, which must be cleaned up to avoid a leak at the
program exit. The same problem exists for both fetch-pack and send-pack,
but not for the usual transport.c code paths, since we already do this
cleanup in disconnect_git().

Fixing this lets us mark t5542 as leak-free for the send-pack side, but
fetch-pack will need some more fixes before we can do the same for
t5539.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fetch-pack: free object filter before exiting

Our fetch_pack_args holds a filter_options struct that may be populated
with allocated strings by the by the "--filter" command-line option. We
must free it before exiting to avoid a leak when the program exits.

The usual fetch code paths that use transport.c don't have the same
leak, because we do the cleanup in disconnect_git().

Fixing this leak lets us mark t5500 as leak-free.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

connect: clear child process before freeing in diagnostic mode

The git_connect() function has a special CONNECT_DIAG_URL mode, where we
stop short of actually connecting to the other side and just print some
parsing details. For URLs that require a child process (like ssh), we
free() the child_process struct but forget to clear it, leaking the
strings we stuffed into its "env" list.

This leak is triggered many times in t5500, which uses "fetch-pack
--diag-url", but we're not yet ready to mark it as leak-free.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fetch-pack: fix leaking sought refs

When calling `fetch_pack()` the caller is expected to pass in a set of
sought-after refs that they want to fetch. This array gets massaged to
not contain duplicate entries, which is done by replacing duplicate refs
with `NULL` pointers. This modifies the caller-provided array, and in
case we do unset any pointers the caller now loses track of that ref and
cannot free it anymore.

Now the obvious fix would be to not only unset these pointers, but to
also free their contents. But this doesn't work because callers continue
to use those refs. Another potential solution would be to copy the array
in `fetch_pack()` so that we dont modify the caller-provided one. But
that doesn't work either because the NULL-ness of those entries is used
by callers to skip over ref entries that we didn't even try to fetch in
`report_unmatched_refs()`.

Instead, we make it the responsibility of our callers to duplicate these
arrays as needed. It ain't pretty, but it works to plug the memory leak.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

shallow: fix leak when unregistering last shallow root

When unregistering a shallow root we shrink the array of grafts by one
and move remaining grafts one to the left. This can of course only
happen when there are any grafts left, because otherwise there is
nothing to move. As such, this code is guarded by a condition that only
performs the move in case there are grafts after the position of the
graft to be unregistered.

By mistake we also put the call to free the unregistered graft into that
condition. But that doesn't make any sense, as we want to always free
the graft when it exists. Fix the resulting memory leak by doing so.

This leak is exposed by t5500, but plugging it does not make the whole
test suite pass.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

http-fetch: clear leaking git-index-pack(1) arguments

We never clear the arguments that we pass to git-index-pack(1). Create a
common exit path and release them there to plug this leak.

This is leak is exposed by t5702, but plugging the leak does not make
the whole test suite pass.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

The 20th batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'jc/pass-repo-to-builtins'

The convention to calling into built-in command implementation has
been updated to pass the repository, if known, together with the
prefix value.

* jc/pass-repo-to-builtins:
  add: pass in repo variable instead of global the_repository
  builtin: remove USE_THE_REPOSITORY for those without the_repository
  builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.h
  builtin: add a repository parameter for builtin functions

Merge branch 'jk/t9001-deflake'

Test fix.

* jk/t9001-deflake:
  t9001: use a more distinct fake BugID

Merge branch 'jk/jump-quickfix-fixes'

A few usability fixes to "git jump" (in contrib/).

* jk/jump-quickfix-fixes:
  git-jump: ignore deleted files in diff mode
  git-jump: always specify column 1 for diff entries

Merge branch 'ak/typofixes'

Trivial typofixes.

* ak/typofixes:
  cbtree: fix a typo
  bloom: fix a typo
  attr: fix a typo

Merge branch 'jk/diag-unexpected-remote-helper-death'

When a remote-helper dies before Git writes to it, SIGPIPE killed
Git silently.  We now explain the situation a bit better to the end
user in our error message.

* jk/diag-unexpected-remote-helper-death:
  print an error when remote helpers die during capabilities

Merge branch 'jc/t5512-sigpipe-fix'

Test fix.

* jc/t5512-sigpipe-fix:
  t5512.40 sometimes dies by SIGPIPE

Merge branch 'ps/environ-wo-the-repository'

Code clean-up.

* ps/environ-wo-the-repository: (21 commits)
  environment: stop storing "core.notesRef" globally
  environment: stop storing "core.warnAmbiguousRefs" globally
  environment: stop storing "core.preferSymlinkRefs" globally
  environment: stop storing "core.logAllRefUpdates" globally
  refs: stop modifying global `log_all_ref_updates` variable
  branch: stop modifying `log_all_ref_updates` variable
  repo-settings: track defaults close to `struct repo_settings`
  repo-settings: split out declarations into a standalone header
  environment: guard state depending on a repository
  environment: reorder header to split out `the_repository`-free section
  environment: move `set_git_dir()` and related into setup layer
  environment: make `get_git_namespace()` self-contained
  environment: move object database functions into object layer
  config: make dependency on repo in `read_early_config()` explicit
  config: document `read_early_config()` and `read_very_early_config()`
  environment: make `get_git_work_tree()` accept a repository
  environment: make `get_graft_file()` accept a repository
  environment: make `get_index_file()` accept a repository
  environment: make `get_object_directory()` accept a repository
  environment: make `get_git_common_dir()` accept a repository
  ...

Sync with Git 2.46.2

Git 2.46.2

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'ma/test-libcurl-prereq' into maint-2.46

Test portability fix.

* ma/test-libcurl-prereq:
  t0211: add missing LIBCURL prereq
  t1517: add missing LIBCURL prereq

Merge branch 'jc/doc-skip-fetch-all-and-prefetch' into maint-2.46

Doc updates.

* jc/doc-skip-fetch-all-and-prefetch:
  doc: remote.*.skip{DefaultUpdate,FetchAll} stops prefetch

Merge branch 'bl/trailers-and-incomplete-last-line-fix' into maint-2.46

The interpret-trailers command failed to recognise the end of the
message when the commit log ends in an incomplete line.

* bl/trailers-and-incomplete-last-line-fix:
  interpret-trailers: handle message without trailing newline

Merge branch 'rj/cygwin-has-dev-tty' into maint-2.46

Cygwin does have /dev/tty support that is needed by things like
single-key input mode.

* rj/cygwin-has-dev-tty:
  config.mak.uname: add HAVE_DEV_TTY to cygwin config section

Merge branch 'rs/diff-exit-code-fix' into maint-2.46

In a few corner cases "git diff --exit-code" failed to report
"changes" (e.g., renamed without any content change), which has
been corrected.

* rs/diff-exit-code-fix:
  diff: report dirty submodules as changes in builtin_diff()
  diff: report copies and renames as changes in run_diff_cmd()

The 19th batch

Merge the topics that have been cooking since 2024-09-13 or so in
'next'.

Let's try a new workflow to update the maintenance track by removing
the "merge ... later to maint" comments from the draft release notes
on the 'master' track.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'jk/git-pm-bare-repo-fix'

In Git 2.39, Git.pm stopped working in a bare repository, which has
been corrected.

* jk/git-pm-bare-repo-fix:
  Git.pm: use "rev-parse --absolute-git-dir" rather than perl code
  Git.pm: fix bare repository search with Directory option

Merge branch 'bb/unicode-width-table-16'

Update the character width table for Unicode 16.

* bb/unicode-width-table-16:
  unicode: update the width tables to Unicode 16

Merge branch 'ma/test-libcurl-prereq'

Test portability fix.

* ma/test-libcurl-prereq:
  t0211: add missing LIBCURL prereq
  t1517: add missing LIBCURL prereq

Merge branch 'jk/interop-test-build-options'

The support to customize build options to adjust for older versions
and/or older systems for the interop tests has been improved.

* jk/interop-test-build-options:
  t/interop: allow per-version make options

Merge branch 'jk/no-openssl-with-openssl-sha1'

The "imap-send" now allows to be compiled with NO_OPENSSL and
OPENSSL_SHA1 defined together.

* jk/no-openssl-with-openssl-sha1:
  imap-send: handle NO_OPENSSL even when openssl exists

Merge branch 'ps/leakfixes-part-6'

More leakfixes.

* ps/leakfixes-part-6: (22 commits)
  builtin/repack: fix leaking keep-pack list
  merge-ort: fix two leaks when handling directory rename modifications
  match-trees: fix leaking prefixes in `shift_tree()`
  builtin/fmt-merge-msg: fix leaking buffers
  builtin/grep: fix leaking object context
  builtin/pack-objects: plug leaking list of keep-packs
  builtin/repack: fix leaking line buffer when packing promisors
  negotiator/skipping: fix leaking commit entries
  shallow: fix leaking members of `struct shallow_info`
  shallow: free grafts when unregistering them
  object: clear grafts when clearing parsed object pool
  gpg-interface: fix misdesigned signing key interfaces
  send-pack: fix leaking push cert nonce
  remote: fix leak in reachability check of a remote-tracking ref
  remote: fix leaking tracking refs
  builtin/submodule--helper: fix leaking refs on push-check
  submodule: fix leaking fetch task data
  upload-pack: fix leaking child process data on reachability checks
  builtin/push: fix leaking refspec query result
  send-pack: fix leaking common object IDs
  ...

Merge branch 'pw/rebase-autostash-fix'

"git rebase --autostash" failed to resurrect the autostashed
changes when the command gets aborted after giving back control
asking for hlep in conflict resolution.

* pw/rebase-autostash-fix:
  rebase: apply and cleanup autostash when rebase fails to start

The eighteenth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'es/chainlint-message-updates'

The error messages from the test script checker have been improved.

* es/chainlint-message-updates:
  chainlint: reduce annotation noise-factor
  chainlint: make error messages self-explanatory
  chainlint: don't be fooled by "?!...?!" in test body

Merge branch 'ps/clar-unit-test'

Import clar unit tests framework libgit2 folks invented for our
use.

* ps/clar-unit-test:
  Makefile: rename clar-related variables to avoid confusion
  clar: add CMake support
  t/unit-tests: convert ctype tests to use clar
  t/unit-tests: convert strvec tests to use clar
  t/unit-tests: implement test driver
  Makefile: wire up the clar unit testing framework
  Makefile: do not use sparse on third-party sources
  Makefile: make hdr-check depend on generated headers
  Makefile: fix sparse dependency on GENERATED_H
  clar: stop including `shellapi.h` unnecessarily
  clar(win32): avoid compile error due to unused `fs_copy()`
  clar: avoid compile error with mingw-w64
  t/clar: fix compatibility with NonStop
  t: import the clar unit testing framework
  t: do not pass GIT_TEST_OPTS to unit tests with prove

ci updates

This batch is solely to unbreak the 32-bit CI jobs that can no
longer work with Ubuntu xenial image that is too ancient.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Sync with 'maint'

Merge branch 'jk/ci-linux32-update'

CI updates

* jk/ci-linux32-update:
  ci: add Ubuntu 16.04 job to GitLab CI
  ci: use regular action versions for linux32 job
  ci: use more recent linux32 image
  ci: unify ubuntu and ubuntu32 dependencies
  ci: drop run-docker scripts

Merge branch 'jc/ci-upload-artifact-and-linux32'

CI started failing completely for linux32 jobs, as the step to
upload failed test directory uses GitHub actions that is deprecated
and is now disabled.  Remove the step so at least we will know if
the tests are passing.

* jc/ci-upload-artifact-and-linux32:
  ci: remove 'Upload failed tests' directories' step from linux32 jobs

Start preparing for Git 2.46.2

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'jk/ci-linux32-update' into maint-2.46

CI updates

* jk/ci-linux32-update:
  ci: add Ubuntu 16.04 job to GitLab CI
  ci: use regular action versions for linux32 job
  ci: use more recent linux32 image
  ci: unify ubuntu and ubuntu32 dependencies
  ci: drop run-docker scripts

Merge branch 'jc/ci-upload-artifact-and-linux32' into maint-2.46

CI started failing completely for linux32 jobs, as the step to
upload failed test directory uses GitHub actions that is deprecated
and is now disabled.  Remove the step so at least we will know if
the tests are passing.

* jc/ci-upload-artifact-and-linux32:
  ci: remove 'Upload failed tests' directories' step from linux32 jobs

Revert "Merge branch 'jc/patch-id' into maint-2.46"

This reverts commit 41c952ebacf7e3369e7bee721f768114d65e50c4,
reversing changes made to 712d970c0145b95ce655773e7cd1676f09dfd215.
Keeping a known breakage for now is better than introducing new
regression(s).

The seventeenth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'jk/ref-filter-trailer-fixes'

Bugfixes and leak plugging in "git for-each-ref --format=..." code
paths.

* jk/ref-filter-trailer-fixes:
  ref-filter: fix leak with unterminated %(if) atoms
  ref-filter: add ref_format_clear() function
  ref-filter: fix leak when formatting %(push:remoteref)
  ref-filter: fix leak with %(describe) arguments
  ref-filter: fix leak of %(trailers) "argbuf"
  ref-filter: store ref_trailer_buf data per-atom
  ref-filter: drop useless cast in trailers_atom_parser()
  ref-filter: strip signature when parsing tag trailers
  ref-filter: avoid extra copies of payload/signature
  t6300: drop newline from wrapped test title

Merge branch 'jc/range-diff-lazy-setup'

Code clean-up.

* jc/range-diff-lazy-setup:
  remerge-diff: clean up temporary objdir at a central place
  remerge-diff: lazily prepare temporary objdir on demand

Merge branch 'ah/apply-3way-ours'

"git apply --3way" learned to take "--ours" and other options.

* ah/apply-3way-ours:
  apply: support --ours, --theirs, and --union for three-way merges

Merge branch 'cp/unit-test-reftable-stack'

Another reftable test migrated to the unit-test framework.

* cp/unit-test-reftable-stack:
  t-reftable-stack: add test for stack iterators
  t-reftable-stack: add test for non-default compaction factor
  t-reftable-stack: use reftable_ref_record_equal() to compare ref records
  t-reftable-stack: use Git's tempfile API instead of mkstemp()
  t: harmonize t-reftable-stack.c with coding guidelines
  t: move reftable/stack_test.c to the unit testing framework

cbtree: fix a typo

Fix a typo in comments.

Signed-off-by: Andrew Kreimer <algonell@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

bloom: fix a typo

Fix a typo in comments.

Signed-off-by: Andrew Kreimer <algonell@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

attr: fix a typo

Fix a typo in comments.

Signed-off-by: Andrew Kreimer <algonell@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t9001: use a more distinct fake BugID

In the test "cc list is sanitized", we feed a commit with a variety of
trailers to send-email, and then check its output to see how it handled
them. For most of them, we are grepping for a specific mention of the
header, but there's a "BugID" header which we expect to be ignored. We
confirm this by grepping for "12345", the fake BugID, and making sure it
is not present.

But we can be fooled by false positives! I just tracked down a flaky
test failure here that was caused by matching this unrelated line in the
output:

  <20240914090449.612345-1-author@example.com>

which will change from run to run based on the time, pid, etc.

Ideally we'd tighten the regex to make this more specifically, but since
the point is that it _shouldn't_ be mentioned, it's hard to say what the
right match would be (e.g., would there be a leading space?).

Instead, let's just choose a match that is much less likely to appear.
The actual content of the header isn't important, since it's supposed to
be ignored.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

git-jump: ignore deleted files in diff mode

If you do something like this:

  rm file_a
  echo change >file_b
  git jump diff

then we'll generate two quickfix entries for the diff, one for each
file. But the one for the deleted file is rather pointless. There's no
content to show since the file is gone, and in fact we open the editor
with the path /dev/null!

In vim, at least, the result is a confusing annoyance: the editor opens
with an empty buffer, and you have to skip past it to the useful
quickfix entry (after scratching your head and figuring out that no,
nothing is broken).

Let's skip such entries entirely. There's nothing useful to show, since
the point is that the file has been deleted.

It is possible that you could be doing a diff whose post-image is not
the working tree, and then you'd perhaps be jumping to the deleted
content (or at least something that was in the same spot). But I don't
think it's worth worrying about that case. For one thing, using git-jump
for such diffs is a bad idea in general, as it's going to sometimes move
you to the wrong spot. And two, a deletion is always going to have one
hunk starting at line 1, which is not that interesting to jump to in the
first place.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

git-jump: always specify column 1 for diff entries

When we generate a quickfix entry for a diff hunk, we provide just the
filename and line number along with the content, like:

  file:1: contents of the line

This can be a problem if the line itself looks like a quickfix header.
For example (and this is adapted from a real-world case that bit me):

  echo 'static_lease 10:11:12:13:14:15:16 10.0.0.1' >file
  git add file
  echo change >file

produces:

  file:1: static_lease 10:11:12:13:14:15:16 10.0.0.1

which is ambiguous. It could be line 1 of "file", or line 11 of the file
"file:1: static_lease 10", and so on. In the case of vim's default
config, it seems to prefer the latter (you can configure "errorformat"
with a variety of patterns, but out of the box it matches some common
ones).

One easy way to fix this is to provide a column number, like:

  file:1:1: static_lease 10:11:12:13:14:15:16 10.0.0.1

which causes vim to prefer line 1 of "file" again (due to the preference
order of the various patterns in the default errorformat).

There are other options. For example, at least in my version of vim,
wrapping the file in quotation marks like:

  "file":1: static_lease 10:11:12:13:14:15:16 10.0.0.1

also works. That perhaps would the right thing even if you had the silly
file name "file:1:1: foo 10". But it's not clear what would happen if
you had a filename with quotes in it.

This feature is inherently scraping text, and there's bound to be some
ambiguities. I don't think it's worth worrying too much about unlikely
filenames, as its the file content that is more likely to introduce
unexpected characters.

So let's just go with the extra ":1" column specifier. We know this is
supported everywhere, as git-jump's "grep" mode already uses it (and
thus doesn't exhibit the same problem).

The "merge" mode is mostly immune to this, as it only matches "<<<<<<<"
conflict marker lines. It's possible of course to have a marker that
says "foo 10:11" later in the line, but in practice these will only have
branches and perhaps file names, so it's probably not worth worrying
about (and fixing it would involve passing --column to the system grep,
which may not be portable).

I also gave some thought as to whether we could put something more
useful than "1" in the column field for diffs. In theory we could find
the first changed character of the line, but this is tricky in practice.
You'd have to correlate before/after lines of the hunk to decide what
changed. So:

  -this is a foo line
  +this is a bar line

is easy (column 11). But:

  -this is a foo line
  +another line
  +this is a bar line

is harder.

This commit certainly doesn't preclude trying to do something more
clever later, but it's a much deeper rabbit hole than just fixing the
syntactic ambiguity.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

print an error when remote helpers die during capabilities

The transport-helper code generally relies on the
remote-helper to provide an informative message to the user
when it encounters an error. In the rare cases where the
helper does not do so, the output can be quite confusing.
E.g.:

  $ git clone https://example.com/foo.git
  Cloning into 'foo'...
  $ echo $?
  128
  $ ls foo
  /bin/ls: cannot access foo: No such file or directory

We tried to address this with 81d340d (transport-helper:
report errors properly, 2013-04-10).

But that makes the common case much more confusing. The
remote helper protocol's method for signaling normal errors
is to simply hang up. So when the helper does encounter a
routine error and prints something to stderr, the extra
error message is redundant and misleading. So we dropped it
again in 266f1fd (transport-helper: be quiet on read errors
from helpers, 2013-06-21).

This puts the uncommon case right back where it started. We
may be able to do a little better, though. It is common for
the helper to die during a "real" command, like fetching the
list of remote refs. It is not common for it to die during
the initial "capabilities" negotiation, right after we
start. Reporting failure here is likely to catch fundamental
problems that prevent the helper from running (and reporting
errors) at all. Anything after that is the responsibility of
the helper itself to report.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Sync with Git 2.46.1

The sixteenth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'bl/trailers-and-incomplete-last-line-fix'

The interpret-trailers command failed to recognise the end of the
message when the commit log ends in an incomplete line.

* bl/trailers-and-incomplete-last-line-fix:
  interpret-trailers: handle message without trailing newline

Merge branch 'rj/cygwin-has-dev-tty'

Cygwin does have /dev/tty support that is needed by things like
single-key input mode.

* rj/cygwin-has-dev-tty:
  config.mak.uname: add HAVE_DEV_TTY to cygwin config section

Merge branch 'rs/diff-exit-code-fix'

In a few corner cases "git diff --exit-code" failed to report
"changes" (e.g., renamed without any content change), which has
been corrected.

* rs/diff-exit-code-fix:
  diff: report dirty submodules as changes in builtin_diff()
  diff: report copies and renames as changes in run_diff_cmd()

Merge branch 'jc/doc-skip-fetch-all-and-prefetch'

Doc updates.

* jc/doc-skip-fetch-all-and-prefetch:
  doc: remote.*.skip{DefaultUpdate,FetchAll} stops prefetch

Merge branch 'ds/doc-wholesale-disabling-advice-messages'

The environment GIT_ADVICE has been intentionally kept undocumented
to discourage its use by interactive users.  Add documentation to
help tool writers.

* ds/doc-wholesale-disabling-advice-messages:
  advice: recommend GIT_ADVICE=0 for tools

Merge branch 'jk/sparse-fdleak-fix'

A file descriptor left open is now properly closed when "git
sparse-checkout" updates the sparse patterns.

* jk/sparse-fdleak-fix:
  sparse-checkout: use fdopen_lock_file() instead of xfdopen()
  sparse-checkout: check commit_lock_file when writing patterns
  sparse-checkout: consolidate cleanup when writing patterns

Merge branch 'ds/scalar-no-tags'

The "scalar clone" command learned the "--no-tags" option.

* ds/scalar-no-tags:
  scalar: add --no-tags option to 'scalar clone'

Git 2.46.1

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'rj/compat-terminal-unused-fix' into maint-2.46

Build fix.

* rj/compat-terminal-unused-fix:
  compat/terminal: mark parameter of git_terminal_prompt() UNUSED

Merge branch 'jc/config-doc-update' into maint-2.46

Docfix.

* jc/config-doc-update:
  git-config.1: fix description of --regexp in synopsis
  git-config.1: --get-all description update

Merge branch 'aa/cat-file-batch-output-doc' into maint-2.46

Docfix.

* aa/cat-file-batch-output-doc:
  docs: explain the order of output in the batched mode of git-cat-file(1)

Merge branch 'cl/config-regexp-docfix' into maint-2.46

Docfix.

* cl/config-regexp-docfix:
  doc: replace 3 dash with correct 2 dash in git-config(1)

Merge branch 'jc/coding-style-c-operator-with-spaces' into maint-2.46

Write down whitespacing rules around C opeators.

* jc/coding-style-c-operator-with-spaces:
  CodingGuidelines: spaces around C operators

Merge branch 'ps/stash-keep-untrack-empty-fix' into maint-2.46

A corner case bug in "git stash" was fixed.

* ps/stash-keep-untrack-empty-fix:
  builtin/stash: fix `--keep-index --include-untracked` with empty HEAD

Merge branch 'ps/index-pack-outside-repo-fix' into maint-2.46

"git verify-pack" and "git index-pack" started dying outside a
repository, which has been corrected.

* ps/index-pack-outside-repo-fix:
  builtin/index-pack: fix segfaults when running outside of a repo

Merge branch 'jk/free-commit-buffer-of-skipped-commits' into maint-2.46

The code forgot to discard unnecessary in-core commit buffer data
for commits that "git log --skip=<number>" traversed but omitted
from the output, which has been corrected.

* jk/free-commit-buffer-of-skipped-commits:
  revision: free commit buffers for skipped commits

add: pass in repo variable instead of global the_repository

With the repository variable available in the builtin function as an
argument, pass this down into helper functions instead of using the
global the_repository.

Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin: remove USE_THE_REPOSITORY for those without the_repository

For builtins that do not operate on a repository, remove
the #define USE_THE_REPOSITORY.

Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.h

Instead of including USE_THE_REPOSITORY_VARIABLE by default on every
builtin, remove it from builtin.h and add it to all the builtins that
include builtin.h (by definition, that means all builtins/*.c).

Also, remove the include statement for repository.h since it gets
brought in through builtin.h.

The next step will be to migrate each builtin
from having to use the_repository.

Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin: add a repository parameter for builtin functions

In order to reduce the usage of the global the_repository, add a
parameter to builtin functions that will get passed a repository
variable.

This commit uses UNUSED on most of the builtin functions, as subsequent
commits will modify the actual builtins to pass the repository parameter
down.

Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t5512.40 sometimes dies by SIGPIPE

The last test in t5512 we recently added seems to be flaky.
Running

    $ make && cd t && sh ./t5512-ls-remote.sh --stress

shows that "git ls-remote foo::bar" exited with status 141, which
means we got a SIGPIPE.  This test piece was introduced by 9e89dcb6
(builtin/ls-remote: fall back to SHA1 outside of a repo, 2024-08-02)
and is pretty much independent from all other tests in the script
(it can even run standalone with everything before it removed).

The transport-helper.c:get_helper() function tries to write to the
helper.  As we can see the helper script is very short and can exit
even before it reads anything, when get_helper() tries to give the
first command, "capabilities", the helper may already be gone.

A trivial fix, presented here, is to make sure that the helper reads
the first command it is given, as what it writes later is a response
to that command.

I however would wonder if the interactions with the helper initiated
by get_helper() should be done on a non-blocking I/O (we do check
the return value from our write(2) system calls, do we?).

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Git.pm: use "rev-parse --absolute-git-dir" rather than perl code

When we open a repository with the "Directory" option, we use "rev-parse
--git-dir" to get the path relative to that directory, and then use
Cwd::abs_path() to make it absolute (since our process working directory
may not be the same).

These days we can just ask for "--absolute-git-dir" instead, which saves
us a little code. That option was added in Git v2.13.0 via a2f5a87626
(rev-parse: add '--absolute-git-dir' option, 2017-02-03). I don't think
we make any promises about running mismatched versions of git and
Git.pm, but even if somebody tries it, that's sufficiently old that it
should be OK.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Git.pm: fix bare repository search with Directory option

When opening a bare repository like:

  Git->repository(Directory => '/path/to/bare.git');

we will incorrectly point the repository object at the _current_
directory, not the one specified by the option.

The bug was introduced by 20da61f25f (Git.pm: trust rev-parse to find
bare repositories, 2022-10-22). Before then, we'd ask "rev-parse
--git-dir" if it was a Git repo, and if it returned anything, we'd
correctly convert that result to an absolute path using File::Spec and
Cwd::abs_path(). If it didn't, we'd guess it might be a bare repository
and find it ourselves, which was wrong (rev-parse should find even a
bare repo, and our search circumvented some of its rules).

That commit dropped most of the custom bare-repo search code in favor of
using "rev-parse --is-bare-repository" and trusting the "--git-dir" it
returned. But it mistakenly left some of the bare-repo code path in
place, which was now broken. That code calls Cwd::abs_path($dir); prior
to 20da61f25f $dir contained the "Directory" option the user passed in.
But afterwards, it contains the output of "rev-parse --git-dir". And
since our tentative rev-parse command is invoked after changing
directory, it will always be the relative path "."! So we'll end up with
the absolute path of the process's current directory, not the Directory
option the caller asked for.

So the non-bare case is correct, but the bare one is broken. Our tests
only check the non-bare one, so we didn't notice. We can fix this by
running the same absolute-path fixup code for both sides.

Helped-by: Rodrigo <rodrigolive@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

ci: add Ubuntu 16.04 job to GitLab CI

In the preceding commits we had to convert the linux32 job to be based
on Ubuntu 20.04 instead of Ubuntu 16.04 due to a limitation in GitHub
Workflows. This was the only job left that still tested against this old
but supported Ubuntu version, and we have no other jobs that test with a
comparatively old Linux distribution.

Add a new job to GitLab CI that tests with Ubuntu 16.04 to cover the
resulting test gap. GitLab doesn't modify Docker images in the same way
GitHub does and thus doesn't fall prey to the same issue. There are two
compatibility issues uncovered by this:

  - Ubuntu 16.04 does not support HTTP/2 in Apache. We thus cannot set
    `GIT_TEST_HTTPD=true`, which would otherwise cause us to fail when
    Apache fails to start.

  - Ubuntu 16.04 cannot use recent JGit versions as they depend on a
    more recent Java runtime than we have available. We thus disable
    installing any kind of optional dependencies that do not come from
    the package manager.

These two restrictions are fine though, as we only really care about
whether Git compiles and runs on such old distributions in the first
place.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

unicode: update the width tables to Unicode 16

Unicode 16 has been announced on 2024-09-10 [0], so update the character
width tables to the new version.

[0] https://blog.unicode.org/2024/09/announcing-unicode-standard-version-160.html

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t/interop: allow per-version make options

Building older versions of Git may require tweaking some build knobs. In
particular, very old versions of Git will fail to build with recent
OpenSSL, because the bignum type switched from a struct to a pointer.

The i5500 interop test uses Git v1.0.0 by default, which triggers this
problem. You can work around it by setting NO_OPENSSL in your
GIT_TEST_MAKE_OPTS variable. But there are two downsides:

  1. You have to know to do this, and it's not at all obvious.

  2. That sets the options for _all_ versions of Git that we build. And
     it's possible for two versions to require conflicting knobs. E.g.,
     building with "make NO_OPENSSL=Nope OPENSSL_SHA1=Yes" causes
     imap-send.c to barf, because it declares a fallback typedef for SSL.
     This is something we may want to fix, but of course many historical
     versions are affected, and the interop scripts should be flexible
     enough to build everything.

So let's introduce per-version make options, along with the ability for
scripts to specify knobs that match their default versions. That should
make everything build out of the box, but also allow testers flexibility
if they are testing interoperability between non-default versions.

We'll set NO_OPENSSL by default for v1.0.0 in i5500. It doesn't have to
worry about the conflict with OPENSSL_SHA1 because imap-send did not
exist back then (but if it did, it could also just explicitly use a
different hash implementation).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Sync with 'maint'

The fifteenth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'kl/cat-file-on-sparse-index'

"git cat-file" works well with the sparse-index, and gets marked as
such.

* kl/cat-file-on-sparse-index:
  builtin/cat-file: mark 'git cat-file' sparse-index compatible
  t1092: allow run_on_* functions to use standard input

Merge branch 'jk/messages-with-excess-lf-fix'

One-line messages to "die" and other helper functions will get LF
added by these helper functions, but many existing messages had an
unnecessary LF at the end, which have been corrected.

* jk/messages-with-excess-lf-fix:
  drop trailing newline from warning/error/die messages

Merge branch 'ps/pack-refs-auto-heuristics'

"git pack-refs --auto" for the files backend was too aggressive,
which has been a bit tamed.

* ps/pack-refs-auto-heuristics:
  refs/files: use heuristic to decide whether to repack with `--auto`
  t0601: merge tests for auto-packing of refs
  wrapper: introduce `log2u()`

Merge branch 'tb/multi-pack-reuse-fix'

A data corruption bug when multi-pack-index is used and the same
objects are stored in multiple packfiles has been corrected.

* tb/multi-pack-reuse-fix:
  builtin/pack-objects.c: do not open-code `MAX_PACK_OBJECT_HEADER`
  pack-bitmap.c: avoid repeated `pack_pos_to_offset()` during reuse
  builtin/pack-objects.c: translate bit positions during pack-reuse
  pack-bitmap: tag bitmapped packs with their corresponding MIDX
  t/t5332-multi-pack-reuse.sh: verify pack generation with --strict

Merge branch 'gt/unit-test-oid-array'

Another unit-test.

* gt/unit-test-oid-array:
  t: port helper/test-oid-array.c to unit-tests/t-oid-array.c

Merge branch 'ps/index-pack-outside-repo-fix'

"git verify-pack" and "git index-pack" started dying outside a
repository, which has been corrected.

* ps/index-pack-outside-repo-fix:
  builtin/index-pack: fix segfaults when running outside of a repo

Merge branch 'jc/mailinfo-header-cleanup'

Code clean-up.

* jc/mailinfo-header-cleanup:
  mailinfo: we parse fixed headers

Another batch of topics for 2.46.1

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'jc/grammo-fixes' into maint-2.46

Doc updates.

* jc/grammo-fixes:
  doc: grammofix in git-diff-tree
  tutorial: grammofix

Merge branch 'jc/tests-no-useless-tee' into maint-2.46

Test fixes.

* jc/tests-no-useless-tee:
  tests: drop use of 'tee' that hides exit status

Merge branch 'jc/how-to-maintain-updates' into maint-2.46

Doc updates.

* jc/how-to-maintain-updates:
  howto-maintain: mention preformatted docs

Merge branch 'ps/bundle-outside-repo-fix' into maint-2.46

"git bundle unbundle" outside a repository triggered a BUG()
unnecessarily, which has been corrected.

* ps/bundle-outside-repo-fix:
  bundle: default to SHA1 when reading bundle headers
  builtin/bundle: have unbundle check for repo before opening its bundle

Merge branch 'jc/patch-id' into maint-2.46

The patch parser in "git patch-id" has been tightened to avoid
getting confused by lines that look like a patch header in the log
message.
cf. <Zqh2T_2RLt0SeKF7@tanuki>

* jc/patch-id:
  patch-id: tighten code to detect the patch header
  patch-id: rewrite code that detects the beginning of a patch
  patch-id: make get_one_patchid() more extensible
  patch-id: call flush_current_id() only when needed
  t4204: patch-id supports various input format

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