Junio C Hamano [Fri, 5 Apr 2024 17:34:23 +0000 (10:34 -0700)]
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()`
Junio C Hamano [Thu, 28 Mar 2024 21:13:50 +0000 (14:13 -0700)]
Merge branch 'jc/show-untracked-false'
The status.showUntrackedFiles configuration variable had a name
that tempts users to set a Boolean value expressed in our usual
"false", "off", and "0", but it only took "no". This has been
corrected so "true" and its synonyms are taken as "normal", while
"false" and its synonyms are taken as "no".
* jc/show-untracked-false:
status: allow --untracked=false and friends
status: unify parsing of --untracked= and status.showUntrackedFiles
Junio C Hamano [Thu, 28 Mar 2024 21:13:50 +0000 (14:13 -0700)]
Merge branch 'ph/diff-src-dst-prefix-config'
"git diff" and friends learned two extra configuration variables,
diff.srcPrefix and diff.dstPrefix.
* ph/diff-src-dst-prefix-config:
diff.*Prefix: use camelCase in the doc and test titles
diff: add diff.srcPrefix and diff.dstPrefix configuration variables
Junio C Hamano [Thu, 28 Mar 2024 21:13:50 +0000 (14:13 -0700)]
Merge branch 'eb/hash-transition'
Work to support a repository that work with both SHA-1 and SHA-256
hash algorithms has started.
* eb/hash-transition: (30 commits)
t1016-compatObjectFormat: add tests to verify the conversion between objects
t1006: test oid compatibility with cat-file
t1006: rename sha1 to oid
test-lib: compute the compatibility hash so tests may use it
builtin/ls-tree: let the oid determine the output algorithm
object-file: handle compat objects in check_object_signature
tree-walk: init_tree_desc take an oid to get the hash algorithm
builtin/cat-file: let the oid determine the output algorithm
rev-parse: add an --output-object-format parameter
repository: implement extensions.compatObjectFormat
object-file: update object_info_extended to reencode objects
object-file-convert: convert commits that embed signed tags
object-file-convert: convert commit objects when writing
object-file-convert: don't leak when converting tag objects
object-file-convert: convert tag objects when writing
object-file-convert: add a function to convert trees between algorithms
object: factor out parse_mode out of fast-import and tree-walk into in object.h
cache: add a function to read an OID of a specific algorithm
tag: sign both hashes
commit: export add_header_signature to support handling signatures on tags
...
builtin/gc: pack refs when using `git maintenance run --auto`
When running `git maintenance run --auto`, then the various subtasks
will only run as needed. Thus, we for example end up only packing loose
objects if we hit a certain threshold.
Interestingly enough, the "pack-refs" task is actually _never_ executed
when the auto-flag is set because it does not have a condition at all.
As 41abfe15d9 (maintenance: add pack-refs task, 2021-02-09) mentions:
The 'auto_condition' function pointer is left NULL for now. We could
extend this in the future to have a condition check if pack-refs
should be run during 'git maintenance run --auto'.
It is not quite clear from that quote whether it is actually intended
that the task doesn't run at all in this mode. Also, no test was added
to verify this behaviour. Ultimately though, it feels quite surprising
that `git maintenance run --auto --task=pack-refs` would quietly never
do anything at all.
In any case, now that we do have the logic in place to let ref backends
decide whether or not to repack refs, it does make sense to wire it up
accordingly. With the "reftable" backend we will thus now perform
auto-compaction, which optimizes the refdb as needed.
But for the "files" backend we now unconditionally pack refs as it does
not yet know to handle the "auto" flag. Arguably, this can be seen as a
bug fix given that previously the task never did anything at all.
Eventually though we should amend the "files" backend to use some
heuristics for auto compaction, as well.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/gc: forward git-gc(1)'s `--auto` flag when packing refs
Forward the `--auto` flag to git-pack-refs(1) when it has been invoked
with this flag itself. This does not change anything for the "files"
backend, which will continue to eagerly pack refs. But it does ensure
that the "reftable" backend only compacts refs as required.
This change does not impact git-maintenance(1) because this command will
in fact never run the pack-refs task when run with `--auto`. This issue
will be addressed in a subsequent commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The ".git/obects/17/" shard is somewhat special because it is used by
git-gc(1) to estimate how many objects there are by extrapolating the
number of objects in that shard, only. In t6500 we thus have a hard
coded set of data that, when written to the object database, result in
blobs starting with that prefix.
We are about to need such "17"-prefixed objects in another test suite.
Extract them into "t/oid-info/hash-info" so that they can be reused by
other tests.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Calling git-pack-refs(1) will unconditionally cause it to pack all
requested refs regardless of the current state of the ref database. For
example:
- With the "files" backend we will end up rewriting the complete
"packed-refs" file even if only a single ref would require
compaction.
- With the "reftable" backend we will end up always compacting all
tables into a single table.
This behaviour can be completely unnecessary depending on the backend
and is thus wasteful.
With the introduction of the `PACK_REFS_AUTO` flag in the preceding
commit we can improve this and let the backends decide for themselves
whether to pack refs in the first place. Expose this functionality via a
new "--auto" flag in git-pack-refs(1), which mirrors the same flag in
both git-gc(1) and git-maintenance(1).
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some of the command line options in `cmd_pack_refs()` require us to
allocate memory. This memory is never released and thus leaking, but we
paper over this leak by declaring the respective variables as `static`
function-level variables, which is somewhat awkward.
Refactor the code to release the allocated memory and drop the `static`
declaration. While at it, remove the useless `flags` variable.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs/reftable: expose auto compaction via new flag
Under normal circumstances, the "reftable" backend will automatically
perform compaction after appending to the stack. It is thus not
necessary and may even be considered wasteful to run git-pack-refs(1) in
"reftable"-backed repositories as it will cause the backend to compact
all tables into a single one. We do exactly that though when running
`git maintenance run --auto` or `git gc --auto`, which gets spawned by
Git after running some specific commands.
The `--auto` mode is typically only executing optimizations as needed.
To do so, we already use several heuristics for the various different
data structures in Git to determine whether to optimize them or not.
We do not use any heuristics for refs though and instead always optimize
them.
Introduce a new `PACK_REFS_AUTO` flag that can be passed to the backend.
When not handled by the backend we will continue to behave the exact
same as we do right now, that is we optimize refs unconditionally. This
is done for the "files" backend for now to retain current behaviour,
even though we may eventually also want to introduce heuristics here.
For the "reftable" backend though we already do have auto-compaction, so
we can easily reuse that logic to implement the new auto-packing flag.
Note that under normal circumstances, this should always end up being a
no-op. After all, we already invoke the code for every single addition
to the stack. But there are special cases where it can still be helpful
to execute the auto-compaction code explicitly:
- Concurrent writers may cause compaction to not run due to locks.
- Callers may decide to disable compaction altogether and then pack
refs at a later point due to various reasons.
- Other implementations of the reftable format may do compaction
differently or even not at all.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The intent of the `PACK_REFS_ALL` flag is to ask the backend to compact
all refs instead of only a subset of them. Thus, this flag gets passed
down to `refs_pack_refs()` via `struct pack_refs_opts::flags`.
But starting with 4fe42f326e (pack-refs: teach pack-refs --include
option, 2023-05-12), the flag's semantics have changed. Instead of being
handled by the respective backends, this flag is now getting handled by
the callers of `refs_pack_refs()` which will add a single glob ("*") to
the list of refs-to-be-packed. Thus, the flag serves no purpose to the
ref backends anymore.
Remove the flag and replace it with a local variable.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs: move `struct pack_refs_opts` to where it's used
The declaration of `struct pack_refs_opts` is in a seemingly random
place. Move it so that it's located right next to its flags and
functions that use it.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The test helper provides a "ref-store <store> pack-refs" wrapper that
more or less directly invokes `refs_pack_refs()`. This helper is only
used in a single test with the "PACK_REFS_PRUNE" and "PACK_REFS_ALL"
flags. Both of these flags can directly be accessed via git-pack-refs(1)
though via the `--all` and `--prune` flags, which makes the helper
superfluous.
Refactor the test to use git-pack-refs(1) instead of the test helper.
Drop the now-unused test helper command.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When git-pack-refs(1) fails in the reftable backend we end up printing
no error message at all, leaving the caller puzzled as to why compaction
has failed. Fix this.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
reftable/stack: gracefully handle failed auto-compaction due to locks
Whenever we commit a new table to the reftable stack we will end up
invoking auto-compaction of the stack to keep the total number of tables
at bay. This auto-compaction may fail though in case at least one of the
tables which we are about to compact is locked. This is indicated by the
compaction function returning `REFTABLE_LOCK_ERROR`. We do not handle
this case though, and thus bubble that return value up the calling
chain, which will ultimately cause a failure.
Fix this bug by ignoring `REFTABLE_LOCK_ERROR`.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
reftable/stack: use error codes when locking fails during compaction
Compaction of a reftable stack may fail gracefully when there is a
concurrent process that writes to the reftable stack and which has thus
locked either the "tables.list" file or one of the tables. This is
expected and can be handled gracefully by some of the callers which
invoke compaction. Thus, to indicate this situation to our callers, we
return a positive return code from `stack_compact_range()` and bubble it
up to the caller.
This kind of error handling is somewhat awkward though as many callers
in the call chain never even think of handling positive return values.
Thus, the result is either that such errors are swallowed by accident,
or that we abort operations with an unhelpful error message.
Make the code more robust by always using negative error codes when
compaction fails, with `REFTABLE_LOCK_ERROR` for the described benign
error case.
Note that only a single callsite knew to handle positive error codes
gracefully in the first place. Subsequent commits will touch up some of
the other sites to handle those errors better.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We currently throw two different errors into a similar-but-different
error code:
- Errors when trying to lock the reftable stack.
- Errors when trying to write to the reftable stack which has been
modified concurrently.
This results in unclear error handling and user-visible error messages.
Create a new `REFTABLE_OUTDATED_ERROR` so that those error conditions
can be clearly told apart from each other. Adjust users of the old
`REFTABLE_LOCK_ERROR` to use the new error code as required.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
reftable/stack: fix error handling in `reftable_stack_init_addition()`
In `reftable_stack_init_addition()` we call `stack_uptodate()` after
having created the lockfile to check whether the stack was modified
concurrently, which is indicated by a positive return code from the
latter function. If so, we return a `REFTABLE_LOCK_ERROR` to the caller
and abort the addition.
The error handling has an off-by-one though because we check whether the
error code is `> 1` instead of `> 0`. Thus, instead of returning the
locking error, we would return a positive value. One of the callers of
`reftable_stack_init_addition()` works around this bug by repeating the
error code check without the off-by-one. But other callers are subtly
broken by this bug.
Fix this by checking for `err > 0` instead. This has the consequence
that `reftable_stack_init_addition()` won't ever return a positive error
code anymore, but will instead return `REFTABLE_LOCK_ERROR` now. Thus,
we can drop the check for a positive error code in `stack_try_add()`
now.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* ps/reftable-reflog-iteration-perf:
refs/reftable: track last log record name via strbuf
reftable/record: use scratch buffer when decoding records
reftable/record: reuse message when decoding log records
reftable/record: reuse refnames when decoding log records
reftable/record: avoid copying author info
reftable/record: convert old and new object IDs to arrays
refs/reftable: reload correct stack when creating reflog iter
Junio C Hamano [Thu, 21 Mar 2024 21:55:13 +0000 (14:55 -0700)]
Merge branch 'jc/safe-implicit-bare'
Users with safe.bareRepository=explicit can still work from within
$GIT_DIR of a seconary worktree (which resides at .git/worktrees/$name/)
of the primary worktree without explicitly specifying the $GIT_DIR
environment variable or the --git-dir=<path> option.
* jc/safe-implicit-bare:
setup: notice more types of implicit bare repositories
Junio C Hamano [Thu, 21 Mar 2024 21:55:12 +0000 (14:55 -0700)]
Merge branch 'ps/reftable-block-search-fix'
The reftable code has its own custom binary search function whose
comparison callback has an unusual interface, which caused the
binary search to degenerate into a linear search, which has been
corrected.
* ps/reftable-block-search-fix:
reftable/block: fix binary search over restart counter
reftable/record: fix memory leak when decoding object records
Junio C Hamano [Thu, 21 Mar 2024 21:55:12 +0000 (14:55 -0700)]
Merge branch 'ps/reftable-stack-tempfile'
The code in reftable backend that creates new table files works
better with the tempfile framework to avoid leaving cruft after a
failure.
* ps/reftable-stack-tempfile:
reftable/stack: register compacted tables as tempfiles
reftable/stack: register lockfiles during compaction
reftable/stack: register new tables as tempfiles
lockfile: report when rollback fails
Jeff King [Wed, 20 Mar 2024 09:48:24 +0000 (05:48 -0400)]
contrib: drop hg-to-git script
The hg-to-git script is full of command injection vulnerabilities
against malicious branch and tag names. It's also old and largely
unmaintained; the last commit was over 4 years ago, and the last code
change before that was from 2013. Users are better off with a modern
remote-helper tool like cinnabar or remote-hg.
So rather than spending time to fix it, let's just get rid of it.
Reported-by: Matthew Rollings <admin@stealthcopter.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 18 Mar 2024 20:24:32 +0000 (13:24 -0700)]
Merge branch 'ps/reftable-stack-tempfile' into ps/pack-refs-auto
* ps/reftable-stack-tempfile:
reftable/stack: register compacted tables as tempfiles
reftable/stack: register lockfiles during compaction
reftable/stack: register new tables as tempfiles
lockfile: report when rollback fails
* jh/trace2-missing-def-param-fix:
trace2: emit 'def_param' set with 'cmd_name' event
trace2: avoid emitting 'def_param' set more than once
t0211: demonstrate missing 'def_param' events for certain commands
The description for `key_value_separator` incorrectly states that this
separator is inserted between trailer lines, which appears likely to
have been incorrectly copied from `separator` when this option was
added.
Update the description to correctly indicate that it is a separator that
appears between the key and the value of each trailer.
Signed-off-by: Brian Lyles <brianmlyles@gmail.com> Acked-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sat, 16 Mar 2024 21:09:47 +0000 (22:09 +0100)]
t-prio-queue: check result array bounds
Avoid reading past the end of the "result" array, which could otherwise
happen if the prio-queue were to yield more items than were put into it
due to an implementation bug, or if the array has not enough entries due
to a test bug.
Also check at the end whether all "result" entries were consumed, which
would not be the case if the prio-queue forgot some entries or the test
definition contained too many.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sat, 16 Mar 2024 20:45:51 +0000 (21:45 +0100)]
t-prio-queue: shorten array index message
If we get an unexpected result, the prio-queue unit test reports it like
this:
# check "result[j++] == show(get)" failed at t/unit-tests/t-prio-queue.c:43
# left: 5
# right: 1
# failed at result[] index 0
That last line repeats "failed" and "result" from the first line.
Shorten it to resemble a similar one in t-ctype and also remove the
incrementation from the first line to avoid possible distractions from
the message of which comparison went wrong where:
Junio C Hamano [Sat, 16 Mar 2024 05:57:22 +0000 (22:57 -0700)]
diff.*Prefix: use camelCase in the doc and test titles
We added documentation for diff.srcPrefix and diff.dstPrefix with
their names properly camelCased, but the diff.noPrefix is listed
there in all lowercase. Also these configuration variables, both
existing ones and the {src,dst}Prefix we recently added, were
spelled in all lowercase in the tests in t4013.
Now we are done with the main change, clean these up.
Signed-off-by: Junio C Hamano <gitster@pobox.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Beat Bolli [Fri, 15 Mar 2024 19:45:58 +0000 (20:45 +0100)]
doc: avoid redundant use of cat
The update-hook-example.txt script uses this anti-pattern twice. Call grep
with the input file name directy. While at it, merge the two consecutive
grep calls.
Signed-off-by: Beat Bolli <dev+git@drbeat.li> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Brian Lyles [Sat, 16 Mar 2024 05:01:24 +0000 (00:01 -0500)]
docs: clarify file options in git-config `--edit`
The description for the `-e`/`--edit` option references scopes
inconsistently: system and global are referenced by their option name
(`--system`/`--global`), but repository (`--local` is not. Additionally,
neither `--worktree` nor `--file` are referenced at all, despite also
being a valid options.
Update the description to mention all four available scopes as well as
`--file`, referencing each consistently by their option name.
Signed-off-by: Brian Lyles <brianmlyles@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jiamu Sun [Thu, 14 Mar 2024 04:00:16 +0000 (04:00 +0000)]
bugreport.c: fix a crash in `git bugreport` with `--no-suffix` option
`git bugreport` does not complain when `--no-suffix` is given, but
it leads to a segmentation fault as the it is not prepared to see a
NULL assigned to the option_suffix variable.
Signed-off-by: Jiamu Sun <barroit@linux.com> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Fri, 15 Mar 2024 23:05:59 +0000 (16:05 -0700)]
Merge branch 'kh/branch-ref-syntax-advice'
When git refuses to create a branch because the proposed branch
name is not a valid refname, an advice message is given to refer
the user to exact naming rules.
* kh/branch-ref-syntax-advice:
branch: advise about ref syntax rules
advice: use double quotes for regular quoting
advice: use backticks for verbatim
advice: make all entries stylistically consistent
t3200: improve test style
Brian C Tracy [Fri, 15 Mar 2024 05:47:31 +0000 (05:47 +0000)]
fuzz: add fuzzer for config parsing
Add a new fuzz target that exercises the parsing of git configs.
The existing git_config_from_mem function is a perfect entry point
for fuzzing as it exercises the same code paths as the rest of the
config parsing functions and offers an easily fuzzable interface.
Config parsing is a useful thing to fuzz because it operates on user
controlled data and is a central component of many git operations.
Signed-off-by: Brian C Tracy <brian.tracy33@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
John Cai [Fri, 15 Mar 2024 04:57:26 +0000 (04:57 +0000)]
t5300: fix test_with_bad_commit()
0f8edf7317 (index-pack: --fsck-objects to take an optional argument for
fsck msgs, 2024-02-01) added a test function test_with_bad_commit() that
contained two bugs. test_expect_fail was used instead of test_must_fail,
and a && was not included at the end of the line.
Fix these two issues in the test.
Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 14 Mar 2024 21:05:25 +0000 (14:05 -0700)]
Merge branch 'rj/complete-worktree-paths-fix'
The logic to complete the command line arguments to "git worktree"
subcommand (in contrib/) has been updated to correctly honor things
like "git -C dir" etc.
Junio C Hamano [Thu, 14 Mar 2024 21:05:24 +0000 (14:05 -0700)]
Merge branch 'jc/test-i18ngrep'
With release 2.44 we got rid of all uses of test_i18ngrep and there
is no in-flight topic that adds a new use of it. Make a call to
test_i18ngrep a hard failure, so that we can remove it at the end
of this release cycle.
* jc/test-i18ngrep:
test_i18ngrep: hard deprecate and forbid its use
Junio C Hamano [Thu, 14 Mar 2024 21:05:24 +0000 (14:05 -0700)]
Merge branch 'la/trailer-api'
Trailer API updates.
Acked-by: Christian Couder <christian.couder@gmail.com>
cf. <CAP8UFD1Zd+9q0z1JmfOf60S2vn5-sD3SafDvAJUzRFwHJKcb8A@mail.gmail.com>
* la/trailer-api:
format_trailers_from_commit(): indirectly call trailer_info_get()
format_trailer_info(): move "fast path" to caller
format_trailers(): use strbuf instead of FILE
trailer_info_get(): reorder parameters
trailer: move interpret_trailers() to interpret-trailers.c
trailer: reorder format_trailers_from_commit() parameters
trailer: rename functions to use 'trailer'
shortlog: add test for de-duplicating folded trailers
trailer: free trailer_info _after_ all related usage
projects / thirdparty / git.git / log
