ls-refs: reject unknown arguments

The v2 ls-refs command may receive extra arguments from the client, one
per pkt-line. The spec is pretty clear that the arguments must come from
a specified set, but we silently ignore any unknown entries. For a
well-behaved client this doesn't matter, but it makes testing and
debugging more confusing. Let's tighten this up to match the spec.

In theory this liberal behavior _could_ be useful for extending the
protocol. But:

  - every other part of the protocol requires that the server first
    indicate that it supports the argument; this includes the fetch and
    object-info commands, plus the "unborn" capability added to ls-refs
    itself

  - it's not a very good extension mechanism anyway; without the server
    advertising support, clients would have no idea if the argument was
    silently ignored, or accepted and simply had no effect

So we're not really losing anything by tightening this.

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

serve: reject commands used as capabilities

Our table of v2 "capabilities" contains everything we might tell the
client we support. But there are differences in how we expect the client
to respond. Some of the entries are true capabilities (i.e., we expect
the client to say "yes, I support this"), and some are ones we expect
them to send as commands (with "command=ls-refs" or similar).

When we receive a capability used as a command, we complain about that.
But when we receive a command used as a capability (e.g., just "ls-refs"
in a pkt-line by itself), we silently ignore it.

This isn't really hurting anything (clients shouldn't send it, and we'll
ignore it), but we can tighten up the protocol to match what we expect
to happen.

There are two new tests here. The first one checks a capability used as
a command, which already passes. The second tests a command as a
capability, which this patch fixes.

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

serve: reject bogus v2 "command=ls-refs=foo"

When we see a line from the client like "command=ls-refs", we parse
everything after the equals sign as a capability, which we check against
our capabilities table. If we don't recognize the command (e.g.,
"command=foo"), we'll reject it.

But in parse_command(), we use the same get_capability() parser for
parsing non-command lines. So if we see "command=ls-refs=foo", we will
feed "ls-refs=foo" to get_capability(), which will say "OK, that's
ls-refs, with value 'foo'". But then we simply ignore the value
entirely.

The client is violating the spec here, which says:

      command = PKT-LINE("command=" key LF)
      key = 1*(ALPHA | DIGIT | "-_")

I.e., the key is not even allowed to have an equals sign in it. Whereas
a real non-command capability does allow a value:

      capability = PKT-LINE(key[=value] LF)

So by reusing the same get_capability() parser, we are mixing up the
"key" and "capability" tokens. However, since that parser tells us
whether it saw an "=", we can still use it; we just need to reject any
input that produces a non-NULL value field.

The current behavior isn't really hurting anything (the client should
never send such a request, and if it does, we just ignore the "value"
part). But since it does violate the spec, let's tighten it up to
prevent any surprising behavior.

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

docs/protocol-v2: clarify some ls-refs ref-prefix details

We've never documented the fact that a client can provide multiple
ref-prefix capabilities. Let's describe the behavior.

We also never discussed the "best effort" nature of the prefixes. The
client side of git.git has always treated them this way, filtering the
result with local patterns. And indeed any client must do this, because
the prefix patterns are not sufficient to express the usual refspecs
(and so for "foo" we ask for "refs/heads/foo", "refs/tags/foo", and so
on).

So this may be considered a change in the spec with respect to client
expectations / requirements, but it's mostly codifying existing
behavior.

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

ls-refs: ignore very long ref-prefix counts

Because each "ref-prefix" capability from the client comes in its own
pkt-line, there's no limit to the number of them that a misbehaving
client may send. We read them all into a strvec, which means the client
can waste arbitrary amounts of our memory by just sending us "ref-prefix
foo" over and over.

One possible solution is to just drop the connection when the limit is
reached. If we set it high enough, then only misbehaving or malicious
clients would hit it. But "high enough" is vague, and it's unfriendly if
we guess wrong and a legitimate client hits this.

