Commit graph

72958 commits

Author SHA1 Message Date
Patrick Steinhardt 2f960dd5fe t0610: make --shared= tests reusable
We have two kinds of `--shared=` tests, one for git-init(1) and one for
git-pack-refs(1). Merge them into a reusable function such that we can
easily add additional testcases with different umasks and flags for the
`--shared=` switch.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-09 14:14:00 -07:00
Junio C Hamano 227b8fd902 Makefile(s): do not enforce "all indents must be done with tab"
Our top-level Makefile follows our generic whitespace rule
established by the top-level .gitattributes file that does not
enforce indent-with-non-tab rule by default, but git-gui is set up
to enforce indent-with-non-tab by default.  With the upcoming change
to GNU make, we no longer can reject (and worse, "fix") a patch that
adds whitespace indented lines to the Makefile, so loosen the rule
there for git-gui/Makefile, too.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-08 16:36:05 -07:00
Taylor Blau 728b9ac0c3 Makefile(s): avoid recipe prefix in conditional statements
In GNU Make commit 07fcee35 ([SV 64815] Recipe lines cannot contain
conditional statements, 2023-05-22) and following, conditional
statements may no longer be preceded by a tab character (which Make
refers to as the recipe prefix).

There are a handful of spots in our various Makefile(s) which will break
in a future release of Make containing 07fcee35. For instance, trying to
compile the pre-image of this patch with the tip of make.git results in
the following:

    $ make -v | head -1 && make
    GNU Make 4.4.90
    config.mak.uname:842: *** missing 'endif'.  Stop.

The kernel addressed this issue in 82175d1f9430 (kbuild: Replace tabs
with spaces when followed by conditionals, 2024-01-28). Address the
issues in Git's tree by applying the same strategy.

When a conditional word (ifeq, ifneq, ifdef, etc.) is preceded by one or
more tab characters, replace each tab character with 8 space characters
with the following:

    find . -type f -not -path './.git/*' -name Makefile -or -name '*.mak' |
      xargs perl -i -pe '
        s/(\t+)(ifn?eq|ifn?def|else|endif)/" " x (length($1) * 8) . $2/ge unless /\\$/
      '

The "unless /\\$/" removes any false-positives (like "\telse \"
appearing within a shell script as part of a recipe).

After doing so, Git compiles on newer versions of Make:

    $ make -v | head -1 && make
    GNU Make 4.4.90
    GIT_VERSION = 2.44.0.414.gfac1dc44ca9
    [...]

    $ echo $?
    0

Reported-by: Dario Gjorgjevski <dario.gjorgjevski@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-08 14:42:32 -07:00
Junio C Hamano 0e0fefb29f config: do not leak excludes_file
The excludes_file variable is marked "const char *", but all the
assignments to it are made with a piece of memory allocated just
for it, and the variable is responsible for owning it.

When "core.excludesfile" is read, the code just lost the previous
value, leaking memory.  Plug it.

The real problem is that the variable is mistyped; our convention
is to never make a variable that owns the piece of memory pointed
by it as "const".  Fixing that would reduce the chance of this kind
of bug happening, and also would make it unnecessary to cast the
constness away while free()ing it, but that would be a much larger
follow-up effort.

Reported-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-08 13:20:24 -07:00
Justin Tobler a949ebd342 reftable/stack: use geometric table compaction
To reduce the number of on-disk reftables, compaction is performed.
Contiguous tables with the same binary log value of size are grouped
into segments. The segment that has both the lowest binary log value and
contains more than one table is set as the starting point when
identifying the compaction segment.

Since segments containing a single table are not initially considered
for compaction, if the table appended to the list does not match the
previous table log value, no compaction occurs for the new table. It is
therefore possible for unbounded growth of the table list. This can be
demonstrated by repeating the following sequence:

git branch -f foo
git branch -d foo

Each operation results in a new table being written with no compaction
occurring until a separate operation produces a table matching the
previous table log value.

Instead, to avoid unbounded growth of the table list, the compaction
strategy is updated to ensure tables follow a geometric sequence after
each operation by individually evaluating each table in reverse index
order. This strategy results in a much simpler and more robust algorithm
compared to the previous one while also maintaining a minimal ordered
set of tables on-disk.

