Re: [PATCH v2 4/5] backfill: add --sparse option

From: Derrick Stolee <stolee@gmail.com>
Date: Feb 3, 2025, 15:11 UTC
Message-ID: <34379f16-9da6-440b-8656-5d580056c36b@gmail.com>
In-Reply-To: <Z4jY4rELRYvS2gOk@pks.im>

On 1/16/25 5:01 AM, Patrick Steinhardt wrote:

> On Fri, Dec 20, 2024 at 04:29:52PM +0000, Derrick Stolee via GitGitGadget wrote:

Show 17 quoted lines

>> +		if (ctx->info->pl) {
>> +			int dtype;
>> +			enum pattern_match_result match;
>> +			match = path_matches_pattern_list(path.buf, path.len,
>> +							  path.buf + base_len, &dtype,
>> +							  ctx->info->pl,
>> +							  ctx->repo->index);
>> +
>> +			if (ctx->info->pl->use_cone_patterns &&
>> +			    match == NOT_MATCHED)
>> +				continue;
>> +			else if (!ctx->info->pl->use_cone_patterns &&
>> +				 type == OBJ_BLOB &&
>> +				 match != MATCHED)
> 
> For my own understanding: is there as pecific reason why one of the
> branches uses `== NOT_MATCHED` whereas the other one uses `!= MATCHED`?

With cone mode sparse-checkout, 'match' could equal MATCHED, MATCHED_RECURSIVE, or UNDECIDED, which we want to be considered all the same case: continue along this path.

When not in cone mode, we can't decide to filter by trees (hence the OBJ_BLOB restriction) and then the result can be MATCHED, NOT_MATCHED, and UNDECIDED. This rule matches the following realization:

  * MATCHED if there is a positive pattern that matches the path.
  * NOT_MATCHED if there is a negative pattern that matches the path.
  * UNDECIDED if no pattern matches the path.

This is subtle, but switching this to "match == NOT_MATCHED" will result in the test failing (and the test is right).

I will make note of this in my commit message in the next version, as well as adding a test that has nested positive and negative patterns.

Thanks, -Stolee