Re: [RFC PATCH] add|rm|mv: fix bug that prevent the update of non-sparse

From: Matheus Tavares <matheus.bernardino@usp.br>
Date: Oct 22, 2021, 04:03 UTC
Message-ID: <20211022040352.16773-1-matheus.bernardino@usp.br>
In-Reply-To: <80b5ba61861193daf7132aa64b65fc7dde90dacb.1634866698.git.matheus.bernardino@usp.br>

On Thu, Oct 21, 2021 at 11:28 PM Matheus Tavares <matheus.bernardino@usp.br> wrote:

Show 40 quoted lines

>
> diff --git a/dir.c b/dir.c
> index a4306ab874..225487a59c 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1516,11 +1517,31 @@ static int path_in_sparse_checkout_1(const char *path,
>  	     !istate->sparse_checkout_patterns->use_cone_patterns))
>  		return 1;
>  
> -	base = strrchr(path, '/');
> -	return path_matches_pattern_list(path, strlen(path), base ? base + 1 : path,
> -					 &dtype,
> -					 istate->sparse_checkout_patterns,
> -					 istate) > 0;
> +	if (istate->sparse_checkout_patterns->use_cone_patterns) {
> +		const char *base = strrchr(path, '/');
> +		return path_matches_pattern_list(path, strlen(path),
> +				base ? base + 1 : path, &dtype,
> +				istate->sparse_checkout_patterns, istate) > 0;
> +	}
> +
> +	for (p = path; ; p++) {
> +		enum pattern_match_result match;
> +
> +		if (*p && *p != '/')
> +			continue;
> +
> +		match  = path_matches_pattern_list(path, p - path,
> +				last_slash ? last_slash + 1 : path, &dtype,
> +				istate->sparse_checkout_patterns, istate);
> +
> +		if (match != UNDECIDED)
> +			ret = match;
> +		if (!*p)
> +			break;
> +		last_slash = p;
> +	}
> +
> +	return ret;
>  }

Of course, after hitting send I realized it would make a lot more sense to start the pattern matching from the full path and only go backwards through the parent dirs until we find the first non-UNDECIDED result. I.e. something like this:

static int path_in_sparse_checkout_1(const char *path,
				     struct index_state *istate,
				     int require_cone_mode)
{
	int dtype = DT_REG;
	enum pattern_match_result ret;
	const char *p, *base;

	/*
	 * We default to accepting a path if there are no patterns or
	 * they are of the wrong type.
	 */
	if (init_sparse_checkout_patterns(istate) ||
	    (require_cone_mode &&
	     !istate->sparse_checkout_patterns->use_cone_patterns))
		return 1;

	if (istate->sparse_checkout_patterns->use_cone_patterns) {
		base = strrchr(path, '/');
		return path_matches_pattern_list(path, strlen(path),
				base ? base + 1 : path, &dtype,
				istate->sparse_checkout_patterns, istate) > 0;
	}

	/*
	 * If the match for the path is UNDECIDED, try to match the parent dir
	 * recursively.
	 */
	for (p = path + strlen(path); p && p > path; p = base) {
		base = memrchr(path, '/', p - path);
		ret = path_matches_pattern_list(path, p - path,
				base ? base + 1 : path, &dtype,
				istate->sparse_checkout_patterns, istate);

		if (ret != UNDECIDED)
			break;
	}

	return ret == UNDECIDED ? NOT_MATCHED : ret;
}

But I will let others comment on the overall idea and/or other alternatives before sending a possible v2.