]> git.ipfire.org Git - thirdparty/git.git/log
thirdparty/git.git
3 years agodoc: interpret-trailers: use input redirection
Kristoffer Haugsbakk [Mon, 1 May 2023 20:02:39 +0000 (22:02 +0200)] 
doc: interpret-trailers: use input redirection

Use input redirection instead of invoking cat(1) on a single file. This
is more straightforward, saves a process, and often makes the line
shorter.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agodoc: interpret-trailers: don’t use heredoc in examples
Kristoffer Haugsbakk [Mon, 1 May 2023 20:02:38 +0000 (22:02 +0200)] 
doc: interpret-trailers: don’t use heredoc in examples

This file contains four instances of trailing spaces from its inception
in commit [1]. These spaces might be intentional, since a user would be
prompted with `> ` in an interactive session. On the one hand, this is a
whitespace error according to `git diff --check`; on the other hand, the
raw documentation—it makes no difference in the rendered output—is just
staying faithful to the simulation of the interactive prompt.

Let’s get rid of these whitespace errors and also make the examples more
friendly to cut-and-paste by replacing the heredocs with files which are
shown with cat(1).

[1]: dfd66ddf5a (Documentation: add documentation for 'git
    interpret-trailers', 2014-10-13)

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosetup: trace bare repository setups
Glen Choo [Mon, 1 May 2023 17:30:37 +0000 (10:30 -0700)] 
setup: trace bare repository setups

safe.bareRepository=explicit is a safer default mode of operation, since
it guards against the embedded bare repository attack [1]. Most end
users don't use bare repositories directly, so they should be able to
set safe.bareRepository=explicit, with the expectation that they can
reenable bare repositories by specifying GIT_DIR or --git-dir.

However, the user might use a tool that invokes Git on bare repositories
without setting GIT_DIR (e.g. "go mod" will clone bare repositories
[2]), so even if a user wanted to use safe.bareRepository=explicit, it
wouldn't be feasible until their tools learned to set GIT_DIR.

To make this transition easier, add a trace message to note when we
attempt to set up a bare repository without setting GIT_DIR. This allows
users and tool developers to audit which of their tools are problematic
and report/fix the issue.  When they are sufficiently confident, they
would switch over to "safe.bareRepository=explicit".

Note that this uses trace2_data_string(), which isn't supported by the
"normal" GIT_TRACE2 target, only _EVENT or _PERF.

[1] https://lore.kernel.org/git/kl6lsfqpygsj.fsf@chooglen-macbookpro.roam.corp.google.com/
[2] https://go.dev/ref/mod

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocontrib/credential: embiggen fixed-size buffer in wincred
Taylor Blau [Mon, 1 May 2023 15:54:06 +0000 (11:54 -0400)] 
contrib/credential: embiggen fixed-size buffer in wincred

As in previous commits, harden the wincred credential helper against the
aforementioned protocol injection attack.

Unlike the approached used for osxkeychain and libsecret, where a
fixed-size buffer was replaced with `getline()`, we must take a
different approach here. There is no `getline()` equivalent in Windows,
and the function is not available to us with ordinary compiler settings.

Instead, allocate a larger (still fixed-size) buffer in which to process
each line. The value of 100 KiB is chosen to match the maximum-length
header that curl will allow, CURL_MAX_HTTP_HEADER.

To ensure that we are reading complete lines at a time, and that we
aren't susceptible to a similar injection attack (albeit with more
padding), ensure that each read terminates at a newline (i.e., that no
line is more than 100 KiB long).

Note that it isn't sufficient to turn the old loop into something like:

    while (len && strchr("\r\n", buf[len - 1])) {
      buf[--len] = 0;
      ends_in_newline = 1;
    }

because if an attacker sends something like:

    [aaaaa.....]\r
    host=example.com\r\n

the credential helper would fill its buffer after reading up through the
first '\r', call fgets() again, and then see "host=example.com\r\n" on
its line.

Note that the original code was written in a way that would trim an
arbitrary number of "\r" and "\n" from the end of the string. We should
get only a single "\n" (since the point of `fgets()` is to return the
buffer to us when it sees one), and likewise would not expect to see
more than one associated "\r". The new code trims a single "\r\n", which
matches the original intent.

[1]: https://curl.se/libcurl/c/CURLOPT_HEADERFUNCTION.html

Tested-by: Matthew John Cheetham <mjcheetham@outlook.com>
Helped-by: Matthew John Cheetham <mjcheetham@outlook.com>
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocontrib/credential: avoid fixed-size buffer in libsecret
Taylor Blau [Mon, 1 May 2023 15:54:03 +0000 (11:54 -0400)] 
contrib/credential: avoid fixed-size buffer in libsecret

The libsecret credential helper reads the newline-delimited
protocol stream one line at a time by repeatedly calling fgets() into a
fixed-size buffer, and is thus affected by the vulnerability described
in the previous commit.

To mitigate this attack, avoid using a fixed-size buffer, and instead
rely on getline() to allocate a buffer as large as necessary to fit the
entire content of the line, preventing any protocol injection.

In most parts of Git we don't assume that every platform has getline().
But libsecret is primarily used on Linux, where we do already assume it
(using a knob in config.mak.uname). POSIX also added getline() in 2008,
so we'd expect other recent Unix-like operating systems to have it
(e.g., FreeBSD also does).

Note that the buffer was already allocated on the heap in this case, but
we'll swap `g_free()` for `free()`, since it will now be allocated by
the system `getline()`, rather than glib's `g_malloc()`.

Tested-by: Jeff King <peff@peff.net>
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocontrib/credential: .gitignore libsecret build artifacts
Taylor Blau [Mon, 1 May 2023 15:54:00 +0000 (11:54 -0400)] 
contrib/credential: .gitignore libsecret build artifacts

The libsecret credential helper does not mark its build artifact as
ignored, so running "make" results in a dirty working tree.

Mark the "git-credential-libsecret" binary as ignored to avoid the above.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocontrib/credential: remove 'gnome-keyring' credential helper
Taylor Blau [Mon, 1 May 2023 15:53:57 +0000 (11:53 -0400)] 
contrib/credential: remove 'gnome-keyring' credential helper

libgnome-keyring was deprecated in 2014 (in favor of libsecret), more
than nine years ago [1].

The credential helper implemented using libgnome-keyring has had a small
handful of commits since 2013, none of which implemented or changed any
functionality. The last commit to do substantial work in this area was
15f7221686 (contrib/git-credential-gnome-keyring.c: support really
ancient gnome-keyring, 2013-09-23), just shy of nine years ago.

This credential helper suffers from the same `fgets()`-related injection
attack (using the new "wwwauth[]" feature) as in the previous commit.
Instead of patching it, let's remove this helper as deprecated.

[1]: https://mail.gnome.org/archives/commits-list/2014-January/msg01585.html

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocontrib/credential: avoid fixed-size buffer in osxkeychain
Taylor Blau [Mon, 1 May 2023 15:53:54 +0000 (11:53 -0400)] 
contrib/credential: avoid fixed-size buffer in osxkeychain

The macOS Keychain-based credential helper reads the newline-delimited
protocol stream one line at a time by repeatedly calling fgets() into a
fixed-size buffer, and is thus affected by the vulnerability described
in the previous commit.

To mitigate this attack, avoid using a fixed-size buffer, and instead
rely on getline() to allocate a buffer as large as necessary to fit the
entire content of the line, preventing any protocol injection.

We solved a similar problem in a5bb10fd5e (config: avoid fixed-sized
buffer when renaming/deleting a section, 2023-04-06) by switching to
strbuf_getline(). We can't do that here because the contrib helpers do
not link with the rest of Git, and so can't use a strbuf. But we can use
the system getline() directly, which works similarly.

In most parts of Git we don't assume that every platform has getline().
But this helper is run only on OS X, and that platform added support in
10.7 ("Lion") which was released in 2011.

Tested-by: Taylor Blau <me@ttaylorr.com>
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot/lib-credential.sh: ensure credential helpers handle long headers
Taylor Blau [Mon, 1 May 2023 15:53:51 +0000 (11:53 -0400)] 
t/lib-credential.sh: ensure credential helpers handle long headers

Add a test ensuring that the "wwwauth[]" field cannot be used to
inject malicious data into the credential helper stream.

Many of the credential helpers in contrib/credential read the
newline-delimited protocol stream one line at a time by repeatedly
calling fgets() into a fixed-size buffer.

