diff mbox series

[v2,9/9] mv: check overwrite for in-to-out move

Message ID 20220805030528.1535376-10-shaoxuan.yuan02@gmail.com (mailing list archive)
State Superseded
Headers show
Series mv: from in-cone to out-of-cone | expand

Commit Message

Shaoxuan Yuan Aug. 5, 2022, 3:05 a.m. UTC
Add checking logic for overwriting when moving from in-cone to
out-of-cone. It is the index version of the original overwrite logic.

Helped-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
---
 builtin/mv.c                  | 12 ++++++++++++
 t/t7002-mv-sparse-checkout.sh |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

Comments

Victoria Dye Aug. 8, 2022, 11:53 p.m. UTC | #1
Shaoxuan Yuan wrote:
> Add checking logic for overwriting when moving from in-cone to
> out-of-cone. It is the index version of the original overwrite logic.
> 
> Helped-by: Derrick Stolee <derrickstolee@github.com>
> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
> ---
>  builtin/mv.c                  | 12 ++++++++++++
>  t/t7002-mv-sparse-checkout.sh |  2 +-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/mv.c b/builtin/mv.c
> index 765a1e8eb5..70996d582f 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -367,6 +367,18 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>  			goto act_on_entry;
>  		}
>  
> +		if (ignore_sparse &&

If '--sparse' is specified...

> +		    (dst_mode & SKIP_WORKTREE_DIR) &&

...and the destination's parent directory is outside the sparse cone...

> +		    index_entry_exists(&the_index, dst, strlen(dst))) {

...and the destination file exists in the index, then we're going to be
overwriting an existing index entry. 

> +			bad = _("destination exists in the index");
> +			if (force) {
> +				if (verbose)
> +					warning(_("overwriting '%s'"), dst);
> +				bad = NULL;
> +			} else {
> +				goto act_on_entry;
> +			}
> +		}

The rest of this aligns with what's done for a normal (exists on-disk)
overwrite. There's not much in the original overwrite-handling logic that
can be reused here (it's checking for overwrite based on 'stat' rather than
the contents of the index), so code structure-wise this makes sense as a
standalone check.

Looking good!

>  		/*
>  		 * We check if the paths are in the sparse-checkout
>  		 * definition as a very final check, since that
> diff --git a/t/t7002-mv-sparse-checkout.sh b/t/t7002-mv-sparse-checkout.sh
> index f0b32a2f70..50bcca583c 100755
> --- a/t/t7002-mv-sparse-checkout.sh
> +++ b/t/t7002-mv-sparse-checkout.sh
> @@ -323,7 +323,7 @@ test_expect_success 'move clean path from in-cone to out-of-cone' '
>  	grep "S folder1/d" actual
>  '
>  
> -test_expect_failure 'move clean path from in-cone to out-of-cone overwrite' '
> +test_expect_success 'move clean path from in-cone to out-of-cone overwrite' '
>  	test_when_finished "cleanup_sparse_checkout" &&
>  	setup_sparse_checkout &&
>  	echo "sub/file1 overwrite" >sub/file1 &&
diff mbox series

Patch

diff --git a/builtin/mv.c b/builtin/mv.c
index 765a1e8eb5..70996d582f 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -367,6 +367,18 @@  int cmd_mv(int argc, const char **argv, const char *prefix)
 			goto act_on_entry;
 		}
 
+		if (ignore_sparse &&
+		    (dst_mode & SKIP_WORKTREE_DIR) &&
+		    index_entry_exists(&the_index, dst, strlen(dst))) {
+			bad = _("destination exists in the index");
+			if (force) {
+				if (verbose)
+					warning(_("overwriting '%s'"), dst);
+				bad = NULL;
+			} else {
+				goto act_on_entry;
+			}
+		}
 		/*
 		 * We check if the paths are in the sparse-checkout
 		 * definition as a very final check, since that
diff --git a/t/t7002-mv-sparse-checkout.sh b/t/t7002-mv-sparse-checkout.sh
index f0b32a2f70..50bcca583c 100755
--- a/t/t7002-mv-sparse-checkout.sh
+++ b/t/t7002-mv-sparse-checkout.sh
@@ -323,7 +323,7 @@  test_expect_success 'move clean path from in-cone to out-of-cone' '
 	grep "S folder1/d" actual
 '
 
-test_expect_failure 'move clean path from in-cone to out-of-cone overwrite' '
+test_expect_success 'move clean path from in-cone to out-of-cone overwrite' '
 	test_when_finished "cleanup_sparse_checkout" &&
 	setup_sparse_checkout &&
 	echo "sub/file1 overwrite" >sub/file1 &&