From: Derrick Stolee Date: Fri, 16 May 2025 15:41:57 GMT Subject: Re: [PATCH v2 08/13] pack-objects: enable --path-walk via config Message-ID: In-Reply-To: On 5/6/25 3:46 PM, Derrick Stolee wrote: > On 5/2/25 7:42 PM, Taylor Blau wrote: >>>   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. >> >> (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). Rereading these comments, I didn't adequately reply to "why use the repo settings?" and the reason comes due to the use of assigning the value in feature.experimental=true. This provides a common place for the logic around both path.usePathWalk and feature.experimental. A similar behavior is already present for pack.useSparse, which was in feature.experimental for a while before it became enabled by default. Thanks, -Stolee