diff mbox series

[v4,2/7] mv: update sparsity after moving from out-of-cone to in-cone

Message ID 20220623114120.12768-3-shaoxuan.yuan02@gmail.com (mailing list archive)
State Superseded
Headers show
Series mv: fix out-of-cone file/directory move logic | expand

Commit Message

Shaoxuan Yuan June 23, 2022, 11:41 a.m. UTC
Originally, "git mv" a sparse file from out-of-cone to
in-cone does not update the moved file's sparsity (remove its
SKIP_WORKTREE bit). And the corresponding cache entry is, unexpectedly,
not checked out in the working tree.

Update the behavior so that:
1. Moving from out-of-cone to in-cone removes the SKIP_WORKTREE bit from
   corresponding cache entry.
2. The moved cache entry is checked out in the working tree to reflect
   the updated sparsity.

Helped-by: Victoria Dye <vdye@github.com>
Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
---
 builtin/mv.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Derrick Stolee June 23, 2022, 3:08 p.m. UTC | #1
On 6/23/2022 7:41 AM, Shaoxuan Yuan wrote:
> Originally, "git mv" a sparse file from out-of-cone to
> in-cone does not update the moved file's sparsity (remove its
> SKIP_WORKTREE bit). And the corresponding cache entry is, unexpectedly,
> not checked out in the working tree.
> 
> Update the behavior so that:
> 1. Moving from out-of-cone to in-cone removes the SKIP_WORKTREE bit from
>    corresponding cache entry.
> 2. The moved cache entry is checked out in the working tree to reflect
>    the updated sparsity.

Since this is a behavior change, can we test it? It would be good
to verify that the new path exists in the worktree after 'git mv'
succeeds.
 
Thanks,
-Stolee
Shaoxuan Yuan June 24, 2022, 8:04 a.m. UTC | #2
On 6/23/2022 11:08 PM, Derrick Stolee wrote:
 > On 6/23/2022 7:41 AM, Shaoxuan Yuan wrote:
 >> Originally, "git mv" a sparse file from out-of-cone to
 >> in-cone does not update the moved file's sparsity (remove its
 >> SKIP_WORKTREE bit). And the corresponding cache entry is, unexpectedly,
 >> not checked out in the working tree.
 >>
 >> Update the behavior so that:
 >> 1. Moving from out-of-cone to in-cone removes the SKIP_WORKTREE bit from
 >>    corresponding cache entry.
 >> 2. The moved cache entry is checked out in the working tree to reflect
 >>    the updated sparsity.
 >
 > Since this is a behavior change, can we test it? It would be good
 > to verify that the new path exists in the worktree after 'git mv'
 > succeeds.

I don't think we can effectively test this based on the change per se.
This change is preparing a correct behavior for the next few
commits, so I'll say it's tested along with the next few commits
(i.e. move "sparse" file/directory ones)?

Thanks & Regards,
Shaoxuan
Derrick Stolee June 27, 2022, 1:55 p.m. UTC | #3
On 6/24/2022 4:04 AM, Shaoxuan Yuan wrote:
> 
> On 6/23/2022 11:08 PM, Derrick Stolee wrote:
>> On 6/23/2022 7:41 AM, Shaoxuan Yuan wrote:
>>> Originally, "git mv" a sparse file from out-of-cone to
>>> in-cone does not update the moved file's sparsity (remove its
>>> SKIP_WORKTREE bit). And the corresponding cache entry is, unexpectedly,
>>> not checked out in the working tree.
>>>
>>> Update the behavior so that:
>>> 1. Moving from out-of-cone to in-cone removes the SKIP_WORKTREE bit from
>>>    corresponding cache entry.
>>> 2. The moved cache entry is checked out in the working tree to reflect
>>>    the updated sparsity.
>>
>> Since this is a behavior change, can we test it? It would be good
>> to verify that the new path exists in the worktree after 'git mv'
>> succeeds.
> 
> I don't think we can effectively test this based on the change per se.
> This change is preparing a correct behavior for the next few
> commits, so I'll say it's tested along with the next few commits
> (i.e. move "sparse" file/directory ones)?

Ah, right. There are other reasons why moving from out-of-cone to
in-cone is blocked at this point in time.

Thanks,
-Stolee
diff mbox series

Patch

diff --git a/builtin/mv.c b/builtin/mv.c
index 83a465ba83..0c0c2b4914 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 "entry.h"
 
 static const char * const builtin_mv_usage[] = {
 	N_("git mv [<options>] <source>... <destination>"),
@@ -304,6 +305,11 @@  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;
+		struct checkout state = CHECKOUT_INIT;
+		state.istate = &the_index;
+
+		if (force)
+			state.force = 1;
 		if (show_only || verbose)
 			printf(_("Renaming %s to %s\n"), src, dst);
 		if (show_only)
@@ -328,6 +334,17 @@  int cmd_mv(int argc, const char **argv, const char *prefix)
 		pos = cache_name_pos(src, strlen(src));
 		assert(pos >= 0);
 		rename_cache_entry_at(pos, dst);
+
+		if ((mode & SPARSE) &&
+		    (path_in_sparse_checkout(dst, &the_index))) {
+			int dst_pos;
+
+			dst_pos = cache_name_pos(dst, strlen(dst));
+			active_cache[dst_pos]->ce_flags &= ~CE_SKIP_WORKTREE;
+
+			if (checkout_entry(active_cache[dst_pos], &state, NULL, NULL))
+				die(_("cannot checkout %s"), ce->name);
+		}
 	}
 
 	if (gitmodules_modified)