When creating 10 thousand references, the new strategy has no
performance impact:

Benchmark 1: update-ref: create refs sequentially (revision = HEAD~)
  Time (mean ± σ):     26.516 s ±  0.047 s    [User: 17.864 s, System: 8.491 s]
  Range (min … max):   26.447 s … 26.569 s    10 runs

Benchmark 2: update-ref: create refs sequentially (revision = HEAD)
  Time (mean ± σ):     26.417 s ±  0.028 s    [User: 17.738 s, System: 8.500 s]
  Range (min … max):   26.366 s … 26.444 s    10 runs

Summary
  update-ref: create refs sequentially (revision = HEAD) ran
    1.00 ± 0.00 times faster than update-ref: create refs sequentially (revision = HEAD~)

Some tests in `t0610-reftable-basics.sh` assert the on-disk state of
tables and are therefore updated to specify the correct new table count.
Since compaction is more aggressive in ensuring tables maintain a
geometric sequence, the expected table count is reduced in these tests.
In `reftable/stack_test.c` tests related to `sizes_to_segments()` are
removed because the function is no longer needed. Also, the
`test_suggest_compaction_segment()` test is updated to better showcase
and reflect the new geometric compaction behavior.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-08 12:11:10 -07:00
Justin Tobler 7c8eb5928f reftable/stack: add env to disable autocompaction
In future tests it will be neccesary to create repositories with a set
number of tables. To make this easier, introduce the
`GIT_TEST_REFTABLE_AUTOCOMPACTION` environment variable that, when set
to false, disables autocompaction of reftables.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-08 12:11:10 -07:00
Justin Tobler bc91330cec reftable/stack: expose option to disable auto-compaction
The reftable stack already has a variable to configure whether or not to
run auto-compaction, but it is inaccessible to users of the library.
There exist use cases where a caller may want to have more control over
auto-compaction.

Move the `disable_auto_compact` option into `reftable_write_options` to
allow external callers to disable auto-compaction. This will be used in
a subsequent commit.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-08 12:11:10 -07:00
Junio C Hamano 836b221391 t1016: local VAR="VAL" fix
The series was based on maint and fixes all the tests that exist
there, but we have acquired a few more.

I suspect that the values assigned in many of these places are $IFS
safe, and this is primarily to squelch the linter than adding a
necessary workaround for buggy dash.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 22:50:11 -07:00
Junio C Hamano 26ba7477d9 t0610: local VAR="VAL" fix
The series was based on maint and fixes all the tests that exist
there, but we have acquired a few more.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 22:50:11 -07:00
Junio C Hamano 8bfe486191 t: teach lint that RHS of 'local VAR=VAL' needs to be quoted
Teach t/check-non-portable-shell.pl that right hand side of the
assignment done with "local VAR=VAL" need to be quoted.  We
deliberately target only VAL that begins with $ so that we can catch

 - $variable_reference and positional parameter reference like $4
 - $(command substitution)
 - ${variable_reference-with_magic}

while excluding

 - $'\n' that is a bash-ism freely usable in t990[23]
 - $(( arithmetic )) whose result should be $IFS safe.
 - $? that also is $IFS safe

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 22:50:11 -07:00
Junio C Hamano e97f4a6d94 t: local VAR="VAL" (quote ${magic-reference})
Future-proof test scripts that do

	local VAR=VAL

without quoting VAL (which is OK in POSIX but broken in some shells)
that is ${magic-"reference to a parameter"}.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 22:50:11 -07:00
Junio C Hamano 7f9f230b7f t: local VAR="VAL" (quote command substitution)
Future-proof test scripts that do

	local VAR=VAL

without quoting VAL (which is OK in POSIX but broken in some shells)
that is a $(command substitution).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 22:50:10 -07:00
Junio C Hamano 341aad8d41 t: local VAR="VAL" (quote positional parameters)
Future-proof test scripts that do

	local VAR=VAL

without quoting VAL (which is OK in POSIX but broken in some shells)
that is a positional parameter, e.g. $4.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 22:50:10 -07:00
Junio C Hamano be34b51049 CodingGuidelines: quote assigned value in 'local var=$val'
Dash bug https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097
lets the shell erroneously perform field splitting on the expansion
of a command substitution during declaration of a local or an extern
variable.

