merge doesn't remove files

merge doesn't remove files

[Gerrit Pape]

Hi, please see http://bugs.debian.org/403104

Shouldn't git remove the original file after merging a commit that moved
the file away?  Here's a simple test case:

$ mkdir repo && cd repo
$ git init-db
defaulting to local storage area
$ echo a >foo && git add foo
$ git commit -a -m'add foo'
Committing initial tree 86de5e13286a8449a8a706a58e63be6781770b12
$ git branch b  
$ git mv foo bar
$ git commit -a -m'move foo'
$ git checkout b
$ echo ttt >>foo
$ git commit -a -m'change foo'
$ git pull . master
Trying really trivial in-index merge...
fatal: Merge requires file-level merging
Nope.
Merging HEAD with adc07a8eac73cdb1cb3764ec50747556ed2abc99
Merging:
92eff19 change foo
adc07a8 move foo
found 1 common ancestor(s):
ba043e5 add foo
Merge made by recursive.
 foo => bar |    0 
 1 files changed, 0 insertions(+), 0 deletions(-)
 rename foo => bar (100%)
$ git status
# On branch refs/heads/b
# Untracked files:
#   (use "git add" to add to commit)
#
#       foo
nothing to commit
$ ls
bar  foo
$ 

Thanks, Gerrit.

Re: merge doesn't remove files

[Junio C Hamano]

Gerrit Pape <pape@smarden.org> writes:

> Shouldn't git remove the original file after merging a commit that moved
> the file away?

I think this has been fixed quite a while ago --- does not seem
to reproduce with the current v1.5.0-rc.

Re: merge doesn't remove files

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Gerrit Pape <pape@smarden.org> writes:
>
>> Shouldn't git remove the original file after merging a commit that moved
>> the file away?
>
> I think this has been fixed quite a while ago --- does not seem
> to reproduce with the current v1.5.0-rc.

Having said that, I think there is a worse problem with
merge-recursive.  It loses untracked files that are not
involved in the merge.

    $ mkdir repo && cd repo
    $ git init-db
    Initialized empty Git repository in .git/
    $ echo a >foo && git add foo
    $ git commit -a -m'add foo'
    Created initial commit da51b094c7c75aef4362e229975be7f376fa00cb
     1 files changed, 1 insertions(+), 0 deletions(-)
     create mode 100644 foo
    $ git branch b
    $ git mv foo bar
    $ git commit -a -m'move foo'
    Created commit 3154467cd539db7a90e04b7291442384222de4a4
     2 files changed, 1 insertions(+), 1 deletions(-)
     create mode 100644 bar
     delete mode 100644 foo

The 'master' branch renames 'foo' to 'bar'

    $ git checkout b
    Switched to branch "b"
    $ echo ttt >>foo
    $ git commit -a -m'change foo'
    Created commit eb14bff5f399b1a9ca6aa450a803833c3f7b7b3a
     1 files changed, 1 insertions(+), 0 deletions(-)

The 'b' branch keeps 'foo' but modifies it.

    $ git checkout master
    Switched to branch "master"
    $ echo frotz >foo

The 'master' branch has an untracked file 'foo' now.

    $ git merge b
     100% (5/5) done
    Merge made by recursive.
     bar |    1 +
     1 files changed, 1 insertions(+), 0 deletions(-)
    $ ls -a
    ./  ../  bar  .git/

Oops.  'foo' is not involved in the merge but is lost.

