threads / discuss / 17548

Recovering the index after a git crash?

Subject: Recovering the index after a git crash?

## tl;dr

9 messages between Feb 4, 2009 and Feb 19, 2009.

replies: 8people: 4as markdown or json

Matthieu Moy· Feb 4, 2009, 08:36 UTC · lore
Hi,

I'm debugging a git assertion failure, which leaves a .git/index.lock in the way. After running the failed command, git refuses to do anything in this directory, complaining with:

  fatal: unable to create '.git/index.lock': File exists

What I'm doing then is just "rm .git/index.lock". Is it safe to do it? I guess I should just make sure there's no git process running, but is there anything else to check?

I'll write a FAQ entry on the wiki with answers, and that would probably be a good idea to give indication to the user directly in the error message too, otherwise, the problem can be blocking for beginners.

Thanks,

<my life> I had to support students using SVN for a month in January, and even when SVN says:

  svn: run 'svn cleanup' to remove locks (type 'svn help cleanup' for details)

it's still hard for many students to understand that they have to run "svn cleanup" and prefer sending me a mail saying "it doesn't work anymore" :-(. </my life>

-- 
Matthieu
Matthieu Moy· Feb 4, 2009, 09:58 UTC · re: Matthieu Moy · lore

[PATCH/RFC] More friendly message when locking the index fails.

Just saying that index.lock exists doesn't tell the user _what_ to do to fix the problem. We should give an indication that it's normally safe to delete index.lock after making sure git isn't running here. ---

> I'll write a FAQ entry on the wiki with answers, and that would
> probably be a good idea to give indication to the user directly in the
> error message too, otherwise, the problem can be blocking for
> beginners.
Something along the lines of this patch maybe?
 builtin-update-index.c |   11 +++++++++--
 lockfile.c             |    9 ++++++++-
 2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/builtin-update-index.c b/builtin-update-index.c
index 5604977..ab8ab8f 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -742,8 +742,15 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		if (newfd < 0) {
 			if (refresh_flags & REFRESH_QUIET)
 				exit(128);
-			die("unable to create '%s.lock': %s",
-			    get_index_file(), strerror(lock_error));
+			if (lock_error == EEXIST) {
+				die("Unable to create '%s.lock': %s\n"
+				    "This probably means a git process crashed in this repository earlier.\n"
+				    "Make sure no other git process is running and remove the file manually.",
+				    get_index_file(), strerror(lock_error));
+			} else {
+				die("Unable to create '%s.lock': %s",
+				    get_index_file(), strerror(lock_error));
+			}
 		}
 		if (write_cache(newfd, active_cache, active_nr) ||
 		    commit_locked_index(lock_file))
diff --git a/lockfile.c b/lockfile.c
index 021c337..bb59f7e 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -159,7 +159,14 @@ int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
 {
 	int fd = lock_file(lk, path, flags);
 	if (fd < 0 && (flags & LOCK_DIE_ON_ERROR))
-		die("unable to create '%s.lock': %s", path, strerror(errno));
+		if (errno == EEXIST) {
+			die("Unable to create '%s.lock': %s\n"
+			    "This probably means a git process crashed in this repository earlier.\n"
+			    "Make sure no other git process is running and remove the file manually.",
+			    path, strerror(errno));
+		} else {
+			die("Unable to create '%s.lock': %s", path, strerror(errno));
+		}
 	return fd;
 }
 
-- 
1.6.1.2.322.g01114.dirty
Sverre Rabbelier· Feb 4, 2009, 11:50 UTC · re: Matthieu Moy · lore

Re: [PATCH/RFC] More friendly message when locking the index fails.

On Wed, Feb 4, 2009 at 10:58, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> +                                   "This probably means a git process crashed in this repository earlier.\n"
> +                                   "Make sure no other git process is running and remove the file manually.",
> +                                   get_index_file(), strerror(lock_error));

How about "If no other git process is currently running, this probably means...." instead? E.g., sometimes I run 'git commit' in one terminal window, and then switch to another before committing (to review the diff for example), which would cause an index.lock file to be present validly.