But we can do better. Since supporting the ref-prefix capability is
optional anyway, the client has to further cull the response based on
their own patterns. So we can simply ignore the patterns once we cross a
certain threshold. Note that we have to ignore _all_ patterns, not just
the ones past our limit (since otherwise we'd send too little data).

The limit here is fairly arbitrary, and probably much higher than anyone
would need in practice. It might be worth limiting it further, if only
because we check it linearly (so with "m" local refs and "n" patterns,
we do "m * n" string comparisons). But if we care about optimizing this,
an even better solution may be a more advanced data structure anyway.

I didn't bother making the limit configurable, since it's so high and
since Git should behave correctly in either case. It wouldn't be too
hard to do, but it makes both the code and documentation more complex.

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

serve: drop "keys" strvec

We collect the set of capabilities the client sends us in a strvec.
While this is usually small, there's no limit to the number of
capabilities the client can send us (e.g., they could just send us
"agent" pkt-lines over and over, and we'd keep adding them to the list).

Since all code has been converted away from using this list, let's get
rid of it. This avoids a potential attack where clients waste our
memory.

Note that we do have to replace it with a flag, because some of the
flush-packet logic checks whether we've seen any valid commands or keys.

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

serve: provide "receive" function for session-id capability

Rather than pulling the session-id string from the list of collected
capabilities, we can handle it as soon as we receive it. This gets us
closer to dropping the collected list entirely.

The behavior should be the same, with one exception. Previously if the
client sent us multiple session-id lines, we'd report only the first.
Now we'll pass each one along to trace2. This shouldn't matter in
practice, since clients shouldn't do that (and if they do, it's probably
sensible to log them all).

As this removes the last caller of the static has_capability(), we can
remove it, as well (and in fact we must to avoid -Wunused-function
complaining).

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

serve: provide "receive" function for object-format capability

We get any "object-format" specified by the client by searching for it
in the collected list of capabilities the client sent. We can instead
just handle it as soon as they send it. This is slightly more efficient,
and gets us one step closer to dropping that collected list.

Note that we do still have to do our final hash check after receiving
all capabilities (because they might not have sent an object-format line
at all, and we still have to check that the default matches our
repository algorithm). Since the check_algorithm() function would now be
down to a single if() statement, I've just inlined it in its only
caller.

There should be no change of behavior here, except for two
broken-protocol cases:

  - if the client sends multiple conflicting object-format capabilities
    (which they should not), we'll now choose the last one rather than
    the first. We could also detect and complain about the duplicates
    quite easily now, which we could not before, but I didn't do so
    here.

  - if the client sends a bogus "object-format" with no equals sign,
    we'll now say so, rather than "unknown object format: ''"

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

serve: add "receive" method for v2 capabilities table

We have a capabilities table that tells us what we should tell the
client we are capable of, and what to do when a client gives us a
particular command (e.g., "command=ls-refs"). But it doesn't tell us
what to do when the client sends us back a capability (e.g.,
"object-format=sha256"). We just collect them all in a strvec and hope
somebody can use them later.

Instead, let's provide a function pointer in the table to act on these.
This will eventually help us avoid collecting the strings, which will be
more efficient and less prone to mischief.

Using the new method is optional, which helps in two ways:

  - we can move existing capabilities over to this new system gradually
    in individual commits

  - some capabilities we don't actually do anything with anyway. For
    example, the client is free to say "agent=git/1.2.3" to us, but we
    do not act on the information in any way.

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

serve: return capability "value" from get_capability()

When the client sends v2 capabilities, they may be simple, like:

  foo

or have a value like:

  foo=bar

(all of the current capabilities actually expect a value, but the
protocol allows for boolean ones).

We use get_capability() to make sure the client's pktline matches a
capability. In doing so, we parse enough to see the "=" and the value
(if any), but we immediately forget it. Nobody cares for now, because they end
up parsing the values out later using has_capability(). But in
preparation for changing that, let's pass back a pointer so the callers
know what we found.

Note that unlike has_capability(), we'll return NULL for a "simple"
capability. Distinguishing these will be useful for some future patches.

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

serve: rename is_command() to parse_command()

The is_command() function not only tells us whether the pktline is a
valid command string, but it also parses out the command (and complains
if we see a duplicate). Let's rename it to make those extra functions
a bit more obvious.

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

Merge branch 'ab/serve-cleanup' into jk/reduce-malloc-in-v2-servers

* ab/serve-cleanup:
  upload-pack: document and rename --advertise-refs
  serve.[ch]: remove "serve_options", split up --advertise-refs code
  {upload,receive}-pack tests: add --advertise-refs tests
  serve.c: move version line to advertise_capabilities()
  serve: move transfer.advertiseSID check into session_id_advertise()
  serve.[ch]: don't pass "struct strvec *keys" to commands
  serve: use designated initializers
  transport: use designated initializers
  transport: rename "fetch" in transport_vtable to "fetch_refs"
  serve: mark has_capability() as static

The fifth batch

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

Merge branch 'ab/help-autocorrect-prompt'

The logic for auto-correction of misspelt subcommands learned to go
interactive when the help.autocorrect configuration variable is set
to 'prompt'.

* ab/help-autocorrect-prompt:
  help.c: help.autocorrect=prompt waits for user action

Merge branch 'cb/ci-build-pedantic'

CI update.

* cb/ci-build-pedantic:
  ci: run a pedantic build as part of the GitHub workflow

Merge branch 'gh/gitweb-branch-sort'

Tie-break branches that point at the same object in the list of
branches on GitWeb to show the one pointed at by HEAD early.

* gh/gitweb-branch-sort:
  gitweb: use HEAD as secondary sort key in git_get_heads_list()

Merge branch 'rs/archive-use-object-id'

Code cleanup.

* rs/archive-use-object-id:
  archive: convert queue_directory to struct object_id

Merge branch 'rs/show-branch-simplify'

Code cleanup.

* rs/show-branch-simplify:
  show-branch: simplify rev_is_head()

Merge branch 'jk/log-warn-on-bogus-encoding'

Doc update plus improved error reporting.

* jk/log-warn-on-bogus-encoding:
  docs: use "character encoding" to refer to commit-object encoding
  logmsg_reencode(): warn when iconv() fails

Merge branch 'cb/remote-ndebug-fix'

Build fix.

* cb/remote-ndebug-fix:
  remote: avoid -Wunused-but-set-variable in gcc with -DNDEBUG

Merge branch 'ab/retire-advice-config'

Code clean up to migrate callers from older advice_config[] based
API to newer advice_if_enabled() and advice_enabled() API.

* ab/retire-advice-config:
  advice: move advice.graftFileDeprecated squashing to commit.[ch]
  advice: remove use of global advice_add_embedded_repo
  advice: remove read uses of most global `advice_` variables
  advice: add enum variants for missing advice variables

Merge branch 'mk/clone-recurse-submodules'

After "git clone --recurse-submodules", all submodules are cloned
but they are not by default recursed into by other commands.  With
submodule.stickyRecursiveClone configuration set, submodule.recurse
configuration is set to true in a repository created by "clone"
with "--recurse-submodules" option.

* mk/clone-recurse-submodules:
  clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled

Merge branch 'ab/mailmap-leakfix'

Leakfix.

* ab/mailmap-leakfix:
  mailmap.c: fix a memory leak in free_mailap_{info,entry}()

Merge branch 'ab/gc-log-rephrase'

A pathname in an advice message has been made cut-and-paste ready.

* ab/gc-log-rephrase:
  gc: remove trailing dot from "gc.log" line

Merge branch 'uk/userdiff-php-enum'

Update the userdiff pattern for PHP.

* uk/userdiff-php-enum:
  userdiff: support enum keyword in PHP hunk header

Merge branch 'tk/fast-export-anonymized-tag-fix'

The output from "git fast-export", when its anonymization feature
is in use, showed an annotated tag incorrectly.

* tk/fast-export-anonymized-tag-fix:
  fast-export: fix anonymized tag using original length

Merge branch 'ba/object-info'

Leakfix.

* ba/object-info:
  protocol-caps.c: fix memory leak in send_info()

Merge branch 'ab/commit-graph-usage'

Fixes on usage message from "git commit-graph".

* ab/commit-graph-usage:
  commit-graph: show "unexpected subcommand" error
  commit-graph: show usage on "commit-graph [write|verify] garbage"
  commit-graph: early exit to "usage" on !argc
  multi-pack-index: refactor "goto usage" pattern
  commit-graph: use parse_options_concat()
  commit-graph: remove redundant handling of -h
  commit-graph: define common usage with a macro

Merge branch 'mh/send-email-reset-in-reply-to'

Even when running "git send-email" without its own threaded
discussion support, a threading related header in one message is
carried over to the subsequent message to result in an unwanted
threading, which has been corrected.

* mh/send-email-reset-in-reply-to:
  send-email: avoid incorrect header propagation

Merge branch 'rs/more-fspathcmp'

Code simplification.

* rs/more-fspathcmp:
  merge-recursive: use fspathcmp() in path_hashmap_cmp()

Merge branch 'sg/set-ceiling-during-tests'

Buggy tests could damage repositories outside the throw-away test
area we created.  We now by default export GIT_CEILING_DIRECTORIES
to limit the damage from such a stray test.

* sg/set-ceiling-during-tests:
  test-lib: set GIT_CEILING_DIRECTORIES to protect the surrounding repository

Merge branch 'jh/sparse-index-resize-fix'

The sparse-index support can corrupt the index structure by storing
a stale and/or uninitialized data, which has been corrected.

* jh/sparse-index-resize-fix:
  sparse-index: copy dir_hash in ensure_full_index()

Merge branch 'es/walken-tutorial-fix'

Typofix.

* es/walken-tutorial-fix:
  doc: fix syntax error and the format of printf

Merge branch 'tb/add-objects-in-unpacked-packs-simplify'

Code simplification with reduced memory usage.

* tb/add-objects-in-unpacked-packs-simplify:
  builtin/pack-objects.c: remove duplicate hash lookup
  builtin/pack-objects.c: simplify add_objects_in_unpacked_packs()
  object-store.h: teach for_each_packed_object to ignore kept packs

Merge branch 'ps/fetch-omit-formatting-under-quiet'

"git fetch --quiet" optimization to avoid useless computation of
info that will never be displayed.

* ps/fetch-omit-formatting-under-quiet:
  fetch: skip formatting updated refs with `--quiet`

Merge branch 'ka/want-ref-in-namespace'

"git upload-pack" which runs on the other side of "git fetch"
forgot to take the ref namespaces into account when handling
want-ref requests.

* ka/want-ref-in-namespace:
  docs: clarify the interaction of transfer.hideRefs and namespaces
  upload-pack.c: treat want-ref relative to namespace
  t5730: introduce fetch command helper

Merge branch 'zh/cherry-pick-advice'

The advice message that "git cherry-pick" gives when it asks
conflicted replay of a commit to be resolved by the end user has
been updated.

* zh/cherry-pick-advice:
  cherry-pick: use better advice message

Merge branch 'js/advise-when-skipping-cherry-picked'

"git rebase" by default skips changes that are equivalent to
commits that are already in the history the branch is rebased onto;
give messages when this happens to let the users be aware of
skipped commits, and also teach them how to tell "rebase" to keep
duplicated changes.

* js/advise-when-skipping-cherry-picked:
  sequencer: advise if skipping cherry-picked commit

The fourth batch

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

Merge branch 'sg/column-nl'

The parser for the "--nl" option of "git column" has been
corrected.

* sg/column-nl:
  column: fix parsing of the '--nl' option

Merge branch 'cb/makefile-apple-clang'

Build update for Apple clang.

* cb/makefile-apple-clang:
  build: catch clang that identifies itself as "$VENDOR clang"
  build: clang version may not be followed by extra words
  build: update detect-compiler for newer Xcode version

Merge branch 'ps/ls-refs-strbuf-optim'

Micro-optimization for the wire protocol driver.

* ps/ls-refs-strbuf-optim:
  ls-refs: reuse buffer when sending refs

Merge branch 'rs/branch-allow-deleting-dangling'

"git branch -D <branch>" used to refuse to remove a broken branch
ref that points at a missing commit, which has been corrected.

* rs/branch-allow-deleting-dangling:
  branch: allow deleting dangling branches with --force

Merge branch 'mt/quiet-with-delayed-checkout'

The delayed checkout code path in "git checkout" etc. were chatty
even when --quiet and/or --no-progress options were given.

* mt/quiet-with-delayed-checkout:
  checkout: make delayed checkout respect --quiet and --no-progress

Merge branch 'rs/xopen-reports-open-failures'

Error diagnostics improvement.

* rs/xopen-reports-open-failures:
  use xopen() to handle fatal open(2) failures
  xopen: explicitly report creation failures

Merge branch 'dd/diff-files-unmerged-fix'

"git diff --relative" segfaulted and/or produced incorrect result
when there are unmerged paths.

* dd/diff-files-unmerged-fix:
  diff-lib: ignore paths that are outside $cwd if --relative asked

Merge branch 'dd/t6300-wo-gpg-fix'

Test fix.

* dd/t6300-wo-gpg-fix:
  t6300: check for cat-file exit status code
  t6300: don't run cat-file on non-existent object

Merge branch 'mh/credential-leakfix'

Leak fix.

* mh/credential-leakfix:
  credential: fix leak in credential_apply_config()

Merge branch 'jk/t5323-no-pack-test-fix'

Test fix.

* jk/t5323-no-pack-test-fix:
  t5323: drop mentions of "master"

Merge branch 'js/maintenance-launchctl-fix'

"git maintenance" scheduler fix for macOS.

* js/maintenance-launchctl-fix:
  maintenance: skip bootout/bootstrap when plist is registered
  maintenance: create `launchctl` configuration using a lock file

Merge branch 'ab/rebase-fatal-fatal-fix'

Error message fix.

* ab/rebase-fatal-fatal-fix:
  rebase: emit one "fatal" in "fatal: fatal: <error>"

Merge branch 'ab/ls-remote-packet-trace'

Debugging aid fix.

* ab/ls-remote-packet-trace:
  ls-remote: set packet_trace_identity(<name>)

Merge branch 'rs/git-mmap-uses-malloc'

mmap() imitation used to call xmalloc() that dies upon malloc()
failure, which has been corrected to just return an error to the
caller to be handled.

* rs/git-mmap-uses-malloc:
  compat: let git_mmap use malloc(3) directly

Merge branch 'ga/send-email-sendmail-cmd'

Test fix.

* ga/send-email-sendmail-cmd:
  t9001: PATH must not use Windows-style paths

Merge branch 'me/t5582-cleanup'

Test fix.

* me/t5582-cleanup:
  t5582: remove spurious 'cd "$D"' line

The third batch

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

Merge branch 'sg/make-fix-ar-invocation'

Build fix.

* sg/make-fix-ar-invocation:
  Makefile: remove archives before manipulating them with 'ar'

Merge branch 'ti/tcsh-completion-regression-fix'

Update to the command line completion (in contrib/) for tcsh.

* ti/tcsh-completion-regression-fix:
  completion: tcsh: Fix regression by drop of wrapper functions

Merge branch 'fc/completion-updates'

Command line completion updates.

* fc/completion-updates:
  completion: bash: add correct suffix in variables
  completion: bash: fix for multiple dash commands
  completion: bash: fix for suboptions with value
  completion: bash: fix prefix detection in branch.*

Merge branch 'pw/rebase-r-fixes'

Various bugs in "git rebase -r" have been fixed.

* pw/rebase-r-fixes:
  rebase -r: fix merge -c with a merge strategy
  rebase -r: don't write .git/MERGE_MSG when fast-forwarding
  rebase -i: add another reword test
  rebase -r: make 'merge -c' behave like reword

Merge branch 'pw/rebase-skip-final-fix'

Checking out all the paths from HEAD during the last conflicted
step in "git rebase" and continuing would cause the step to be
skipped (which is expected), but leaves MERGE_MSG file behind in
$GIT_DIR and confuses the next "git commit", which has been
corrected.

* pw/rebase-skip-final-fix:
  rebase --continue: remove .git/MERGE_MSG
  rebase --apply: restore some tests
  t3403: fix commit authorship

Merge branch 'cb/ci-use-upload-artifacts-v1'

Use upload-artifacts v1 (instead of v2) for 32-bit linux, as the
new version has a blocker bug for that architecture.

* cb/ci-use-upload-artifacts-v1:
  ci: use upload-artifacts v1 for dockerized jobs

Merge branch 'jk/commit-edit-fixup-fix'

"git commit --fixup" now works with "--edit" again, after it was
broken in v2.32.

* jk/commit-edit-fixup-fix:
  commit: restore --edit when combined with --fixup

Merge branch 'ps/connectivity-optim'

The revision traversal API has been optimized by taking advantage
of the commit-graph, when available, to determine if a commit is
reachable from any of the existing refs.

* ps/connectivity-optim:
  revision: avoid hitting packfiles when commits are in commit-graph
  commit-graph: split out function to search commit position
  revision: stop retrieving reference twice
  connected: do not sort input revisions
  revision: separate walk and unsorted flags

remote: avoid -Wunused-but-set-variable in gcc with -DNDEBUG

In make_remote(), we store the return value of hashmap_put() and check
it using assert(), but don't otherwise use it. If Git is compiled with
NDEBUG, then the assert() becomes a noop, and nobody looks at the
variable at all. This causes some compilers to produce warnings.

Let's switch it instead to a BUG(). This accomplishes the same thing,
but is always compiled in (and we don't have to worry about the cost;
the check is cheap, and this is not a hot code path).

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

gc: remove trailing dot from "gc.log" line

Remove the trailing dot from the warning we emit about gc.log. It's
common for various terminal UX's to allow the user to select "words",
and by including the trailing dot a user wanting to select the path to
gc.log will need to manually remove the trailing dot.

Such a user would also probably need to adjust the path if it e.g. had
spaces in it, but this should address this very common case.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Suggested-by: Jan Judas <snugar.i@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

docs: clarify the interaction of transfer.hideRefs and namespaces

Expand the section about namespaces in the documentation of
`transfer.hideRefs` to point out the subtle differences between
`upload-pack` and `receive-pack`.

ffcfb68176 (upload-pack.c: treat want-ref relative to namespace,
2021-07-30) taught `upload-pack` to reject `want-ref`s for hidden refs,
which is now mentioned. It is clarified that at no point the name of a
hidden ref is revealed, but the object id it points to may.

Signed-off-by: Kim Altintop <kim@eagain.st>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

upload-pack.c: treat want-ref relative to namespace

When 'upload-pack' runs within the context of a git namespace, treat any
'want-ref' lines the client sends as relative to that namespace.

Also check if the wanted ref is hidden via 'hideRefs'. If it is hidden,
respond with an error as if the ref didn't exist.

Helped-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Kim Altintop <kim@eagain.st>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t5730: introduce fetch command helper

Assembling a "raw" fetch command to be fed directly to "test-tool serve-v2"
is extracted into a test helper.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kim Altintop <kim@eagain.st>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

mailmap.c: fix a memory leak in free_mailap_{info,entry}()

In the free_mailmap_entry() code added in 0925ce4d49 (Add map_user()
and clear_mailmap() to mailmap, 2009-02-08) the intent was clearly to
clear the "me" structure, but while we freed parts of the
mailmap_entry structure, we didn't free the structure itself. The same
goes for the "mailmap_info" structure.

This brings the number of SANITIZE=leak failures in t4203-mailmap.sh
down from 50 to 49. Not really progress as far as the number of
failures is concerned, but as far as I can tell this fixes all leaks
in mailmap.c itself. There's still users of it such as builtin/log.c
that call read_mailmap() without a clear_mailmap(), but that's on
them.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

userdiff: support enum keyword in PHP hunk header

"enum" keyword will be introduced in PHP 8.1.
https://wiki.php.net/rfc/enumerations

Signed-off-by: USAMI Kenta <tadsan@zonu.me>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fast-export: fix anonymized tag using original length

Commit 7f4075949686 (fast-export: tighten anonymize_mem() interface to
handle only strings, 2020-06-23) changed the interface used in anonymizing
strings, but failed to update the size of annotated tag messages to match
the new anonymized string.

As a result, exporting tags having messages longer than 13 characters
would create output that couldn't be parsed by fast-import,
as the data length indicated was larger than the data output.

Reset the message size when anonymizing, and add a tag with a "long"
message to the test.

Signed-off-by: Tal Kelrich <hasturkun@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

protocol-caps.c: fix memory leak in send_info()

Fix a memory leak in a2ba162cda (object-info: support for retrieving
object info, 2021-04-20) which appears to have been based on a
misunderstanding of how the pkt-line.c API works. There is no need to
strdup() input to packet_writer_write(), it's just a printf()-like
format function.

This fixes a potentially large memory leak, since the number of OID
lines the "object-info" call can be arbitrarily large (or a small one
if the request is small).

This makes t5701-git-serve.sh pass again under SANITIZE=leak, as it
did before a2ba162cda2.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Acked-by: Bruno Albuquerque <bga@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit-graph: show "unexpected subcommand" error

Bring the "commit-graph" command in line with the error output and
general pattern in cmd_multi_pack_index().

Let's test for that output, and also cover the same potential bug as
was fixed in the multi-pack-index command in
88617d11f9d (multi-pack-index: fix potential segfault without
sub-command, 2021-07-19).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit-graph: show usage on "commit-graph [write|verify] garbage"

Change the parse_options() invocation in the commit-graph code to
error on unknown leftover argv elements, in addition to the existing
and implicit erroring via parse_options() on unknown options.

We'd already error in cmd_commit_graph() on e.g.:

    git commit-graph unknown verify
    git commit-graph --unknown verify

But here we're calling parse_options() twice more for the "write" and
"verify" subcommands. We did not do the same checking for leftover
argv elements there. As a result we'd silently accept garbage in these
subcommands, let's not do that.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit-graph: early exit to "usage" on !argc

Rather than guarding all of the !argc with an additional "if" arm
let's do an early goto to "usage". This also makes it clear that
"save_commit_buffer" is not needed in this case.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

multi-pack-index: refactor "goto usage" pattern

Refactor the "goto usage" pattern added in
cd57bc41bbc (builtin/multi-pack-index.c: display usage on unrecognized
command, 2021-03-30) and 88617d11f9d (multi-pack-index: fix potential
segfault without sub-command, 2021-07-19) to maintain the same
brevity, but in a form that doesn't run afoul of the recommendation in
CodingGuidelines about braces:

    When there are multiple arms to a conditional and some of them
    require braces, enclose even a single line block in braces for
    consistency[...]

Let's also change "argv == 0" to juts "!argv", per:

    Do not explicitly compare an integral value with constant 0 or
    '\0', or a pointer value with constant NULL[...]

I'm changing this because in a subsequent commit I'll make
builtin/commit-graph.c use the same pattern, having the two similarly
structured commands match aids readability.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit-graph: use parse_options_concat()

Make use of the parse_options_concat() so we don't need to copy/paste
common options like --object-dir.

This is inspired by a similar change to "checkout" in 2087182272
(checkout: split options[] array in three pieces, 2019-03-29), and the
same pattern in the multi-pack-index command, see
60ca94769ce (builtin/multi-pack-index.c: split sub-commands,
2021-03-30).

A minor behavior change here is that now we're going to list both
--object-dir and --progress first, before we'd list --progress along
with other options.

Co-authored-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit-graph: remove redundant handling of -h

If we don't handle the -h option here like most parse_options() users
we'll fall through and it'll do the right thing for us.

I think this code added in 4ce58ee38d (commit-graph: create
git-commit-graph builtin, 2018-04-02) was always redundant,
parse_options() did this at the time, and the commit-graph code never
used PARSE_OPT_NO_INTERNAL_HELP.

We don't need a test for this, it's tested by the t0012-help.sh test
added in d691551192a (t0012: test "-h" with builtins, 2017-05-30).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit-graph: define common usage with a macro

Share the usage message between these three variables by using a
macro. Before this new options needed to copy/paste the usage
information, see e.g. 809e0327f5 (builtin/commit-graph.c: introduce
'--max-new-filters=<n>', 2020-09-18).

See b25b727494f (builtin/multi-pack-index.c: define common usage with
a macro, 2021-03-30) for another use of this pattern (but on-list this
one came first).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sequencer: advise if skipping cherry-picked commit

Silently skipping commits when rebasing with --no-reapply-cherry-picks
(currently the default behavior) can cause user confusion. Issue
warnings when this happens, as well as advice on how to preserve the
skipped commits.

These warnings and advice are displayed only when using the (default)
"merge" rebase backend.

Update the git-rebase docs to mention the warnings and advice.

Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

The second batch

The most significant of this batch is of course "merge -sort".
Thanks, Elijah and everybody who helped the topic.

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

Merge branch 'cb/ci-freebsd-update'

Update FreeBSD CI job

* cb/ci-freebsd-update:
  ci: update freebsd 12 cirrus job

Merge branch 'tl/traverse-non-commits-rename'

Meh.

* tl/traverse-non-commits-rename:
  list-objects.c: rename "traverse_trees_and_blobs" to "traverse_non_commits"

Merge branch 'bc/t5607-avoid-broken-test-fail-prereqs'

The current implementation of GIT_TEST_FAIL_PREREQS is broken in
that checking for the lack of a prerequisite would not work.  Avoid
the use of "if ! test_have_prereq X" in a test script.

* bc/t5607-avoid-broken-test-fail-prereqs:
  t5607: avoid using prerequisites to select algorithm

Merge branch 'th/userdiff-more-java'

The userdiff pattern for "java" language has been updated.

* th/userdiff-more-java:
  userdiff: improve java hunk header regex

Merge branch 'jk/range-diff-fixes'

"git range-diff" code clean-up.

* jk/range-diff-fixes:
  range-diff: use ssize_t for parsed "len" in read_patches()
  range-diff: handle unterminated lines in read_patches()
  range-diff: drop useless "offset" variable from read_patches()

Merge branch 'jk/apply-binary-hunk-parsing-fix'

"git apply" miscounted the bytes and failed to read to the end of
binary hunks.

* jk/apply-binary-hunk-parsing-fix:
  apply: keep buffer/size pair in sync when parsing binary hunks

Merge branch 'jc/userdiff-pattern-hint'

Remind developers that the userdiff patterns should be kept simple
and permissive, assuming that the contents they apply are always
syntactically correct.

* jc/userdiff-pattern-hint:
  userdiff: comment on the builtin patterns

Merge branch 'cb/builtin-merge-format-string-fix'

Code clean-up.

* cb/builtin-merge-format-string-fix:
  builtin/merge: avoid -Wformat-extra-args from ancient Xcode

Merge branch 'js/log-protocol-version'

Debugging aid.

* js/log-protocol-version:
  connect, protocol: log negotiated protocol version

Merge branch 'en/ort-becomes-the-default'

Use `ort` instead of `recursive` as the default merge strategy.

* en/ort-becomes-the-default:
  Update docs for change of default merge backend
  Change default merge backend from recursive to ort

Merge branch 'en/merge-strategy-docs'

Documentation updates.

* en/merge-strategy-docs:
  Update error message and code comment
  merge-strategies.txt: add coverage of the `ort` merge strategy
  git-rebase.txt: correct out-of-date and misleading text about renames
  merge-strategies.txt: fix simple capitalization error
  merge-strategies.txt: avoid giving special preference to patience algorithm
  merge-strategies.txt: do not imply using copy detection is desired
  merge-strategies.txt: update wording for the resolve strategy
  Documentation: edit awkward references to `git merge-recursive`
  directory-rename-detection.txt: small updates due to merge-ort optimizations
  git-rebase.txt: correct antiquated claims about --rebase-merges

Merge branch 'en/pull-conflicting-options'

"git pull" had various corner cases that were not well thought out
around its --rebase backend, e.g. "git pull --ff-only" did not stop
but went ahead and rebased when the history on other side is not a
descendant of our history.  The series tries to fix them up.

* en/pull-conflicting-options:
  pull: fix handling of multiple heads
  pull: update docs & code for option compatibility with rebasing
  pull: abort by default when fast-forwarding is not possible
  pull: make --rebase and --no-rebase override pull.ff=only
  pull: since --ff-only overrides, handle it first
  pull: abort if --ff-only is given and fast-forwarding is impossible
  t7601: add tests of interactions with multiple merge heads and config
  t7601: test interaction of merge/rebase/fast-forward flags and options

clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled

Based on current experience, when running git clone --recurse-submodules,
developers do not expect other commands such as pull or checkout to run
recursively into active submodules. However, setting submodule.recurse=true
at this step could make for a simpler workflow by eliminating the need for
the --recurse-submodules option in subsequent commands. To collect more
data on developers' preference in regards to making submodule.recurse=true
a default config value in the future, deploy this feature under the opt in
submodule.stickyRecursiveClone flag.

Signed-off-by: Mahi Kolla <mkolla2@illinois.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

send-email: avoid incorrect header propagation

If multiple independent patches are sent with send-email, even if the
"In-Reply-To" and "References" headers are not managed by --thread or
--in-reply-to, their values may be propagated from prior patches to
subsequent patches with no such headers defined.

To mitigate this and potential future issues, make sure all global
patch-specific variables are always either handled by
command-specific code (e.g. threading), or are reset to their default
values for every iteration.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fetch: skip formatting updated refs with `--quiet`

When fetching, Git will by default print a list of all updated refs in a
nicely formatted table. In order to come up with this table, Git needs
to iterate refs twice: first to determine the maximum column width, and
a second time to actually format these changed refs.

While this table will not be printed in case the user passes `--quiet`,
we still go out of our way and do all these steps. In fact, we even do
more work compared to not passing `--quiet`: without the flag, we will
skip all references in the column width computation which have not been
updated, but if it is set we will now compute widths for all refs.

Fix this issue by completely skipping both preparation of the format and
formatting data for display in case the user passes `--quiet`, improving
performance especially with many refs. The following benchmark shows a
nice speedup for a quiet mirror-fetch in a repository with 2.3M refs:

    Benchmark #1: HEAD~: git-fetch
      Time (mean ± σ):     26.929 s ±  0.145 s    [User: 24.194 s, System: 4.656 s]
      Range (min … max):   26.692 s … 27.068 s    5 runs

    Benchmark #2: HEAD: git-fetch
      Time (mean ± σ):     25.189 s ±  0.094 s    [User: 22.556 s, System: 4.606 s]
      Range (min … max):   25.070 s … 25.314 s    5 runs

    Summary
      'HEAD: git-fetch' ran
        1.07 ± 0.01 times faster than 'HEAD~: git-fetch'

While at it, this patch also fixes `adjust_refcol_width()` such that it
skips unchanged refs in case the user passed `--quiet`, where verbosity
will be negative. While this function won't be called anymore if so,
this brings the comment in line with actual code. Furthermore, needless
`verbosity >= 0` checks are now removed in `store_updated_refs()`: we
never print to the `note` buffer anymore in case `verbosity < 0`, so we
won't end up in that code block anyway.

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

merge-recursive: use fspathcmp() in path_hashmap_cmp()

Call fspathcmp() instead of open-coding it.  This shortens the code and
makes it less repetitive.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

test-lib: set GIT_CEILING_DIRECTORIES to protect the surrounding repository

Every once in a while a test somehow manages to escape from its trash
directory and modifies the surrounding repository, whether because of
a bug in git itself, a bug in a test [1], or e.g. when trying to run
tests with a shell that is, in general, unable to run our tests [2].

Set GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/.." as an additional
safety measure to protect the surrounding repository at least from
modifications by git commands executed in the tests (assuming that
handling of ceiling directories during repository discovery is not
broken, and, of course, it won't save us from regular shell commands,
e.g. 'cd .. && rm -f ...').

[1] e.g. https://public-inbox.org/git/20210423051255.GD2947267@szeder.dev
[2] $ git symbolic-ref HEAD
    refs/heads/master
    $ ksh ./t2011-checkout-invalid-head.sh
    [... a lot of "not ok" ...]
    $ git symbolic-ref HEAD
    refs/heads/other

    (In short: 'ksh' doesn't support the 'local' builtin command,
    which is used by 'test_oid', causing it to return with error
    whenever it's called, leaving ZERO_OID set to empty, so when the
    test 'checkout main from invalid HEAD' runs 'echo $ZERO_OID
    >.git/HEAD' it writes a corrupt (not invalid) HEAD, and subsequent
    git commands don't recognize the repository in the trash directory
    anymore, but operate on the surrounding repo.)

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: fix syntax error and the format of printf

Fix syntax and correct the format of printf in MyFirstObjectWalk.txt

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