Re: [PATCH] git-mv: succeed even if source is a prefix of destination

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, 16 Aug 2006, Fredrik Kuivinen wrote:

> On Wed, Aug 16, 2006 at 02:20:32AM +0200, Johannes Schindelin wrote:
> > 
> > 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 <Johannes.Schindelin@xxxxxx>
---
 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

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]