Permalink
Comparing changes
Choose two branches to see what’s changed or to start a new pull request.
If you need to, you can also or
learn more about diff comparisons.
Open a pull request
Create a new pull request by comparing changes across two branches. If you need to, you can also .
Learn more about diff comparisons here.
...
- 8 commits
- 1 file changed
- 1 contributor
Commits on Sep 13, 2023
-
builtin/repack.c: extract structure to store existing packs
The repack machinery needs to keep track of which packfiles were present in the repository at the beginning of a repack, segmented by whether or not each pack is marked as kept. The names of these packs are stored in two `string_list`s, corresponding to kept- and non-kept packs, respectively. As a consequence, many functions within the repack code need to take both `string_list`s as arguments, leading to code like this: ret = write_cruft_pack(&cruft_po_args, packtmp, pack_prefix, cruft_expiration, &names, &existing_nonkept_packs, /* <- */ &existing_kept_packs); /* <- */ Wrap up this pair of `string_list`s into a single structure that stores both. This saves us from having to pass both string lists separately, and prepares for adding additional fields to this structure. Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]> -
builtin/repack.c: extract marking packs for deletion
At the end of a repack (when given `-d`), Git attempts to remove any packs which have been made "redundant" as a result of the repacking operation. For example, an all-into-one (`-A` or `-a`) repack makes every pre-existing pack which is not marked as kept redundant. Geometric repacks (with `--geometric=<n>`) make any packs which were rolled up redundant, and so on. But before deleting the set of packs we think are redundant, we first check to see whether or not we just wrote a pack which is identical to any one of the packs we were going to delete. When this is the case, Git must avoid deleting that pack, since it matches a pack we just wrote (so deleting it may cause the repository to become corrupt). Right now we only process the list of non-kept packs in a single pass. But a future change will split the existing non-kept packs further into two lists: one for cruft packs, and another for non-cruft packs. Factor out this routine to prepare for calling it twice on two separate lists in a future patch. Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
-
builtin/repack.c: extract redundant pack cleanup for --geometric
To reduce the complexity of the already quite-long `cmd_repack()` implementation, extract out the parts responsible for deleting redundant packs from a geometric repack out into its own sub-routine. Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
-
builtin/repack.c: extract redundant pack cleanup for existing packs
To remove redundant packs at the end of a repacking operation, Git uses its `remove_redundant_pack()` function in a loop over the set of pre-existing, non-kept packs. In a later commit, we will split this list into two, one for pre-existing cruft pack(s), and another for non-cruft pack(s). Prepare for this by factoring out the routine to loop over and delete redundant packs into its own function. Instead of calling `remove_redundant_pack()` directly, we now will call `remove_redundant_existing_packs()`, which itself dispatches a call to `remove_redundant_packs_1()`. Note that the geometric repacking code will still call `remove_redundant_pack()` directly, but see the previous commit for more details. Having `remove_redundant_packs_1()` exist as a separate function may seem like overkill in this patch. However, a later patch will call `remove_redundant_packs_1()` once over two separate lists, so this refactoring sets us up for that. Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
-
builtin/repack.c: extract
has_existing_non_kept_packs()When there is: - at least one pre-existing packfile (which is not marked as kept), - repacking with the `-d` flag, and - not doing a cruft repack , then we pass a handful of additional options to the inner `pack-objects` process, like `--unpack-unreachable`, `--keep-unreachable`, and `--pack-loose-unreachable`, in addition to marking any packs we just wrote for promisor remotes as kept in-core (with `--keep-pack`, as opposed to the presence of a ".keep" file on disk). Because we store both cruft and non-cruft packs together in the same `existing.non_kept_packs` list, it suffices to check its `nr` member to see if it is zero or not. But a following change will store cruft- and non-cruft packs separately, meaning this check would break as a result. Prepare for this by extracting this part of the check into a new helper function called `has_existing_non_kept_packs()`. This patch does not introduce any functional changes, but prepares us to make a more isolated change in a subsequent patch. Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
-
builtin/repack.c: store existing cruft packs separately
When repacking with the `--write-midx` option, we invoke the function `midx_included_packs()` in order to produce the list of packs we want to include in the resulting MIDX. This list is comprised of: - existing .keep packs - any pack(s) which were written earlier in the same process - any unchanged packs when doing a `--geometric` repack - any cruft packs Prior to this patch, we stored pre-existing cruft and non-cruft packs together (provided those packs are non-kept). This meant we needed an additional bit to indicate which non-kept pack(s) were cruft versus those that aren't. But alternatively we can store cruft packs in a separate list, avoiding the need for this extra bit, and simplifying the code below. Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
-
builtin/repack.c: avoid directly inspecting "util"
The `->util` field corresponding to each string_list_item is used to track the existence of some pack at the beginning of a repack operation was originally intended to be used as a bitfield. This bitfield tracked: - (1 << 0): whether or not the pack should be deleted - (1 << 1): whether or not the pack is cruft The previous commit removed the use of the second bit, but a future patch (from a different series than this one) will introduce a new use of it. So we could stop treating the util pointer as a bitfield and instead start treating it as if it were a boolean. But this would require some backtracking when that later patch is applied. Instead, let's avoid touching the ->util field directly, and instead introduce convenience functions like: - pack_mark_for_deletion() - pack_is_marked_for_deletion() Helped-by: Junio C Hamano <[email protected]> Helped-by: Jeff King <[email protected]> Helped-by: Patrick Steinhardt <[email protected]> Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
-
builtin/repack.c: extract common cruft pack loop
When generating the list of packs to store in a MIDX (when given the `--write-midx` option), we include any cruft packs both during --geometric and non-geometric repacks. But the rules for when we do and don't have to check whether any of those cruft packs were queued for deletion differ slightly between the two cases. But the two can be unified, provided there is a little bit of extra detail added in the comment to clarify when it is safe to avoid checking for any pending deletions (and why it is OK to do so even when not required). Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
Loading
This comparison is taking too long to generate.
Unfortunately it looks like we can’t render this comparison for you right now. It might be too big, or there might be something weird with your repository.
You can try running this command locally to see the comparison on your machine:
git diff 6a8bb340f2d73636ab739fbe457943e00b322376...c6a0468f824f458acea442450d204a3d01d1aa9b