-- 
Cheers,

Sverre Rabbelier
Mikael Magnusson· Feb 6, 2009, 13:38 UTC · re: Matthieu Moy · lore

Re: Recovering the index after a git crash?

2009/2/4 Matthieu Moy <Matthieu.Moy@imag.fr>:
Show 11 quoted lines
> Hi,
>
> I'm debugging a git assertion failure, which leaves a .git/index.lock
> in the way. After running the failed command, git refuses to do
> anything in this directory, complaining with:
>
>  fatal: unable to create '.git/index.lock': File exists
>
> What I'm doing then is just "rm .git/index.lock". Is it safe to do it?
> I guess I should just make sure there's no git process running, but is
> there anything else to check?

It should be safe, yes. Often it is even safe to remove .git/index itself, since you can restore it from the latest commit with 'git reset'.

-- 
Mikael Magnusson
Matthieu Moy· Feb 6, 2009, 14:36 UTC · re: Matthieu Moy · lore

[PATCH] More friendly message when locking the index fails.

Just saying that index.lock exists doesn't tell the user _what_ to do to fix the problem. We should give an indication that it's normally safe to delete index.lock after making sure git isn't running here.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Sverre Rabbelier <srabbelier@gmail.com> writes:
Show 5 quoted lines
> How about "If no other git process is currently running, this probably
> means...." instead? E.g., sometimes I run 'git commit' in one terminal
> window, and then switch to another before committing (to review the
> diff for example), which would cause an index.lock file to be present
> validly.
Right, here's an updated version.
 builtin-update-index.c |   12 ++++++++++--
 lockfile.c             |   10 +++++++++-
 2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/builtin-update-index.c b/builtin-update-index.c
index 5604977..23b97db 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -742,8 +742,16 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		if (newfd < 0) {
 			if (refresh_flags & REFRESH_QUIET)
 				exit(128);
-			die("unable to create '%s.lock': %s",
-			    get_index_file(), strerror(lock_error));
+			if (lock_error == EEXIST) {
+				die("Unable to create '%s.lock': %s.\n\n"
+				    "If no other git process is currently running, this probably means a\n"
+				    "git process crashed in this repository earlier. Make sure no other git\n"
+				    "process is running and remove the file manually to continue.",
+				    get_index_file(), strerror(lock_error));
+			} else {
+				die("Unable to create '%s.lock': %s",
+				    get_index_file(), strerror(lock_error));
+			}
 		}
 		if (write_cache(newfd, active_cache, active_nr) ||
 		    commit_locked_index(lock_file))
diff --git a/lockfile.c b/lockfile.c
index 021c337..9bde859 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -159,7 +159,15 @@ int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
 {
 	int fd = lock_file(lk, path, flags);
 	if (fd < 0 && (flags & LOCK_DIE_ON_ERROR))
-		die("unable to create '%s.lock': %s", path, strerror(errno));
+		if (errno == EEXIST) {
+			die("Unable to create '%s.lock': %s.\n\n"
+			    "If no other git process is currently running, this probably means a\n"
+			    "git process crashed in this repository earlier. Make sure no other git\n"
+			    "process is running and remove the file manually to continue.",
+			    path, strerror(errno));
+		} else {
+			die("Unable to create '%s.lock': %s", path, strerror(errno));
+		}
 	return fd;
 }
 
-- 
1.6.1.2.351.g032a4.dirty
Matthieu Moy· Feb 6, 2009, 15:09 UTC · re: Matthieu Moy · lore

[PATCH v3] More friendly message when locking the index fails.

Just saying that index.lock exists doesn't tell the user _what_ to do to fix the problem. We should give an indication that it's normally safe to delete index.lock after making sure git isn't running here.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Oops, I hadn't noticed, but the previous version triggered a warning
because of lack of braces on the if. Sorry.
 builtin-update-index.c |   12 ++++++++++--
 lockfile.c             |   13 +++++++++++--
 2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/builtin-update-index.c b/builtin-update-index.c
