threads / discuss / 14420

git-new-workdir should not share .git/rr-cache/MERGE_RR

Subject: git-new-workdir should not share .git/rr-cache/MERGE_RR

## tl;dr

5 messages between Jul 12, 2008 and Jul 13, 2008.

replies: 4people: 4as markdown or json

Kalle Olavi Niemitalo· Jul 12, 2008, 11:27 UTC · lore

The contrib/workdir/git-new-workdir script makes .git/rr-cache in each workdir a symlink to the shared .git/rr-cache directory. This lets git-rerere share conflict resolutions between workdirs. However, that directory also contains .git/rr-cache/MERGE_RR, which lists the files for which conflict resolutions should be saved in the next commit. It is thus possible that git-merge in one workdir records conflicts to the shared MERGE_RR file, and git-commit in another workdir saves the staged files to rr-cache as conflict resolutions, even though those files are from a different branch and never saw the merge. Such invalid conflict resolutions need then be separately deleted. This happened to me, with git version 1.5.6.

I don't see any way to modify the symlinks made by git-new-workdir so that new SHA-1 directories in .git/rr-cache would be shared but .git/rr-cache/MERGE_RR would not. On IRC, "gitte" suggested changing Git to use $GIT_DIR/MERGE_RR instead of $GIT_DIR/rr-cache/MERGE_RR. I suppose compatibility with people's existing repositories would require the modified Git to keep reading $GIT_DIR/rr-cache/MERGE_RR too, so that Git could be painlessly upgraded during a merge, but it is not obvious to me how lock files should then work.

Alternatively, the SHA-1 directories in .git/rr-cache could be moved to a subdirectory; then git-new-workdir could be changed to symlink only that subdirectory.

Johannes Schindelin· Jul 12, 2008, 14:56 UTC · re: Kalle Olavi Niemitalo · lore

[PATCH 1/2] Move MERGE_RR from .git/rr-cache/ into .git/

If you want to reuse the rerere cache in another repository, and set a symbolic link to it, you do not want to have the two repositories interfer with each other by accessing the _same_ MERGE_RR.

For example, if you use contrib/git-new-workdir to set up a second working directory, and you have a conflict in one working directory, but commit in the other working directory first, the wrong "resolution" will be recorded.

The easy solution is to move MERGE_RR out of the rr-cache/ directory, which also corresponds with the notion that rr-cache/ contains cached resolutions, not some intermediate temporary states.

Noticed by Kalle Olavi Niemitalo.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
	Shawn, I Cc:ed you because of 2/2.
	On Sat, 12 Jul 2008, Kalle Olavi Niemitalo wrote:
	> I don't see any way to modify the symlinks made by 
	> git-new-workdir so that new SHA-1 directories in .git/rr-cache
	> would be shared but .git/rr-cache/MERGE_RR would not.  On IRC, 
	> "gitte" suggested changing Git to use $GIT_DIR/MERGE_RR instead of 
	> $GIT_DIR/rr-cache/MERGE_RR.
	"gitte" actually expected that a patch would not be that hard.
	> I suppose compatibility with people's existing repositories 
	> would require the modified Git to keep reading 
	> $GIT_DIR/rr-cache/MERGE_RR too, so that Git could be painlessly 
	> upgraded during a merge, but it is not obvious to me how lock files 
	> should then work.
	"rerere" is not perfect.  Thus, I suspect that we can just leave 
	an existing MERGE_RR alone, and the user just has to record the 
	resolution another time.  Too bad.
 branch.c          |    2 +-
 builtin-rerere.c  |    2 +-
 t/t4200-rerere.sh |    6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/branch.c b/branch.c
