Re: [GSOC PATCH v4 2/3] environment: move access to "core.sparsecheckoutcone" into repo_settings

[Junio C Hamano <gitster@pobox.com>]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> The setting "core.sparsecheckoutcone" is stored in the global
> `core_sparse_checkout_cone` and is populated in config.c. Refactor the
> code to store it in the variable `sparse_checkout_cone` in the struct
> `repo_settings`.
> It's fine not to lazily load it from the config, as the variable
> is used quite commonly.
>
> This change is part of an ongoing effort to eliminate global variables,
> improve modularity and help libify the codebase.
>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---

I think "the correctness guarantee comes from a different place now.
How are we making sure that these accesses are correct?" comment
applies equally here.

>  builtin/grep.c            |  2 +-
>  builtin/mv.c              |  2 +-

Looking at the output from

    $ git grep -n -e prepare_repo_settings \*.c

there are many builtin/*.c that makes a call to the function on
the_repository fairly early in its start-up sequence.  Unlike many
others these two do not seem to have any.

>  builtin/sparse-checkout.c | 28 ++++++++++++++--------------

This one does, immediately after calling git_config(), so it should
be fairly safe.

>  dir.c                     |  2 +-
>  sparse-index.c            |  2 +-

These two also need correctness guarantee.

You'd need to make sure any potential caller of the helper functions
have called prepare_repo_settings().  Those who wrote an access to
the global variable in the original would already have made sure
that the callers would have already read the configuration file, but
with the new code, that is no longer a guarantee for correctness.