threads / patch / 43658

patchRe: [PATCH] git-mv: search more precisely for source directory in index

Subject: Re: [PATCH] git-mv: search more precisely for source directory in index

## tl;dr

3 messages between Dec 3, 2006 and Dec 3, 2006. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Sergey Vlasov· Dec 3, 2006, 13:57 UTC · lore

[BUG] git-mv fails with "source directory is empty" when it should not

Hello!

In some cases git-mv fails with "source directory is empty" when this is definitely not the case.

Example (run in an empty directory):

$ git init-db defaulting to local storage area $ mkdir ab; touch ab.c; touch ab/d $ git add . $ git commit -m 'initial' Committing initial tree f2dd231ec22a1d4376d4a6eadf49d87dd34ac2a0 $ git mv ab a fatal: source directory is empty, source=ab, destination=a

However, at this point the source directory is definitely not empty:

$ git ls-tree -r HEAD 100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 ab.c 100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 ab/d $ ls -ogR .: total 4 drwxr-xr-x 2 4096 Dec 3 16:47 ab -rw-r--r-- 1 0 Dec 3 16:47 ab.c

./ab: total 0 -rw-r--r-- 1 0 Dec 3 16:47 d $ git status nothing to commit

This happens both in v1.4.4.1 and in current 'master' (v1.4.4.1-g278fcd7).

-- 
Sergey Vlasov

Johannes Schindelin· Dec 3, 2006, 19:42 UTC · re: Sergey Vlasov · lore

[PATCH] git-mv: search more precisely for source directory in index

A move of a directory should find the entries in the index by searching for the name _including_ the slash. Otherwise, the directory can be shadowed by a file when it matches the prefix and is lexicographically smaller, e.g. "ab.c" shadows "ab/".

Noticed by Sergey Vlasov.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

	Good catch. Thanks!

 builtin-mv.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

Show changes to builtin-mv.c +7 −4

diff --git a/builtin-mv.c b/builtin-mv.c
index 54dd3bf..d14a4a7 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -146,21 +146,24 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 				&& lstat(dst, &st) == 0)
 			bad = "cannot move directory over file";
 		else if (src_is_dir) {
+			const char *src_w_slash = add_slash(src);
+			int len_w_slash = length + 1;
 			int first, last;
 
 			modes[i] = WORKING_DIRECTORY;
 
-			first = cache_name_pos(src, length);
+			first = cache_name_pos(src_w_slash, len_w_slash);
 			if (first >= 0)
-				die ("Huh? %s/ is in index?", src);
+				die ("Huh? %.*s is in index?",
+						len_w_slash, src_w_slash);
 
 			first = -1 - first;
 			for (last = first; last < active_nr; last++) {
 				const char *path = active_cache[last]->name;
-				if (strncmp(path, src, length)
-						|| path[length] != '/')
+				if (strncmp(path, src_w_slash, len_w_slash))
 					break;
 			}
+			free((char *)src_w_slash);
 
 			if (last - first < 1)
 				bad = "source directory is empty";

-- 
1.4.4.1.g317bd

Sergey Vlasov· Dec 3, 2006, 20:04 UTC · re: Johannes Schindelin · lore

On Sun, Dec 03, 2006 at 08:42:47PM +0100, Johannes Schindelin wrote:

Show 5 quoted lines

> 
> A move of a directory should find the entries in the index by
> searching for the name _including_ the slash. Otherwise, the
> directory can be shadowed by a file when it matches the prefix
> and is lexicographically smaller, e.g. "ab.c" shadows "ab/".

Thanks - seems to work now, and the existing tests are not broken.

Show 43 quoted lines

> Noticed by Sergey Vlasov.
> 
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> ---
> 
> 	Good catch. Thanks!
> 
>  builtin-mv.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/builtin-mv.c b/builtin-mv.c
> index 54dd3bf..d14a4a7 100644
> --- a/builtin-mv.c
> +++ b/builtin-mv.c
> @@ -146,21 +146,24 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>  				&& lstat(dst, &st) == 0)
>  			bad = "cannot move directory over file";
>  		else if (src_is_dir) {
> +			const char *src_w_slash = add_slash(src);
> +			int len_w_slash = length + 1;
>  			int first, last;
>  
>  			modes[i] = WORKING_DIRECTORY;
>  
> -			first = cache_name_pos(src, length);
> +			first = cache_name_pos(src_w_slash, len_w_slash);
>  			if (first >= 0)
> -				die ("Huh? %s/ is in index?", src);
> +				die ("Huh? %.*s is in index?",
> +						len_w_slash, src_w_slash);
>  
>  			first = -1 - first;
>  			for (last = first; last < active_nr; last++) {
>  				const char *path = active_cache[last]->name;
> -				if (strncmp(path, src, length)
> -						|| path[length] != '/')
> +				if (strncmp(path, src_w_slash, len_w_slash))
>  					break;
>  			}
> +			free((char *)src_w_slash);
>  
>  			if (last - first < 1)
>  				bad = "source directory is empty";