The explanation was stolen from ebee5580 (parallel-checkout: avoid
dash local bug in tests, 2021-06-06).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 22:50:05 -07:00
Junio C Hamano 7e3a9c23d6 CodingGuidelines: describe "export VAR=VAL" rule
https://lore.kernel.org/git/201307081121.22769.tboegi@web.de/
resulted in 9968ffff (test-lint: detect 'export FOO=bar',
2013-07-08) to add a rule to t/check-non-portable-shell.pl script to
reject

	export VAR=VAL

and suggest us to instead write it as two statements, i.e.,

	VAR=VAL
	export VAR

This however was not spelled out in the CodingGuidelines document.

We may want to re-evaluate the rule since it is from ages ago, but
for now, let's make the written rule and what the automation
enforces consistent.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 22:48:12 -07:00
Steven Jeuris ec0e3075d2 userdiff: better method/property matching for C#
- Support multi-line methods by not requiring closing parenthesis.
- Support multiple generics (comma was missing before).
- Add missing `foreach`, `lock` and  `fixed` keywords to skip over.
- Remove `instanceof` keyword, which isn't C#.
- Also detect non-method keywords not positioned at the start of a line.
- Added tests; none existed before.

The overall strategy is to focus more on what isn't expected for
method/property definitions, instead of what is, but is fully optional.

Signed-off-by: Steven Jeuris <steven.jeuris@gmail.com>
Acked-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 15:21:43 -07:00
René Scharfe 9720d23e8c date: make DATE_MODE thread-safe
date_mode_from_type() modifies a static variable and returns a pointer
to it.  This is not thread-safe.  Most callers of date_mode_from_type()
use it via the macro DATE_MODE and pass its result on to functions like
show_date(), which take a const pointer and don't modify the struct.

Avoid the static storage by putting the variable on the stack and
returning the whole struct date_mode.  Change functions that take a
constant pointer to expect the whole struct instead.

Reduce the cost of passing struct date_mode around on 64-bit systems
by reordering its members to close the hole between the 32-bit wide
.type and the 64-bit aligned .strftime_fmt as well as the alignment
hole at the end.  sizeof reports 24 before and 16 with this change
on x64.  Keep .type at the top to still allow initialization without
designator -- though that's only done in a single location, in
builtin/blame.c.

Signed-off-by: René Scharfe <l.s.r@web.de>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 15:21:14 -07:00
René Scharfe c63adab961 usage: report vsnprintf(3) failure
vreportf(), which is used e.g. by die() and warning() by default, calls
vsnprintf(3) to format the message to report.  If that call fails, it
only prints the prefix, e.g. "fatal: " or "warning: ".  This at least
informs users that they were supposed to get a message and reveals its
severity, but leaves them wondering what it may have been about.

Here's an example where vreportf() tries to print a message with a 2GB
string, which is too much for vsnprintf(3):

  $ perl -le 'print "create refs/heads/", "a"x2**31' | git update-ref --stdin
  fatal:

At least report the formatting error along with the offending message
(unformatted) to indicate why that message is empty.  Use fprintf(3)
instead of error() to get the message out directly and avoid recursing
back into vreportf().

With this patch we get:

  $ perl -le 'print "create refs/heads/", "a"x2**31' | git update-ref --stdin
  error: unable to format message: invalid ref format: %s
  fatal:

... which allows users to at least get an idea of what went wrong.

Suggested-by: Jeff King <peff@peff.net>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 15:16:27 -07:00
Jeff King 92a209bf24 remote-curl: add Transfer-Encoding header only for older curl
As of curl 7.66.0, we don't need to manually specify a "chunked"
Transfer-Encoding header. Instead, modern curl deduces the need for it
in a POST that has a POSTFIELDSIZE of -1 and uses READFUNCTION rather
than POSTFIELDS.

