Re: [PATCH] git-mv: succeed even if source is a prefix of destination
From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:42:37
Subsystem:
the rest · Maintainer:
Linus Torvalds
Hi, On Wed, 16 Aug 2006, Fredrik Kuivinen wrote:
On Wed, Aug 16, 2006 at 02:20:32AM +0200, Johannes Schindelin wrote:quoted
As noted by Fredrik Kuivinen, without this patch, git-mv fails on git-mv README README-renamed because "README" is a prefix of "README-renamed".Thank you. 'git-mv README README-renamed' works for me too now. However, there still seems to be some minor problem with git-mv. $ git mv t t fatal: renaming t failed: Invalid argument $ git mv t t/ fatal: renaming t failed: Invalid argument $ git mv t/ t/ fatal: cannot move directory over file, source=t/, destination=t/ $ git mv t/ t fatal: cannot move directory over file, source=t/, destination=t/ I kind of expected to get 'can not move directory into itself' in all of those cases. At least the same error messages should be given in all cases. It looks like we need some kind of path normalization before we do those tests.
I kind of hoped it was not necessary to do this, since get_pathspec() does a rudimentary version of it (BTW git-mv.perl got it wrong: it substituted "./" by "", which would fail for a directory name like "endsWithADot."). It was a little more involved: -- 8< -- [PATCH] git-mv: add more path normalization We already use the normalization from get_pathspec(), but now we also remove a trailing slash. So, git mv some_path/ into_some_path/ works now. Also, move the "can not move directory into itself" test before the subdirectory expansion. Signed-off-by: Johannes Schindelin <redacted> --- builtin-mv.c | 25 ++++++++++++++++--------- 1 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/builtin-mv.c b/builtin-mv.c
index e7b5eb7..c0c8764 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c@@ -17,12 +17,19 @@ static const char builtin_mv_usage[] = static const char **copy_pathspec(const char *prefix, const char **pathspec, int count, int base_name) { + int i; const char **result = xmalloc((count + 1) * sizeof(const char *)); memcpy(result, pathspec, count * sizeof(const char *)); result[count] = NULL; - if (base_name) { - int i; - for (i = 0; i < count; i++) { + for (i = 0; i < count; i++) { + int length = strlen(result[i]); + if (length > 0 && result[i][length - 1] == '/') { + char *without_slash = xmalloc(length); + memcpy(without_slash, result[i], length - 1); + without_slash[length] = '\0'; + result[i] = without_slash; + } + if (base_name) { const char *last_slash = strrchr(result[i], '/'); if (last_slash) result[i] = last_slash + 1;
@@ -129,6 +136,12 @@ int cmd_mv(int argc, const char **argv, if (lstat(source[i], &st) < 0) bad = "bad source"; + if (!bad && + (length = strlen(source[i])) >= 0 && + !strncmp(destination[i], source[i], length) && + (destination[i][length] == 0 || destination[i][length] == '/')) + bad = "can not move directory into itself"; + if (S_ISDIR(st.st_mode)) { const char *dir = source[i], *dest_dir = destination[i]; int first, last, len = strlen(dir);
@@ -204,12 +217,6 @@ int cmd_mv(int argc, const char **argv, } } - if (!bad && - (length = strlen(source[i])) >= 0 && - !strncmp(destination[i], source[i], length) && - (destination[i][length] == 0 || destination[i][length] == '/')) - bad = "can not move directory into itself"; - if (!bad && cache_name_pos(source[i], strlen(source[i])) < 0) bad = "not under version control";
--
1.4.2.gf71ee