This assumes that each line is no more than 1024 characters long, since
each iteration of the loop assumes that it is parsing starting at the
beginning of a new line in the stream. However, similar to a5bb10fd5e
(config: avoid fixed-sized buffer when renaming/deleting a section,
2023-04-06), if a line is longer than 1024 characters, a malicious actor
can embed another command within an existing line, bypassing the usual
checks introduced in 9a6bbee800 (credential: avoid writing values with
newlines, 2020-03-11).

As with the problem fixed in that commit, specially crafted input can
cause the helper to return the credential for the wrong host, letting an
attacker trick the victim into sending credentials for one host to
another.

Luckily, all parts of the credential helper protocol that are available
in a tagged release of Git are immune to this attack:

  - "protocol" is restricted to known values, and is thus immune.

  - "host" is immune because curl will reject hostnames that have a '='
    character in them, which would be required to carry out this attack.

  - "username" is immune, because the buffer characters to fill out the
    first `fgets()` call would pollute the `username` field, causing the
    credential helper to return nothing (because it would match a
    username if present, and the username of the credential to be stolen
    is likely not 1024 characters).

  - "password" is immune because providing a password instructs
    credential helpers to avoid filling credentials in the first place.

  - "path" is similar to username; if present, it is not likely to match
    any credential the victim is storing. It's also not enabled by
    default; the victim would have to set credential.useHTTPPath
    explicitly.

However, the new "wwwauth[]" field introduced via 5f2117b24f
(credential: add WWW-Authenticate header to cred requests, 2023-02-27)
can be used to inject data into the credential helper stream. For
example, running:

    {
      printf 'HTTP/1.1 401\r\n'
      printf 'WWW-Authenticate: basic realm='
      perl -e 'print "a" x 1024'
      printf 'host=victim.com\r\n'
    } | nc -Nlp 8080

in one terminal, and then:

    git clone http://localhost:8080

in another would result in a line like:

    wwwauth[]=basic realm=aaa[...]aaahost=victim.com

being sent to the credential helper. If we tweak that "1024" to align
our output with the helper's buffer size and the rest of the data on the
line, it can cause the helper to see "host=victim.com" on its own line,
allowing motivated attackers to exfiltrate credentials belonging to
"victim.com".

The below test demonstrates these failures and provides us with a test
to ensure that our fix is correct. That said, it has a couple of
shortcomings:

  - it's in t0303, since that's the only mechanism we have for testing
    random helpers. But that means nobody is going to run it under
    normal circumstances.

  - to get the attack right, it has to line up the stuffed name with the
    buffer size, so we depend on the exact buffer size. I parameterized
    it so it could be used to test other helpers, but in practice it's
    not likely for anybody to do that.

Still, it's the best we can do, and will help us confirm the presence of
the problem (and our fixes) in the new few patches.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocredential.c: store "wwwauth[]" values in `credential_read()`
Taylor Blau [Mon, 1 May 2023 15:53:48 +0000 (11:53 -0400)] 
credential.c: store "wwwauth[]" values in `credential_read()`

Teach git-credential to read "wwwauth[]" value(s) when parsing the
output of a credential helper.

These extra headers are not needed for Git's own HTTP support to use the
feature internally, but the feature would not be available for a
scripted caller (say, git-remote-mediawiki providing the header in the
same way).

As a bonus, this also makes it easier to use wwwauth[] in synthetic
credential inputs in our test suite.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosend-email: detect empty blank lines in command output
Maxim Cournoyer [Mon, 1 May 2023 14:38:48 +0000 (10:38 -0400)] 
send-email: detect empty blank lines in command output

The email format does not allow blank lines in headers; detect such
input and report it as malformed and add a test for it.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosend-email: add --header-cmd, --no-header-cmd options
Maxim Cournoyer [Mon, 1 May 2023 14:38:47 +0000 (10:38 -0400)] 
send-email: add --header-cmd, --no-header-cmd options

Sometimes, adding a header different than CC or TO is desirable; for
example, when using Debbugs, it is best to use 'X-Debbugs-Cc' headers
to keep people in CC; this is an example use case enabled by the new
'--header-cmd' option.

The header unfolding logic is extracted to a subroutine so that it can
be reused; a test is added for coverage.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosend-email: extract execute_cmd from recipients_cmd
Maxim Cournoyer [Mon, 1 May 2023 14:38:46 +0000 (10:38 -0400)] 
send-email: extract execute_cmd from recipients_cmd

This refactor is to pave the way for the addition of the new
'--header-cmd' option to the send-email command.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot/t3501-revert-cherry-pick.sh: clarify scope of the file
Oswald Buddenhagen [Sun, 30 Apr 2023 10:00:34 +0000 (12:00 +0200)] 
t/t3501-revert-cherry-pick.sh: clarify scope of the file

The file started out as a test for picks and reverts with renames, but
has been subsequently populated with all kinds of basic tests, in
accordance with its generic name. Adjust the description to reflect
that.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'tb/enable-cruft-packs-by-default'
Junio C Hamano [Fri, 28 Apr 2023 23:03:03 +0000 (16:03 -0700)] 
Merge branch 'tb/enable-cruft-packs-by-default'

When "gc" needs to retain unreachable objects, packing them into
cruft packs (instead of exploding them into loose object files) has
been offered as a more efficient option for some time.  Now the use
of cruft packs has been made the default and no longer considered
an experimental feature.

* tb/enable-cruft-packs-by-default:
  repository.h: drop unused `gc_cruft_packs`
  builtin/gc.c: make `gc.cruftPacks` enabled by default
  t/t9300-fast-import.sh: prepare for `gc --cruft` by default
  t/t6500-gc.sh: add additional test cases
  t/t6500-gc.sh: refactor cruft pack tests
  t/t6501-freshen-objects.sh: prepare for `gc --cruft` by default
  t/t5304-prune.sh: prepare for `gc --cruft` by default
  builtin/gc.c: ignore cruft packs with `--keep-largest-pack`
  builtin/repack.c: fix incorrect reference to '-C'
  pack-write.c: plug a leak in stage_tmp_packfiles()

3 years agoThe fifteenth batch
Junio C Hamano [Fri, 28 Apr 2023 22:22:24 +0000 (15:22 -0700)] 
The fifteenth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'jk/gpg-trust-level-fix'
Junio C Hamano [Fri, 28 Apr 2023 23:03:03 +0000 (16:03 -0700)] 
Merge branch 'jk/gpg-trust-level-fix'

The "%GT" placeholder for the "--format" option of "git log" and
friends caused BUG() to trigger on a commit signed with an unknown
key, which has been corrected.

* jk/gpg-trust-level-fix:
  gpg-interface: set trust level of missing key to "undefined"

3 years agosend-email docs: Remove mention of discontinued gmail feature
Jouke Witteveen [Sat, 1 Oct 2022 10:46:09 +0000 (12:46 +0200)] 
send-email docs: Remove mention of discontinued gmail feature

Support for "less secure apps" ended May 30, 2022.

This effectively reverts 155067a (git-send-email.txt: mention less secure
app access with Gmail, 2021-01-08).

Signed-off-by: Jouke Witteveen <j.witteveen@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomessages: capitalization and punctuation exceptions
Oswald Buddenhagen [Fri, 28 Apr 2023 12:56:49 +0000 (14:56 +0200)] 
messages: capitalization and punctuation exceptions

These are conscious violations of the usual rules for error messages,
based on this reasoning:

 - If an error message is directly followed by another sentence, it
   needs to be properly terminated with a period, lest the grammar
   looks broken and becomes hard to read.

 - That second sentence isn't actually an error message any more, so
   it should abide to conventional language rules for good looks and
   legibility. Arguably, these should be converted to advice
   messages (which the user can squelch, too), but that's a much
   bigger effort to get right.

 - Neither of these apply to the first hunk in do_exec(), but this
   two-line message looks just too much like a real sentence to not
   terminate it. Also, leaving it alone would make it asymmetrical
   to the other hunk.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosequencer: actually translate report in do_exec()
Oswald Buddenhagen [Fri, 28 Apr 2023 12:56:48 +0000 (14:56 +0200)] 
sequencer: actually translate report in do_exec()

N_() is meant to be used on strings that are subsequently _()'d, which
isn't the case here.