That version is recent enough that we can't just drop the header; we
need to do so conditionally. Since it's only a single line, it seems
like the simplest thing would just be to keep setting it unconditionally
(after all, the #ifdefs are much longer than the actual code). But
there's another wrinkle: HTTP/2.

Curl may choose to use HTTP/2 under the hood if the server supports it.
And in that protocol, we do not use the chunked encoding for streaming
at all. Most versions of curl handle this just fine by recognizing and
removing the header. But there's a regression in curl 8.7.0 and 8.7.1
where it doesn't, and large requests over HTTP/2 are broken (which t5559
notices). That regression has since been fixed upstream, but not yet
released.

Make the setting of this header conditional, which will let Git work
even with those buggy curl versions. And as a bonus, it serves as a
reminder that we can eventually clean up the code as we bump the
supported curl versions.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 14:45:19 -07:00
Junio C Hamano 19981daefd The fifteenth batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 10:49:49 -07:00
Junio C Hamano dce1e0b6da Merge branch 'jk/core-comment-string'
core.commentChar used to be limited to a single byte, but has been
updated to allow an arbitrary multi-byte sequence.

* jk/core-comment-string:
  config: add core.commentString
  config: allow multi-byte core.commentChar
  environment: drop comment_line_char compatibility macro
  wt-status: drop custom comment-char stringification
  sequencer: handle multi-byte comment characters when writing todo list
  find multi-byte comment chars in unterminated buffers
  find multi-byte comment chars in NUL-terminated strings
  prefer comment_line_str to comment_line_char for printing
  strbuf: accept a comment string for strbuf_add_commented_lines()
  strbuf: accept a comment string for strbuf_commented_addf()
  strbuf: accept a comment string for strbuf_stripspace()
  environment: store comment_line_char as a string
  strbuf: avoid shadowing global comment_line_char name
  commit: refactor base-case of adjust_comment_line_char()
  strbuf: avoid static variables in strbuf_add_commented_lines()
  strbuf: simplify comment-handling in add_lines() helper
  config: forbid newline as core.commentChar
2024-04-05 10:49:49 -07:00
Junio C Hamano 3256584c36 Merge branch 'rs/config-comment'
"git config" learned "--comment=<message>" option to leave a
comment immediately after the "variable = value" on the same line
in the configuration file.

* rs/config-comment:
  config: allow tweaking whitespace between value and comment
  config: fix --comment formatting
  config: add --comment option to add a comment
2024-04-05 10:49:49 -07:00
Junio C Hamano 7424fb7797 Merge branch 'ps/pack-refs-auto' into jt/reftable-geometric-compaction
* ps/pack-refs-auto:
  builtin/gc: pack refs when using `git maintenance run --auto`
  builtin/gc: forward git-gc(1)'s `--auto` flag when packing refs
  t6500: extract objects with "17" prefix
  builtin/gc: move `struct maintenance_run_opts`
  builtin/pack-refs: introduce new "--auto" flag
  builtin/pack-refs: release allocated memory
  refs/reftable: expose auto compaction via new flag
  refs: remove `PACK_REFS_ALL` flag
  refs: move `struct pack_refs_opts` to where it's used
  t/helper: drop pack-refs wrapper
  refs/reftable: print errors on compaction failure
  reftable/stack: gracefully handle failed auto-compaction due to locks
  reftable/stack: use error codes when locking fails during compaction
  reftable/error: discern locked/outdated errors
  reftable/stack: fix error handling in `reftable_stack_init_addition()`
2024-04-05 10:34:23 -07:00
René Scharfe 2b1f456adf apply: don't leak fd on fdopen() error
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 10:09:26 -07:00
Derrick Stolee a816ccd642 fetch: return when parsing submodule.recurse
When parsing config keys, the normal pattern is to return 0 after
completing the logic for a specific config key, since no other key will
match. One instance, for "submodule.recurse", was missing this case in
builtin/fetch.c.

This is a very minor change, and will have minimal impact to
performance. This particular block was edited recently in 56e8bb4fb4
(fetch: use `fetch_config` to store "fetch.recurseSubmodules" value,
2023-05-17), which led to some hesitation that perhaps this omission was
on purpose.

