Merge branch 'sy/mv-out-of-cone'

"git mv A B" in a sparsely populated working tree can be asked to
move a path from a directory that is "in cone" to another directory
that is "out of cone".  Handling of such a case has been improved.

* sy/mv-out-of-cone:
  builtin/mv.c: fix possible segfault in add_slash()
  mv: check overwrite for in-to-out move
  advice.h: add advise_on_moving_dirty_path()
  mv: cleanup empty WORKING_DIRECTORY
  mv: from in-cone to out-of-cone
  mv: remove BOTH from enum update_mode
  mv: check if <destination> is a SKIP_WORKTREE_DIR
  mv: free the with_slash in check_dir_in_index()
  mv: rename check_dir_in_index() to empty_dir_has_sparse_contents()
  t7002: add tests for moving from in-cone to out-of-cone

1 files changed, 134 insertions, 27 deletions

diff --git a/builtin/mv.c b/builtin/mv.c
index 4729bb1a1a..3413ad1c9b 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -21,7 +21,6 @@ static const char * const builtin_mv_usage[] = {
 };
 
 enum update_mode {
-	BOTH = 0,
 	WORKING_DIRECTORY = (1 << 1),
 	INDEX = (1 << 2),
 	SPARSE = (1 << 3),
@@ -72,7 +71,7 @@ static const char **internal_prefix_pathspec(const char *prefix,
 static const char *add_slash(const char *path)
 {
 	size_t len = strlen(path);
-	if (path[len - 1] != '/') {
+	if (len && path[len - 1] != '/') {
 		char *with_slash = xmalloc(st_add(len, 2));
 		memcpy(with_slash, path, len);
 		with_slash[len++] = '/';
@@ -125,16 +124,15 @@ static int index_range_of_same_dir(const char *src, int length,
 }
 
 /*
- * 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.
+ * Given the path of a directory that does not exist on-disk, check whether the
+ * directory contains any entries in the index with the SKIP_WORKTREE flag
+ * enabled.
+ * Return 1 if such index entries exist.
+ * Return 0 otherwise.
  */
-static int check_dir_in_index(const char *name)
+static int empty_dir_has_sparse_contents(const char *name)
 {
+	int ret = 0;
 	const char *with_slash = add_slash(name);
 	int length = strlen(with_slash);
 
@@ -144,14 +142,18 @@ static int check_dir_in_index(const char *name)
 	if (pos < 0) {
 		pos = -pos - 1;
 		if (pos >= the_index.cache_nr)
-			return 1;
+			goto free_return;
 		ce = active_cache[pos];
 		if (strncmp(with_slash, ce->name, length))
-			return 1;
+			goto free_return;
 		if (ce_skip_worktree(ce))
-			return 0;
+			ret = 1;
 	}
-	return 1;
+
+free_return:
+	if (with_slash != name)
+		free((char *)with_slash);
+	return ret;
 }
 
 int cmd_mv(int argc, const char **argv, const char *prefix)
@@ -168,12 +170,17 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 	const char **source, **destination, **dest_path, **submodule_gitfile;
-	enum update_mode *modes;
+	const char *dst_w_slash;
+	const char **src_dir = NULL;
+	int src_dir_nr = 0, src_dir_alloc = 0;
+	struct strbuf a_src_dir = STRBUF_INIT;
+	enum update_mode *modes, dst_mode = 0;
 	struct stat st;
 	struct string_list src_for_dst = STRING_LIST_INIT_NODUP;
 	struct lock_file lock_file = LOCK_INIT;
 	struct cache_entry *ce;
 	struct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;
+	struct string_list dirty_paths = STRING_LIST_INIT_NODUP;
 
 	git_config(git_default_config, NULL);
 
@@ -198,6 +205,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	if (argc == 1 && is_directory(argv[0]) && !is_directory(argv[1]))
 		flags = 0;
 	dest_path = internal_prefix_pathspec(prefix, argv + argc, 1, flags);
+	dst_w_slash = add_slash(dest_path[0]);
 	submodule_gitfile = xcalloc(argc, sizeof(char *));
 
 	if (dest_path[0][0] == '\0')
@@ -205,12 +213,31 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 		destination = internal_prefix_pathspec(dest_path[0], argv, argc, DUP_BASENAME);
 	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);
 	} else {
-		if (argc != 1)
+		if (!path_in_sparse_checkout(dst_w_slash, &the_index) &&
+		    empty_dir_has_sparse_contents(dst_w_slash)) {
+			destination = internal_prefix_pathspec(dst_w_slash, argv, argc, DUP_BASENAME);
+			dst_mode = SKIP_WORKTREE_DIR;
+		} else if (argc != 1) {
 			die(_("destination '%s' is not a directory"), dest_path[0]);
-		destination = dest_path;
+		} else {
+			destination = dest_path;
+			/*
+			 * <destination> is a file outside of sparse-checkout
+			 * cone. Insist on cone mode here for backward
+			 * compatibility. We don't want dst_mode to be assigned
+			 * for a file when the repo is using no-cone mode (which
+			 * is deprecated at this point) sparse-checkout. As
+			 * SPARSE here is only considering cone-mode situation.
+			 */
+			if (!path_in_cone_mode_sparse_checkout(destination[0], &the_index))
+				dst_mode = SPARSE;
+		}
+	}
+	if (dst_w_slash != dest_path[0]) {
+		free((char *)dst_w_slash);
+		dst_w_slash = NULL;
 	}
 
 	/* Checking */
@@ -232,7 +259,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 			if (pos < 0) {
 				const char *src_w_slash = add_slash(src);
 				if (!path_in_sparse_checkout(src_w_slash, &the_index) &&
-				    !check_dir_in_index(src)) {
+				    empty_dir_has_sparse_contents(src)) {
 					modes[i] |= SKIP_WORKTREE_DIR;
 					goto dir_check;
 				}
@@ -290,6 +317,10 @@ dir_check:
 
 			/* last - first >= 1 */
 			modes[i] |= WORKING_DIRECTORY;
+
+			ALLOC_GROW(src_dir, src_dir_nr + 1, src_dir_alloc);
+			src_dir[src_dir_nr++] = src;
+
 			n = argc + last - first;
 			REALLOC_ARRAY(source, n);
 			REALLOC_ARRAY(destination, n);
@@ -346,6 +377,18 @@ dir_check:
 			goto act_on_entry;
 		}
 
+		if (ignore_sparse &&
+		    (dst_mode & (SKIP_WORKTREE_DIR | SPARSE)) &&
+		    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
@@ -396,6 +439,7 @@ remove_entry:
 		const char *src = source[i], *dst = destination[i];
 		enum update_mode mode = modes[i];
 		int pos;
+		int sparse_and_dirty = 0;
 		struct checkout state = CHECKOUT_INIT;
 		state.istate = &the_index;
 
@@ -406,6 +450,7 @@ remove_entry:
 		if (show_only)
 			continue;
 		if (!(mode & (INDEX | SPARSE | SKIP_WORKTREE_DIR)) &&
+		    !(dst_mode & (SKIP_WORKTREE_DIR | SPARSE)) &&
 		    rename(src, dst) < 0) {
 			if (ignore_errors)
 				continue;
@@ -425,20 +470,81 @@ remove_entry:
 
 		pos = cache_name_pos(src, strlen(src));
 		assert(pos >= 0);
+		if (!(mode & SPARSE) && !lstat(src, &st))
+			sparse_and_dirty = ce_modified(active_cache[pos], &st, 0);
 		rename_cache_entry_at(pos, dst);
 
-		if ((mode & SPARSE) &&
-		    (path_in_sparse_checkout(dst, &the_index))) {
-			int dst_pos;
+		if (ignore_sparse &&
+		    core_apply_sparse_checkout &&
+		    core_sparse_checkout_cone) {
+			/*
+			 * NEEDSWORK: we are *not* paying attention to
+			 * "out-to-out" move (<source> is out-of-cone and
+			 * <destination> is out-of-cone) at this point. It
+			 * should be added in a future patch.
+			 */
+			if ((mode & SPARSE) &&
+			    path_in_sparse_checkout(dst, &the_index)) {
+				/* from out-of-cone to in-cone */
+				int dst_pos = cache_name_pos(dst, strlen(dst));
+				struct cache_entry *dst_ce = active_cache[dst_pos];
+
+				dst_ce->ce_flags &= ~CE_SKIP_WORKTREE;
+
+				if (checkout_entry(dst_ce, &state, NULL, NULL))
+					die(_("cannot checkout %s"), dst_ce->name);
+			} else if ((dst_mode & (SKIP_WORKTREE_DIR | SPARSE)) &&
+				   !(mode & SPARSE) &&
+				   !path_in_sparse_checkout(dst, &the_index)) {
+				/* from in-cone to out-of-cone */
+				int dst_pos = cache_name_pos(dst, strlen(dst));
+				struct cache_entry *dst_ce = active_cache[dst_pos];
 
-			dst_pos = cache_name_pos(dst, strlen(dst));
-			active_cache[dst_pos]->ce_flags &= ~CE_SKIP_WORKTREE;
+				/*
+				 * if src is clean, it will suffice to remove it
+				 */
+				if (!sparse_and_dirty) {
+					dst_ce->ce_flags |= CE_SKIP_WORKTREE;
+					unlink_or_warn(src);
+				} else {
+					/*
+					 * if src is dirty, move it to the
+					 * destination and create leading
+					 * dirs if necessary
+					 */
+					char *dst_dup = xstrdup(dst);
+					string_list_append(&dirty_paths, dst);
+					safe_create_leading_directories(dst_dup);
+					FREE_AND_NULL(dst_dup);
+					rename(src, dst);
+				}
+			}
+		}
+	}
 
-			if (checkout_entry(active_cache[dst_pos], &state, NULL, NULL))
-				die(_("cannot checkout %s"), active_cache[dst_pos]->name);
+	/*
+	 * cleanup the empty src_dirs
+	 */
+	for (i = 0; i < src_dir_nr; i++) {
+		int dummy;
+		strbuf_addstr(&a_src_dir, src_dir[i]);
+		/*
+		 * if entries under a_src_dir are all moved away,
+		 * recursively remove a_src_dir to cleanup
+		 */
+		if (index_range_of_same_dir(a_src_dir.buf, a_src_dir.len,
+					    &dummy, &dummy) < 1) {
+			remove_dir_recursively(&a_src_dir, 0);
 		}
+		strbuf_reset(&a_src_dir);
 	}
 
+	strbuf_release(&a_src_dir);
+	free(src_dir);
+
+	if (dirty_paths.nr)
+		advise_on_moving_dirty_path(&dirty_paths);
+
 	if (gitmodules_modified)
 		stage_updated_gitmodules(&the_index);
 
@@ -447,6 +553,7 @@ remove_entry:
 		die(_("Unable to write new index file"));
 
 	string_list_clear(&src_for_dst, 0);
+	string_list_clear(&dirty_paths, 0);
 	UNLEAK(source);
 	UNLEAK(dest_path);
 	free(submodule_gitfile);