Re: [PATCH] t/t7001-mv.sh: Propose ability to use git-mv on conflicting entries
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:03
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH] t/t7001-mv.sh: Propose ability to use git-mv on conflicting entries · Petr Baudis <hidden>
- 2016-06-15 · Re: [PATCH] t/t7001-mv.sh: Propose ability to use git-mv on conflicting entries · Junio C Hamano <hidden>
Petr Baudis [off-list ref] writes:
I don't really know if it is ok to make "feature requests" like this by adding failing testcases...
Of course it depends on who you are. It's not Ok for somebody like you, who is known to be competent, and certainly it is not Ok to do it on a command that you know you care more about than I do. I've neglected builtin-mv.c since its introduction, mostly because I never say "git mv" myself (in other words, I haven't cared very much how broken it was. For one thing, its indentation style is peculiar and is hard to read and maintain).
+# Rationale: I cannot git mv around a conflicted file. This is unnecessary +# restriction in case another part of conflict resolution requires me to +# move the file around.
Yes, I would agree this is a reasonable thing to support. Something like this patch, perhaps. --- builtin-mv.c | 21 ++++++++++++++------- t/t7001-mv.sh | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/builtin-mv.c b/builtin-mv.c
index 4f65b5a..cc9e505 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c@@ -96,7 +96,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) /* Checking */ for (i = 0; i < argc; i++) { const char *src = source[i], *dst = destination[i]; - int length, src_is_dir; + int length, src_is_dir, pos; const char *bad = NULL; if (show_only)
@@ -177,7 +177,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix) } else bad = "Cannot overwrite"; } - } else if (cache_name_pos(src, length) < 0) + } else if (((pos = cache_name_pos(src, length)) < 0) && + strcmp(active_cache[-1 - pos]->name, src)) bad = "not under version control"; else if (string_list_has_string(&src_for_dst, dst)) bad = "multiple sources for the same target";
@@ -202,7 +203,6 @@ int cmd_mv(int argc, const char **argv, const char *prefix) for (i = 0; i < argc; i++) { const char *src = source[i], *dst = destination[i]; enum update_mode mode = modes[i]; - int pos; if (show_only || verbose) printf("Renaming %s to %s\n", src, dst); if (!show_only && mode != INDEX &&
@@ -212,10 +212,17 @@ int cmd_mv(int argc, const char **argv, const char *prefix) if (mode == WORKING_DIRECTORY) continue; - pos = cache_name_pos(src, strlen(src)); - assert(pos >= 0); - if (!show_only) - rename_cache_entry_at(pos, dst); + if (!show_only) { + while (1) { + int pos = cache_name_pos(src, strlen(src)); + if (pos < 0) + pos = -1 - pos; + if ((active_nr <= pos) || + strcmp(active_cache[pos]->name, src)) + break; + rename_cache_entry_at(pos, dst); + } + } } if (active_cache_changed) {
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index b0fa407..d538f88 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh@@ -173,6 +173,27 @@ test_expect_success 'git mv should not change sha1 of moved cache entry' ' rm -f dirty dirty2 +cat >expect <<\EOT +100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 1 staged +100755 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 2 staged +100755 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 3 staged +EOT + +test_expect_success 'git mv should move all stages of cache entry' ' + rm -fr .git && + git init && + >staged && + git update-index --index-info <expect && + git ls-files --stage >actual && + test_cmp expect actual && + git mv staged staged-mv && + sed "s/staged/staged-mv/" <expect >expect-2 && + git ls-files --stage >actual && + test_cmp expect-2 actual +' + +rm -f expect expect-2 staged actual staged-mv + test_expect_success 'git mv should overwrite symlink to a file' ' rm -fr .git &&