Bug? - Unexpected lock file error

Bug? - Unexpected lock file error

[Eric Raible]

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

Re: Bug? - Unexpected lock file error

[Junio C Hamano]

"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.

Re: Bug? - Unexpected lock file error

[Eric Raible]

On Fri, Sep 5, 2008 at 4:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "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.

Re: Bug? - Unexpected lock file error

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> "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.

Re: Bug? - Unexpected lock file error

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

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.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[Miklos Vajna]

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:
> > 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

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

[Junio C Hamano]

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.