Re: [PATCH 03/13] setup: unify setup of shallow file

[Justin Tobler <jltobler@gmail.com>]

On 26/06/30 01:47PM, Patrick Steinhardt wrote:
> It is possible to configure an arbitrary "shallow" file via two
> mechanisms, and the respective logic to handle these is split across two
> locations:
> 
>   - Via the "GIT_SHALLOW_FILE" environment variable, which is handled in
>     `setup_git_env_internal()`.
> 
>   - Via the global "--shallow-file=" command line option, which is
>     handled in `handle_options()`.

Ok.

> We can rather easily unify this logic by not configuring the shallow
> file in `handle_options()`, but instead overwriting the environment
> variable. The environment variable itself is then handled inside of
> `apply_repository_format()`, which is responsible for configuring a
> discovered Git directory.

What is supposed to be the correct order for processing shallow file
configuration here? Does this mean that the `--shallow-file` option now
overwrites the environment variable? Was this how it already was?

> This new logic is similar in nature to how we handle the other global
> options already, all of which end up setting an environment variable.
> So for one this gives us more consistency. But more importantly, this
> change means that `the_repository` will not contain any relevant state
> anymore before we hit `apply_repository_format()` once we're at the end
> of this patch series. Consequently, it will become possible for us to
> completely discard `the_repository` and populate it anew.

I can't say that I'm a fan of using environment variables to store
global state in this manner, but I guess if there is precdent and this
is making us more consistent, it is probably fine. I guess the other
option would be to store the read configuration is some intermediate
structure to be applied later, but that may not be worth it here.

-Justin