Re: [PATCH v3 6/8] attr: be careful about sparse directories

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Stolee,

On Tue, 17 Aug 2021, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <dstolee@microsoft.com>
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  attr.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/attr.c b/attr.c
> index d029e681f28..a1009f78029 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -14,6 +14,7 @@
>  #include "utf8.h"
>  #include "quote.h"
>  #include "thread-utils.h"
> +#include "dir.h"
>
>  const char git_attr__true[] = "(builtin)true";
>  const char git_attr__false[] = "\0(builtin)false";
> @@ -744,6 +745,19 @@ static struct attr_stack *read_attr_from_index(struct index_state *istate,
>  	if (!istate)
>  		return NULL;
>
> +	/*
> +	 * In the case of cone-mode sparse-checkout, getting the
> +	 * .gitattributes file from a directory is meaningless: all
> +	 * contained paths will be sparse if the .gitattributes is also
> +	 * sparse. In the case of a sparse index, it is critical that we
> +	 * don't go looking for one as it will expand the index.
> +	 */
> +	init_sparse_checkout_patterns(istate);
At first I thought that `init_sparse_checkout_patterns()` is called by
`path_in_sparse_checkout()` below, and therefore would not be necessary.

But it is! Without it, we have no way to test whether `use_cone_patterns`
is set, because, well, it gets set by `init_sparse_checkout_patterns()`.

Would it therefore make sense to refactor the code to have a
`path_in_sparse_checkout_cone()` function? Or add a flag
`only_in_cone_mode` as function parameter to `path_in_sparse_checkout()`?

Ciao,
Dscho

> +	if (istate->sparse_checkout_patterns &&
> +	    istate->sparse_checkout_patterns->use_cone_patterns &&
> +	    path_in_sparse_checkout(path, istate) == NOT_MATCHED)

> +		return NULL;
> +
>  	buf = read_blob_data_from_index(istate, path, NULL);
>  	if (!buf)
>  		return NULL;
> --
> gitgitgadget
>
>