threads / bug / 15390

Bug? - Unexpected lock file error

Subject: Bug? - Unexpected lock file error

## tl;dr

7 messages between Sep 5, 2008 and Sep 7, 2008.

replies: 6people: 3as markdown or json

Eric Raible· Sep 5, 2008, 23:22 UTC · lore

In playing around with creating new unatached branches I came across a seemingly bogus "fatal: unable to create '.git/index.lock': File exists" error message.

Now I understand that the final merge below is a no-op at best but the resulting error message is less than intuitive.

This is in "git version 1.6.0.1.319.g9f32b.dirty" (msysgit/devel, but I doubt that makes a difference).

Comments?

# Make a couple of versions of 'a' in main repo mkdir foo cd foo git init -q touch a for i in a b c; do echo $i > a && git add a && git commit -q -m $i; done

# Make a new repo and push it into main repo (to make an unattached branch) mkdir empty cd empty git init -q git commit -q --allow-empty -m empty git push .. master:empty

# Trigger "fatal: unable to create '.git/index.lock': File exists" error cd .. git merge empty

If 'empty' has any commits then the merge happens as expected. Is this just another special case involving initial commits that I should just get over? Or is there something more substantive going on here?

- Eric
Junio C Hamano· Sep 5, 2008, 23:47 UTC · re: Eric Raible · lore

Re: Bug? - Unexpected lock file error

"Eric Raible" <raible@gmail.com> writes:
> If 'empty' has any commits then the merge happens as expected.

I think it is a bug in the git-merge rewritten in C, in "Already uptodate!" codepath.

Eric Raible· Sep 6, 2008, 00:05 UTC · re: Junio C Hamano · lore

Re: Bug? - Unexpected lock file error

On Fri, Sep 5, 2008 at 4:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 6 quoted lines
> "Eric Raible" <raible@gmail.com> writes:
>
>> If 'empty' has any commits then the merge happens as expected.
>
> I think it is a bug in the git-merge rewritten in C, in "Already
> uptodate!" codepath.

And the ""Already uptodate!" codepath isn't even triggered when we're up-to-date but 'empty' has at even one actual commit. In that case we get a friendly "Already up-to-date" and no fatal error.

Junio C Hamano· Sep 6, 2008, 00:09 UTC · re: Junio C Hamano · lore

Re: Bug? - Unexpected lock file error

Junio C Hamano <gitster@pobox.com> writes:
Show 6 quoted lines
> "Eric Raible" <raible@gmail.com> writes:
>
>> If 'empty' has any commits then the merge happens as expected.
>
> I think it is a bug in the git-merge rewritten in C, in "Already
> uptodate!" codepath.

Try reverting 18668f5 (builtin-merge: avoid run_command_v_opt() for recursive and subtree, 2008-08-28). That patch seems to be seriously broken.

Miklos Vajna· Sep 6, 2008, 02:12 UTC · re: Junio C Hamano · lore

Re: Bug? - Unexpected lock file error

On Fri, Sep 05, 2008 at 05:09:32PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> > I think it is a bug in the git-merge rewritten in C, in "Already
> > uptodate!" codepath.

I can't reproduce it with master, so I don't think this problem was introduced by the C rewrite.

> Try reverting 18668f5 (builtin-merge: avoid run_command_v_opt() for
> recursive and subtree, 2008-08-28).  That patch seems to be seriously
> broken.
Yes, that works around the problem. I'll try to send a fix later today.
Miklos Vajna· Sep 6, 2008, 16:29 UTC · re: Miklos Vajna · lore

[PATCH] builtin-merge: release the lockfile in try_merge_strategy()

Once we committed the locked index, we should release the lockfile. In most cases this is done automatically when the process ends, but this is not true in this case.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Sat, Sep 06, 2008 at 04:12:23AM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
Show 6 quoted lines
> > Try reverting 18668f5 (builtin-merge: avoid run_command_v_opt() for
> > recursive and subtree, 2008-08-28).  That patch seems to be
> > seriously broken.
>
> Yes, that works around the problem. I'll try to send a fix later
> today.
Here is - I think - the proper prefix.
 builtin-merge.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index b857cf6..bb09e6f 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -576,6 +576,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 				(write_cache(index_fd, active_cache, active_nr) ||
 				 commit_locked_index(lock)))
 			die ("unable to write %s", get_index_file());
+		rollback_lock_file(lock);
 		return clean ? 0 : 1;
 	} else {
 		args = xmalloc((4 + commit_list_count(common) +

-- 
1.6.0.1
Junio C Hamano· Sep 7, 2008, 05:44 UTC · re: Miklos Vajna · lore

Re: [PATCH] builtin-merge: release the lockfile in try_merge_strategy()

Miklos Vajna <vmiklos@frugalware.org> writes:
> Once we committed the locked index, we should release the lockfile. In
> most cases this is done automatically when the process ends, but this is
> not true in this case.
Thanks.

← back to recent threads