Re: [GSOC PATCH v5 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns'

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Ayush

On 02/07/2025 00:53, Ayush Chandekar wrote:
> On Tue, Jul 1, 2025 at 6:48 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>> On 30/06/2025 20:27, Ayush Chandekar wrote:
>>>
>>>    void clear_skip_worktree_from_present_files(struct index_state *istate)
>>>    {
>>> +     int sparse_expect_files_outside_of_patterns = 0;
>>> +     repo_config_get_bool(istate->repo, "sparse.expectfilesoutsideofpatterns",
>>> +             &sparse_expect_files_outside_of_patterns);
>>
>> This changes the user facing behavior if
>> sparse.expectfilesoutsideofpatterns is not a valid boolean value.
>> Currently git will error out when it first starts because that config
>> value is parsed by git_default_config() which is called by almost all
>> git commands. This means that if someone sets an invalid value they get
>> timely feedback that the value is invalid and git dies before doing
>> anything. Now, if the value is invalid, git will only die if this
>> function is called and it is likely to die in the middle of a command.
> 
> Yes, I get your point. However, if we look at settings which are
> shifted to `struct repo_settings`, the behaviour is to set a
> fallback/default value in case of an invalid input, instead of
> throwing an error. This is done inside the `prepare_repo_settings()`
> function, which is often called in the middle of a process.

I'm a bit confused by this and I'm not quite sure what you're saying for 
a couple of reasons. Firstly this patch is not adding a new member to 
struct repo_settings, it is parsing the config directly and will die() 
in git_config_bool() if the config value is invalid. Secondly 
prepare_repo_settings() ends up calling git_config_bool() and so will 
also die if the config value is invalid rather than setting a default 
value. In the case of prepare_repo_settings() commands that do not want 
to die in the middle of an operation can call that function early on 
before they start doing any real work. Looking at the output of "git 
grep prepare_repo_settings()" many do exactly that. Here there is no 
option for a command to die() early on invalid config values if it wants to.

Thanks

Phillip