git update-index not delete lock file when using different worktree

git update-index not delete lock file when using different worktree

[Yue Lin Ho]

This is a [issue from
TortoiseGit](https://code.google.com/p/tortoisegit/issues/detail?id=2233).
After doing some test, I report it here.
The following is the testing information I have tested.

### Folder Structure
```
Test
  |-- myrepo
  |     |-- bar.txt
  |     |-- foo.txt
  |
  |-- myrepo.git
        |-- .git
```
Testing repository is
[here](https://code.google.com/p/tortoisegit/issues/detail?id=2233#c2).

### Using different worktree
Set the config file (in the .git folder)
```
[core]
	worktree = ../../myrepo
```

### Test 1 - Git Bash
```
User@PC /d/Repo/myrepo.git (master)
$ git --version
git version 1.9.4.msysgit.0

User@PC /d/Repo/myrepo.git (master)
$ git update-index --refresh
fatal: Unable to write new index file
```
D:\Repo\myrepo.git\\**index.lock** is not deleted.

### Test 2 - Git 2.0.0
Copy testing repository into ```C:\msysgit\MyTest```

Execute```msys.bat```
```$ vagrant up```
```$ vagrant ssh```
```vagrant@precise64:/vagrant/git$ cd /vagrant```
```vagrant@precise64:/vagrant$ cd mytest/myrepo.git```
```
vagrant@precise64:/vagrant/mytest/myrepo.git$ git --version
git version 2.0.0
```
```
vagrant@precise64:/vagrant/mytest/myrepo.git$ git update-index --refresh
fatal: Unable to write new index file
```
Also, **index.lock is not deleted.**

### Test 3 - gitdll of TortoiseGit 
(git version 1.9.0)
Tracing the source code into **compat/mingw.c**
line 289 : xutftowcs_canonical_path() get the value of var. wpathname
```
D:\Repo\myrepo\.git\index.lock
```
It should be
```
D:\Repo\myrepo.git\.git\index.lock
```

line 294 : _wunlink() try to delete the
file.(```D:\Repo\myrepo\.git\index.lock```)
line 295 : GetLastError() return 3(ERROR_PATH_NOT_FOUND)
(Actually, there is no ```D:\Repo\myrepo\.git``` folder.)





--
View this message in context: http://git.661346.n2.nabble.com/git-update-index-not-delete-lock-file-when-using-different-worktree-tp7615300.html
Sent from the git mailing list archive at Nabble.com.

Re: git update-index not delete lock file when using different worktree

[Yue Lin Ho]

I see that refresh() of update_index.c calls setup_work_tree() to change dir
to working tree.
And the dir is not changed back to git dir before commit_lock_file() or
rollback_lock_file() is called.

So, commit_lock_file() rename file failed.
or rollback_lock_file() delete file failed.



--
View this message in context: http://git.661346.n2.nabble.com/git-update-index-not-delete-lock-file-when-using-different-worktree-tp7615300p7615306.html
Sent from the git mailing list archive at Nabble.com.

Re: git update-index not delete lock file when using different worktree

[Duy Nguyen]

On Thu, Jul 17, 2014 at 01:27:08AM -0700, Yue Lin Ho wrote:
> I see that refresh() of update_index.c calls setup_work_tree() to change dir
> to working tree.
> And the dir is not changed back to git dir before commit_lock_file() or
> rollback_lock_file() is called.
> 
> So, commit_lock_file() rename file failed.
> or rollback_lock_file() delete file failed.

I think you're on the right track. Although the problem is
hold_locked_index() in cmd_update_index() when the index path is
relative. After setup_work_tree(), the saved path points to a wrong
place and can't be unlinked anymore.

This patch seems to fix it

-- 8< --
diff --git a/lockfile.c b/lockfile.c
index 2a800ce..69fe837 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -264,7 +264,8 @@ int commit_lock_file(struct lock_file *lk)
 
 int hold_locked_index(struct lock_file *lk, int die_on_error)
 {
-	return hold_lock_file_for_update(lk, get_index_file(),
+	return hold_lock_file_for_update(lk,
+					 absolute_path(get_index_file()),
 					 die_on_error
 					 ? LOCK_DIE_ON_ERROR
 					 : 0);
-- 8< --

We could turn all lockfile's path absolute when setup_work_tree()
moves pwd, but that seems dangerous without looking through how all
lockfiles are used.

--
Duy

Re: git update-index not delete lock file when using different worktree

[Yue Lin Ho]

Hi Duy:

I tested your patch. It works. :)
(only one case.)

Thank you.

There are 26 hold_locked_index() in these files:

Line  475 of builtin\add.c
Line 4234 of \builtin\apply.c
Line  259 of \builtin\checkout.c
Line  448 of \builtin\checkout.c
Line  139 of \builtin\checkout-index.c
Line  643 of \builtin\clone.c
Line  323 of \builtin\commit.c
Line  362 of \builtin\commit.c
Line  383 of \builtin\commit.c
Line  434 of \builtin\commit.c
Line 1295 of \builtin\commit.c
Line  479 of \builtin\describe.c
Line  211 of \builtin\diff.c
Line  660 of \builtin\merge.c
Line  700 of \builtin\merge.c
Line   88 of \builtin\mv.c
Line  152 of \builtin\read-tree.c
Line  338 of \builtin\reset.c
Line  296 of \builtin\rm.c
Line  808 of \builtin\update-index.c
Line  588 of \cache-tree.c
Line   75 of \merge.c
Line 2004 of \merge-recursive.c
Line  482 of \rerere.c
Line  301 of \sequencer.c
Line  671 of \sequencer.c

Yue Lin




--
View this message in context: http://git.661346.n2.nabble.com/git-update-index-not-delete-lock-file-when-using-different-worktree-tp7615300p7615378.html
Sent from the git mailing list archive at Nabble.com.