The previous commit simplified the way that revision.c parses ".." and
"..." range operators. But there's roughly similar code in rev-parse.
This is less likely to trigger a segfault, as there is no library
function which we'd pass a string literal to, but it still causes the
compiler to complain about laundering away constness via strstr().
Let's give it the same treatment, copying the left-hand side of the
range operator into its own string.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The logic to count objects has been cleaned up.
* ps/object-counting:
odb: introduce generic object counting
odb/source: introduce generic object counting
object-file: generalize counting objects
object-file: extract logic to approximate object count
packfile: extract logic to count number of objects
odb: stop including "odb/source.h"
Further work on incremental repacking using MIDX/bitmap
* tb/incremental-midx-part-3.2:
midx: enable reachability bitmaps during MIDX compaction
midx: implement MIDX compaction
t/helper/test-read-midx.c: plug memory leak when selecting layer
midx-write.c: factor fanout layering from `compute_sorted_entries()`
midx-write.c: enumerate `pack_int_id` values directly
midx-write.c: extract `fill_pack_from_midx()`
midx-write.c: introduce `midx_pack_perm()` helper
midx: do not require packs to be sorted in lexicographic order
midx-write.c: introduce `struct write_midx_opts`
midx-write.c: don't use `pack_perm` when assigning `bitmap_pos`
t/t5319-multi-pack-index.sh: fix copy-and-paste error in t5319.39
git-multi-pack-index(1): align SYNOPSIS with 'git multi-pack-index -h'
git-multi-pack-index(1): remove non-existent incompatibility
builtin/multi-pack-index.c: make '--progress' a common option
midx: introduce `midx_get_checksum_hex()`
midx: rename `get_midx_checksum()` to `midx_get_checksum_hash()`
midx: mark `get_midx_checksum()` arguments as const
Add a coccinelle rule to break the build when "struct strbuf" gets
passed by value.
* dd/cocci-do-not-pass-strbuf-by-value:
stash: do not pass strbuf by value
coccinelle: detect struct strbuf passed by value
Reduce system overhead "git upload-pack" spends on relaying "git
pack-objects" output to the "git fetch" running on the other end of
the connection.
* ps/upload-pack-buffer-more-writes:
builtin/pack-objects: reduce lock contention when writing packfile data
csum-file: drop `hashfd_throughput()`
csum-file: introduce `hashfd_ext()`
sideband: use writev(3p) to send pktlines
wrapper: introduce writev(3p) wrappers
compat/posix: introduce writev(3p) wrapper
upload-pack: reduce lock contention when writing packfile data
upload-pack: prefer flushing data over sending keepalive
upload-pack: adapt keepalives based on buffering
upload-pack: fix debug statement when flushing packfile data
"git history" learned the "split" subcommand.
* ps/history-split:
builtin/history: implement "split" subcommand
builtin/history: split out extended function to create commits
cache-tree: allow writing in-memory index as tree
add-patch: allow disabling editing of hunks
add-patch: add support for in-memory index patching
add-patch: remove dependency on "add-interactive" subsystem
add-patch: split out `struct interactive_options`
add-patch: split out header from "add-interactive.h"
"git fast-import" learned to optionally replace signature on
commits whose signatures get invalidated due to replaying by
signing afresh.
* jt/fast-import-sign-again:
fast-import: add mode to sign commits with invalid signatures
gpg-interface: allow sign_buffer() to use default signing key
commit: remove unused forward declaration
The code in "git help" that shows configuration items in sorted
order was awkwardly organized and prone to bugs.
* ac/help-sort-correctly:
help: cleanup the contruction of keys_uniq
Instead of hardcoded 'origin', use the configured default remote
when fetching from submodules.
* ng/submodule-default-remote:
submodule: fetch missing objects from default remote
The run_command() API lost its implicit dependencyon the singleton
`the_repository` instance.
* bk/run-command-wo-the-repository:
run-command: wean auto_maintenance() functions off the_repository
run-command: wean start_command() off the_repository
The way combined list-object filter options are parsed has been
revamped.
* dd/list-objects-filter-options-wo-strbuf-split:
list-objects-filter-options: avoid strbuf_split_str()
worktree: do not pass strbuf by value
"git repo structure" command learns to report maximum values on
various aspects of objects it inspects.
* jt/repo-structure-extrema:
builtin/repo: find tree with most entries
builtin/repo: find commit with most parents
builtin/repo: add OID annotations to table output
builtin/repo: collect largest inflated objects
builtin/repo: add helper for printing keyvalue output
builtin/repo: update stats for each object
save_untracked_files() takes its 'files' parameter as struct strbuf
by value. Passing a strbuf by value copies the struct but shares
the underlying buffer between caller and callee, risking a dangling
pointer and double-free if the callee reallocates.
The function needs both the buffer and its length for
pipe_command(), so a plain const char * is not sufficient here.
Switch the parameter to struct strbuf * and update the caller to
pass a pointer.
Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When running `git pack-objects --stdout` we feed the data through
`hashfd_ext()` with a progress meter and a smaller-than-usual buffer
length of 8kB so that we can track throughput more granularly. But as
packfiles tend to be on the larger side, this small buffer size may
cause a ton of write(3p) syscalls.
Originally, the buffer we used in `hashfd()` was 8kB for all use cases.
This was changed though in 2ca245f8be (csum-file.h: increase hashfile
buffer size, 2021-05-18) because we noticed that the number of writes
can have an impact on performance. So the buffer size was increased to
128kB, which improved performance a bit for some use cases.
But the commit didn't touch the buffer size for `hashd_throughput()`.
The reasoning here was that callers expect the progress indicator to
update frequently, and a larger buffer size would of course reduce the
update frequency especially on slow networks.
While that is of course true, there was (and still is, even though it's
now a call to `hashfd_ext()`) only a single caller of this function in
git-pack-objects(1). This command is responsible for writing packfiles,
and those packfiles are often on the bigger side. So arguably:
- The user won't care about increments of 8kB when packfiles tend to
be megabytes or even gigabytes in size.
- Reducing the number of syscalls would be even more valuable here
than it would be for multi-pack indices, which was the benchmark
done in the mentioned commit, as MIDXs are typically significantly
smaller than packfiles.
- Nowadays, many internet connections should be able to transfer data
at a rate significantly higher than 8kB per second.
Update the buffer to instead have a size of `LARGE_PACKET_DATA_MAX - 1`,
which translates to ~64kB. This limit was chosen because `git
pack-objects --stdout` is most often used when sending packfiles via
git-upload-pack(1), where packfile data is chunked into pktlines when
using the sideband. Furthermore, most internet connections should have a
bandwidth signifcantly higher than 64kB/s, so we'd still be able to
observe progress updates at a rate of at least once per second.
This change significantly reduces the number of write(3p) syscalls from
355,000 to 44,000 when packing the Linux repository. While this results
in a small performance improvement on an otherwise-unused system, this
improvement is mostly negligible. More importantly though, it will
reduce lock contention in the kernel on an extremely busy system where
we have many processes writing data at once.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `hashfd_throughput()` function is used by a single callsite in
git-pack-objects(1). In contrast to `hashfd()`, this function uses a
progress meter to measure throughput and a smaller buffer length so that
the progress meter can provide more granular metrics.
We're going to change that caller in the next commit to be a bit more
specific to packing objects. As such, `hashfd_throughput()` will be a
somewhat unfitting mechanism for any potential new callers.
Drop the function and replace it with a call to `hashfd_ext()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
With git-fast-import(1), handling of signed commits is controlled via
the `--signed-commits=<mode>` option. When an invalid signature is
encountered, a user may want the option to sign the commit again as
opposed to just stripping the signature. To facilitate this, introduce a
"sign-if-invalid" mode for the `--signed-commits` option. Optionally, a
key ID may be explicitly provided in the form
`sign-if-invalid[=<keyid>]` to specify which signing key should be used
when signing invalid commit signatures.
Note that to properly support interoperability mode when signing commit
signatures, the commit buffer must be created in both the repository and
compatability object formats to generate the appropriate signatures
accordingly. As currently implemented, the commit buffer for the
compatability object format is not reconstructed and thus signing
commits in interoperability mode is not yet supported. Support may be
added in the future.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback
logic to get the default configured signing key when a key is not
provided and handles generating the commit signature accordingly. This
signing operation is not really specific to commits as any arbitrary
buffer can be signed. Also, in a subsequent commit, this same logic is
reused by git-fast-import(1) when signing commits with invalid
signatures.
Remove the `sign_commit_to_strbuf()` helper from "commit.c" and extend
`sign_buffer()` in "gpg-interface.c" to support using the default key as
a fallback when the `SIGN_BUFFER_USE_DEFAULT_KEY` flag is provided. Call
sites are updated accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The object source API is getting restructured to allow plugging new
backends.
* ps/odb-sources:
odb/source: make `begin_transaction()` function pluggable
odb/source: make `write_alternate()` function pluggable
odb/source: make `read_alternates()` function pluggable
odb/source: make `write_object_stream()` function pluggable
odb/source: make `write_object()` function pluggable
odb/source: make `freshen_object()` function pluggable
odb/source: make `for_each_object()` function pluggable
odb/source: make `read_object_stream()` function pluggable
odb/source: make `read_object_info()` function pluggable
odb/source: make `close()` function pluggable
odb/source: make `reprepare()` function pluggable
odb/source: make `free()` function pluggable
odb/source: introduce source type for robustness
odb: move reparenting logic into respective subsystems
odb: embed base source in the "files" backend
odb: introduce "files" source
odb: split `struct odb_source` into separate header
"git for-each-repo" started from a secondary worktree did not work
as expected, which has been corrected.
* ds/for-each-repo-w-worktree:
for-each-repo: simplify passing of parameters
for-each-repo: work correctly in a worktree
run-command: extract sanitize_repo_env helper
for-each-repo: test outside of repo context
A bit of OIDmap API enhancement and cleanup.
* sk/oidmap-clear-with-custom-free-func:
builtin/rev-list: migrate missing_objects cleanup to oidmap_clear_with_free()
oidmap: make entry cleanup explicit in oidmap_clear
The 'cmd_mktree()' function already receives a 'struct repository *repo'
pointer, but it was previously marked as UNUSED.
Pass the 'repo' pointer down to 'mktree_line()' and 'write_tree()'.
Consequently, remove the 'USE_THE_REPOSITORY_VARIABLE' macro, replace
usages of 'the_repository', and swap 'parse_oid_hex()' with its context-aware
version 'parse_oid_hex_algop()'.
This refactoring is safe because 'cmd_mktree()' is registered with the
'RUN_SETUP' flag in 'git.c', which guarantees that the command is
executed within a initialized repository, ensuring that the passed 'repo'
pointer is never 'NULL'.
Signed-off-by: Tian Yuchen <cat@malon.dev>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar to the preceding commit, introduce counting of objects on the
object database level, replacing the logic that we have in
`repo_approximate_object_count()`.
Note that the function knows to cache the object count. It's unclear
whether this cache is really required as we shouldn't have that many
cases where we count objects repeatedly. But to be on the safe side the
caching mechanism is retained, with the only excepting being that we
also have to use the passed flags as caching key.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Generalize the function introduced in the preceding commit to not only
be able to approximate the number of loose objects, but to also provide
an accurate count. The behaviour can be toggled via a new flag.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In "builtin/gc.c" we have some logic that checks whether we need to
repack objects. This is done by counting the number of objects that we
have and checking whether it exceeds a certain threshold. We don't
really need an accurate object count though, which is why we only
open a single object directory shard and then extrapolate from there.
Extract this logic into a new function that is owned by the loose object
database source. This is done to prepare for a subsequent change, where
we'll introduce object counting on the object database source level.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "odb.h" header currently includes the "odb/source.h" file. This is
somewhat roundabout though: most callers shouldn't have to care about
the `struct odb_source`, but should rather use the ODB-level functions.
Furthermore, it means that a couple of definitions have to live on the
source level even though they should be part of the generic interface.
Reverse the relation between "odb/source.h" and "odb.h" and move the
enums and typedefs that relate to the generic interfaces back into
"odb.h". Add the necessary includes to all files that rely on the
transitive include.
Suggested-by: Justin Tobler <jltobler@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The prepare_auto_maintenance() relies on the_repository to read
configurations. Since run_auto_maintenance() calls
prepare_auto_maintenance(), it also implicitly depends the_repository.
Add 'struct repository *' as a parameter to both functions and update
all callers to pass the_repository.
With no global repository dependencies left in this file, remove the
USE_THE_REPOSITORY_VARIABLE macro.
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Burak Kaan Karaçay <bkkaracay@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The start_command() relies on the_repository due to the
close_object_store flag in 'struct child_process'. When this flag is
set, start_command() closes the object store associated with
the_repository before spawning a child process.
To eliminate this dependency, replace the 'close_object_store' with the
new 'struct object_database *odb_to_close' field. This allows callers to
specify the object store that needs to be closed.
Suggested-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Burak Kaan Karaçay <bkkaracay@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
construction of keys_uniq depends on sort operation
executed on keys before processing, which does not
gurantee that keys_uniq will be sorted.
refactor the code to shift the sort operation after
the processing to remove dependency on key's sort operation
and strictly maintain the sorted order of keys_uniq.
move strbuf init and release out of loop to reuse same buffer.
dedent sort -u and sed in tests and replace grep with sed, to
avoid piping grep's output to sed.
Suggested-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
Signed-off-by: Amisha Chhajed <amishhhaaaa@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The submodule_summary_callback() function currently uses a raw malloc()
which could lead to a NULL pointer dereference.
Standardize this by replacing malloc() with xmalloc() for error handling.
To improve maintainability, use sizeof(*temp) instead of the struct name,
and drop the typecast of void pointer assignment.
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
write_worktree_linking_files() takes two struct strbuf parameters by
value, even though it only reads path strings from them.
Passing a strbuf by value is misleading and dangerous. The structure
carries a pointer to its underlying character array; caller and callee
end up sharing that storage. If the callee ever causes the strbuf to
be reallocated, the caller's copy becomes a dangling pointer, which
results in a double-free when the caller does strbuf_release().
The function only needs the string values, not the strbuf machinery.
Switch it to take const char * and update all callers to pass .buf.
Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The parse-options API learned to notice an options[] array with
duplicated long options.
* rs/parse-options-duplicated-long-options:
parseopt: check for duplicate long names and numerical options
pack-objects: remove duplicate --stdin-packs definition
Allow hook commands to be defined (possibly centrally) in the
configuration files, and run multiple of them for the same hook
event.
* ar/config-hooks:
hook: add -z option to "git hook list"
hook: allow out-of-repo 'git hook' invocations
hook: allow event = "" to overwrite previous values
hook: allow disabling config hooks
hook: include hooks from the config
hook: add "git hook list" command
hook: run a list of hooks to prepare for multihook support
hook: add internal state alloc/free callbacks
The configuration variable format.noprefix did not behave as a
proper boolean variable, which has now been fixed and documented.
* kh/format-patch-noprefix-is-boolean:
doc: diff-options.adoc: make *.noprefix split translatable
doc: diff-options.adoc: show format.noprefix for format-patch
format-patch: make format.noprefix a boolean
* ps/odb-sources:
odb/source: make `begin_transaction()` function pluggable
odb/source: make `write_alternate()` function pluggable
odb/source: make `read_alternates()` function pluggable
odb/source: make `write_object_stream()` function pluggable
odb/source: make `write_object()` function pluggable
odb/source: make `freshen_object()` function pluggable
odb/source: make `for_each_object()` function pluggable
odb/source: make `read_object_stream()` function pluggable
odb/source: make `read_object_info()` function pluggable
odb/source: make `close()` function pluggable
odb/source: make `reprepare()` function pluggable
odb/source: make `free()` function pluggable
odb/source: introduce source type for robustness
odb: move reparenting logic into respective subsystems
odb: embed base source in the "files" backend
odb: introduce "files" source
odb: split `struct odb_source` into separate header
"git add <submodule>" has been taught to honor
submodule.<name>.ignore that is set to "all" (and requires "git add
-f" to override it).
* cs/add-skip-submodule-ignore-all:
Documentation: update add --force option + ignore=all config
tests: fix existing tests when add an ignore=all submodule
tests: t2206-add-submodule-ignored: ignore=all and add --force tests
read-cache: submodule add need --force given ignore=all configuration
read-cache: update add_files_to_cache take param ignored_too
Use the hook API to replace ad-hoc invocation of hook scripts via
the run_command() API.
* ar/run-command-hook-take-2:
builtin/receive-pack: avoid spinning no-op sideband async threads
receive-pack: convert receive hooks to hook API
receive-pack: convert update hooks to new API
run-command: poll child input in addition to output
hook: add jobs option
reference-transaction: use hook API instead of run-command
transport: convert pre-push to hook API
hook: allow separate std[out|err] streams
hook: convert 'post-rewrite' hook in sequencer.c to hook API
hook: provide stdin via callback
run-command: add stdin callback for parallelization
run-command: add helper for pp child states
t1800: add hook output stream tests
git history reword expects a single valid revision argument and errors
out if it doesn't get it. In that case the struct rev_info passed to
release_revisions() for cleanup is still uninitialized, which can result
in attempts to free(3) random pointers. Avoid that by initializing the
structure.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a new --trailer=<trailer> option to git rebase to append trailer
lines to each rewritten commit message (merge backend only).
Because the apply backend does not provide a commit-message filter,
reject --trailer when --apply is in effect and require the merge backend
instead.
This option implies --force-rebase so that fast-forwarded commits are
also rewritten. Validate trailer arguments early to avoid starting an
interactive rebase with invalid input.
Add integration tests covering error paths and trailer insertion across
non-interactive and interactive rebases.
Signed-off-by: Li Chen <me@linux.beauty>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now that amend_file_with_trailers() expects raw trailer lines, do not
store argv-style "--trailer=<trailer>" strings in git commit and git
tag.
Parse --trailer using OPT_STRVEC so trailer_args contains only the
trailer value, and drop the temporary prefix stripping in
amend_file_with_trailers().
Signed-off-by: Li Chen <me@linux.beauty>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Introduce amend_strbuf_with_trailers() to apply trailer additions to a
message buffer via process_trailers(), avoiding the need to run git
interpret-trailers as a child process.
Update amend_file_with_trailers() to use the in-process helper and
rewrite the target file via tempfile+rename, preserving the previous
in-place semantics. As the trailers are no longer added in a separate
process and trailer_config_init() die()s on missing config values it
is called early on in cmd_commit() and cmd_tag() so that they die()
early before writing the message file. The trailer arguments are now
also sanity checked.
Keep existing callers unchanged by continuing to accept argv-style
--trailer=<trailer> entries and stripping the prefix before feeding the
in-process implementation.
Signed-off-by: Li Chen <me@linux.beauty>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move create_in_place_tempfile() and process_trailers() from
builtin/interpret-trailers.c into trailer.c and expose it via trailer.h.
This reverts most of ae0ec2e0e0 (trailer: move interpret_trailers()
to interpret-trailers.c, 2024-03-01) and lets other call sites reuse
the same trailer rewriting logic.
Signed-off-by: Li Chen <me@linux.beauty>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refactor create_in_place_tempfile() in preparation for moving it
to tralier.c. Change the return type to return a `struct tempfile*`
instead of a `FILE*` so that we can remove the file scope tempfile
variable. Since 076aa2cbda (tempfile: auto-allocate tempfiles on
heap, 2017-09-05) it has not been necessary to make tempfile varibales
static so this is safe. Also use error() and return NULL in place of
die() so the caller can exit gracefully and use find_last_dir_sep()
rather than strchr() to find the parent directory.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Extract the trailer rewriting logic into a helper that appends to an
output strbuf.
Update interpret_trailers() to handle file I/O only: read input once,
call the helper, and write the buffered result.
This separation makes it easier to move the helper into trailer.c in the
next commit.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Li Chen <me@linux.beauty>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "files" backend is implemented as a pointer in the `struct
odb_source`. This contradicts our typical pattern for pluggable backends
like we use it for example in the ref store or for object database
streams, where we typically embed the generic base structure in the
specialized implementation. This pattern has a couple of small benefits:
- We avoid an extra allocation.
- We hide implementation details in the generic structure.
- We can easily downcast from a generic backend to the specialized
structure and vice versa because the offsets are known at compile
time.
- It becomes trivial to identify locations where we depend on backend
specific logic because the cast needs to be explicit.
Refactor our "files" object database source to do the same and embed the
`struct odb_source` in the `struct odb_source_files`.
There are still a bunch of sites in our code base where we do have to
access internals of the "files" backend. The intent is that those will
go away over time, but this will certainly take a while. Meanwhile,
provide a `odb_source_files_downcast()` function that can convert a
generic source into a "files" source.
As we only have a single source the downcast succeeds unconditionally
for now. Eventually though the intent is to make the cast `BUG()` in
case the caller requests to downcast a non-"files" backend to a "files"
backend.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King