index 5604977..23b97db 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -742,8 +742,16 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		if (newfd < 0) {
 			if (refresh_flags & REFRESH_QUIET)
 				exit(128);
-			die("unable to create '%s.lock': %s",
-			    get_index_file(), strerror(lock_error));
+			if (lock_error == EEXIST) {
+				die("Unable to create '%s.lock': %s.\n\n"
+				    "If no other git process is currently running, this probably means a\n"
+				    "git process crashed in this repository earlier. Make sure no other git\n"
+				    "process is running and remove the file manually to continue.",
+				    get_index_file(), strerror(lock_error));
+			} else {
+				die("Unable to create '%s.lock': %s",
+				    get_index_file(), strerror(lock_error));
+			}
 		}
 		if (write_cache(newfd, active_cache, active_nr) ||
 		    commit_locked_index(lock_file))
diff --git a/lockfile.c b/lockfile.c
index 021c337..c812596 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -158,8 +158,17 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
 int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
 {
 	int fd = lock_file(lk, path, flags);
-	if (fd < 0 && (flags & LOCK_DIE_ON_ERROR))
-		die("unable to create '%s.lock': %s", path, strerror(errno));
+	if (fd < 0 && (flags & LOCK_DIE_ON_ERROR)) {
+		if (errno == EEXIST) {
+			die("Unable to create '%s.lock': %s.\n\n"
+			    "If no other git process is currently running, this probably means a\n"
+			    "git process crashed in this repository earlier. Make sure no other git\n"
+			    "process is running and remove the file manually to continue.",
+			    path, strerror(errno));
+		} else {
+			die("Unable to create '%s.lock': %s", path, strerror(errno));
+		}
+	}
 	return fd;
 }
 
-- 
1.6.1.2.351.g80eb2.dirty
Junio C Hamano· Feb 7, 2009, 00:37 UTC · re: Matthieu Moy · lore

Re: [PATCH v3] More friendly message when locking the index fails.

Matthieu Moy <Matthieu.Moy@imag.fr> writes:
Show 8 quoted lines
> Just saying that index.lock exists doesn't tell the user _what_ to do
> to fix the problem. We should give an indication that it's normally
> safe to delete index.lock after making sure git isn't running here.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> Oops, I hadn't noticed, but the previous version triggered a warning
> because of lack of braces on the if. Sorry.
Perhaps it is a good idea to introduce
NORETURN void unable_to_lock_index_die(const char *path, int err)
{
	if (err == EEXIST)
	        die(...);
 	else
        	die(...);
}
in lockfile.c and call it from these two places you are touching?
Show 58 quoted lines
>  builtin-update-index.c |   12 ++++++++++--
>  lockfile.c             |   13 +++++++++++--
>  2 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/builtin-update-index.c b/builtin-update-index.c
> index 5604977..23b97db 100644
> --- a/builtin-update-index.c
> +++ b/builtin-update-index.c
> @@ -742,8 +742,16 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  		if (newfd < 0) {
>  			if (refresh_flags & REFRESH_QUIET)
>  				exit(128);
> -			die("unable to create '%s.lock': %s",
> -			    get_index_file(), strerror(lock_error));
> +			if (lock_error == EEXIST) {
> +				die("Unable to create '%s.lock': %s.\n\n"
> +				    "If no other git process is currently running, this probably means a\n"
> +				    "git process crashed in this repository earlier. Make sure no other git\n"
> +				    "process is running and remove the file manually to continue.",
> +				    get_index_file(), strerror(lock_error));
> +			} else {
> +				die("Unable to create '%s.lock': %s",
> +				    get_index_file(), strerror(lock_error));
> +			}
>  		}
>  		if (write_cache(newfd, active_cache, active_nr) ||
>  		    commit_locked_index(lock_file))
> diff --git a/lockfile.c b/lockfile.c
> index 021c337..c812596 100644
> --- a/lockfile.c
> +++ b/lockfile.c
> @@ -158,8 +158,17 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
>  int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
>  {
>  	int fd = lock_file(lk, path, flags);
> -	if (fd < 0 && (flags & LOCK_DIE_ON_ERROR))
> -		die("unable to create '%s.lock': %s", path, strerror(errno));
> +	if (fd < 0 && (flags & LOCK_DIE_ON_ERROR)) {
> +		if (errno == EEXIST) {
> +			die("Unable to create '%s.lock': %s.\n\n"
> +			    "If no other git process is currently running, this probably means a\n"
> +			    "git process crashed in this repository earlier. Make sure no other git\n"
> +			    "process is running and remove the file manually to continue.",
> +			    path, strerror(errno));
> +		} else {
> +			die("Unable to create '%s.lock': %s", path, strerror(errno));
> +		}
> +	}
>  	return fd;
>  }
>  
> -- 
> 1.6.1.2.351.g80eb2.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthieu Moy· Feb 19, 2009, 12:54 UTC · re: Junio C Hamano · lore

