Re: [PATCH v4 7/7] mv: add check_dir_in_index() and solve general dir check issue
- From
Derrick Stolee <derrickstolee@github.com>
- Date
- Jun 23, 2022, 15:14 UTC
- 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:
Show 46 quoted lines
> 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