[pull] master from git:master - #239
Merged
Merged
Conversation
…Timeout Concurrent config writers race for the ".lock" file, which is taken with open(O_EXCL) and no retry, so the losers fail right away with "could not lock config file". This shows up with parallel "git worktree add -b" against the same repository: each one writes a couple of branch.* keys and the losers fail at random. Worse, "git worktree add" doesn't propagate that failure to its exit code, so the tracking config is silently dropped. (The swallowed error is a separate bug.) Retry instead of giving up on the first EEXIST. The lock is only held while rewriting a small file, so the loser only has to wait out the other writers. Same approach as 4ff0f01 (refs: retry acquiring reference locks for 100ms, 2017-08-21). On the semantics: the on-disk config is read only after the lock is taken, so writers touching different keys can't lose each other's change. Writers touching the same key still get last-writer-wins, but that is already the case today and would need a compare-and-swap config API to fix. The retry only turns hard failures into successes. Default to 1000ms, like core.packedRefsTimeout: same shape of problem, one shared file everyone serializes through. A larger timeout only costs anything when a stale lock is left behind by a crash, which is rare; a smaller one fails spuriously on slow filesystems (NTFS has been seen needing more than 100ms). Make it configurable as core.configLockTimeout. There is no chicken-and-egg problem: we read the config before we lock it. microsoft/git carries a similar patch (core.configWriteLockTimeoutMS, default off) for Scalar's tests. Defaulting to non-zero here because the worktree case fails silently. Helped-by: Patrick Steinhardt <ps@pks.im> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Jörg Thalheim <joerg@thalheim.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>
As part of the ongoing libification effort, dynamically allocated global configuration variables are being moved into 'struct repo_config_values'. To prevent memory leaks, we need a destructor to free these heap-allocated variables when a repository instance is torn down. Introduce 'repo_config_values_clear()' in environment.c and invoke it from 'repo_clear()' in repository.c. As a starting point, update this new function to handle the cleanup of 'attributes_file'. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The global variable 'excludes_file' is used to track the path to the global ignore file. If this variable is NULL, 'setup_standard_excludes()' in 'dir.c' forcefully evaluates and assigns the XDG default path to it. Continue the libification effort by encapsulating this lazy-loading fallback logic into a proper getter and moving the variable into 'struct repo_config_values'. Since 'excludes_file' is a dynamically allocated string, it requires proper heap memory management. It is safely freed using the newly introduced 'repo_config_values_clear()' function when the repository is torn down. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The global variable 'editor_program' holds the path to the user's preferred editor. Move 'editor_program' into 'struct repo_config_values' to continue the libification effort. There have been discussions on whether external programs like editors truly need to be configured on a per-repository basis within the same process. While a single process might rarely invoke different editors, this migration is necessary for two reasons: 1. Developers frequently use different toolchains for different projects. Per-repo configuration respects this. 2. Moving this string into 'repo_config_values' eliminates mutable global state. As the codebase moves toward becoming a long-running processes, managing multiple repositories concurrently must not overwrite each other's program configurations. No standalone getter function is introduced. Callers directly access the field via 'repo_config_values()'. Heap memory is safely reclaimed in 'repo_config_values_clear()'. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The 'pager_program' variable is currently defined as a file-scoped static string in pager.c. Move it into 'struct repo_config_values'. The configuration parsing logic remains strictly within pager.c to respect subsystem boundaries. The read/write operations are simply redirected to the repository-specific structure using 'repo_config_values()'. All current callers indeed pass 'the_repository', so this new enforcement does not harm them. Similar to the recent editor_program migration, no standalone getter is introduced to keep the code minimal. The dynamically allocated memory is now managed by 'repo_config_values_clear()'. On top of that, fix memory leaks in pager.c while we are at it. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The global variable 'askpass_program' stores the path to the program used to prompt the user for credentials. Move it into repo_config_values to continue the libification effort. While it is uncommon for a single process to require different askpass programs for different repositories, maintaining this value as a mutable global string is a blocker for libification. Global heap-allocated strings introduce thread-safety issues in a multi-repo environment. Move 'askpass_program' into 'struct repo_config_values' to eliminate this global state. The memory is now safely managed and freed via 'repo_config_values_clear()'. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
…ewhitespace The global variables 'apply_default_whitespace' and 'apply_default_ignorewhitespace' are used to store the default whitespace configuration for 'git apply'. Move these variables into 'struct repo_config_values' to continue the libification effort. Dynamically allocated strings fetched via 'repo_config_get_string()' are now tracked per-repository and safely freed in 'repo_config_values_clear()'. As part of this transition, update 'git_apply_config()' to accept a 'struct repository *' argument rather than relying on the 'the_repository' global. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The global variable 'push_default' specifies the default behavior of 'git push' when no explicit refspec is provided. Move 'push_default' into 'struct repo_config_values' to continue the libification effort. While 'enum push_default_type' ideally belongs in 'remote.h', moving it there introduces a circular dependency chain: remote.h -> hash.h -> repository.h -> environment.h. Therefore, the enum definition is kept in 'environment.h' just above 'struct repo_config_values' with a NEEDSWORK comment for future cleanup. Modify the configuration parsing in environment.c to update the per-repository structure directly, and update caller across the codebase to access the value via 'repo_config_values()'. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The global variable 'autorebase' dictates whether a newly created branch should be configured to automatically rebase by default. Move it into 'struct repo_config_values' to continue the libification effort. The 'enum rebase_setup_type' definition is moved higher up in 'environment.h' so that it is visible to the repository-specific structure. The default state AUTOREBASE_NEVER is now correctly initialized in 'repo_config_values_init()'. Configuration parsing in 'git_default_branch_config()' is updated to write directly to the repository's configuration instance. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The global variable 'object_creation_mode' controls how Git creates object files, specifically determining whether to use hardlinks or renames when moving temporary files into the object database. Move it into 'struct repo_config_values' to continue the libification effort. Move the 'enum object_creation_mode' definition higher up in 'environment.h' to ensure it is visible to the structure. Initialize the per-repository value to its default macro value OBJECT_CREATION_MODE inside 'repo_config_values_init()'. Update configuration parsing in 'git_default_core_config()' to write directly to the repository-specific configuration structure. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The configurations in 'struct config_values_private_' are not all parsed in 'git_default_config()'. For example, 'pager_program' is now parsed in 'pager.c'. Therefore, update the comment. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add variants of create_tempfile_mode() that handle arbitrary repositories. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the config setting core.sharedRepository from the ref store base repository at hand instead of from the_repository. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add variants of hold_lock_file_for_update_timeout_mode() that handle arbitrary repositories. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove the compatibility wrappers create_tempfile_mode() and create_tempfile() that have become unused. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the config setting core.sharedRepository from the repository at hand instead of from the_repository. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diff index lines have included a trailing file mode since ec1fcc1 (Show original and resulting blob object info in diff output, 2005-10-07) when the old and new file modes match: index <old>..<new> 100644 gitweb recognizes that trailing mode before it tries to shorten and link the object IDs. This appends the file-type annotation first, but the object-ID matcher requires the ID range to end the line. As a result, this common form keeps both full object IDs as plain text. That is inconsistent with other hash displays and makes commitdiff output wider than necessary. Recent gitweb changes have fixed mobile overflow in log, commit, blob and diff views; leaving two full object IDs in this header preserves an avoidable long line in the diff header. * gitweb/gitweb.perl: Remove the trailing mode before matching the index IDs, then append it again after the IDs have been shortened and linked. This preserves the mode display while letting ordinary and combined index lines use the existing object-ID formatting paths. * t/t9502-gitweb-standalone-parse-output.sh: Add coverage for that common form by rendering a commitdiff for a regular file modification. Check that the visible index line contains linked short blob IDs followed by the mode and file-type annotation, and that the full unlinked form is not emitted. Signed-off-by: Travor Liu <travor_lzh@outlook.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
'git receive-pack' has been refactored to use ODB transaction interfaces instead of directly managing 'tmp_objdir' for staging incoming objects, bringing it closer to being ODB backend agnostic. * jt/receive-pack-use-odb-transactions: builtin/receive-pack: stage incoming objects via ODB transactions builtin/receive-pack: drop redundant tmpdir env odb/transaction: introduce ODB transaction flags odb/transaction: add transaction env interface odb/transaction: propagate commit errors odb/transaction: propagate begin errors object-file: propagate files transaction errors object-file: drop check for inflight transactions object-file: embed transaction flush logic in commit function object-file: rename files transaction fsync function object-file: rename files transaction prepare function
Repositories can have a compatibility hash configured, which means that such a repository is expected to maintain a mapping between canonical and compatibility object hashes. Maintaining this mapping is the responsibility of the object database sources, where we either store them as part of the loose objects map or in packfile indices v3 (once we gain support for this feature). But besides storing these compatibility hashes, the sources are also responsible for generating the compatibility hash in the first place. This is somewhat unnecessary though, as the compatibility hash should be computed the same no matter which source is being used. The consequence is that we need to duplicate this functionality across the different backends, which does not make a lot of sense. Refactor the code so that we instead compute the compatibility hash in `odb_write_object_ext()` and then pass the computed value to the sources. No callers need adjustment as there are none that write objects via the source interfaces directly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the next commit we're about to change how objects are being written into the object database source. Prepare for this refactoring by introducing a wrapper function into our unit tests so that we don't have to adjust all callsites. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Same as in a preceding commit, compute the object hash in `odb_write_object_ext()` so that we can unify this logic. Besides unification, this change also allows us to lift the object existence check out of the "loose" backend into the generic layer, which will happen in the next commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Before writing a new loose object we first check whether the object
already exists in any of the sources attached to the object database.
This results in a couple of issues:
- We have a layering violation, where the source needs to be aware of
objects stored in any of the other sources.
- Every backend would have to reimplement this check, which feels
somewhat pointless.
- It is not possible to easily write an object into a source in case
the same object already exists in another source.
Refactor the code and lift up the object existence check from the
"loose" backend into the generic ODB layer. No callers need adjustment
as none of them write via a specific source, but via the ODB layer.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function `force_object_loose()` is used to loosen packed objects before repacking. It passes the pack's mtime along so that the newly written loose object inherits the same timestamp. This matters for object pruning, which uses the mtime to determine whether an object is old enough to be pruned. In a subsequent commit, `force_object_loose()` will be converted to use the generic `odb_source_write_object()` interface instead of calling `write_loose_object()` directly. But the generic interface doesn't yet support setting a specific mtime, which makes it impossible to implement the logic as of now. Prepare for the change by introducing a new `mtime` parameter to this function that we plumb through the stack. If set, the backends are instructed to set the object's mtime accordingly. If unset, the backends are expected to use the current time instead. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We return an error when converting the given object to the compatibility hash algorithm fails. This early return causes a memory leak though, because we don't free the content buffer that we've already read before via `odb_read_object_info_extended()`. Plug the memory leak by creating a common exit path where the buffer gets free'd. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When repacking objects we may end up "loosening" objects via `force_objects_loose()`. The implementation of this logic still sits with "object-file.c" even though it is ultimately an implementation detail of the "files" backend. Moving this logic around is non-trivial though as we depend on `write_loose_object()`, which is an internal implementation detail of how we write loose objects. Until now it wasn't possible to use the generic function `odb_source_write_object()` though, because the "loose" implementation thereof would skip writing the object in case it already exists in any other source. This restriction was lifted over the preceding commits though, where this object existence check is now handled on the object database level and not on the individual source level anymore. Consequently, it is now possible to use generic interfaces. Refactor the code accordingly so that we can move the logic around in a subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the preceding commits we have refactored `force_object_loose()` to not call internal functions anymore for writing the object. Instead, it now only uses generic functions that are accessible to all callers. Consequently, we can now easily move the function to its only caller, which is git-pack-objects(1). Do so. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The logic to write loose objects is split up across "object-file.c" and
"odb/source-loose.c". This split is somewhat weird, but it is the result
of two things:
- `force_object_loose()` used to reach into internals of how exactly
we write objects.
- The logic of writing objects is intertwined with potentially
starting a transaction.
We have refactored `force_object_loose()` over preceding commits to work
via generic interfaces now, so this reason doesn't exist anymore. But
the second reason still does, as our management of "files" transactions
and their ad-hoc creation is still very messy. This area definitely
requires further work, and that work is indeed ongoing.
That being said, we can already move the writing logic into the "loose"
backend rather easily. All we have to do is to expose two functions that
relate to the transactions.
Expose these two functions and move the writing logic into the "loose"
backend accordingly so that it becomes more self-contained. Note that
this requires us to drop a reference to `the_repository` in favor of
using the source's repository in `start_loose_object_common()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The 'read-cache.c' file already includes 'environment.h', which provides the extern declarations for variables like 'trust_executable_bit' and 'has_symlinks'. Remove the redundant extern declarations inside 'st_mode_from_ce()' to clean up the code. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The ce_mode_from_stat() function is a performance-critical static inline helper in 'read-cache.h'. As we migrate configuration variables into the repository struct, this helper needs access to the repository context. Update the signature of ce_mode_from_stat() to take a 'struct repository *' parameter, and update all callers to pass the appropriate repository instance. To prepare for the overhead of replacing cheap global variable accesses with getter functions, the boolean expressions are reordered to evaluate 'S_ISREG(mode)' first. While at it, add a comment for ce_mode_from_stat(). Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the global 'trust_executable_bit' configuration into the repository-specific 'repo_config_values' struct. To ensure code readability, the getter function 'repo_trust_executable_bit()' has been introduced. Callers access this configuration by passing in 'repo' when possible, and explicitly fall back to 'the_repository' the rest of time. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the global 'has_symlinks' configuration into the repository-specific 'repo_config_values' struct. Introduce 'repo_has_symlinks()' getter for readability. Callers access this configuration by passing in 'repo' when possible, and explicitly fall back to 'the_repository' the rest of the time. Introduce 'platform_has_symlinks()' macro to allow platform specific-customization, primarily to help MinGW. Platforms can override this in their respective headers. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The pathspec parser computes `len` and `nowildcard_len` from `item.match`, which includes any prefix added when a command is run from a subdirectory. `item.original` can still contain the shorter, unprefixed argument. Using `item.original + item.nowildcard_len` in `pathspec_needs_expanded_index()` can therefore read past the end of the allocation. AddressSanitizer reports a heap-buffer-overflow for prefixed wildcard pathspecs passed to `git rm` and `git reset` with a sparse index. The mismatch dates back to 4d1cfc1 ("reset: make --mixed sparse-aware", 2021-11-29), which introduced the helper using `item.original`. b29ad38 ("pathspec.h: move pathspec_needs_expanded_index() from reset.c to here", 2022-08-07) later moved it to `pathspec.c` and preserved the affected comparisons. Use `item.match` consistently when checking whether a pathspec can match a sparse-directory entry. Add coverage for prefixed wildcard pathspecs so both commands keep the index sparse. Signed-off-by: Ted Nyman <tnyman@openai.com> Reviewed-by: Taylor Blau <ttaylorr@openai.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
`git stash push -- <pathspec>` expands a sparse index before checking whether the pathspec matches any tracked paths. This is unnecessary when the pathspec is wholly inside the sparse-checkout cone and makes a path-limited stash proportional to the size of the full index. Use `pathspec_needs_expanded_index()` to expand only when a pathspec can match part of a sparse-directory entry, as `git rm` and `git reset` already do. Keep the full-index behavior for pathspecs that need it. Add compatibility coverage for literal, prefixed, wildcard, file, multiple, staged, and missing pathspecs. Add the corresponding path-limited stash case to p2000. On a cone-mode repository with 349,525 tracked paths and 49 sparse index entries, the best of three runs changed from 18.87s to 0.06s. Trace2 reported four index expansions before this change and none after it. Signed-off-by: Ted Nyman <tnyman@openai.com> Reviewed-by: Taylor Blau <ttaylorr@openai.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a built-in userdiff driver for the Swift programming language so that
diff hunk headers and word diffs work out of the box for ".swift" files.
The funcname pattern is built for Swift's own declaration grammar: an
optional run of attributes ("@objc", "@available(iOS 13, *)", ...),
followed by an optional run of lowercase modifiers ("public", "static",
"final", ...), followed by a declaration keyword (func, class, struct,
enum, protocol, extension, actor, init, deinit, subscript). The keyword
is followed by a boundary that allows whitespace, "(" (init/subscript),
"?" or "!" (failable init), or "<" (generics), while still acting as a
word boundary so e.g. "initialize(" does not match.
The word regex recognizes Swift identifiers, hexadecimal, octal, binary,
integer and floating-point literals, and the language's operators.
Signed-off-by: Shlok Kulshreshtha <diy2903@gmail.com>
Acked-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
disconnect_helper() only frees data inside of the if(data->helper) block [1]. When the transport is disconnected without the helper being fully started, data->name allocated in transport_helper_init() is never freed. Move FREE_AND_NULL(data->name) outside the conditional block so it's always freed on disconnect. [1]: https://lore.kernel.org/git/05fbadbae2184479c87c37675dde7bd79b3e32ab.1716465556.git.ps@pks.im/ Mentored-by: Karthik Nayak <karthik.188@gmail.com> Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Declare loop counters in the for statement when they are only used within the loop body, limiting their scope and improving readability. While updating the loop counters, use size_t instead of int for counters that iterate over object counts. Update the 'nr' parameter of dispatch_calls() to size_t as all callers already pass a value of that type. Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Eric Ju <eric.peijian@gmail.com> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Extract utility functions from the cat-file's test script 't1006-cat-file.sh' into a new 'lib-cat-file.sh' dedicated library file. A subsequent commit will need these functions. This improves the code reuse and readability, enabling future cat-file tests to share these helpers without duplicating code. While at it update the style of this line to follow coding guidelines: . "$TEST_DIRECTORY/lib-loose.sh" to . "$TEST_DIRECTORY"/lib-loose.sh Signed-off-by: Eric Ju <eric.peijian@gmail.com> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
write_fetch_command_and_capabilities() is moved to 'connect.c' in a
subsequent commit. To prepare for that, drop the static variable usage
of advertise_sid.
Currently advertise_sid is set in fetch_pack_config() by reading
"transfer.advertisesid". It is used in three places:
1. In do_fetch_pack(), to clear it when the server lacks support:
if (!server_supports("session-id"))
advertise_sid = 0;
2. In find_common(), to advertise the session id over protocol v0/v1:
if (advertise_sid)
strbuf_addf(&c, " session-id=%s", trace2_session_id());
3. In write_fetch_command_and_capabilities(), to advertise it over
protocol v2:
if (advertise_sid && server_supports_v2("session-id"))
packet_buf_write(req_buf, "session-id=%s", trace2_session_id());
About 1, the check only guards the v0/v1 path, and the v2 path
already checks server support inline in its condition. Follow the
same pattern and fold the check into the condition in find_common().
About 2 and 3, replace the static variable with a local read via
repo_config_get_bool() in each function.
Because repo_config_get_bool() leaves advertise_sid as is if it is not
set, initialize it to 0, matching its default.
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
hash_algo_by_name() returns unsigned int, but it is stored in hash_algo variable as int. This goes unnoticed because of: DISABLE_SIGN_COMPARE_WARNINGS On 'fetch-pack.c' On a subsequent commit this function will be moved to 'connect.c' that would notice this. Change hash_algo variable type to match its return type, also make it const because they are never modified. Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a subsequent commit write_fetch_command_and_capabilities() will be refactored to a more general-purpose function, making it more accessible to additional commands in the future. Move write_fetch_command_and_capabilities() to 'connect.c', where there are similar purpose functions. Because string_list is only used as a pointer, use a forward declaration [1]. [1]: https://lore.kernel.org/git/Z0RIqUAoEob8lGfM@pks.im/ Helped-by: Jonathan Tan <jonathantanmy@google.com> Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Eric Ju <eric.peijian@gmail.com> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refactor write_fetch_command_and_capabilities(), enabling it to serve both fetch and additional commands. In this context, "command" refers to the "operations" supported by Git's wire protocol Documentation/gitprotocol-v2.adoc, such as a Git subcommand (e.g., git-fetch(1)) or a server-side operation like "object-info" as implemented in commit a2ba162 (object-info: support for retrieving object info, 2021-04-20). Refactor the function signature to accept a command instead of the hardcoded "fetch". Helped-by: Jonathan Tan <jonathantanmy@google.com> Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Eric Ju <eric.peijian@gmail.com> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are some variables initialized at the start of the do_fetch_pack_v2() state machine. Currently, they are initialized in FETCH_CHECK_LOCAL, which is the initial state set at the beginning of the function. However, a subsequent patch will allow for another initial state, while still requiring these initialized variables. Move the initialization to be before the state machine, so that they are set regardless of the initial state. Note that there is no change in behavior, because we're moving code from the beginning of the first state to just before the execution of the state machine. Helped-by: Jonathan Tan <jonathantanmy@google.com> Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Eric Ju <eric.peijian@gmail.com> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
…quested Currently, send_info() only checks for existence when the attribute 'size' is also requested. Requesting a bare OID, without attributes only echoes back the OID. Extract the existence check to be done regardless of the number of attributes requested. While at it, introduce a wrapper called get_object_info() similar to odb_read_object_info() that returns OBJ_BAD on fail and adds OBJECT_INFO_SKIP_FETCH_OBJECT and OBJECT_INFO_QUICK flags. OBJECT_INFO_SKIP_FETCH_OBJECT is so a server with a partial clone doesn't trigger fetching objects when it gets an object-info request with an OID that is not available locally. A server should only report what it has locally. Tighten the condition used to determine whether an object is recognized. get_object_info() returns OBJ_BAD for unknown objects, but OBJ_NONE (0) can also mean "not found". Change the check from '< 0' to '<= OBJ_NONE' to cover both as unrecognized. With this patch, a bare OID has two possible responses: 1. Recognized OID: the server answers with "<OID>" 2. Unrecognized OID: the server answers with "<OID> SP" Update the object-info section in 'gitprotocol-v2.adoc': - Require full obj-oid explicitly. - Fix parentheses. - Define obj-size explicitly. - Make obj-size optional in obj-info and document the behavior for unrecognized object IDs. - Describe the attr header as zero or more pkt-lines, one per attribute, matching what the server implements. A request with no attributes gets no header. Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In order for a client to know what object-info components a server can provide, advertise supported object-info features. This allows a client to decide whether to query the server for object-info or fetch as a fallback. Helped-by: Jonathan Tan <jonathantanmy@google.com> Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Eric Ju <eric.peijian@gmail.com> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sometimes, it is beneficial to retrieve information about an object without downloading it entirely. The server-side logic for this functionality was implemented in commit "a2ba162cda (object-info: support for retrieving object info, 2021-04-20)." And the wire format is documented at https://git-scm.com/docs/protocol-v2#_object_info. Introduce client-side support for the object-info capability. Add its own function for object-info separate from existing fetch infrastructure. Currently, the client supports requesting a list of OIDs with the size attribute from a v2 server. If the server does not advertise this feature (i.e., transfer.advertiseobjectinfo is set to false), the client returns an error and exits. Note that: 1. The entire request is written into req_buf before being sent to the remote. This approach follows the pattern used in the send_fetch_request() logic within 'fetch-pack.c'. Streaming the request is not addressed in this patch. 2. A new field 'unrecognized' has been added to object_info. This new field is set at fetch_object_info() when the object is unrecognized by the server. Helped-by: Jonathan Tan <jonathantanmy@google.com> Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Eric Ju <eric.peijian@gmail.com> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since the info command in cat-file --batch-command prints object
info for a given object, it is natural to add another command in
cat-file --batch-command to print object info for a given object
from a remote.
Add remote-object-info command to cat-file --batch-command.
While info takes object ids one at a time, this creates overhead when
making requests to a server. So remote-object-info instead can take
multiple object ids at once.
The cat-file --batch-command command is generally implemented in the
following manner:
- Receive and parse input from user
- Call respective function attached to command
- Get object info, print object info
In --buffer mode, this changes to:
- Receive and parse input from user
- Store respective function attached to command in a queue
- After flush, loop through commands in queue
- Call respective function attached to command
- Get object info, print object info
Notice how the getting and printing of object info is accomplished one
at a time. As described above, this creates a problem for making
requests to a server. Therefore, remote-object-info is implemented in
the following manner:
- Receive and parse input from user
If command is remote-object-info:
- Get object info from remote
- Loop through and print each object info
Else:
- Call respective function attached to command
- Parse input, get object info, print object info
And finally for --buffer mode remote-object-info:
- Receive and parse input from user
- Store respective function attached to command in a queue
- After flush, loop through commands in queue:
If command is remote-object-info:
- Get object info from remote
- Loop through and print each object info
Else:
- Call respective function attached to command
- Get object info, print object info
To summarize, remote-object-info gets object info from the remote and
then loops through the object info passed in, printing the info.
In order for remote-object-info to avoid remote communication
overhead in the non-buffer mode, the objects are passed in as such:
remote-object-info <remote> <oid> <oid> ... <oid>
rather than
remote-object-info <remote> <oid>
remote-object-info <remote> <oid>
...
remote-object-info <remote> <oid>
Placeholders in the format are validated against an allow-list of the
atoms the remote path supports: "objectname" and "objectsize".
Unsupported atoms expand to an empty string, honoring how for-each-ref
handles known but inapplicable atoms.
Without this, atoms like %(objecttype) would mark data->info.typep and
because the server only sends size, type_name() would later crash.
As extra safety, even outside of the remote path, initialize
expand_data's type to OBJ_BAD and handle type_name() returning NULL.
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
[pablo: added the atom allow-list validation]
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The static allow-list in expand_atom() is hardcoded to allow only
"objectname" and "objectsize" for remote queries. This works because,
up to this point, servers will either support object-info with name
and size or they do not support them at all.
As object-info gains new capabilities, we cannot expect different
servers with different Git versions to have the same object-info
capabilities. Therefore, the client needs to adapt its allow-list to
what the server advertises.
The client now:
1. Requests the protocol option that the placeholder refers to (i.e.
"size" for "%(objectsize)").
2. Drops any requested option that the server does not advertise in
fetch_object_info().
3. Maps the remaining advertised options back to their placeholders and
populates remote_allowed_atoms.
4. Uses remote_allowed_atoms in expand_atom(), preserving the previous
behavior for supported placeholders.
For example, if the client requests "%(objectsize) %(objecttype)" and
the server only supports 'size', then the client only requests 'size'.
The server returns the size (i.e "42") "%(objectsize)" is expanded
normally while "%(objecttype)" expands to an empty string:
"42 "
Note that the empty string expansion is only for known but unsupported
placeholders. "%(objectcolor)" which doesn't exist would die().
This honors what for-each-ref does for known but inapplicable atoms
(placeholders).
Move object_info_options out of get_remote_info() so the caller which
has data can select what options will be requested instead of requesting
always size.
Move batch_object_write() out so output is always produced.
If there are no supported attributes, the output is a blank line.
Include "type" in the object_info_options even though the client does
not yet know how to parse the server's "type" capability.
As a result, "type" is always filtered out, allowing the tests to verify
that known but unsupported placeholders expand to an empty string.
Since the filter removes options by swapping with the last element,
the list is no longer kept sorted. Drop the pre-sort in
fetch_object_info_via_pack() and use the unsorted string_list lookup
for the response header. This has no effect in performance as the list
can only be two entries long ('size' and 'type').
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The tempfile and lockfile APIs have been refactored to stop depending
on the 'the_repository' global variable, and their callers have been
updated to use the repository-aware variants.
* rs/tempfile-wo-the-repository:
use repo_hold_lock_file_for_update{,_mode,_timeout}() with custom repos
tempfile: stop using the_repository
lockfile: add repo_hold_lock_file_for_update{,_timeout}{,_mode}()
refs/packed: use repo_create_tempfile()
tempfile: add repo_create_tempfile{,_mode}()
The 'trust_executable_bit' (coming from the 'core.filemode' configuration) has been migrated into 'struct repo_config_values' to tie it to a specific repository instance. * ty/migrate-trust-executable-bit: environment: move has_symlinks into repo_config_values environment: move trust_executable_bit into repo_config_values read-cache: pass 'repo' to 'ce_mode_from_stat()' read-cache: remove redundant extern declarations
The 'excludes_file' and various other global configuration variables (including 'editor_program', 'pager_program', 'askpass_program', and 'push_default') have been migrated into the per-repository structure. * ty/migrate-excludes-file: repository: adjust the comment of config_values_private_ environment: move object_creation_mode into repo_config_values environment: move autorebase into repo_config_values environment: move push_default into repo_config_values environment: migrate apply_default_whitespace and apply_default_ignorewhitespace environment: move askpass_program into repo_config_values environment: move pager_program into repo_config_values environment: move editor_program into repo_config_values environment: move excludes_file into repo_config_values repository: introduce repo_config_values_clear()
Userdiff patterns for Swift have been added, with support for Swift-specific constructs such as attributes, modifiers, failable initializers, and generics. * sk/userdiff-swift: userdiff: add support for Swift
The 'git stash push' command has been optimized to avoid unnecessary sparse index expansion when pathspecs are wholly inside the sparse-checkout cone. Also, a potential out-of-bounds read in the sparse-index expansion check helper pathspec_needs_expanded_index() has been fixed by consistently using the parsed, prefixed path. * tn/stash-avoid-sparse-index-expansion: stash: avoid sparse-index expansion for in-cone paths pathspec: use match for sparse-index expansion checks
Configuration file locking has been updated to retry for a short period, avoiding failures when multiple processes attempt to update the configuration simultaneously. * jt/config-lock-timeout: config: retry acquiring config.lock, configurable via core.configLockTimeout
The 'remote-object-info' command has been added to 'git cat-file --batch-command', allowing clients to request object metadata (currently size) from a remote server via protocol v2 without downloading the entire object. Format placeholders are dynamically filtered on the client based on server-advertised capabilities, returning empty strings for inapplicable or unsupported fields. * ps/cat-file-remote-object-info: cat-file: make remote-object-info allow-list adapt to the server cat-file: add remote-object-info to batch-command transport: add client support for object-info serve: advertise object-info feature protocol-caps: check object existence regardless of the attributes requested fetch-pack: move fetch initialization connect: make write_fetch_command_and_capabilities() more generic fetch-pack: move write_fetch_command_and_capabilities() to connect.c fetch-pack: use unsigned int for hash_algo variable fetch-pack: drop the static advertise_sid variable t1006: extract helper functions into new 'lib-cat-file.sh' cat-file: declare loop counter inside for() transport-helper: fix memory leak of helper on disconnect
The object ID shortening and linking in the 'commitdiff' view of 'gitweb' has been corrected to work even when the index line carries a trailing file mode. * tl/gitweb-shorten-hashes-with-modes: gitweb: shorten index hashes with trailing file modes
The logic to write loose objects has been refactored and moved from 'object-file.c' to the loose backend source file 'odb/source-loose.c', making the loose backend more self-contained. This is achieved by first refactoring force_object_loose() to use generic ODB write interfaces instead of loose-backend internals. * ps/odb-move-loose-object-writing: object-file: move logic to write loose objects object-file: move `force_object_loose()` object-file: force objects loose via generic interface object-file: fix memory leak in `force_object_loose()` odb: support setting mtime when writing objects odb: lift object existence check out of the "loose" backend odb: compute object hash in `odb_write_object_ext()` t/u-odb-inmemory: implement wrapper for writing objects odb: compute compat object ID in `odb_write_object_ext()`
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )