Re: [PATCH v2 08/13] pack-objects: enable --path-walk via config
- From
Derrick Stolee <stolee@gmail.com>
- Date
- May 6, 2025, 19:46 UTC
- Message-ID
- <ea342443-2c71-476d-ab45-86cbe711fec7@gmail.com>
- In-Reply-To
- <aBVYa9SzXk7eguIz@nand.local>
On 5/2/25 7:42 PM, Taylor Blau wrote:
> On Mon, Mar 24, 2025 at 03:22:44PM +0000, Derrick Stolee via GitGitGadget wrote:
Show 13 quoted lines
>> +pack.usePathWalk:: >> + When true, git will default to using the '--path-walk' option in >> + 'git pack-objects' when the '--revs' option is present. This >> + algorithm groups objects by path to maximize the ability to >> + compute delta chains across historical versions of the same >> + object. This may disable other options, such as using bitmaps to >> + enumerate objects. >> + > > Same note here as in the previous patch, I think it's fine to refer > readers to the git-pack-objects[1] documentation instead of repeating > yourself here. (And it removes the risk of these three descriptions > falling out of sync with one another).
Sure.
Show 22 quoted lines
>> pack.preferBitmapTips::
>> When selecting which commits will receive bitmaps, prefer a
>> commit at the tip of any reference that is a suffix of any value
>> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
>> index a6b8a78d42a..0ea85754c52 100644
>> --- a/builtin/pack-objects.c
>> +++ b/builtin/pack-objects.c
>> @@ -4652,6 +4652,9 @@ int cmd_pack_objects(int argc,
>> if (use_bitmap_index > 0 ||
>> !use_internal_rev_list)
>> path_walk = 0;
>> + else if (the_repository->gitdir &&
>> + the_repository->settings.pack_use_path_walk)
>> + path_walk = 1;
>> else
>> path_walk = git_env_bool("GIT_TEST_PACK_PATH_WALK", 0);
>> }
>
> The limited diff context makes it hard for me to tell for sure, but this
> takes place after git_config(), right? If so, I think we can avoid using
> the repository settings machinery here and just use the config API
> directly.As I'm paging this thread back into my memory, I internally thought "obviously the test demonstrates that it works..."
Show 7 quoted lines
> (FWIW, I typically think of repository settings as a way to expose > config information to some part of the codebase that doesn't otherwise > have easy access to, e.g., a static field that was set by a git_config() > callback). > > Separate from the above: should this have a sanity test to makes sure > that we read the config setting correctly?
...but of course I neglected to create such a test. Will fix.
Thanks, -Stolee