From: Derrick Stolee <derrickstolee@github.com>
Date: Thu, 23 Jun 2022 15:14:18 GMT
Subject: Re: [PATCH v4 7/7] mv: add check_dir_in_index() and solve general dir check issue
Message-ID: <17a34570-f823-7908-1ea5-8e7bf8680b23@github.com>
In-Reply-To: <20220623114120.12768-8-shaoxuan.yuan02@gmail.com>

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@github.com>
> Helped-by: Derrick Stolee <derrickstolee@github.com>
> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
> ---
>  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.

> +static int check_dir_in_index(const char *name)

Thanks,
-Stolee