The affected construct is a bit questionable from an i18n perspective,
as it pieces together a sentence from separate strings. However, it
doesn't appear to be that bad, as the "assembly instructions" are in a
translatable message as well. Lacking specific complaints from
translators, it doesn't seem worth changing this.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agococci: codify authoring and reviewing practices
Glen Choo [Thu, 27 Apr 2023 22:22:23 +0000 (22:22 +0000)] 
cocci: codify authoring and reviewing practices

These practices largely reflect what we are already doing on the mailing
list, which should help new Coccinelle authors and reviewers get up to
speed.

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agococci: add headings to and reword README
Glen Choo [Thu, 27 Apr 2023 22:22:22 +0000 (22:22 +0000)] 
cocci: add headings to and reword README

- Drop "examples" since we actually use the patches.
- Drop sentences that could be headings instead

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoThe fourteenth batch
Junio C Hamano [Thu, 27 Apr 2023 23:00:32 +0000 (16:00 -0700)] 
The fourteenth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'fc/doc-checkout-markup-updates'
Junio C Hamano [Thu, 27 Apr 2023 23:00:59 +0000 (16:00 -0700)] 
Merge branch 'fc/doc-checkout-markup-updates'

Doc mark-up update.

* fc/doc-checkout-markup-updates:
  doc: git-checkout: reorganize examples
  doc: git-checkout: trivial callout cleanup

3 years agoMerge branch 'fc/doc-use-datestamp-in-commit'
Junio C Hamano [Thu, 27 Apr 2023 23:00:59 +0000 (16:00 -0700)] 
Merge branch 'fc/doc-use-datestamp-in-commit'

Instead of the time the formatter was run, show the timestamp
recorded in the commit in the documentation.

* fc/doc-use-datestamp-in-commit:
  doc: set actual revdate for manpages

3 years agoMerge branch 'ds/fsck-pack-revindex'
Junio C Hamano [Thu, 27 Apr 2023 23:00:59 +0000 (16:00 -0700)] 
Merge branch 'ds/fsck-pack-revindex'

"git fsck" learned to validate the on-disk pack reverse index files.

* ds/fsck-pack-revindex:
  fsck: validate .rev file header
  fsck: check rev-index position values
  fsck: check rev-index checksums
  fsck: create scaffolding for rev-index checks

3 years agoMerge branch 'tb/pack-revindex-on-disk'
Junio C Hamano [Thu, 27 Apr 2023 23:00:59 +0000 (16:00 -0700)] 
Merge branch 'tb/pack-revindex-on-disk'

The on-disk reverse index that allows mapping from the pack offset
to the object name for the object stored at the offset has been
enabled by default.

* tb/pack-revindex-on-disk:
  t: invert `GIT_TEST_WRITE_REV_INDEX`
  config: enable `pack.writeReverseIndex` by default
  pack-revindex: introduce `pack.readReverseIndex`
  pack-revindex: introduce GIT_TEST_REV_INDEX_DIE_ON_DISK
  pack-revindex: make `load_pack_revindex` take a repository
  t5325: mark as leak-free
  pack-write.c: plug a leak in stage_tmp_packfiles()

3 years agoparse_commit(): describe more date-parsing failure modes
Jeff King [Thu, 27 Apr 2023 08:17:24 +0000 (04:17 -0400)] 
parse_commit(): describe more date-parsing failure modes

The previous few commits improved the parsing of dates in malformed
commit objects. But there's one big case left implicit: we may still
feed garbage to parse_timestamp(). This is preferable to trying to be
more strict, but let's document the thinking in a comment.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoparse_commit(): handle broken whitespace-only timestamp
Jeff King [Thu, 27 Apr 2023 08:17:15 +0000 (04:17 -0400)] 
parse_commit(): handle broken whitespace-only timestamp

The comment in parse_commit_date() claims that parse_timestamp() will
not walk past the end of the buffer we've been given, since it will hit
the newline at "eol" and stop. This is usually true, when dateptr
contains actual numbers to parse. But with a line like:

   committer name <email>   \n

with just whitespace, and no numbers, parse_timestamp() will consume
that newline as part of the leading whitespace, and we may walk past our
"tail" pointer (which itself is set from the "size" parameter passed in
to parse_commit_buffer()).