Sadly, a similar case was fixed in merge-resolve where our
branch has already removed a file, had an untracked file of the
same name in the working tree, and merging an old branch that
still had that path.

    $ git init
    Initialized empty Git repository in .git/
    $ echo a >foo ; echo b >bar ; git add .
    $ git commit -m 'initial'
    Created initial commit 7925ec23146d4bffa1078a2c220df2c2f9935ca8
     2 files changed, 2 insertions(+), 0 deletions(-)
     create mode 100644 bar
     create mode 100644 foo
    $ git branch b
    $ rm foo ; echo ttt >>bar
    $ git commit -a -m 'remove foo and update bar'
    Created commit 7fbbec4cb0d4af95090e0ba4f1fb8c9630d8e438
     2 files changed, 1 insertions(+), 1 deletions(-)
     delete mode 100644 foo
    $ git checkout b
    Switched to branch "b"
    $ echo c >baz ; git add .
    $ git commit -m 'side'
    Created commit 272ef105f43df5e3629d6915001476651cafa139
     1 files changed, 1 insertions(+), 0 deletions(-)
     create mode 100644 baz
    $ git checkout master
    Switched to branch "master"
    $ ls -a
    ./  ../  bar  .git/
    $ echo throw >foo
    $ git merge -s resolve b
    Trying really trivial in-index merge...
    fatal: Merge requires file-level merging
    Nope.
    Trying simple merge.
    Merge made by resolve.
     baz |    1 +
     1 files changed, 1 insertions(+), 0 deletions(-)
     create mode 100644 baz
    $ ls -a
    ./  ../  bar  baz  foo  .git/
    $ cat foo
    throw

Fortunately merge-recursive gets this case right, but when a
rename is involved it does not.

Re: merge doesn't remove files

[Shawn O. Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> Gerrit Pape <pape@smarden.org> writes:
> 
> > Shouldn't git remove the original file after merging a commit that moved
> > the file away?
> 
> I think this has been fixed quite a while ago --- does not seem
> to reproduce with the current v1.5.0-rc.

Indeed, this commit of mine was supposed to fix it:

  commit 8371234ecaaf6e14fe3f2082a855eff1bbd79ae9
  Author: Shawn Pearce <spearce@spearce.org>
  Date:   Wed Dec 13 05:42:44 2006 -0500

Its in 1.5.0-rc0, but nothing earlier.

-- 
Shawn.

Re: merge doesn't remove files

[Shawn O. Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> Having said that, I think there is a worse problem with
> merge-recursive.  It loses untracked files that are not
> involved in the merge.

I tracked it down to my commit 8371234e which was supposed to fix
the bug that started this thread.

Apparently what I did was make merge-recursive remove the file from
the working directory if it was renamed, even though it wasn't in
the index when the merge started.

So merge-recursive is only deleting untracked files which are in
the merge base, but aren't in one of the heads involved in the merge.

I'm looking at patching this.  Clearly we should only delete the
old file from the working directory during a rename if the tree-ish
which was supposed to match the working directory had the file
in it.  But that tree-ish doesn't have to be the first non-base
argument to merge-recursive, does it?  In practice it usually is,
but can I assume it in the code?

I'm looking at something like the following:

diff --git a/merge-recursive.c b/merge-recursive.c
index fa320eb..bbb52a7 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -891,7 +891,7 @@ static int process_renames(struct path_list *a_renames,
                        struct diff_filespec src_other, dst_other;
                        int try_merge, stage = a_renames == renames1 ? 3: 2;
 
-                       remove_file(1, ren1_src, index_only);
+                       remove_file(1, ren1_src, index_only || stage == 3);
 
                        hashcpy(src_other.sha1, ren1->src_entry->stages[stage].s
                        src_other.mode = ren1->src_entry->stages[stage].mode;

-- 
Shawn.

Re: merge doesn't remove files

[Linus Torvalds]

On Sat, 3 Feb 2007, Shawn O. Pearce wrote:
> 
> But that tree-ish doesn't have to be the first non-base
> argument to merge-recursive, does it?  In practice it usually is,
> but can I assume it in the code?

Yes, the first branch is supposed to be the "current" one. That's the one 
we compare to the index, and that we should already error out on if the 
index doesn't match the tree. So yeah, a file should ever be removed only 
if it existed in the old index (and by implication in the first branch, 
since otherwise we should have failed the merge as not having a clean 
tree anyway).

		Linus