Re: [WIP v2 5/5] mv: use update_sparsity() after touching sparse contents

[Victoria Dye <vdye@github.com>]

Shaoxuan Yuan wrote:
> Originally, "git mv" a sparse file/directory from out/in-cone to
> in/out-cone does not update the sparsity following the sparse-checkout
> patterns.
> 

I generally agree with the intent here - that, if you move a non-sparse file
out-of-cone, it should become sparse (and vice versa). However, that result
can be reached by simply flipping the 'SKIP_WORKTREE' bit(s) on the
resultant index entry/entries (which you already have, since they're renamed
with 'rename_cache_entry_at()' below). 

Note that you'll also probably need to check out the file(s) (if moving into
the cone) or remove them from disk (if moving out of cone). If you don't,
files moved into cone will appear "deleted" on-disk, and files moved
out-of-cone that still appear on disk will have 'SKIP_WORKTREE'
automatically disabled (see [1]).

For reference, I'd advise against reapplying the sparsity patterns - as you
do below - because involves a much more expensive traversal of the entire
repository. It also has the possibly unwanted side effect of resetting the
'SKIP_WORKTREE' bit to match the sparse patterns on *all* files, not just
the one(s) you moved. 

[1] https://lore.kernel.org/git/11d46a399d26c913787b704d2b7169cafc28d639.1642175983.git.gitgitgadget@gmail.com/

> Use update_sparsity() after touching sparse contents, so the sparsity
> will be updated after the move.
> 
> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
> ---
>  builtin/mv.c                  | 19 +++++++++++++++++++
>  t/t7002-mv-sparse-checkout.sh | 16 ++++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/builtin/mv.c b/builtin/mv.c
> index e64f251a69..2c02120941 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -13,6 +13,7 @@
>  #include "string-list.h"
>  #include "parse-options.h"
>  #include "submodule.h"
> +#include "unpack-trees.h"
>  
>  static const char * const builtin_mv_usage[] = {
>  	N_("git mv [<options>] <source>... <destination>"),
> @@ -158,6 +159,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>  {
>  	int i, flags, gitmodules_modified = 0;
>  	int verbose = 0, show_only = 0, force = 0, ignore_errors = 0, ignore_sparse = 0;
> +	int sparse_moved = 0;
>  	struct option builtin_mv_options[] = {
>  		OPT__VERBOSE(&verbose, N_("be verbose")),
>  		OPT__DRY_RUN(&show_only, N_("dry run")),
> @@ -376,6 +378,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>  		const char *src = source[i], *dst = destination[i];
>  		enum update_mode mode = modes[i];
>  		int pos;
> +		if (!sparse_moved && mode & (SPARSE | SKIP_WORKTREE_DIR))
> +			sparse_moved = 1;
>  		if (show_only || verbose)
>  			printf(_("Renaming %s to %s\n"), src, dst);
>  		if (show_only)
> @@ -403,6 +407,21 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>  		rename_cache_entry_at(pos, dst);
>  	}
>  
> +	if (sparse_moved) {
> +		struct unpack_trees_options o;
> +		memset(&o, 0, sizeof(o));
> +		o.verbose_update = isatty(2);
> +		o.update = 1;
> +		o.head_idx = -1;
> +		o.src_index = &the_index;
> +		o.dst_index = &the_index;
> +		o.skip_sparse_checkout = 0;
> +		o.pl = the_index.sparse_checkout_patterns;
> +		setup_unpack_trees_porcelain(&o, "mv");
> +		update_sparsity(&o);
> +		clear_unpack_trees_porcelain(&o);
> +	}
> +
>  	if (gitmodules_modified)
>  		stage_updated_gitmodules(&the_index);
>  
> diff --git a/t/t7002-mv-sparse-checkout.sh b/t/t7002-mv-sparse-checkout.sh
> index cf2f5dc46f..1fd3e3c0fc 100755
> --- a/t/t7002-mv-sparse-checkout.sh
> +++ b/t/t7002-mv-sparse-checkout.sh
> @@ -287,6 +287,22 @@ test_expect_success 'refuse to move sparse file to existing destination' '
>  	test_cmp expect stderr
>  '
>  
> +# Need fix.
> +#
> +# The *expected* behavior:
> +#
> +# Using --sparse to accept a sparse file, --force to overwrite the destination.
> +# The folder1/file1 should replace the sub/file1 without error.
> +#
> +# The *actual* behavior:
> +#
> +# It emits a warning:
> +#
> +# warning: Path ' sub/file1
> +# ' already present; will not overwrite with sparse update.
> +# After fixing the above paths, you may want to run `git sparse-checkout
> +# reapply`.
> +
>  test_expect_failure 'move sparse file to existing destination with --force and --sparse' '
>  	git sparse-checkout disable &&
>  	git reset --hard &&

This error is (I think) part of 'update_sparsity()'. If you change the
approach to only modifying the 'SKIP_WORKTREE' bit, hopefully you'll get the
behavior you're looking for.