[PATCH] More friendly message when locking the index fails.

Just saying that index.lock exists doesn't tell the user _what_ to do to fix the problem. We should give an indication that it's normally safe to delete index.lock after making sure git isn't running here.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Show 11 quoted lines
> Perhaps it is a good idea to introduce
> 
> NORETURN void unable_to_lock_index_die(const char *path, int err)
> {
> 	if (err == EEXIST)
> 	        die(...);
>  	else
>         	die(...);
> }
> 
> in lockfile.c and call it from these two places you are touching?
Here it is!
 builtin-update-index.c |    3 +--
 cache.h                |    1 +
 lockfile.c             |   16 +++++++++++++++-
 3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/builtin-update-index.c b/builtin-update-index.c
index 5604977..dd43d5b 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -742,8 +742,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		if (newfd < 0) {
 			if (refresh_flags & REFRESH_QUIET)
 				exit(128);
-			die("unable to create '%s.lock': %s",
-			    get_index_file(), strerror(lock_error));
+			unable_to_lock_index_die(get_index_file(), lock_error);
 		}
 		if (write_cache(newfd, active_cache, active_nr) ||
 		    commit_locked_index(lock_file))
diff --git a/cache.h b/cache.h
index 37dfb1c..bb5a190 100644
--- a/cache.h
+++ b/cache.h
@@ -484,6 +484,7 @@ struct lock_file {
 };
 #define LOCK_DIE_ON_ERROR 1
 #define LOCK_NODEREF 2
+extern NORETURN void unable_to_lock_index_die(const char *path, int err);
 extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
 extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
 extern int commit_lock_file(struct lock_file *);
diff --git a/lockfile.c b/lockfile.c
index 021c337..1db1a2f 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -155,11 +155,25 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
 	return lk->fd;
 }
 
+
+NORETURN void unable_to_lock_index_die(const char *path, int err)
+{
+	if (errno == EEXIST) {
+		die("Unable to create '%s.lock': %s.\n\n"
+		    "If no other git process is currently running, this probably means a\n"
+		    "git process crashed in this repository earlier. Make sure no other git\n"
+		    "process is running and remove the file manually to continue.",
+		    path, strerror(err));
+	} else {
+		die("Unable to create '%s.lock': %s", path, strerror(err));
+	}
+}
+
 int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
 {
 	int fd = lock_file(lk, path, flags);
 	if (fd < 0 && (flags & LOCK_DIE_ON_ERROR))
-		die("unable to create '%s.lock': %s", path, strerror(errno));
+		unable_to_lock_index_die(path, errno);
 	return fd;
 }
 
-- 
1.6.2.rc1.14.g7f87d
Sverre Rabbelier· Feb 19, 2009, 14:12 UTC · re: Matthieu Moy · lore

Re: [PATCH] More friendly message when locking the index fails.

Heya,
On Thu, Feb 19, 2009 at 13:54, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> Just saying that index.lock exists doesn't tell the user _what_ to do
> to fix the problem. We should give an indication that it's normally
> safe to delete index.lock after making sure git isn't running here.

Surely we need a 'git cleanup' which will attempt to clean up such mess, fail miserably most of the time and might even corrupt your repository further! Oh wait, wrong VCS...

-- 
Cheers,

Sverre Rabbelier

← back to recent threads