However, no later cases within git_fetch_config() will match the key if
equal to "submodule.recurse" and neither will any key matches within the
catch-all git_default_config().

Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 09:55:21 -07:00
René Scharfe 708f7e0590 path: remove mksnpath()
Remove the function mksnpath(), which has become unused.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 09:49:38 -07:00
René Scharfe 9126cb3186 apply: avoid fixed-size buffer in create_one_file()
PATH_MAX is not always a hard limit and 'path' in create_one_file()
could be longer -- it's taken from the patch file and allocated
dynamically.  Allocate the name of the temporary file on the heap as
well instead of using a fixed-size buffer to avoid that arbitrary limit.

Resist the temptation of using the more convenient mkpath() to avoid
introducing a dependency on a static variable deep inside the apply
machinery.

Take care to work around (arguably buggy) implementations of free(3)
that modify errno, by calling it only after using the errno value.

Suggested-by: Jeff King <peff@peff.net>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05 09:49:36 -07:00
Ghanshyam Thakkar 7de13cfef3 builtin/add: error out when passing untracked path with -u
When passing untracked path with -u option, it silently succeeds.
There is no error message and the exit code is zero. This is
inconsistent with other instances of git commands where the expected
argument is a known path. In those other instances, we error out when
the path is not known.

Fix this by passing a character array to add_files_to_cache() to
collect the pathspec matching information and report the error if a
pathspec does not match any cache entry. Also add a testcase to cover
this scenario.

Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 14:55:22 -07:00
Ghanshyam Thakkar ac5946e624 builtin/commit: error out when passing untracked path with -i
When we provide a pathspec which does not match any tracked path
alongside --include, we do not error like without --include. If there
is something staged, it will commit the staged changes and ignore the
pathspec which does not match any tracked path. And if nothing is
staged, it will print the status. Exit code is 0 in both cases (unlike
without --include). This is also described in the TODO comment before
the relevant testcase.

Fix this by passing a character array to add_files_to_cache() to
collect the pathspec matching information and error out if the given
path is untracked. Also, amend the testcase to check for the error
message and remove the TODO comment.

Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 14:55:22 -07:00
Junio C Hamano 86829f3f3e revision: optionally record matches with pathspec elements
Unlike "git add" and other end-user facing commands, where it is
diagnosed as an error to give a pathspec with an element that does
not match any path, the diff machinery does not care if some
elements of the pathspec do not match.  Given that the diff
machinery is heavily used in pathspec-limited "git log" machinery,
and it is common for a path to come and go while traversing the
project history, this is usually a good thing.

However, in some cases we would want to know if all the pathspec
elements matched.  For example, "git add -u <pathspec>" internally
uses the machinery used by "git diff-files" to decide contents from
what paths to add to the index, and as an end-user facing command,
"git add -u" would want to report an unmatched pathspec element.

Add a new .ps_matched member next to the .prune_data member in
"struct rev_info" so that we can optionally keep track of the use of
.prune_data pathspec elements that can be inspected by the caller.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 14:55:21 -07:00
Matthias Aßhauer 2406bf5fc5 Win32: detect unix socket support at runtime
Windows 10 build 17063 introduced support for unix sockets to Windows.
bb390b1 (git-compat-util: include declaration for unix sockets in
windows, 2021-09-14) introduced a way to build git with unix socket
support on Windows, but you still had to decide at build time which
Windows version the compiled executable was supposed to run on.

We can detect at runtime wether the operating system supports unix
sockets and act accordingly for all supported Windows versions.

This fixes https://github.com/git-for-windows/git/issues/3892

Signed-off-by: Matthias Aßhauer <mha1993@live.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 14:54:28 -07:00
Junio C Hamano 7774cfed62 The fourteenth batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 10:56:20 -07:00
Junio C Hamano 17381ab62a Merge branch 'bl/cherry-pick-empty'
Allow git-cherry-pick(1) to automatically drop redundant commits via
a new `--empty` option, similar to the `--empty` options for
git-rebase(1) and git-am(1). Includes a soft deprecation of
`--keep-redundant-commits` as well as some related docs changes and
sequencer code cleanup.

* bl/cherry-pick-empty:
  cherry-pick: add `--empty` for more robust redundant commit handling
  cherry-pick: enforce `--keep-redundant-commits` incompatibility
  sequencer: do not require `allow_empty` for redundant commit options
  sequencer: handle unborn branch with `--allow-empty`
  rebase: update `--empty=ask` to `--empty=stop`
  docs: clean up `--empty` formatting in git-rebase(1) and git-am(1)
  docs: address inaccurate `--empty` default with `--exec`
