summaryrefslogtreecommitdiffstats
path: root/t/t9145-git-svn-master-branch.sh (unfollow)
Commit message (Collapse)AuthorFilesLines
2024-12-06The fourteenth batchJunio C Hamano1-0/+6
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04midx: inline the `MIDX_MIN_SIZE` definitionKarthik Nayak1-5/+1
The `MIDX_MIN_SIZE` definition is used to check the midx_size in `local_multi_pack_index_one`. This definition relies on the `the_hash_algo` global variable. Inline this and remove the global variable usage. With this, remove `USE_THE_REPOSITORY_VARIABLE` usage from `midx.c`. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04midx: pass down `hash_algo` to functions using global variablesKarthik Nayak5-29/+37
The functions `get_split_midx_filename_ext()`, `get_midx_filename()` and `get_midx_filename_ext()` use `hash_to_hex()` which internally uses the `the_hash_algo` global variable. Remove this dependency on global variables by passing down the `hash_algo` through to the functions mentioned and instead calling `hash_to_hex_algop()` along with the obtained `hash_algo`. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04midx: pass `repository` to `load_multi_pack_index`Karthik Nayak3-10/+13
The `load_multi_pack_index` function in midx uses `the_repository` variable to access the `repository` struct. Modify the function and its callee's to send the `repository` field. This moves usage of `the_repository` to the `test-read-midx.c` file. While that is not optimal, it is okay, since the upcoming commits will slowly move the usage of `the_repository` up the layers and remove it eventually. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04midx: cleanup internal usage of `the_repository` and `the_hash_algo`Karthik Nayak1-22/+27
In the `midx.c` file, there are multiple usages of `the_repository` and `the_hash_algo` within static functions of the file. Some of the usages can be simply swapped out with the available `repository` struct. While some of them can be swapped out by passing the repository to the required functions. This leaves out only some other usages of `the_repository` and `the_hash_algo` in the file in non-static functions, which we'll tackle in upcoming commits. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04midx-write: pass down repository to `write_midx_file[_only]`Karthik Nayak4-23/+17
In a previous commit, we passed the repository field to all subcommands in the `builtin/` directory. Utilize this to pass the repository field down to the `write_midx_file[_only]` functions to remove the usage of `the_repository` global variables. With this, all usage of global variables in `midx-write.c` is removed, hence, remove the `USE_THE_REPOSITORY_VARIABLE` guard from the file. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04write-midx: add repository field to `write_midx_context`Karthik Nayak1-18/+20
The struct `write_midx_context` is used to pass context for creating MIDX files. Add the repository field here to ensure that most functions within `midx-write.c` have access to the field and can use that instead of the global `the_repository` variable. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04midx-write: use `revs->repo` inside `read_refs_snapshot`Karthik Nayak1-1/+2
The function `read_refs_snapshot()` uses `parse_oid_hex()`, which relies on the global `the_hash_algo` variable. Let's instead use `parse_oid_hex_algop()` and provide the hash algo via `revs->repo`. Also, while here, fix a missing newline after the function's definition. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04midx-write: pass down repository to static functionsKarthik Nayak1-26/+31
In 'midx-write.c' there are a lot of static functions which use global variables `the_repository` or `the_hash_algo`. In a follow up commit, the repository variable will be added to `write_midx_context`, which some of the functions can use. But for functions which do not have access to this struct, pass down the required information from non-static functions `write_midx_file` and `write_midx_file_only`. This requires that the function `hash_to_hex` is also replaced with `hash_to_hex_algop` since the former internally accesses the `the_hash_algo` global variable. This ensures that the usage of global variables is limited to these non-static functions, which will be cleaned up in a follow up commit. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04The thirteenth batchJunio C Hamano1-12/+22
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04t9300: test verification of renamed pathsJeff King1-2/+9
Commit da91a90c2f (fast-import: disallow more path components, 2024-11-30) added two separate verify_path() calls (one for added/modified files, and one for renames/copies). But our tests only exercise the first one. Let's protect ourselves against regressions by tweaking one of the tests to rename into the bad path. There are adjacent tests that will stay as additions, so now both calls are covered. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04packfile.c: remove unnecessary prepare_packed_git() callTaylor Blau1-1/+0
In 454ea2e4d7 (treewide: use get_all_packs, 2018-08-20) we converted existing calls to both: - get_packed_git(), as well as - the_repository->objects->packed_git , to instead use the new get_all_packs() function. In the instance that this commit addresses, there was a preceding call to prepare_packed_git(), which dates all the way back to 660c889e46 (sha1_file: add for_each iterators for loose and packed objects, 2014-10-15) when its caller (for_each_packed_object()) was first introduced. This call could have been removed in 454ea2e4d7, since get_all_packs() itself calls prepare_packed_git(). But the translation in 454ea2e4d7 was (to the best of my knowledge) a find-and-replace rather than inspecting each individual caller. Having an extra prepare_packed_git() call here is harmless, since it will notice that we have already set the 'packed_git_initialized' field and the call will be a noop. So we're only talking about a few dozen CPU cycles to set up and tear down the stack frame. But having a lone prepare_packed_git() call immediately before a call to get_all_packs() confused me, so let's remove it as redundant to avoid more confusion in the future. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04midx: add repository to `multi_pack_index` structKarthik Nayak3-35/+59
The `multi_pack_index` struct represents the MIDX for a repository. Here, we add a pointer to the repository in this struct, allowing direct use of the repository variable without relying on the global `the_repository` struct. With this addition, we can determine the repository associated with a `bitmap_index` struct. A `bitmap_index` points to either a `packed_git` or a `multi_pack_index`, both of which have direct repository references. To support this, we introduce a static helper function, `bitmap_repo`, in `pack-bitmap.c`, which retrieves a repository given a `bitmap_index`. With this, we clear up all usages of `the_repository` within `pack-bitmap.c` and also remove the `USE_THE_REPOSITORY_VARIABLE` definition. Bringing us another step closer to remove all global variable usage. Although this change also opens up the potential to clean up `midx.c`, doing so would require additional refactoring to pass the repository struct to functions where the MIDX struct is created: a task better suited for future patches. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04config: make `packed_git_(limit|window_size)` non-global variablesKarthik Nayak7-30/+35
The variables `packed_git_window_size` and `packed_git_limit` are global config variables used in the `packfile.c` file. Since it is only used in this file, let's change it from being a global config variable to a local variable for the subsystem. With this, we rid `packfile.c` from all global variable usage and this means we can also remove the `USE_THE_REPOSITORY_VARIABLE` guard from the file. Helped-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04config: make `delta_base_cache_limit` a non-global variableKarthik Nayak11-14/+39
The `delta_base_cache_limit` variable is a global config variable used by multiple subsystems. Let's make this non-global, by adding this variable independently to the subsystems where it is used. First, add the setting to the `repo_settings` struct, this provides access to the config in places where the repository is available. Use this in `packfile.c`. In `index-pack.c` we add it to the `pack_idx_option` struct and its constructor. While the repository struct is available here, it may not be set because `git index-pack` can be used without a repository. In `gc.c` add it to the `gc_config` struct and also the constructor function. The gc functions currently do not have direct access to a repository struct. These changes are made to remove the usage of `delta_base_cache_limit` as a global variable in `packfile.c`. This brings us one step closer to removing the `USE_THE_REPOSITORY_VARIABLE` definition in `packfile.c` which we complete in the next patch. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04packfile: pass down repository to `for_each_packed_object`Karthik Nayak15-35/+44
The function `for_each_packed_object` currently relies on the global variable `the_repository`. To eliminate global variable usage in `packfile.c`, we should progressively shift the dependency on the_repository to higher layers. Let's remove its usage from this function and closely related function `is_promisor_object`. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04packfile: pass down repository to `has_object[_kept]_pack`Karthik Nayak11-17/+21
The functions `has_object[_kept]_pack` currently rely on the global variable `the_repository`. To eliminate global variable usage in `packfile.c`, we should progressively shift the dependency on the_repository to higher layers. Let's remove its usage from these functions and any related ones. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04packfile: pass down repository to `odb_pack_name`Karthik Nayak6-14/+14
The function `odb_pack_name` currently relies on the global variable `the_repository`. To eliminate global variable usage in `packfile.c`, we should progressively shift the dependency on the_repository to higher layers. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04packfile: pass `repository` to static function in the fileKarthik Nayak1-7/+8
Some of the static functions in the `packfile.c` access global variables, which can simply be avoided by passing the `repository` struct down to them. Let's do that. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04packfile: use `repository` from `packed_git` directlyKarthik Nayak1-23/+27
In the previous commit, we introduced the `repository` structure inside `packed_git`. This provides an alternative route instead of using the global `the_repository` variable. Let's modify `packfile.c` now to use this field wherever possible instead of relying on the global state. There are still a few instances of `the_repository` usage in the file, where there is no struct `packed_git` locally available, which will be fixed in the following commits. Helped-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04packfile: add repository to struct `packed_git`Karthik Nayak10-16/+30
The struct `packed_git` holds information regarding a packed object file. Let's add the repository variable to this object, to represent the repository that this packfile belongs to. This helps remove dependency on the global `the_repository` object in `packfile.c` by simply using repository information now readily available in the struct. We do need to consider that a packfile could be part of the alternates of a repository, but considering that we only have one repository struct and also that we currently anyways use 'the_repository', we should be OK with this change. We also modify `alloc_packed_git` to ensure that the repository is added to newly created `packed_git` structs. This requires modifying the function and all its callee to pass the repository object down the levels. Helped-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-02Documentation/git-update-ref.txt: add missing wordKristoffer Haugsbakk1-4/+4
Add missing word “that” in the phrase “after verifying that”, like what was done in 1b2dfb70504 (Documentation/git-update-ref.txt: drop “flag”, 2024-10-21) Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-02Documentation/git-bundle.txt: fix word join typoKristoffer Haugsbakk1-1/+1
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-02fast-import: disallow more path componentsElijah Newren3-7/+91
Instead of just disallowing '.' and '..', make use of verify_path() to ensure that fast-import will disallow anything we wouldn't allow into the index, such as anything under .git/, .gitmodules as a symlink, or a dos drive prefix on Windows. Since a few fast-export and fast-import tests that tried to stress-test the correct handling of quoting relied on filenames that fail is_valid_win32_path(), such as spaces or periods at the end of filenames or backslashes within the filename, turn off core.protectNTFS for those tests to ensure they keep passing. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-02worktree: refactor `repair_worktree_after_gitdir_move()`Caleb White2-25/+26
This refactors `repair_worktree_after_gitdir_move()` to use the new `write_worktree_linking_files` function. It also preserves the relativity of the linking files; e.g., if an existing worktree used absolute paths then the repaired paths will be absolute (and visa-versa). `repair_worktree_after_gitdir_move()` is used to repair both sets of worktree linking files if the `.git` directory is moved during a re-initialization using `git init`. This also adds a test case for reinitializing a repository that has relative worktrees. Signed-off-by: Caleb White <cdwhite3@pm.me> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-02worktree: add relative cli/config options to `repair` commandCaleb White5-33/+76
This teaches the `worktree repair` command to respect the `--[no-]relative-paths` CLI option and `worktree.useRelativePaths` config setting. If an existing worktree with an absolute path is repaired with `--relative-paths`, the links will be replaced with relative paths, even if the original path was correct. This allows a user to covert existing worktrees between absolute/relative as desired. To simplify things, both linking files are written when one of the files needs to be repaired. In some cases, this fixes the other file before it is checked, in other cases this results in a correct file being written with the same contents. Signed-off-by: Caleb White <cdwhite3@pm.me> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-02worktree: add relative cli/config options to `move` commandCaleb White4-16/+39
This teaches the `worktree move` command to respect the `--[no-]relative-paths` CLI option and `worktree.useRelativePaths` config setting. If an existing worktree is moved with `--relative-paths` the new path will be relative (and visa-versa). Signed-off-by: Caleb White <cdwhite3@pm.me> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-02worktree: add relative cli/config options to `add` commandCaleb White7-49/+95
This introduces the `--[no-]relative-paths` CLI option and `worktree.useRelativePaths` configuration setting to the `worktree add` command. When enabled these options allow worktrees to be linked using relative paths, enhancing portability across environments where absolute paths may differ (e.g., containerized setups, shared network drives). Git still creates absolute paths by default, but these options allow users to opt-in to relative paths if desired. The t2408 test file is removed and more comprehensive tests are written for the various worktree operations in their own files. Signed-off-by: Caleb White <cdwhite3@pm.me> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-02worktree: add `write_worktree_linking_files()` functionCaleb White2-0/+48
A new helper function, `write_worktree_linking_files()`, centralizes the logic for computing and writing either relative or absolute paths, based on the provided configuration. This function accepts `strbuf` pointers to both the worktree’s `.git` link and the repository’s `gitdir`, and then writes the appropriate path to each. The `relativeWorktrees` extension is automatically set when a worktree is linked with relative paths. Signed-off-by: Caleb White <cdwhite3@pm.me> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-02worktree: refactor infer_backlink returnCaleb White1-7/+8
The previous round[1] was merged a bit early before reviewer feedback could be applied. This correctly indents a code block and updates the `infer_backlink` function to return `-1` on failure and strbuf.len on success. [1]: https://lore.kernel.org/git/20241007-wt_relative_paths-v3-0-622cf18c45eb@pm.me Signed-off-by: Caleb White <cdwhite3@pm.me> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-02worktree: add `relativeWorktrees` extensionCaleb White5-0/+16
A new extension, `relativeWorktrees`, is added to indicate that at least one worktree in the repository has been linked with relative paths. This ensures older Git versions do not attempt to automatically prune worktrees with relative paths, as they would not not recognize the paths as being valid. Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Caleb White <cdwhite3@pm.me> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-02setup: correctly reinitialize repository versionCaleb White2-12/+26
When reinitializing a repository, Git does not account for extensions other than `objectformat` and `refstorage` when determining the repository version. This can lead to a repository being downgraded to version 0 if extensions are set, causing Git future operations to fail. This patch teaches Git to check if other extensions are defined in the config to ensure that the repository version is set correctly. Signed-off-by: Caleb White <cdwhite3@pm.me> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-29CodingGuidelines: a handful of error message guidelinesJunio C Hamano1-3/+17
It is more efficient to have something in the coding guidelines document to point at, when we want to review and comment on a new message in the codebase to make sure it "fits" in the set of existing messages. Let's write down established best practice we are aware of. Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-28transport: propagate fsck configuration during bundle fetchJustin Tobler2-0/+27
When fetching directly from a bundle, fsck message severity configuration is not propagated to the underlying git-index-pack(1). It is only capable of enabling or disabling fsck checks entirely. This does not align with the fsck behavior for fetches through git-fetch-pack(1). Use the fsck config parsing from fetch-pack to populate fsck message severity configuration and wire it through to `unbundle()` to enable the same fsck verification as done through fetch-pack. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-28fetch-pack: split out fsck config parsingJustin Tobler2-8/+29
When `fetch_pack_config()` is invoked, fetch-pack configuration is parsed from the config. As part of this operation, fsck message severity configuration is assigned to the `fsck_msg_types` global variable. This is optionally used to configure the downstream git-index-pack(1) when the `--strict` option is specified. The same parsed fsck message severity configuration is also needed outside of fetch-pack. Instead of exposing/relying on the existing global state, split out the fsck config parsing logic into `fetch_pack_fsck_config()` and expose it. In a subsequent commit, this is used to provide fsck configuration when invoking `unbundle()`. For `fetch_pack_fsck_config()` to discern between errors and unhandled config variables, the return code when `git_config_path()` errors is changed to a different value also indicating success. This frees up the previous return code to now indicate the provided config variable was unhandled. The behavior remains functionally the same. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-28bundle: support fsck message configurationJustin Tobler2-6/+14
If the `VERIFY_BUNDLE_FLAG` is set during `unbundle()`, the git-index-pack(1) spawned is configured with the `--fsck-options` flag to perform fsck verification. With this flag enabled, there is not a way to configure fsck message severity though. Extend the `unbundle_opts` type to store fsck message severity configuration and update `unbundle()` to conditionally append it to the `--fsck-objects` flag if provided. This enables `unbundle()` call sites to support optionally setting the severity for specific fsck messages. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-28bundle: add bundle verification options typeJustin Tobler5-9/+22
When `unbundle()` is invoked, fsck verification may be configured by passing the `VERIFY_BUNDLE_FSCK` flag. This mechanism allows fsck checks on the bundle to be enabled or disabled entirely. To facilitate more fine-grained fsck configuration, additional context must be provided to `unbundle()`. Introduce the `unbundle_opts` type, which wraps the existing `verify_bundle_flags`, to facilitate future extension of `unbundle()` configuration. Also update `unbundle()` and its call sites to accept this new options type instead of the flags directly. The end behavior is functionally the same, but allows for the set of configurable options to be extended. This is leveraged in a subsequent commit to enable fsck message severity configuration. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26The twelfth batchJunio C Hamano1-0/+21
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26ref-cache: fix invalid free operation in `free_ref_entry`shejialuo1-1/+2
In cfd971520e (refs: keep track of unresolved reference value in iterators, 2024-08-09), we added a new field "referent" into the "struct ref" structure. In order to free the "referent", we unconditionally freed the "referent" by simply adding a "free" statement. However, this is a bad usage. Because when ref entry is either directory or loose ref, we will always execute the following statement: free(entry->u.value.referent); This does not make sense. We should never access the "entry->u.value" field when "entry" is a directory. However, the change obviously doesn't break the tests. Let's analysis why. The anonymous union in the "ref_entry" has two members: one is "struct ref_value", another is "struct ref_dir". On a 64-bit machine, the size of "struct ref_dir" is 32 bytes, which is smaller than the 48-byte size of "struct ref_value". And the offset of "referent" field in "struct ref_value" is 40 bytes. So, whenever we create a new "ref_entry" for a directory, we will leave the offset from 40 bytes to 48 bytes untouched, which means the value for this memory is zero (NULL). It's OK to free a NULL pointer, but this is merely a coincidence of memory layout. To fix this issue, we now ensure that "free(entry->u.value.referent)" is only called when "entry->flag" indicates that it represents a loose reference and not a directory to avoid the invalid memory operation. Signed-off-by: shejialuo <shejialuo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26refs/reftable: reuse iterators when reading refsPatrick Steinhardt1-3/+29
When reading references the reftable backend has to: 1. Create a new ref iterator. 2. Seek the iterator to the record we're searching for. 3. Read the record. We cannot really avoid the last two steps, but re-creating the iterator every single time we want to read a reference is kind of expensive and a waste of resources. We couldn't help it in the past though because it was not possible to reuse iterators. But starting with 5bf96e0c39 (reftable/generic: move seeking of records into the iterator, 2024-05-13) we have split up the iterator lifecycle such that creating the iterator and seeking are two different concerns. Refactor the code such that we cache iterators in the reftable backend. This cache is invalidated whenever the respective stack is reloaded such that we know to recreate the iterator in that case. This leads to a sizeable speedup when creating many refs, which requires a lot of random reference reads: Benchmark 1: update-ref: create many refs (refcount = 100000, revision = master) Time (mean ± σ): 1.793 s ± 0.010 s [User: 0.954 s, System: 0.835 s] Range (min … max): 1.781 s … 1.811 s 10 runs Benchmark 2: update-ref: create many refs (refcount = 100000, revision = HEAD) Time (mean ± σ): 1.680 s ± 0.013 s [User: 0.846 s, System: 0.831 s] Range (min … max): 1.664 s … 1.702 s 10 runs Summary update-ref: create many refs (refcount = 100000, revision = HEAD) ran 1.07 ± 0.01 times faster than update-ref: create many refs (refcount = 100000, revision = master) While 7% is not a huge win, you have to consider that the benchmark is _writing_ data, so _reading_ references is only one part of what we do. Flame graphs show that we spend around 40% of our time reading refs, so the speedup when reading refs is approximately ~2.5x that. I could not find better benchmarks where we perform a lot of random ref reads. You can also see a sizeable impact on memory usage when creating 100k references. Before this change: HEAP SUMMARY: in use at exit: 19,112,538 bytes in 200,170 blocks total heap usage: 8,400,426 allocs, 8,200,256 frees, 454,367,048 bytes allocated After this change: HEAP SUMMARY: in use at exit: 674,416 bytes in 169 blocks total heap usage: 7,929,872 allocs, 7,929,703 frees, 281,509,985 bytes allocated As an additional factor, this refactoring opens up the possibility for more performance optimizations in how we re-seek iterators. Any change that allows us to optimize re-seeking by e.g. reusing data structures would thus also directly speed up random reads. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26reftable/merged: drain priority queue on reseekPatrick Steinhardt2-0/+75
In 5bf96e0c39 (reftable/generic: move seeking of records into the iterator, 2024-05-13) we have refactored the reftable codebase such that iterators can be initialized once and then re-seeked multiple times. This feature is used by 1869525066 (refs/reftable: wire up support for exclude patterns, 2024-09-16) in order to skip records based on exclude patterns provided by the caller. The logic to re-seek the merged iterator is insufficient though because we don't drain the priority queue on a re-seek. This means that the queue may contain stale entries and thus reading the next record in the queue will return the wrong entry. While this is an obvious bug, it is harmless in the context of above exclude patterns: - If the queue contained stale entries that match the pattern then the caller would already know to filter out such refs. This is because our codebase is prepared to handle backends that don't have a way to efficiently implement exclude patterns. - If the queue contained stale entries that don't match the pattern we'd eventually filter out any duplicates. This is because the reftable code discards items with the same ref name and sorts any remaining entries properly. So things happen to work in this context regardless of the bug, and there is no other use case yet where we re-seek iterators. We're about to introduce a caching mechanism though where iterators are reused by the reftable backend, and that will expose the bug. Fix the issue by draining the priority queue when seeking and add a testcase that surfaces the issue. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26reftable/stack: add mechanism to notify callers on reloadPatrick Steinhardt2-0/+13
Reftable stacks are reloaded in two cases: - When calling `reftable_stack_reload()`, if the stat-cache tells us that the stack has been modified. - When committing a reftable addition. While callers can figure out the second case, they do not have a mechanism to figure out whether `reftable_stack_reload()` led to an actual reload of the on-disk data. All they can do is thus to assume that data is always being reloaded in that case. Improve the situation by introducing a new `on_reload()` callback to the reftable options. If provided, the function will be invoked every time the stack has indeed been reloaded. This allows callers to invalidate data that depends on the current stack data. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26refs/reftable: refactor reflog expiry to use reftable backendPatrick Steinhardt1-8/+5
Refactor the callback function that expires reflog entries in the reftable backend to use `reftable_backend_read_ref()` instead of accessing the reftable stack directly. This ensures that the function will benefit from the new caching layer that we're about to introduce. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26refs/reftable: refactor reading symbolic refs to use reftable backendPatrick Steinhardt1-7/+4
Refactor the callback function that reads symbolic references in the reftable backend to use `reftable_backend_read_ref()` instead of accessing the reftable stack directly. This ensures that the function will benefit from the new caching layer that we're about to introduce. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26refs/reftable: read references via `struct reftable_backend`Patrick Steinhardt1-63/+59
Refactor `read_ref_without_reload()` to accept `struct reftable_backend` as parameter instead of `struct reftable_stack`. Rename the function to `reftable_backend_read_ref()` to clarify its scope and move it close to other functions operating on `struct reftable_backend`. This change allows us to implement an additional caching layer when reading refs where we can reuse reftable iterators. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26refs/reftable: figure out hash via `reftable_stack`Patrick Steinhardt1-7/+19
The function `read_ref_without_reload()` accepts a ref store as input only so that we can figure out the hash function used by it. This is duplicate information though because the reftable stack knows about its hash function, too. Drop the superfluous parameter to simplify the calling convention a bit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26reftable/stack: add accessor for the hash IDPatrick Steinhardt2-0/+8
Add an accessor function that allows callers to access the hash ID of a reftable stack. This function will be used in a subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26refs/reftable: handle reloading stacks in the reftable backendPatrick Steinhardt1-58/+126
When accessing a stack we almost always have to reload the stack before reading data from it. This is mostly because Git does not have a notification mechanism for when underlying data has been changed, and thus we are forced to opportunistically reload the stack every single time to account for any changes that may have happened concurrently. Handle the reload internally in `backend_for()`. For one this forces callsites to think about whether or not they need to reload the stack. But second this makes the logic to access stacks more self-contained by letting the `struct reftable_backend` manage themselves. Update callsites where we don't reload the stack to document why we don't. In some cases it's unclear whether it is the right thing to do in the first place, but fixing that is outside of the scope of this patch series. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26refs/reftable: encapsulate reftable stackPatrick Steinhardt1-59/+76
The reftable ref store needs to keep track of multiple stacks, one for the main worktree and an arbitrary number of stacks for worktrees. This is done by storing pointers to `struct reftable_stack`, which we then access directly. Wrap the stack in a new `struct reftable_backend`. This will allow us to attach more data to each respective stack in subsequent commits. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26builtin: pass repository to sub commandsKarthik Nayak17-135/+239
In 9b1cb5070f (builtin: add a repository parameter for builtin functions, 2024-09-13) the repository was passed down to all builtin commands. This allowed the repository to be passed down to lower layers without depending on the global `the_repository` variable. Continue this work by also passing down the repository parameter from the command to sub-commands. This will help pass down the repository to other subsystems and cleanup usage of global variables like 'the_repository' and 'the_hash_algo'. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>