git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Out-of-repository file remove error
@ 2008-07-19  8:23 Nick Andrew
  2008-07-19 16:21 ` [PATCH] builtin-rm: fix index lock file path Olivier Marin
  0 siblings, 1 reply; 3+ messages in thread
From: Nick Andrew @ 2008-07-19  8:23 UTC (permalink / raw)
  To: git

git rm seems to have a problem with removing a file from a repository
when the repository .git and working tree are not in the current
directory. It leaves an index.lock file.

Here's a script to show the bug:

mkdir Bugtest
cd Bugtest
git init
date > newfile
git add newfile
git commit -m 'Added' newfile
cd ..
git --git-dir=Bugtest/.git --work-tree=Bugtest rm newfile
ls -l Bugtest/.git/index.lock

Output:

Initialized empty Git repository in .../Bugtest/.git/
Created initial commit 43dec15: Added
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 newfile
rm 'newfile'
fatal: Unable to write new index file
-rw-r--r-- 1 nick nick 32 Jul 19 18:20 Bugtest/.git/index.lock

I tested on:

git version 1.5.6.2
git version 1.5.6.3.440.g9d8f

Nick.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] builtin-rm: fix index lock file path
  2008-07-19  8:23 Out-of-repository file remove error Nick Andrew
@ 2008-07-19 16:21 ` Olivier Marin
  2008-07-19 16:24   ` [PATCH V2] " Olivier Marin
  0 siblings, 1 reply; 3+ messages in thread
From: Olivier Marin @ 2008-07-19 16:21 UTC (permalink / raw)
  To: Nick Andrew; +Cc: Junio C Hamano, Git Mailing List

From: Olivier Marin <dkr@freesurf.fr>

When hold_locked_index() is called with a relative git_dir and you are
outside the work tree, the lock file become relative to the current
directory. So when later setup_work_tree() change the current directory
it breaks lock file path and commit_locked_index() fails.

This patch move index locking code after setup_work_tree() call to make
lock file relative to the working tree as it should be and add a test
case.

Noticed by Nick Andrew.

Signed-off-by: Olivier Marin <dkr@freesurf.fr>
---
 builtin-rm.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin-rm.c b/builtin-rm.c
index 56454ec..ee8247b 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -146,11 +146,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config, NULL);
 
-	newfd = hold_locked_index(&lock_file, 1);
-
-	if (read_cache() < 0)
-		die("index file corrupt");
-
 	argc = parse_options(argc, argv, builtin_rm_options, builtin_rm_usage, 0);
 	if (!argc)
 		usage_with_options(builtin_rm_usage, builtin_rm_options);
@@ -158,6 +153,11 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	if (!index_only)
 		setup_work_tree();
 
+	newfd = hold_locked_index(&lock_file, 1);
+
+	if (read_cache() < 0)
+		die("index file corrupt");
+
 	pathspec = get_pathspec(prefix, argv);
 	seen = NULL;
 	for (i = 0; pathspec[i] ; i++)
-- 
1.5.6.3.440.g489d7

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH V2] builtin-rm: fix index lock file path
  2008-07-19 16:21 ` [PATCH] builtin-rm: fix index lock file path Olivier Marin
@ 2008-07-19 16:24   ` Olivier Marin
  0 siblings, 0 replies; 3+ messages in thread
From: Olivier Marin @ 2008-07-19 16:24 UTC (permalink / raw)
  To: Olivier Marin; +Cc: Nick Andrew, Junio C Hamano, Git Mailing List

From: Olivier Marin <dkr@freesurf.fr>

When hold_locked_index() is called with a relative git_dir and you are
outside the work tree, the lock file become relative to the current
directory. So when later setup_work_tree() change the current directory
it breaks lock file path and commit_locked_index() fails.

This patch move index locking code after setup_work_tree() call to make
lock file relative to the working tree as it should be and add a test
case.

Noticed by Nick Andrew.

Signed-off-by: Olivier Marin <dkr@freesurf.fr>
---

 The same with the test case!

 builtin-rm.c  |   10 +++++-----
 t/t3600-rm.sh |   12 ++++++++++++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/builtin-rm.c b/builtin-rm.c
index 56454ec..ee8247b 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -146,11 +146,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config, NULL);
 
-	newfd = hold_locked_index(&lock_file, 1);
-
-	if (read_cache() < 0)
-		die("index file corrupt");
-
 	argc = parse_options(argc, argv, builtin_rm_options, builtin_rm_usage, 0);
 	if (!argc)
 		usage_with_options(builtin_rm_usage, builtin_rm_options);
@@ -158,6 +153,11 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	if (!index_only)
 		setup_work_tree();
 
+	newfd = hold_locked_index(&lock_file, 1);
+
+	if (read_cache() < 0)
+		die("index file corrupt");
+
 	pathspec = get_pathspec(prefix, argv);
 	seen = NULL;
 	for (i = 0; pathspec[i] ; i++)
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 316775e..79c06ad 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -217,4 +217,16 @@ test_expect_success 'Remove nonexistent file returns nonzero exit status' '
 	test_must_fail git rm nonexistent
 '
 
+test_expect_success 'Call "rm" from outside the work tree' '
+	mkdir repo &&
+	cd repo &&
+	git init &&
+	echo something > somefile &&
+	git add somefile &&
+	git commit -m "add a file" &&
+	(cd .. &&
+	 git --git-dir=repo/.git --work-tree=repo rm somefile) &&
+	test_must_fail git ls-files --error-unmatch somefile
+'
+
 test_done
-- 
1.5.6.3.440.g489d7

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2008-07-19 16:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-19  8:23 Out-of-repository file remove error Nick Andrew
2008-07-19 16:21 ` [PATCH] builtin-rm: fix index lock file path Olivier Marin
2008-07-19 16:24   ` [PATCH V2] " Olivier Marin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).