2024-04-03 10:56:20 -07:00
Junio C Hamano d988e80bd3 Merge branch 'bl/pretty-shorthand-config-fix'
The "--pretty=<shortHand>" option of the commands in the "git log"
family, defined as "[pretty] shortHand = <expansion>" should have
been looked up case insensitively, but was not, which has been
corrected.

* bl/pretty-shorthand-config-fix:
  pretty: find pretty formats case-insensitively
  pretty: update tests to use `test_config`
2024-04-03 10:56:20 -07:00
Junio C Hamano 4cc302e886 Merge branch 'rs/strbuf-expand-bad-format'
Code clean-up.

* rs/strbuf-expand-bad-format:
  cat-file: use strbuf_expand_bad_format()
  factor out strbuf_expand_bad_format()
2024-04-03 10:56:20 -07:00
Junio C Hamano f046355ec3 Merge branch 'rs/midx-use-strvec-pushf'
Code clean-up.

* rs/midx-use-strvec-pushf:
  midx: use strvec_pushf() for pack-objects base name
2024-04-03 10:56:20 -07:00
Junio C Hamano 188e94250a Merge branch 'pb/test-scripts-are-build-targets'
The t/README file now gives a hint on running individual tests in
the "t/" directory with "make t<num>-*.sh t<num>-*.sh".

* pb/test-scripts-are-build-targets:
  t/README: mention test files are make targets
2024-04-03 10:56:19 -07:00
Junio C Hamano e4193dcf12 Merge branch 'ds/grep-doc-updates'
Documentation updates.

* ds/grep-doc-updates:
  grep docs: describe --no-index further and improve formatting a bit
  grep docs: describe --recurse-submodules further and improve formatting a bit
2024-04-03 10:56:19 -07:00
Junio C Hamano e76218cad3 Merge branch 'az/grep-group-error-message-update'
Error message clarification.

* az/grep-group-error-message-update:
  grep: improve errors for unmatched ( and )
2024-04-03 10:56:19 -07:00
Junio C Hamano eda72ddc18 Merge branch 'jc/release-notes-entry-experiment'
Introduce an experimental protocol for contributors to propose the
topic description to be used in the "What's cooking" report, the
merge commit message for the topic, and in the release notes and
document it in the SubmittingPatches document.

* jc/release-notes-entry-experiment:
  SubmittingPatches: release-notes entry experiment
2024-04-03 10:56:19 -07:00
Junio C Hamano e139bb1006 Merge branch 'jk/remote-helper-object-format-option-fix'
The implementation and documentation of "object-format" option
exchange between the Git itself and its remote helpers did not
quite match, which has been corrected.

* jk/remote-helper-object-format-option-fix:
  transport-helper: send "true" value for object-format option
  transport-helper: drop "object-format <algo>" option
  transport-helper: use write helpers more consistently
2024-04-03 10:56:18 -07:00
Taylor Blau b494b1ce39 t/t7700-repack.sh: fix test breakages with GIT_TEST_MULTI_PACK_INDEX=1
There are a handful of related test breakages which are found when
running t/t7700-repack.sh with GIT_TEST_MULTI_PACK_INDEX set to "1" in
your environment.

Both test failures are the result of something like:

    git repack --write-midx --write-bitmap-index [...] &&

    test_path_is_file $midx &&
    test_path_is_file $midx-$(midx_checksum $objdir).bitmap

, where we repack instructing Git to write a new MIDX and corresponding
MIDX bitamp.

