* Re: Two bugs with renaming
2008-03-19 23:21 Two bugs with renaming John Goerzen
@ 2008-03-20 0:51 ` Junio C Hamano
2008-03-20 2:06 ` Jeff King
2008-03-20 2:30 ` John Goerzen
2008-03-20 0:56 ` Jeff King
` (2 subsequent siblings)
3 siblings, 2 replies; 9+ messages in thread
From: Junio C Hamano @ 2008-03-20 0:51 UTC (permalink / raw)
To: John Goerzen; +Cc: git
John Goerzen <jgoerzen@complete.org> writes:
> # Now here comes bug #1...
Please try 1.5.5-rc0 or newer. I think Linus's unpack_trees() updates,
even though it was sort of a rocky road to get there, addresses this.
Namely, v1.5.5-rc0~25 was the commit that fixed this issue.
> # Set up bug #2
This hasn't been addressed, I think.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Two bugs with renaming
2008-03-20 0:51 ` Junio C Hamano
@ 2008-03-20 2:06 ` Jeff King
2008-03-20 2:30 ` John Goerzen
1 sibling, 0 replies; 9+ messages in thread
From: Jeff King @ 2008-03-20 2:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Goerzen, git
On Wed, Mar 19, 2008 at 05:51:45PM -0700, Junio C Hamano wrote:
> > # Set up bug #2
>
> This hasn't been addressed, I think.
Hmm. It looks like threeway_merge is getting bogus input. In a regular
merge (e.g., branch changes file "file", master adds new file "other",
branch merges master), threeway merge sees:
- call 1: index=file, head=file, remote=file
- call 2: index=NULL, head=NULL, remote=other
But if the change to file is a D/F conflict (as in the scenario John
described), we get:
- call 1: index=files, head=files, remote=""
- call 2: index=files.upstream/delete.me, head=NULL, remote=NULL
and it barfs because index != head in the second call. But the "" entry
in the first call makes me wonder if this is the same "lists getting out
of sync" problem as before.
This is as far as I got. I don't have any more time to look at it
tonight, unfortunately.
-Peff
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Two bugs with renaming
2008-03-20 0:51 ` Junio C Hamano
2008-03-20 2:06 ` Jeff King
@ 2008-03-20 2:30 ` John Goerzen
1 sibling, 0 replies; 9+ messages in thread
From: John Goerzen @ 2008-03-20 2:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Wednesday 19 March 2008 7:51:45 pm Junio C Hamano wrote:
> John Goerzen <jgoerzen@complete.org> writes:
> > # Now here comes bug #1...
>
> Please try 1.5.5-rc0 or newer. I think Linus's unpack_trees() updates,
> even though it was sort of a rocky road to get there, addresses this.
>
> Namely, v1.5.5-rc0~25 was the commit that fixed this issue.
Correct, bug #1 is gone with current git master.
>
> > # Set up bug #2
>
> This hasn't been addressed, I think.
Also correct. Bug #2 is still present with current git master. It shows:
jgoerzen@katherina:/tmp/testrepo$ git merge master
error: Entry 'files.upstream/delete.me' would be overwritten by merge. Cannot
merge.
fatal: merging of trees 5cec043802758b3a4cd617905c395a9f12bf89a2 and
9671b5181cb0649f39cfae372af1aed56a24010d failed
Merge with strategy recursive failed.
Is there any other information I can provide to assist with tracking that one
down?
-- John
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Two bugs with renaming
2008-03-19 23:21 Two bugs with renaming John Goerzen
2008-03-20 0:51 ` Junio C Hamano
@ 2008-03-20 0:56 ` Jeff King
2008-03-20 1:24 ` Björn Steinbrink
2008-03-20 4:12 ` Linus Torvalds
3 siblings, 0 replies; 9+ messages in thread
From: Jeff King @ 2008-03-20 0:56 UTC (permalink / raw)
To: John Goerzen; +Cc: git
On Wed, Mar 19, 2008 at 06:21:27PM -0500, John Goerzen wrote:
> Bug #1 causes git to refuse to change to a different branch, claiming
> that uncommitted changes exist, even when git status says there are none.
> [...]
> Tested with Git 1.5.4.4.
I tried to reproduce this with current 'master', and it seems to be
magically fixed. There has been some work on unpack-trees lately, so it
might be a fallout from that.
> Bug #2 causes git to refuse to merge unrelated changes.
> [...]
> lrwxrwxrwx 1 jgoerzen jgoerzen 16 Mar 19 18:14 files -> /tmp/nonexistant
> drwxr-xr-x 2 jgoerzen jgoerzen 22 Mar 19 18:14 files.upstream
> jgoerzen@katherina:/tmp/testrepo$ git merge master
> fatal: Entry 'files/delete.me' would be overwritten by merge. Cannot merge.
> Merge with strategy recursive failed.
Hrm, that should work, I think. I haven't kept up with the recent
changes, so I'll have to take some time to investigate.
-Peff
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Two bugs with renaming
2008-03-19 23:21 Two bugs with renaming John Goerzen
2008-03-20 0:51 ` Junio C Hamano
2008-03-20 0:56 ` Jeff King
@ 2008-03-20 1:24 ` Björn Steinbrink
2008-03-20 4:12 ` Linus Torvalds
3 siblings, 0 replies; 9+ messages in thread
From: Björn Steinbrink @ 2008-03-20 1:24 UTC (permalink / raw)
To: John Goerzen; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 893 bytes --]
On 2008.03.19 18:21:27 -0500, John Goerzen wrote:
> Hi folks,
>
> I have a transcript of a Git session that illustrates two odd bugs
> with Git renaming. Command output is truncated except where
> interesting.
I have another one. Well, not necessarily a bug, but at least something
that looks like it could be improved. I found that while trying to
reproduce bug #1 (which doesn't show up here, as I'm using
1.5.5-something).
It seems to boil down to the fact that even when git detected a rename
during a merge, it still uses the original file name to check for
conflicts, causing somewhat bogus conflicts.
Test script is attached, when I run that, git correctly applies the
changes from foo/file in master to foo2/file in "other", but
nevertheless it complains that "foo" conflicts, although the directory
is effectively empty and should therefore not be of any interest to git.
Björn
[-- Attachment #2: t.sh --]
[-- Type: application/x-sh, Size: 306 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Two bugs with renaming
2008-03-19 23:21 Two bugs with renaming John Goerzen
` (2 preceding siblings ...)
2008-03-20 1:24 ` Björn Steinbrink
@ 2008-03-20 4:12 ` Linus Torvalds
2008-03-20 4:22 ` Linus Torvalds
2008-03-20 4:45 ` Junio C Hamano
3 siblings, 2 replies; 9+ messages in thread
From: Linus Torvalds @ 2008-03-20 4:12 UTC (permalink / raw)
To: John Goerzen; +Cc: git
On Wed, 19 Mar 2008, John Goerzen wrote:
>
> #######
> # Set up bug #2
>
> jgoerzen@katherina:/tmp/testrepo$ echo foo > foo
> jgoerzen@katherina:/tmp/testrepo$ git add foo
> jgoerzen@katherina:/tmp/testrepo$ git commit -m 'Added foo'
> jgoerzen@katherina:/tmp/testrepo$ git checkout testbranch
> Switched to branch "testbranch"
> jgoerzen@katherina:/tmp/testrepo$ ls -l
> total 0
> lrwxrwxrwx 1 jgoerzen jgoerzen 16 Mar 19 18:14 files -> /tmp/nonexistant
> drwxr-xr-x 2 jgoerzen jgoerzen 22 Mar 19 18:14 files.upstream
> jgoerzen@katherina:/tmp/testrepo$ git merge master
> fatal: Entry 'files/delete.me' would be overwritten by merge. Cannot merge.
> Merge with strategy recursive failed.
Ok, so if I read this right, you have a D/F conflict, with the current
branch having a file (or rather, a symlink) called "files", and the branch
you are trying to merge has a *directory* called "files" in it, and the
merge gets really unhappy about that conflict.
Now, arguably, it should just see them as two independent issues, and then
the rename detection will notice that the "files/delete.me" file got
renamed as "files.upstream/delete.me", so *after* rename detection there
will be no D/F conflict in the end result, but we see the conflict before
that all even happens.
Ho humm.
With the new unpack-trees logic it's pretty easy to *not* unpack with DF
conflicts (add a flag that tells us to use "base_name_compare()" instead
of "df_name_compare()" in do_compare_entry()), and maybe we can then make
builtin-merge-recursive.c set that flag. But then builtin-merge-recursive
would also have to understand about D/F conflicts itself (since now
unpack_trees() wouldn't give them as conflicts any more).
Linus
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Two bugs with renaming
2008-03-20 4:12 ` Linus Torvalds
@ 2008-03-20 4:22 ` Linus Torvalds
2008-03-20 4:45 ` Junio C Hamano
1 sibling, 0 replies; 9+ messages in thread
From: Linus Torvalds @ 2008-03-20 4:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
On Wed, 19 Mar 2008, Linus Torvalds wrote:
>
> With the new unpack-trees logic it's pretty easy to *not* unpack with DF
> conflicts (add a flag that tells us to use "base_name_compare()" instead
> of "df_name_compare()" in do_compare_entry()), and maybe we can then make
> builtin-merge-recursive.c set that flag. [...]
Looking at that, the first thing we should probably do is to make those
existing flags be bitfields rather than "int" before we add even more
flags there.
Hmm?
Linus
---
unpack-trees.h | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/unpack-trees.h b/unpack-trees.h
index 50453ed..ad8cc65 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -9,16 +9,16 @@ typedef int (*merge_fn_t)(struct cache_entry **src,
struct unpack_trees_options *options);
struct unpack_trees_options {
- int reset;
- int merge;
- int update;
- int index_only;
- int nontrivial_merge;
- int trivial_merges_only;
- int verbose_update;
- int aggressive;
- int skip_unmerged;
- int gently;
+ unsigned int reset:1,
+ merge:1,
+ update:1,
+ index_only:1,
+ nontrivial_merge:1,
+ trivial_merges_only:1,
+ verbose_update:1,
+ aggressive:1,
+ skip_unmerged:1,
+ gently:1;
const char *prefix;
int pos;
struct dir_struct *dir;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: Two bugs with renaming
2008-03-20 4:12 ` Linus Torvalds
2008-03-20 4:22 ` Linus Torvalds
@ 2008-03-20 4:45 ` Junio C Hamano
1 sibling, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2008-03-20 4:45 UTC (permalink / raw)
To: Linus Torvalds; +Cc: John Goerzen, git
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Now, arguably, it should just see them as two independent issues, and then
> the rename detection will notice that the "files/delete.me" file got
> renamed as "files.upstream/delete.me", so *after* rename detection there
> will be no D/F conflict in the end result, but we see the conflict before
> that all even happens.
The common ancestor had files/delete.me.
Our side (test_branch) deleted files/delete.me, created
files.upstream/delete.me and created files.
Their side (master)kept files/delete.me without changing, created foo.
So I would think we should not even trigger D/F conflict at all.
- files/delete.me is deleted by only one side (our side) and it should go.
- files and files.upstream/delete.me are created by only one side (ours)
and it should stay.
- foo is created by only one side (theirs) and it should come.
However, we wanted to allow policy decision to happen after read-tree, and
we traditionally kept "one side removes" case as an internal conflict in
the index, later to be resolved by merge-one-file (and merge-recursive).
So I think the resulting index should look like this:
100644 xxxxxxx 0 files.upstream/delete.me
100644 xxxxxxx 1 files/delete.me
100644 xxxxxxx 3 files/delete.me
120000 xxxxxxx 0 files
100644 xxxxxxx 0 foo
or (if we also want to leave policy decision for "one side adds" case to
merge-one-file and merge-recursive) even:
100644 xxxxxxx 1 files/delete.me
100644 xxxxxxx 3 files/delete.me
120000 xxxxxxx 2 files
100644 xxxxxxx 2 files.upstream/delete.me
100644 xxxxxxx 2 foo
But that is not what is happening here. In fact, if you did not have
"files" in the test branch, here is what you will see:
100644 xxxxxxx 0 files.upstream/delete.me
100644 xxxxxxx 1 files/delete.me
100644 xxxxxxx 3 files/delete.me
100644 xxxxxxx 0 foo
and merge-recursive knows how to match up the first three entries and if
there are changes between stages #1 and #3 of files/delete.me, that is
carried forward to files.upstream/delete.me
Your unpack_trees() is bug-to-bug compatibile with Daniel's that is in
1.5.4. Both "read-tree -m -u" bails out with the same error, without
even leaving higher stage entries in the index.
^ permalink raw reply [flat|nested] 9+ messages in thread