On 7/19/2022 9:28 AM, Shaoxuan Yuan wrote:
> Originally, <destination> is assumed to be in the working tree. If it is
> not found as a directory, then it is determined to be either a regular file
> path, or error out if used under the second form (move into a directory)
> of 'git-mv'. Such behavior is not ideal, mainly because Git does not
> look into the index for <destination>, which could potentially be a
> SKIP_WORKTREE_DIR, which we need to determine for the later "moving from
> in-cone to out-of-cone" patch.
>
> Change the logic so that Git first check if <destination> is a directory
> with all its contents sparsified (a SKIP_WORKTREE_DIR). If yes,
> then treat <destination> as a directory exists in the working tree, and
"treat <destination> as a directory exists in the working tree" is a bit
akward, and the rest of the sentence struggles with that. Starting with
"If yes," we could rewrite as follows:
If <destination> is such a sparse directory, then we should modify the
index the same way as we would if this were a non-sparse directory. We
must be careful to ensure that the <destination> is marked with
SKIP_WORKTREE_DIR.
(Note that I don't equate this as doing the same thing, just operating on
the index.)
> If no, continue the original checking logic.
I think this part doesn't need to be there, but I don't feel strongly
about it.
> Also add a `dst_w_slash` to reuse the result from `add_slash()`, which
> was everywhere and can be simplified.
> else if (!lstat(dest_path[0], &st) &&
> S_ISDIR(st.st_mode)) {
> - dest_path[0] = add_slash(dest_path[0]);
> - destination = internal_prefix_pathspec(dest_path[0], argv, argc, DUP_BASENAME);
> + destination = internal_prefix_pathspec(dst_w_slash, argv, argc, DUP_BASENAME);
Hm. I find it interesting how this works even if the directory is _not_
present in the index. Is there a test that checks this kind of setup?
git init &&
>file &&
git add file &&
git commit -m init &&
mkdir dir &&
git mv file dir/
Locally, this indeed works with the following 'git status' detail:
renamed: file -> dir/file
> } else {
> - if (argc != 1)
> + if (!path_in_sparse_checkout(dst_w_slash, &the_index) &&
> + !check_dir_in_index(dst_w_slash)) {
> + destination = internal_prefix_pathspec(dst_w_slash, argv, argc, DUP_BASENAME);
> + dst_mode |= SKIP_WORKTREE_DIR;
Looks like we are assigning dst_mode here, but not using it. I wonder if
you'll get a compiler error if you build this patch with DEVELOPER=1.
You can test all of the commits in your series using this command:
git rebase -x "make -j12 DEVELOPER=1" <base>
> + if (dst_w_slash != dest_path[0])
> + free((char *)dst_w_slash);
Good that you are freeing this here. You might also want to set it to NULL
just in case.
Thanks,
-Stolee