The error occurs when GIT_TEST_MULTI_PACK_INDEX=1 is found in the
enviornment. This causes Git to write out a second MIDX (after
processing the builtin's `--write-midx` argument) which is identical to
the first, but does not request a bitmap (since we did not set the
GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP variable in the environment).

Since c528e17966 (pack-bitmap: write multi-pack bitmaps, 2021-08-31),
the MIDX machinery will drop an existing MIDX bitmap when rewriting an
identical MIDX which does not itself request a corresponding bitmap,
which is similar to the way repack itself behaves in the pack-bitmap
case.

Correct these issues (which date back to [1] and [2], respectively) by
explicitly setting GIT_TEST_MULTI_PACK_INDEX to zero before running each
command.

In the future, we should consider removing GIT_TEST_MULTI_PACK_INDEX,
and in general clean up unused GIT_TEST_-variables. But that is a larger
effort, and this ensures that we can cleanly run:

    $ GIT_TEST_MULTI_PACK_INDEX=1 make test

in the meantime.

[1]: 324efc90d1 (builtin/repack.c: pass `--refs-snapshot` when writing
  bitmaps, 2021-10-01)

[2]: 197443e80a (repack: don't remove .keep packs with
  `--pack-kept-objects`, 2022-10-17).

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 10:45:36 -07:00
Patrick Steinhardt d51d8cc368 reftable/block: avoid decoding keys when searching restart points
When searching over restart points in a block we decode the key of each
of the records, which results in a memory allocation. This is quite
pointless though given that records it restart points will never use
prefix compression and thus store their keys verbatim in the block.

Refactor the code so that we can avoid decoding the keys, which saves us
some allocations.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 09:16:50 -07:00
Patrick Steinhardt cd75790707 reftable/record: extract function to decode key lengths
We're about to refactor the binary search over restart points so that it
does not need to fully decode the record keys anymore. To do so we will
need to decode the record key lengths, which is non-trivial logic.

Extract the logic to decode these lengths from `refatble_decode_key()`
so that we can reuse it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 09:16:50 -07:00
Patrick Steinhardt f9e88544f5 reftable/block: fix error handling when searching restart points
When doing the binary search over restart points in a block we need to
decode the record keys. This decoding step can result in an error when
the block is corrupted, which we indicate to the caller of the binary
search by setting `args.error = 1`. But the only caller that exists
mishandles this because it in fact performs the error check before
calling `binsearch()`.

Fix this bug by checking for errors at the right point in time.
Furthermore, refactor `binsearch()` so that it aborts the search in case
the callback function returns a negative value so that we don't
needlessly continue to search the block.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 09:16:50 -07:00
Patrick Steinhardt 77307a61d6 reftable/block: refactor binary search over restart points
When seeking a record in our block reader we perform a binary search
over the block's restart points so that we don't have to do a linear
scan over the whole block. The logic to do so is quite intricate though,
which makes it hard to understand.

Improve documentation and rename some of the functions and variables so
that the code becomes easier to understand overall. This refactoring
should not result in any change in behaviour.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 09:16:50 -07:00
Patrick Steinhardt 21767925b0 reftable/refname: refactor binary search over refnames
It is comparatively hard to understand how exactly the binary search
over refnames works given that the function and variable names are not
exactly easy to grasp. Rename them to make this more obvious. This
should not result in any change in behaviour.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 09:16:50 -07:00
Patrick Steinhardt e8b808258e reftable/basics: improve binsearch() test
The `binsearch()` test is somewhat weird in that it doesn't explicitly
spell out its expectations. Instead it does so in a rather ad-hoc way
with some hard-to-understand computations.

Refactor the test to spell out the needle as well as expected index for
all testcases. This refactoring highlights that the `binsearch_func()`
is written somewhat weirdly to find the first integer smaller than the
needle, not smaller or equal to it. Adjust the function accordingly.

While at it, rename the callback function to better convey its meaning.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 09:16:49 -07:00
Patrick Steinhardt 3e7b36d129 reftable/basics: fix return type of binsearch() to be size_t
The `binsearch()` function can be used to find the first element for
which a callback functions returns a truish value. But while the array
size is of type `size_t`, the function in fact returns an `int` that is
supposed to index into that array.

Fix the function signature to return a `size_t`. This conversion does
not change any semantics given that the function would only ever return
a value in the range `[0, sz]` anyway.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03 09:16:49 -07:00
René Scharfe 543b2a1083 t-prio-queue: simplify using compound literals
Test names like "basic" are mentioned seven times in the code (ignoring
case): Twice when defining the input and result macros, thrice when
defining the test function, and twice again when calling it.  Reduce
that to a single time by using compound literals to pass the input and
result arrays via TEST_INPUT to test_prio_queue().

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-02 13:41:32 -07:00