index 56e9492..b1e59f2 100644
--- a/branch.c
+++ b/branch.c
@@ -166,7 +166,7 @@ void create_branch(const char *head,
 void remove_branch_state(void)
 {
 	unlink(git_path("MERGE_HEAD"));
-	unlink(git_path("rr-cache/MERGE_RR"));
+	unlink(git_path("MERGE_RR"));
 	unlink(git_path("MERGE_MSG"));
 	unlink(git_path("SQUASH_MSG"));
 }
diff --git a/builtin-rerere.c b/builtin-rerere.c
index 69c3a52..1db2e0c 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -429,7 +429,7 @@ static int setup_rerere(struct path_list *merge_rr)
 	if (!is_rerere_enabled())
 		return -1;
 
-	merge_rr_path = xstrdup(git_path("rr-cache/MERGE_RR"));
+	merge_rr_path = xstrdup(git_path("MERGE_RR"));
 	fd = hold_lock_file_for_update(&write_lock, merge_rr_path, 1);
 	read_rr(merge_rr);
 	return fd;
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index cf10557..b5a4202 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -57,7 +57,7 @@ test_expect_success 'conflicting merge' '
 	! git merge first
 '
 
-sha1=$(sed -e 's/	.*//' .git/rr-cache/MERGE_RR)
+sha1=$(sed -e 's/	.*//' .git/MERGE_RR)
 rr=.git/rr-cache/$sha1
 test_expect_success 'recorded preimage' "grep ^=======$ $rr/preimage"
 
@@ -143,7 +143,7 @@ test_expect_success 'rerere kicked in' "! grep ^=======$ a1"
 test_expect_success 'rerere prefers first change' 'test_cmp a1 expect'
 
 rm $rr/postimage
-echo "$sha1	a1" | perl -pe 'y/\012/\000/' > .git/rr-cache/MERGE_RR
+echo "$sha1	a1" | perl -pe 'y/\012/\000/' > .git/MERGE_RR
 
 test_expect_success 'rerere clear' 'git rerere clear'
 
@@ -190,7 +190,7 @@ test_expect_success 'file2 added differently in two branches' '
 	git add file2 &&
 	git commit -m version2 &&
 	! git merge fourth &&
-	sha1=$(sed -e "s/	.*//" .git/rr-cache/MERGE_RR) &&
+	sha1=$(sed -e "s/	.*//" .git/MERGE_RR) &&
 	rr=.git/rr-cache/$sha1 &&
 	echo Cello > file2 &&
 	git add file2 &&
-- 
1.5.6.2.511.ge432a
Johannes Schindelin· Jul 12, 2008, 14:56 UTC · re: Johannes Schindelin · lore

[PATCH 2/2 (for GIT-GUI)] git-gui: MERGE_RR lives in .git/ directly with newer Git versions

Now that MERGE_RR was moved out of .git/rr-cache/, we have to delete it somewhere else. Just in case somebody wants to use a newer git-gui with an older Git, the file .git/rr-cache/MERGE_RR is removed, too (if it exists).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-gui/lib/merge.tcl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/lib/merge.tcl b/lib/merge.tcl
index cc26b07..5c01875 100644
--- a/lib/merge.tcl
+++ b/lib/merge.tcl
@@ -257,6 +257,7 @@ proc _reset_wait {fd} {
 
 		catch {file delete [gitdir MERGE_HEAD]}
 		catch {file delete [gitdir rr-cache MERGE_RR]}
+		catch {file delete [gitdir MERGE_RR]}
 		catch {file delete [gitdir SQUASH_MSG]}
 		catch {file delete [gitdir MERGE_MSG]}
 		catch {file delete [gitdir GITGUI_MSG]}
-- 
1.5.6.2.511.ge432a
Shawn O. Pearce· Jul 13, 2008, 01:16 UTC · re: Johannes Schindelin · lore

Re: [PATCH 2/2 (for GIT-GUI)] git-gui: MERGE_RR lives in .git/ directly with newer Git versions

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 7 quoted lines
> 
> Now that MERGE_RR was moved out of .git/rr-cache/, we have to delete
> it somewhere else.  Just in case somebody wants to use a newer git-gui
> with an older Git, the file .git/rr-cache/MERGE_RR is removed, too (if
> it exists).
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Ack, I can pull this into git-gui.git. But I want to make sure Junio is going to take 1/2 into git.git.

Show 16 quoted lines
>  git-gui/lib/merge.tcl |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/merge.tcl b/lib/merge.tcl
> index cc26b07..5c01875 100644
> --- a/lib/merge.tcl
> +++ b/lib/merge.tcl
> @@ -257,6 +257,7 @@ proc _reset_wait {fd} {
>  
>  		catch {file delete [gitdir MERGE_HEAD]}
>  		catch {file delete [gitdir rr-cache MERGE_RR]}
> +		catch {file delete [gitdir MERGE_RR]}
>  		catch {file delete [gitdir SQUASH_MSG]}
>  		catch {file delete [gitdir MERGE_MSG]}
>  		catch {file delete [gitdir GITGUI_MSG]}
> -- 
-- 
Shawn.
Junio C Hamano· Jul 13, 2008, 08:58 UTC · re: Shawn O. Pearce · lore

Re: [PATCH 2/2 (for GIT-GUI)] git-gui: MERGE_RR lives in .git/ directly with newer Git versions

"Shawn O. Pearce" <spearce@spearce.org> writes:
Show 11 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> 
>> Now that MERGE_RR was moved out of .git/rr-cache/, we have to delete
>> it somewhere else.  Just in case somebody wants to use a newer git-gui
>> with an older Git, the file .git/rr-cache/MERGE_RR is removed, too (if
>> it exists).
>> 
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Ack, I can pull this into git-gui.git.  But I want to make sure
> Junio is going to take 1/2 into git.git.
Will do.

← back to recent threads