Re: [PATCH] git-mv: succeed even if source is a prefix of destination
Hi,
On Wed, 16 Aug 2006, Fredrik Kuivinen wrote:
Show 28 quoted lines
> 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@gmx.de>
---
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
Fredrik Kuivinen