On 6/23/2022 11:14 PM, Derrick Stolee wrote:
> On 6/23/2022 7:41 AM, Shaoxuan Yuan wrote:
>> Originally, moving a <source> directory which is not on-disk due
>> to its existence outside of sparse-checkout cone, "giv mv" command
>> errors out with "bad source".
>>
>> Add a helper check_dir_in_index() function to see if a directory
>> name exists in the index. Also add a SKIP_WORKTREE_DIR bit to mark
>> such directories.
>>
>> Change the checking logic, so that such <source> directory makes
>> "giv mv" command warns with "advise_on_updating_sparse_paths()"
>> instead of "bad source"; also user now can supply a "--sparse" flag so
>> this operation can be carried out successfully.
>>
>> Helped-by: Victoria Dye <vdye@xxxxxxxxxx>
>> Helped-by: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@xxxxxxxxx>
>> ---
>> builtin/mv.c | 50 ++++++++++++++++++++++++++++++-----
>> t/t7002-mv-sparse-checkout.sh | 4 +--
>> 2 files changed, 46 insertions(+), 8 deletions(-)
>>
>> diff --git a/builtin/mv.c b/builtin/mv.c
>> index aa29da4337..b5d0d8ef4f 100644
>> --- a/builtin/mv.c
>> +++ b/builtin/mv.c
>> @@ -25,6 +25,7 @@ enum update_mode {
>> WORKING_DIRECTORY = (1 << 1),
>> INDEX = (1 << 2),
>> SPARSE = (1 << 3),
>> + SKIP_WORKTREE_DIR = (1 << 4),
>> };
>>
>> #define DUP_BASENAME 1
>> @@ -123,6 +124,36 @@ static int index_range_of_same_dir(const char
*src, int length,
>> return last - first;
>> }
>>
>> +/*
>> + * Check if an out-of-cone directory should be in the index.
Imagine this case
>> + * that all the files under a directory are marked with
'CE_SKIP_WORKTREE' bit
>> + * and thus the directory is sparsified.
>> + *
>> + * Return 0 if such directory exist (i.e. with any of its contained
files not
>> + * marked with CE_SKIP_WORKTREE, the directory would be present in
working tree).
>> + * Return 1 otherwise.
>> + */
>
> This description and the implementation seems like it will work
> even if the path exists as a sparse directory in a sparse index.
>
> It would be good to consider testing this kind of move for a
> directory on the sparse boundary (where it would be a sparse
> directory in a sparse index) _and_ if it is deeper than the
> boundary (so the sparse index would expand in the cache_name_pos()
> method). These tests can be written now for correctness, but later
> the first case can be updated to use the 'ensure_not_expanded'
> helper in t1092.
I'm a bit confused here. Shouldn't we turn on the sparse-index
feature for 'mv' before adding sparse-index related tests? Since this
series does not go into sparse-index, I'm not sure how the tests can
pass. Perhaps we can test about this in the future sparse-index
integration series, no?
Thanks & Regards,
Shaoxuan