In practice this can't cause us to walk off the end of an array, because
we always add an extra NUL byte to the end of objects we load from disk
(as a defense against exactly this kind of bug). However, you can see
the behavior in action when "committer" is the final header (which it
usually is, unless there's an encoding) and the subject line can be
parsed as an integer. We walk right past the newline on the committer
line, as well as the "\n\n" separator, and mistake the subject for the
timestamp.

We can solve this by trimming the whitespace ourselves, making sure that
it has some non-whitespace to parse. Note that we need to be a bit
careful about the definition of "whitespace" here, as our isspace()
doesn't match exotic characters like vertical tab or formfeed. We can
work around that by checking for an actual number (see the in-code
comment). This is slightly more restrictive than the current code, but
in practice the results are either the same (we reject "foo" as "0", but
so would parse_timestamp()) or extremely unlikely even for broken
commits (parse_timestamp() would allow "\v123" as "123", but we'll now
make it "0").

I did also allow "-" here, which may be controversial, as we don't
currently support negative timestamps. My reasoning was two-fold. One,
the design of parse_timestamp() is such that we should be able to easily
switch it to handling signed values, and this otherwise creates a
hard-to-find gotcha that anybody doing that work would get tripped up
on. And two, the status quo is that we currently parse them, though the
result of course ends up as a very large unsigned value (which is likely
to just get clamped to "0" for display anyway, since our date routines
can't handle it).

The new test checks the commit parser (via "--until") for both vanilla
spaces and the vertical-tab case. I also added a test to check these
against the pretty-print formatter, which uses split_ident_line().  It's
not subject to the same bug, because it already insists that there be
one or more digits in the timestamp.

Helped-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoparse_commit(): parse timestamp from end of line
Jeff King [Thu, 27 Apr 2023 08:14:09 +0000 (04:14 -0400)] 
parse_commit(): parse timestamp from end of line

To find the committer timestamp, we parse left-to-right looking for the
closing ">" of the email, and then expect the timestamp right after
that. But we've seen some broken cases in the wild where this fails, but
we _could_ find the timestamp with a little extra work. E.g.:

  Name <Name<email>> 123456789 -0500

This means that features that rely on the committer timestamp, like
--since or --until, will treat the commit as happening at time 0 (i.e.,
1970).

This is doubly confusing because the pretty-print parser learned to
handle these in 03818a4a94 (split_ident: parse timestamp from end of
line, 2013-10-14). So printing them via "git show", etc, makes
everything look normal, but --until, etc are still broken (despite the
fact that that commit explicitly mentioned --until!).

So let's use the same trick as 03818a4a94: find the end of the line, and
parse back to the final ">". In theory we could use split_ident_line()
here, but it's actually a bit more strict. In particular, it requires a
valid time-zone token, too. That should be present, of course, but we
wouldn't want to break --until for cases that are working currently.

We might want to teach split_ident_line() to become more lenient there,
but it would require checking its many callers (since right now they can
assume that if date_start is non-NULL, so is tz_start).

So for now we'll just reimplement the same trick in the commit parser.

The test is in t4212, which already covers similar cases, courtesy of
03818a4a94. We'll just adjust the broken commit to munge both the author
and committer timestamps. Note that we could match (author|committer)
here, but alternation can't be used portably in sed. Since we wouldn't
expect to see ">" except as part of an ident line, we can just match
that character on any line.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot4212: avoid putting git on left-hand side of pipe
Jeff King [Thu, 27 Apr 2023 08:14:06 +0000 (04:14 -0400)] 
t4212: avoid putting git on left-hand side of pipe

We wouldn't expect cat-file to fail here, but it's good practice to
avoid putting git on the upstream of a pipe, as we otherwise ignore its
exit code.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobanned.h: mark `strtok()` and `strtok_r()` as banned
Taylor Blau [Mon, 24 Apr 2023 22:20:26 +0000 (18:20 -0400)] 
banned.h: mark `strtok()` and `strtok_r()` as banned

`strtok()` has a couple of drawbacks that make it undesirable to have
any new instances. In addition to being thread-unsafe, it also
encourages confusing data flows, where `strtok()` may be called from
multiple functions with its first argument as NULL, making it unclear
from the immediate context which string is being tokenized.

Now that we have removed all instances of `strtok()` from the tree,
let's ban `strtok()` to avoid introducing new ones in the future. If new
callers should arise, they are encouraged to use
`string_list_split_in_place()` (and `string_list_remove_empty_items()`,
if applicable).

string_list_split_in_place() is not a perfect drop-in replacement
for `strtok_r()`, particularly if the caller is processing a string with
an arbitrary number of tokens, and wants to process each token one at a
time.

But there are no instances of this in Git's tree which are more
well-suited to `strtok_r()` than the friendlier
`string_list_split_in_place()`, so ban `strtok_r()`, too.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agonegotiator/skipping: fix some problems in mark_common()
Han Xin [Wed, 26 Apr 2023 13:15:04 +0000 (21:15 +0800)] 
negotiator/skipping: fix some problems in mark_common()

The mark_common() method in negotiator/skipping.c was converted
from recursive to iterative in 4654134976f (negotiator/skipping:
avoid stack overflow, 2022-10-25), but there is some more work
to do:

1. prio_queue() should be used with clear_prio_queue(), otherwise there
   will be a memory leak.
2. It does not do duplicate protection before prio_queue_put().
   (The COMMON bit would work here, too.)
3. When it translated from recursive to iterative it kept "return"
   statements that should probably be "continue" statements.
4. It does not attempt to parse commits, and instead returns
   immediately when finding an unparsed commit. This is something
   that it did in its original version, so maybe it is by design,
   but it doesn't match the doc comment for the method.

Helped-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Han Xin <hanxin.hx@bytedance.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agonegotiator/default: avoid stack overflow
Han Xin [Wed, 26 Apr 2023 13:15:03 +0000 (21:15 +0800)] 
negotiator/default: avoid stack overflow

mark_common() in negotiator/default.c may overflow the stack due to
recursive function calls. Avoid this by instead recursing using a
heap-allocated data structure.

This is the same case as 4654134976f (negotiator/skipping: avoid
stack overflow, 2022-10-25)

Reported-by: Xin Xing <xingxin.xx@bytedance.com>
Signed-off-by: Han Xin <hanxin.hx@bytedance.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoHandle some compiler versions containing a dash
Mike Hommey [Wed, 26 Apr 2023 00:48:43 +0000 (09:48 +0900)] 
Handle some compiler versions containing a dash

The version reported by e.g. x86_64-w64-mingw32-gcc on Debian bullseye
looks like:
  gcc version 10-win32 20210110 (GCC)

This ends up with detect-compiler failing with:
  ./detect-compiler: 30: test: Illegal number: 10-win32

This change removes the two known suffixes known to exist in GCC versions
in Debian: -win32 and -posix.

Signed-off-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agodoc: GIT_DEFAULT_HASH is and will be ignored during "clone"
Junio C Hamano [Wed, 26 Apr 2023 15:13:55 +0000 (08:13 -0700)] 
doc: GIT_DEFAULT_HASH is and will be ignored during "clone"

The phrasing "is currently ignored" was prone to be misinterpreted
as if we were wishing if it were honored.  Rephrase it to make it
clear that the experimental variable will be ignored.

In the longer term, after/when we allow incremental/over-the-wire
migration of the object-format, i.e. cloning from an SHA-1
repository to create an SHA-256 repository (or vice versa) and
fetching and pushing between them would bidirectionally convert the
object format on the fly, it is likely that we would teach a new
option "--object-format" to "git clone" to say "you would use
whatever object format the origin uses by default, but this time, I
am telling you to use this format on our side, doing on-the-fly
object format conversion as needed".  So it is perfectly OK to
ignore the settings of this experimental variable, even after such
an extension happens that makes it necessary for us to have a way to
create a new repository that uses different object format from the
origin repository.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoThe thirteenth batch
Junio C Hamano [Tue, 25 Apr 2023 20:17:56 +0000 (13:17 -0700)] 
The thirteenth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'ps/fix-geom-repack-with-alternates'
Junio C Hamano [Tue, 25 Apr 2023 20:56:20 +0000 (13:56 -0700)] 
Merge branch 'ps/fix-geom-repack-with-alternates'

Geometric repacking ("git repack --geometric=<n>") in a repository
that borrows from an alternate object database had various corner
case bugs, which have been corrected.

* ps/fix-geom-repack-with-alternates:
  repack: disable writing bitmaps when doing a local repack
  repack: honor `-l` when calculating pack geometry
  t/helper: allow chmtime to print verbosely without modifying mtime
  pack-objects: extend test coverage of `--stdin-packs` with alternates
  pack-objects: fix error when same packfile is included and excluded
  pack-objects: fix error when packing same pack twice
  pack-objects: split out `--stdin-packs` tests into separate file
  repack: fix generating multi-pack-index with only non-local packs
  repack: fix trying to use preferred pack in alternates
  midx: fix segfault with no packs and invalid preferred pack

3 years agoMerge branch 'rj/send-email-validate-hook-count-messages'
Junio C Hamano [Tue, 25 Apr 2023 20:56:20 +0000 (13:56 -0700)] 
Merge branch 'rj/send-email-validate-hook-count-messages'

The sendemail-validate validate hook learned to pass the total
number of input files and where in the sequence each invocation is
via environment variables.

* rj/send-email-validate-hook-count-messages:
  send-email: export patch counters in validate environment

3 years agoMerge branch 'jk/protocol-cap-parse-fix'
Junio C Hamano [Tue, 25 Apr 2023 20:56:20 +0000 (13:56 -0700)] 
Merge branch 'jk/protocol-cap-parse-fix'

The code to parse capability list for v0 on-wire protocol fell into
an infinite loop when a capability appears multiple times, which
has been corrected.

* jk/protocol-cap-parse-fix:
  v0 protocol: use size_t for capability length/offset
  t5512: test "ls-remote --heads --symref" filtering with v0 and v2
  t5512: allow any protocol version for filtered symref test
  t5512: add v2 support for "ls-remote --symref" test
  v0 protocol: fix sha1/sha256 confusion for capabilities^{}
  t5512: stop referring to "v1" protocol
  v0 protocol: fix infinite loop when parsing multi-valued capabilities

3 years agoMerge branch 'en/header-split-cache-h'
Junio C Hamano [Tue, 25 Apr 2023 20:56:19 +0000 (13:56 -0700)] 
Merge branch 'en/header-split-cache-h'

Header clean-up.

* en/header-split-cache-h: (24 commits)
  protocol.h: move definition of DEFAULT_GIT_PORT from cache.h
  mailmap, quote: move declarations of global vars to correct unit
  treewide: reduce includes of cache.h in other headers
  treewide: remove double forward declaration of read_in_full
  cache.h: remove unnecessary includes
  treewide: remove cache.h inclusion due to pager.h changes
  pager.h: move declarations for pager.c functions from cache.h
  treewide: remove cache.h inclusion due to editor.h changes
  editor: move editor-related functions and declarations into common file
  treewide: remove cache.h inclusion due to object.h changes
  object.h: move some inline functions and defines from cache.h
  treewide: remove cache.h inclusion due to object-file.h changes
  object-file.h: move declarations for object-file.c functions from cache.h
  treewide: remove cache.h inclusion due to git-zlib changes
  git-zlib: move declarations for git-zlib functions from cache.h
  treewide: remove cache.h inclusion due to object-name.h changes
  object-name.h: move declarations for object-name.c functions from cache.h
  treewide: remove unnecessary cache.h inclusion
  treewide: be explicit about dependence on mem-pool.h
  treewide: be explicit about dependence on oid-array.h
  ...

3 years agoSync with Git 2.40.1
Junio C Hamano [Tue, 25 Apr 2023 05:31:32 +0000 (22:31 -0700)] 
Sync with Git 2.40.1

3 years agot/helper/test-json-writer.c: avoid using `strtok()`
Taylor Blau [Mon, 24 Apr 2023 22:20:23 +0000 (18:20 -0400)] 
t/helper/test-json-writer.c: avoid using `strtok()`

Apply similar treatment as in the previous commit to remove usage of
`strtok()` from the "oidmap" test helper.

Each of the different commands that the "json-writer" helper accepts
pops the next space-delimited token from the current line and interprets
it as a string, integer, or double (with the exception of the very first
token, which is the command itself).

To accommodate this, split the line in place by the space character, and
pass the corresponding string_list to each of the specialized `get_s()`,
`get_i()`, and `get_d()` functions.

`get_i()` and `get_d()` are thin wrappers around `get_s()` that convert
their result into the appropriate type by either calling `strtol()` or
`strtod()`, respectively. In `get_s()`, we mark the token as "consumed"
by incrementing the `consumed_nr` counter, indicating how many tokens we
have read up to that point.

Because each of these functions needs the string-list parts, the number
of tokens consumed, and the line number, these three are wrapped up in
to a struct representing the line state.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot/helper/test-oidmap.c: avoid using `strtok()`
Taylor Blau [Mon, 24 Apr 2023 22:20:20 +0000 (18:20 -0400)] 
t/helper/test-oidmap.c: avoid using `strtok()`

Apply similar treatment as in the previous commit to remove usage of
`strtok()` from the "oidmap" test helper.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot/helper/test-hashmap.c: avoid using `strtok()`
Taylor Blau [Mon, 24 Apr 2023 22:20:17 +0000 (18:20 -0400)] 
t/helper/test-hashmap.c: avoid using `strtok()`

Avoid using the non-reentrant `strtok()` to separate the parts of each
incoming command. Instead of replacing it with `strtok_r()`, let's
instead use the more friendly pair of `string_list_split_in_place()` and
`string_list_remove_empty_items()`.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agostring-list: introduce `string_list_setlen()`
Taylor Blau [Mon, 24 Apr 2023 22:20:14 +0000 (18:20 -0400)] 
string-list: introduce `string_list_setlen()`

It is sometimes useful to reduce the size of a `string_list`'s list of
items without having to re-allocate them. For example, doing the
following:

    struct strbuf buf = STRBUF_INIT;
    struct string_list parts = STRING_LIST_INIT_NO_DUP;
    while (strbuf_getline(&buf, stdin) != EOF) {
      parts.nr = 0;
      string_list_split_in_place(&parts, buf.buf, ":", -1);
      /* ... */
    }
    string_list_clear(&parts, 0);

is preferable over calling `string_list_clear()` on every iteration of
the loop. This is because `string_list_clear()` causes us free our
existing `items` array. This means that every time we call
`string_list_split_in_place()`, the string-list internals re-allocate
the same size array.

Since in the above example we do not care about the individual parts
after processing each line, it is much more efficient to pretend that
there aren't any elements in the `string_list` by setting `list->nr` to
0 while leaving the list of elements allocated as-is.

This allows `string_list_split_in_place()` to overwrite any existing
entries without needing to free and re-allocate them.

However, setting `list->nr` manually is not safe in all instances. There
are a couple of cases worth worrying about:

  - If the `string_list` is initialized with `strdup_strings`,
    truncating the list can lead to overwriting strings which are
    allocated elsewhere. If there aren't any other pointers to those
    strings other than the ones inside of the `items` array, they will
    become unreachable and leak.

    (We could ourselves free the truncated items between
    string_list->items[nr] and `list->nr`, but no present or future
    callers would benefit from this additional complexity).

  - If the given `nr` is larger than the current value of `list->nr`,
    we'll trick the `string_list` into a state where it thinks there are
    more items allocated than there actually are, which can lead to
    undefined behavior if we try to read or write those entries.

Guard against both of these by introducing a helper function which
guards assignment of `list->nr` against each of the above conditions.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agostring-list: multi-delimiter `string_list_split_in_place()`
Taylor Blau [Mon, 24 Apr 2023 22:20:10 +0000 (18:20 -0400)] 
string-list: multi-delimiter `string_list_split_in_place()`

Enhance `string_list_split_in_place()` to accept multiple characters as
delimiters instead of a single character.

Instead of using `strchr(2)` to locate the first occurrence of the given
delimiter character, `string_list_split_in_place_multi()` uses
`strcspn(2)` to move past the initial segment of characters comprised of
any characters in the delimiting set.

When only a single delimiting character is provided, `strpbrk(2)` (which
is implemented with `strcspn(2)`) has equivalent performance to
`strchr(2)`. Modern `strcspn(2)` implementations treat an empty
delimiter or the singleton delimiter as a special case and fall back to
calling strchrnul(). Both glibc[1] and musl[2] implement `strcspn(2)`
this way.

This change is one step to removing `strtok(2)` from the tree. Note that
`string_list_split_in_place()` is not a strict replacement for
`strtok()`, since it will happily turn sequential delimiter characters
into empty entries in the resulting string_list. For example:

    string_list_split_in_place(&xs, "foo:;:bar:;:baz", ":;", -1)

would yield a string list of:

    ["foo", "", "", "bar", "", "", "baz"]

Callers that wish to emulate the behavior of strtok(2) more directly
should call `string_list_remove_empty_items()` after splitting.

To avoid regressions for the new multi-character delimter cases, update
t0063 in this patch as well.

[1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=string/strcspn.c;hb=glibc-2.37#l35
[2]: https://git.musl-libc.org/cgit/musl/tree/src/string/strcspn.c?h=v1.2.3#n11

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoblame: use different author name for fake commit generated by --contents
Jacob Keller [Mon, 24 Apr 2023 19:35:08 +0000 (12:35 -0700)] 
blame: use different author name for fake commit generated by --contents

When the --contents option is used with git blame, and the contents of
the file have lines which can't be annotated by the history being
blamed, the user will see an author of "Not Committed Yet". This is
similar to the way blame handles working tree contents when blaming
without a revision.

This is slightly confusing since this data isn't the working copy and
while it is technically "not committed yet", its also coming from an
external file. Replace this author name with "External file
(--contents)" to better differentiate such lines from actual working
copy lines.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Glen Choo <chooglen@google.com>
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot1300: add tests for missing keys
Andrei Rybak [Sun, 23 Apr 2023 13:46:49 +0000 (15:46 +0200)] 
t1300: add tests for missing keys

There are several tests in t1300-config.sh that validate failing
invocations of "git config".  However, there are no tests that check
what happens when "git config" is asked to retrieve a value for a
missing key.

Add tests that check this for various combinations of "<section>.<key>"
and "<section>.<subsection>.<key>".

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot1300: check stderr for "ignores pairs" tests
Andrei Rybak [Sun, 23 Apr 2023 13:46:48 +0000 (15:46 +0200)] 
t1300: check stderr for "ignores pairs" tests

Tests "git config ignores pairs ..." in t1300-config.sh validate that
"git config" ignores various kinds of supplied pairs of environment
variables GIT_CONFIG_KEY_* GIT_CONFIG_VALUE_* depending on
GIT_CONFIG_COUNT.  By "ignores" here we mean that "git config" abides by
the value of environment variable GIT_CONFIG_COUNT and doesn't use
key-value pairs outside of the supplied GIT_CONFIG_COUNT when trying to
produce a value for config key "pair.one".

These tests also validate that "git config" doesn't complain about
mismatched environment variables to standard error.  This is validated
by redirecting the standard error to a file called "error" and asserting
that it is empty.  However, two of these tests incorrectly redirect to
standard output while calling the file "error", and test 'git config
ignores pairs exceeding count' doesn't validate standard error at all.

Fix these tests by redirecting standard error to file "error" and
asserting its emptiness.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot1300: drop duplicate test
Andrei Rybak [Sun, 23 Apr 2023 13:46:47 +0000 (15:46 +0200)] 
t1300: drop duplicate test

There are two almost identical tests called 'git config ignores pairs
with zero count' in file t1300-config.sh.  Drop the first of these and
keep the one that contains more assertions.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: fix calling merge_finalize() with no intermediate merge
Elijah Newren [Sat, 22 Apr 2023 20:22:10 +0000 (20:22 +0000)] 
merge-ort: fix calling merge_finalize() with no intermediate merge

If some code sets up the data structures for a merge, but then never
actually performs one before calling merge_finalize(), then
merge_finalize() wouldn't notice that result->priv was NULL and
return early, resulting in following that NULL pointer and getting
a segfault.  There is currently no code in the git codebase that does
this, but this issue was found during testing of some proposed patches
that had the following structure:

    struct merge_options merge_opt;
    struct merge_result result;

    init_merge_options(&merge_opt, the_repository);
    memset(&result, 0, sizeof(result));

    <do N merges, for some value of N>

    merge_finalize(&merge_opt, &result);

where some flags could cause the code to have N=0, i.e. doing no merges.
Add a check for result->priv being NULL and return early to avoid a
segfault in these kinds of cases.

While at it, ensure the FREE_AND_NULL() in the function does something
useful with the nulling aspect, namely sets result->priv to NULL rather
than a mere temporary.

Reported-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoreftable: ensure git-compat-util.h is the first (indirect) include
Elijah Newren [Sat, 22 Apr 2023 20:17:29 +0000 (20:17 +0000)] 
reftable: ensure git-compat-util.h is the first (indirect) include

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agodiff.h: reduce unnecessary includes
Elijah Newren [Sat, 22 Apr 2023 20:17:28 +0000 (20:17 +0000)] 
diff.h: reduce unnecessary includes

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoobject-store.h: reduce unnecessary includes
Elijah Newren [Sat, 22 Apr 2023 20:17:27 +0000 (20:17 +0000)] 
object-store.h: reduce unnecessary includes

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocommit.h: reduce unnecessary includes
Elijah Newren [Sat, 22 Apr 2023 20:17:26 +0000 (20:17 +0000)] 
commit.h: reduce unnecessary includes

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofsmonitor: reduce includes of cache.h
Elijah Newren [Sat, 22 Apr 2023 20:17:25 +0000 (20:17 +0000)] 
fsmonitor: reduce includes of cache.h

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocache.h: remove unnecessary headers
Elijah Newren [Sat, 22 Apr 2023 20:17:24 +0000 (20:17 +0000)] 
cache.h: remove unnecessary headers

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotreewide: remove cache.h inclusion due to previous changes
Elijah Newren [Sat, 22 Apr 2023 20:17:23 +0000 (20:17 +0000)] 
treewide: remove cache.h inclusion due to previous changes

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocache,tree: move basic name compare functions from read-cache to tree
Elijah Newren [Sat, 22 Apr 2023 20:17:22 +0000 (20:17 +0000)] 
cache,tree: move basic name compare functions from read-cache to tree

None of base_name_compare(), df_name_compare(), or name_compare()
depended upon a cache_entry or index_state in any way.  By moving these
functions to tree.h, half a dozen other files can stop depending upon
cache.h (though that change will be made in a later commit).

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocache,tree: move cmp_cache_name_compare from tree.[ch] to read-cache.c
Elijah Newren [Sat, 22 Apr 2023 20:17:21 +0000 (20:17 +0000)] 
cache,tree: move cmp_cache_name_compare from tree.[ch] to read-cache.c

Since cmp_cache_name_compare() was comparing cache_entry structs, it
was associated with the cache rather than with trees.  Move the
function.  As a side effect, we can make cache_name_stage_compare()
static as well.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agohash-ll.h: split out of hash.h to remove dependency on repository.h
Elijah Newren [Sat, 22 Apr 2023 20:17:20 +0000 (20:17 +0000)] 
hash-ll.h: split out of hash.h to remove dependency on repository.h

hash.h depends upon and includes repository.h, due to the definition and
use of the_hash_algo (defined as the_repository->hash_algo).  However,
most headers trying to include hash.h are only interested in the layout
of the structs like object_id.  Move the parts of hash.h that do not
depend upon repository.h into a new file hash-ll.h (the "low level"
parts of hash.h), and adjust other files to use this new header where
the convenience inline functions aren't needed.

This allows hash.h and object.h to be fairly small, minimal headers.  It
also exposes a lot of hidden dependencies on both path.h (which was
brought in by repository.h) and repository.h (which was previously
implicitly brought in by object.h), so also adjust other files to be
more explicit about what they depend upon.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotree-diff.c: move S_DIFFTREE_IFXMIN_NEQ define from cache.h
Elijah Newren [Sat, 22 Apr 2023 20:17:19 +0000 (20:17 +0000)] 
tree-diff.c: move S_DIFFTREE_IFXMIN_NEQ define from cache.h

S_DIFFTREE_IFXMIN_NEQ is *only* used in tree-diff.c, so there is no
point exposing it in cache.h.  Move it to tree-diff.c.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agodir.h: move DTYPE defines from cache.h
Elijah Newren [Sat, 22 Apr 2023 20:17:18 +0000 (20:17 +0000)] 
dir.h: move DTYPE defines from cache.h

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoversioncmp.h: move declarations for versioncmp.c functions from cache.h
Elijah Newren [Sat, 22 Apr 2023 20:17:17 +0000 (20:17 +0000)] 
versioncmp.h: move declarations for versioncmp.c functions from cache.h

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agows.h: move declarations for ws.c functions from cache.h
Elijah Newren [Sat, 22 Apr 2023 20:17:16 +0000 (20:17 +0000)] 
ws.h: move declarations for ws.c functions from cache.h

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomatch-trees.h: move declarations for match-trees.c functions from cache.h
Elijah Newren [Sat, 22 Apr 2023 20:17:15 +0000 (20:17 +0000)] 
match-trees.h: move declarations for match-trees.c functions from cache.h

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopkt-line.h: move declarations for pkt-line.c functions from cache.h
Elijah Newren [Sat, 22 Apr 2023 20:17:14 +0000 (20:17 +0000)] 
pkt-line.h: move declarations for pkt-line.c functions from cache.h

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobase85.h: move declarations for base85.c functions from cache.h
Elijah Newren [Sat, 22 Apr 2023 20:17:13 +0000 (20:17 +0000)] 
base85.h: move declarations for base85.c functions from cache.h

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocopy.h: move declarations for copy.c functions from cache.h
Elijah Newren [Sat, 22 Apr 2023 20:17:12 +0000 (20:17 +0000)] 
copy.h: move declarations for copy.c functions from cache.h

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoserver-info.h: move declarations for server-info.c functions from cache.h
Elijah Newren [Sat, 22 Apr 2023 20:17:11 +0000 (20:17 +0000)] 
server-info.h: move declarations for server-info.c functions from cache.h

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopackfile.h: move pack_window and pack_entry from cache.h
Elijah Newren [Sat, 22 Apr 2023 20:17:10 +0000 (20:17 +0000)] 
packfile.h: move pack_window and pack_entry from cache.h

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosymlinks.h: move declarations for symlinks.c functions from cache.h
Elijah Newren [Sat, 22 Apr 2023 20:17:09 +0000 (20:17 +0000)] 
symlinks.h: move declarations for symlinks.c functions from cache.h

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotreewide: be explicit about dependence on strbuf.h
Elijah Newren [Sat, 22 Apr 2023 20:17:08 +0000 (20:17 +0000)] 
treewide: be explicit about dependence on strbuf.h

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agofetch_bundle_uri(): drop pointless NULL check
Jeff King [Sat, 22 Apr 2023 13:56:46 +0000 (09:56 -0400)] 
fetch_bundle_uri(): drop pointless NULL check

We check if "uri" is NULL, but it cannot be since we'd have segfaulted
earlier in the function when we unconditionally called xstrdup() on it.

In theory we might want to soften that xstrdup() to handle this case,
but even before the code which added it via c23f592117 (bundle-uri:
fetch a list of bundles, 2022-10-12), we'd have fed NULL to
fetch_bundle_uri_internal(), which would also segfault.

The extra check isn't hurting anything, but it does cause Coverity to
complain, and it may mislead somebody reading the code into thinking
that a NULL uri is something we're prepared to handle.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agonotes: clean up confusing NULL checks in init_notes()
Jeff King [Sat, 22 Apr 2023 13:55:43 +0000 (09:55 -0400)] 
notes: clean up confusing NULL checks in init_notes()

Coverity complains that we check whether "notes_ref" is NULL, but it was
already implied to be non-NULL earlier in the function. And this is
true; since b9342b3fd63 (refs: add array of ref namespaces, 2022-08-05),
we call xstrdup(notes_ref) unconditionally, which would segfault if it
was NULL.

But that commit is actually doing the right thing. Even if NULL is
passed into the function, we'll use default_notes_ref() as a fallback,
which will never return NULL (it tries a few options, but its last
resort is a string literal). Ironically, the "!notes_ref" check was
added by the same commit that added the fallback: 709f79b0894 (Notes
API: init_notes(): Initialize the notes tree from the given notes ref,
2010-02-13). So this check never did anything.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoThe twelfth batch
Junio C Hamano [Fri, 21 Apr 2023 21:57:45 +0000 (14:57 -0700)] 
The twelfth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'ow/ref-filter-omit-empty'
Junio C Hamano [Fri, 21 Apr 2023 22:35:05 +0000 (15:35 -0700)] 
Merge branch 'ow/ref-filter-omit-empty'

"git branch --format=..." and "git format-patch --format=..."
learns "--omit-empty" to hide refs that whose formatting result
becomes an empty string from the output.

* ow/ref-filter-omit-empty:
  branch, for-each-ref, tag: add option to omit empty lines

3 years agoMerge branch 'ah/format-patch-thread-doc'
Junio C Hamano [Fri, 21 Apr 2023 22:35:04 +0000 (15:35 -0700)] 
Merge branch 'ah/format-patch-thread-doc'

Doc update.

* ah/format-patch-thread-doc:
  format-patch: correct documentation of --thread without an argument

3 years agoMerge branch 'rn/sparse-describe'
Junio C Hamano [Fri, 21 Apr 2023 22:35:04 +0000 (15:35 -0700)] 
Merge branch 'rn/sparse-describe'

"git describe --dirty" learns to work better with sparse-index.

* rn/sparse-describe:
  describe: enable sparse index for describe

3 years agoMerge branch 'rs/archive-from-subdirectory-fixes'
Junio C Hamano [Fri, 21 Apr 2023 22:35:04 +0000 (15:35 -0700)] 
Merge branch 'rs/archive-from-subdirectory-fixes'

"git archive" run from a subdirectory mishandled attributes and
paths outside the current directory.

* rs/archive-from-subdirectory-fixes:
  archive: improve support for running in subdirectory

3 years agoMerge branch 'fc/doc-stop-using-manversion'
Junio C Hamano [Fri, 21 Apr 2023 22:35:04 +0000 (15:35 -0700)] 
Merge branch 'fc/doc-stop-using-manversion'

Doc build simplification.

* fc/doc-stop-using-manversion:
  doc: simplify man version

3 years agocredential: new attribute oauth_refresh_token
M Hickford [Fri, 21 Apr 2023 09:47:59 +0000 (09:47 +0000)] 
credential: new attribute oauth_refresh_token

Git authentication with OAuth access token is supported by every popular
Git host including GitHub, GitLab and BitBucket [1][2][3]. Credential
helpers Git Credential Manager (GCM) and git-credential-oauth generate
OAuth credentials [4][5]. Following RFC 6749, the application prints a
link for the user to authorize access in browser. A loopback redirect
communicates the response including access token to the application.

For security, RFC 6749 recommends that OAuth response also includes
expiry date and refresh token [6]. After expiry, applications can use
the refresh token to generate a new access token without user
reauthorization in browser. GitLab and BitBucket set the expiry at two
hours [2][3]. (GitHub doesn't populate expiry or refresh token.)

However the Git credential protocol has no attribute to store the OAuth
refresh token (unrecognised attributes are silently discarded). This
means that the user has to regularly reauthorize the helper in browser.
On a browserless system, this is particularly intrusive, requiring a
second device.

Introduce a new attribute oauth_refresh_token. This is especially
useful when a storage helper and a read-only OAuth helper are configured
together. Recall that `credential fill` calls each helper until it has a
non-expired password.

```
[credential]
helper = storage  # eg. cache or osxkeychain
helper = oauth
```

The OAuth helper can use the stored refresh token forwarded by
`credential fill` to generate a fresh access token without opening the
browser. See
https://github.com/hickford/git-credential-oauth/pull/3/files
for an implementation tested with this patch.

Add support for the new attribute to credential-cache. Eventually, I
hope to see support in other popular storage helpers.

Alternatives considered: ask helpers to store all unrecognised
attributes. This seems excessively complex for no obvious gain.
Helpers would also need extra information to distinguish between
confidential and non-confidential attributes.

Workarounds: GCM abuses the helper get/store/erase contract to store the
refresh token during credential *get* as the password for a fictitious
host [7] (I wrote this hack). This workaround is only feasible for a
monolithic helper with its own storage.

[1] https://github.blog/2012-09-21-easier-builds-and-deployments-using-git-over-https-and-oauth/
[2] https://docs.gitlab.com/ee/api/oauth2.html#access-git-over-https-with-access-token
[3] https://support.atlassian.com/bitbucket-cloud/docs/use-oauth-on-bitbucket-cloud/#Cloning-a-repository-with-an-access-token
[4] https://github.com/GitCredentialManager/git-credential-manager
[5] https://github.com/hickford/git-credential-oauth
[6] https://datatracker.ietf.org/doc/html/rfc6749#section-5.1
[7] https://github.com/GitCredentialManager/git-credential-manager/blob/66b94e489ad8cc1982836355493e369770b30211/src/shared/GitLab/GitLabHostProvider.cs#L207

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocompletion: suppress unwanted unescaping of `read`
Edwin Kofler [Thu, 20 Apr 2023 22:38:00 +0000 (07:38 +0900)] 
completion: suppress unwanted unescaping of `read`

The function `__git_eread`, which reads the first line from the file,
calls the `read` builtin without passing the flag option `-r`.  When
the `read` builtin is called without the flag `-r`, it processes the
backslash escaping in the text that it reads.  For this reason, it is
generally considered the best practice to always use the `read`
builtin with flag `-r` unless one intensionally processes the
backslash escaping.  For the present case in git-prompt.sh, in fact,
all the occurrences of the calls of `__git_eread` intend to read the
literal content of the first lines.

To make it read the first line literally, pass the flag `-r` to the
`read` builtin in the function `__git_eread`.

Signed-off-by: Edwin Kofler <edwin@kofler.dev>
Signed-off-by: Koichi Murase <myoga.murase@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agococci: remove 'unused.cocci'
SZEDER Gábor [Thu, 20 Apr 2023 20:53:50 +0000 (22:53 +0200)] 
cocci: remove 'unused.cocci'

When 'unused.cocci' was added in 4f40f6cb73 (cocci: add and apply a
rule to find "unused" strbufs, 2022-07-05) it found three unused
strbufs, and when it was generalized in the next commit it managed to
find an unused string_list as well.  That's four unused variables in
over 17 years, so apparently we rarely make this mistake.

Unfortunately, applying 'unused.cocci' is quite expensive, e.g. it
increases the from-scratch runtime of 'make coccicheck' by over 5:30
minutes or over 160%:

  $ make -s cocciclean
  $ time make -s coccicheck
      * new spatch flags

  real    8m56.201s
  user    0m0.420s
  sys     0m0.406s
  $ rm contrib/coccinelle/unused.cocci contrib/coccinelle/tests/unused.*
  $ make -s cocciclean
  $ time make -s coccicheck
      * new spatch flags

  real    3m23.893s
  user    0m0.228s
  sys     0m0.247s

That's a lot of runtime spent for not much in return, and arguably an
unused struct instance sneaking in is not that big of a deal to
justify the significantly increased runtime.

Remove 'unused.cocci', because we are not getting our CPU cycles'
worth.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogittutorial: wrap literal examples in backticks
Martin Ågren [Sat, 15 Apr 2023 17:29:11 +0000 (19:29 +0200)] 
gittutorial: wrap literal examples in backticks

Our coding guidelines prefer literal examples to be wrapped in
`backticks` to typeset them in monospace.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogittutorial: drop early mention of origin
Martin Ågren [Sat, 15 Apr 2023 17:29:10 +0000 (19:29 +0200)] 
gittutorial: drop early mention of origin

We don't have an origin at this point in the tutorial, so "Your branch
is up to date" won't actually show up in the output of `git status`.

This line was introduced in 8942821ec0 ("gittutorial: fix output of 'git
status'", 2014-11-13) in what looks like a mistake -- that commit mostly
just wanted to remove leading '#' characters.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoThe eleventh batch
Junio C Hamano [Thu, 20 Apr 2023 20:41:15 +0000 (13:41 -0700)] 
The eleventh batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'gc/better-error-when-local-clone-fails-with-symlink'
Junio C Hamano [Thu, 20 Apr 2023 21:33:36 +0000 (14:33 -0700)] 
Merge branch 'gc/better-error-when-local-clone-fails-with-symlink'

"git clone --local" stops copying from an original repository that
has symbolic links inside its $GIT_DIR; an error message when that
happens has been updated.

* gc/better-error-when-local-clone-fails-with-symlink:
  clone: error specifically with --local and symlinked objects

3 years agoMerge branch 'ar/t2024-checkout-output-fix'
Junio C Hamano [Thu, 20 Apr 2023 21:33:36 +0000 (14:33 -0700)] 
Merge branch 'ar/t2024-checkout-output-fix'

Test fix.

* ar/t2024-checkout-output-fix:
  t2024: fix loose/strict local base branch DWIM test

3 years agoMerge branch 'rs/get-tar-commit-id-use-defined-const'
Junio C Hamano [Thu, 20 Apr 2023 21:33:36 +0000 (14:33 -0700)] 
Merge branch 'rs/get-tar-commit-id-use-defined-const'

Code clean-up to replace a hardcoded constant with a CPP macro.

* rs/get-tar-commit-id-use-defined-const:
  get-tar-commit-id: use TYPEFLAG_GLOBAL_HEADER instead of magic value

3 years agoMerge branch 'rs/remove-approxidate-relative'
Junio C Hamano [Thu, 20 Apr 2023 21:33:35 +0000 (14:33 -0700)] 
Merge branch 'rs/remove-approxidate-relative'

The approxidate() API has been simplified by losing an extra
function that did the same thing as another one.

* rs/remove-approxidate-relative:
  date: remove approxidate_relative()

3 years agoMerge branch 'rs/userdiff-multibyte-regex'
Junio C Hamano [Thu, 20 Apr 2023 21:33:35 +0000 (14:33 -0700)] 
Merge branch 'rs/userdiff-multibyte-regex'

The userdiff regexp patterns for various filetypes that are built
into the system have been updated to avoid triggering regexp errors
from UTF-8 aware regex engines.

* rs/userdiff-multibyte-regex:
  userdiff: support regexec(3) with multi-byte support

3 years agosend-email: expose header information to git-send-email's sendemail-validate hook
Michael Strawbridge [Wed, 19 Apr 2023 20:27:03 +0000 (16:27 -0400)] 
send-email: expose header information to git-send-email's sendemail-validate hook

To allow further flexibility in the Git hook, the SMTP header
information of the email which git-send-email intends to send, is now
passed as the 2nd argument to the sendemail-validate hook.

As an example, this can be useful for acting upon keywords in the
subject or specific email addresses.

Cc: Luben Tuikov <luben.tuikov@amd.com>
Cc: Junio C Hamano <gitster@pobox.com>
Cc: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Acked-by: Luben Tuikov <luben.tuikov@amd.com>
Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosend-email: refactor header generation functions
Michael Strawbridge [Wed, 19 Apr 2023 20:27:02 +0000 (16:27 -0400)] 
send-email: refactor header generation functions

Split process_file and send_message into easier to use functions.
Making SMTP header information widely available.

Cc: Luben Tuikov <luben.tuikov@amd.com>
Cc: Junio C Hamano <gitster@pobox.com>
Cc: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Acked-by: Luben Tuikov <luben.tuikov@amd.com>
Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogpg-interface: set trust level of missing key to "undefined"
Jeff King [Wed, 19 Apr 2023 01:29:57 +0000 (21:29 -0400)] 
gpg-interface: set trust level of missing key to "undefined"

In check_signature(), we initialize the trust_level field to "-1", with
the idea that if gpg does not return a trust level at all (if there is
no signature, or if the signature is made by an unknown key), we'll
use that value. But this has two problems:

  1. Since the field is an enum, it's up to the compiler to decide what
     underlying storage to use, and it only has to fit the values we've
     declared. So we may not be able to store "-1" at all. And indeed,
     on my system (linux with gcc), the resulting enum is an unsigned
     32-bit value, and -1 becomes 4294967295.

     The difference may seem academic (and you even get "-1" if you pass
     it to printf("%d")), but it means that code like this:

       status |= sigc->trust_level < configured_min_trust_level;

     does not necessarily behave as expected. This turns out not to be a
     bug in practice, though, because we keep the "-1" only when gpg did
     not report a signature from a known key, in which case the line
     above:

       status |= sigc->result != 'G';

     would always set status to non-zero anyway. So only a 'G' signature
     with no parsed trust level would cause a problem, which doesn't
     seem likely to trigger (outside of unexpected gpg behavior).

  2. When using the "%GT" format placeholder, we pass the value to
     gpg_trust_level_to_str(), which complains that the value is out of
     range with a BUG(). This behavior was introduced by 803978da49
     (gpg-interface: add function for converting trust level to string,
     2022-07-11). Before that, we just did a switch() on the enum, and
     anything that wasn't matched would end up as the empty string.

     Curiously, solving this by naively doing:

       if (level < 0)
               return "";

     in that function isn't sufficient. Because of (1) above, the
     compiler can (and does in my case) actually remove that conditional
     as dead code!

We can solve both by representing this state as an enum value. We could
do this by adding a new "unknown" value. But this really seems to match
the existing "undefined" level well. GPG describes this as "Not enough
information for calculation".

We have tests in t7510 that trigger this case (verifying a signature
from a key that we don't have, and then checking various %G
placeholders), but they didn't notice the BUG() because we didn't look
at %GT for that case! Let's make sure we check all %G placeholders for
each case in the formatting tests.

The interesting ones here are "show unknown signature with custom
format" and "show lack of signature with custom format", both of which
would BUG() before, and now turn %GT into "undefined". Prior to
803978da49 they would have turned it into the empty string, but I think
saying "undefined" consistently is a reasonable outcome, and probably
makes life easier for anyone parsing the output (and any such parser had
to be ready to see "undefined" already).

The other modified tests produce the same output before and after this
patch, but now we're consistently checking both %G? and %GT in all of
them.

Signed-off-by: Jeff King <peff@peff.net>
Reported-by: Rolf Eike Beer <eb@emlix.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agodoc: git-checkout: reorganize examples
Felipe Contreras [Tue, 18 Apr 2023 07:00:48 +0000 (01:00 -0600)] 
doc: git-checkout: reorganize examples

The examples are an ordered list, however, they are complex enough that
a callout is inside example 1, and that confuses the parsers as the list
continuation (`+`) is unclear (are we continuing the previous list item,
or the previous callout?).

We could use an open block as the asciidoctor documentation suggests,
but that has a tiny formatting issue (a newline is missing).

To simplify things for everyone (the reader, the writer, and the parser)
let's use subsections.

After this change, the HTML documentation generated with asciidoc has
the right indentation.

Cc: Jeff King <peff@peff.net>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agodoc: git-checkout: trivial callout cleanup
Felipe Contreras [Tue, 18 Apr 2023 07:00:47 +0000 (01:00 -0600)] 
doc: git-checkout: trivial callout cleanup

The callouts are directly tied to the listing above, remove spaces to
make it clear they are one and the same.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agorepository.h: drop unused `gc_cruft_packs`
Taylor Blau [Tue, 18 Apr 2023 20:41:00 +0000 (16:41 -0400)] 
repository.h: drop unused `gc_cruft_packs`

As of the previous commit, all callers that need to read the value of
`gc.cruftPacks` do so outside without using the `repo_settings` struct,
making its `gc_cruft_packs` unused. Drop it accordingly.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobuiltin/gc.c: make `gc.cruftPacks` enabled by default
Taylor Blau [Tue, 18 Apr 2023 20:40:57 +0000 (16:40 -0400)] 
builtin/gc.c: make `gc.cruftPacks` enabled by default

Back in 5b92477f89 (builtin/gc.c: conditionally avoid pruning objects
via loose, 2022-05-20), `git gc` learned the `--cruft` option and
`gc.cruftPacks` configuration to opt-in to writing cruft packs when
collecting or pruning unreachable objects.

Cruft packs were introduced with the merge in a50036da1a (Merge branch
'tb/cruft-packs', 2022-06-03). They address the problem of "loose object
explosions", where Git will write out many individual loose objects when
there is a large number of unreachable objects that have not yet aged
past `--prune=<date>`.

Instead of keeping track of those unreachable yet recent objects via
their loose object file's mtime, cruft packs collect all unreachable
objects into a single pack with a corresponding `*.mtimes` file that
acts as a table to store the mtimes of all unreachable objects. This
prevents the need to store unreachable objects as loose as they age out
of the repository, and avoids the problem of loose object explosions.

Beyond avoiding loose object explosions, cruft packs also act as a more
efficient mechanism to store unreachable objects as they age out of a
repository. This is because pairs of similar unreachable objects serve
as delta bases for one another.

In 5b92477f89, the feature was introduced as experimental. Since then,
GitHub has been running these patches in every repository generating
hundreds of millions of cruft packs along the way. The feature is
battle-tested, and avoids many pathological cases such as above. Users
who either run `git gc` manually, or via `git maintenance` can benefit
from having cruft packs.

As such, enable cruft pack generation to take place by default (by
making `gc.cruftPacks` have the default of "true" rather than "false).

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>