Am able to delete a file with no trace in the log

We just had this come up. A file was accidentally removed during a  
merge operation, and we could find no clue in the gitk and git log  
output as to which change the deletion occurred in. We found the  
original file addition in a change; but no other mention of this file.

This seems like a bug. The log should show the file being deleted.

This is pretty easy to reproduce. Just run the script below. Here,  
pretend "c" is the central repo to which two users are pushing/ 
pulling. The users are using repositories r1 and r2.

# create and init repos
md r1
md r2
md c
cd c
git init --bare
cd ..
cd r1 && git init && cd ..
cd r2 && git init && cd ..

# create a file to conflict later
cd r1
echo hello > a.txt
git add .
git commit -m "add file"
git push ../c master

# sync other repo
cd ../r2
git pull ../c

# make conflicting change
echo helloworld > a.txt
git commit -a -m "change file in r2"

# and add a file
echo bye > b.txt
git add .
git commit -m "add file b"
git push ../c

# Meanwhile, r1 has been working
cd ../r1
echo hellogit > a.txt
git commit -a -m "change file in r1"
git pull ../c

# edit a.txt to resolve merge
vi a.txt

# User in r1 erroneously thinks "ooh, I didn't edit b.txt, I don't  
want that in my commit!"
# This is especially easy to do in git gui. That's what our user did.
# Or, the user could legitimately remove the file as part of the  
merge. Probably
# they should git rm, but our user didn't.
git reset HEAD b.txt

# But he thinks, "I did merge a.txt"
git add a.txt
git commit -m "Merged"
git push ../c


# Now user in r2 wants to pull r1's changes
cd ../r2
git pull ../c

# File b.txt deleted! Woah! How did that happen?
# gitk and git log show nothing about the deletion.
# There seems to be no evidence about who, how, why, or when the file  
got deleted.
# So it's hard to track down which user mis-used the system and  
educate them.

Cheers,
Graham Perks.

On Tue, 2 Jun 2009, Graham Perks wrote:

We just had this come up. A file was accidentally removed during a merge
operation, and we could find no clue in the gitk and git log output as to
which change the deletion occurred in. We found the original file addition in
a change; but no other mention of this file.

This seems like a bug. The log should show the file being deleted.

The manual for git log suggests the -c or --cc options ought to display
the information you want, but it doesn't seem to work in practice.

Tony.
-- 
f.anthony.n.finch  [off-list ref]  http://dotat.at/
GERMAN BIGHT HUMBER: SOUTHWEST 5 TO 7. MODERATE OR ROUGH. SQUALLY SHOWERS.
MODERATE OR GOOD.

On Tue, Jun 02, 2009 at 02:33:14PM -0500, Graham Perks wrote:

# File b.txt deleted! Woah! How did that happen?
# gitk and git log show nothing about the deletion.
# There seems to be no evidence about who, how, why, or when the file got 
deleted.
# So it's hard to track down which user mis-used the system and educate 
them.

I think this is a funny interaction with the way the diffs for merge
commits are shown. The diff you are looking for is definitely available.
In your example:

  # compare the second parent of the merge to the merge result
  $ git diff-tree HEAD^2 HEAD
  :100644 100644 31e0fce560e96c8b357f5b8630c7d8fbeb0a3ec8 2dc6d98afe942589307d7c0166971b3a2ec8706d M      a.txt
  :100644 000000 b023018cabc396e7692c70bbf5784a93d3f738ab 0000000000000000000000000000000000000000 D      b.txt

But it doesn't show up in "git log". I believe this is because the rule
for what to show in a merge commit is "if content is exactly the same as
one of the parents, it's not interesting". That is, deleting "b.txt"
from the second parent ends up being exactly as it is in the first
parent -- nonexistent. So git has no idea that you deleted "b.txt"
accidentally, and it was not simply part of the conflict resolution.

So I think this is working as intended, and is not a bug exactly. But
certainly the behavior leaves something to be desired for actually
tracking down the source of the change later on. I don't think there is
a way to get "git show $merge" to show the deletion, and nor does it
show up under "git log -- b.txt". Even worse, the latter produces no
output at all for your example (you need "--full-history" to tell it to
follow both parents of a merge).

I wonder if we need some kind of "--verbose-merges" option to tell the
diff engine that we really are interested in all of the changes that
happened in a merge. But maybe we even have something and I don't know
about it.

-Peff

On Tue, 2 Jun 2009, Jeff King wrote:

But it doesn't show up in "git log". I believe this is because the rule
for what to show in a merge commit is "if content is exactly the same as
one of the parents, it's not interesting".

Correct.

What happens is that "git log" with a filename will always simplify the 
history to the side that matches. And yes, "matching" can and does include 
"doesn't exist in child, doesn't exist in parent"

Now, I admit that in this case the matching heuristic is dubious, and 
maybe we should consider "does not exist in result" to not match any 
parent. We already think that "all new" is special ("REV_TREE_NEW" vs 
"REV_TREE_DIFFERENT"), so maybe we should think that "all deleted" is also 
special ("REV_TREE_DEL")

		Linus

On Wed, Jun 3, 2009 at 3:25 AM, Linus Torvalds
[off-list ref] wrote:

On Tue, 2 Jun 2009, Jeff King wrote:

quoted

But it doesn't show up in "git log". I believe this is because the rule
for what to show in a merge commit is "if content is exactly the same as
one of the parents, it's not interesting".

Correct.

What happens is that "git log" with a filename will always simplify the
history to the side that matches. And yes, "matching" can and does include
"doesn't exist in child, doesn't exist in parent"

Now, I admit that in this case the matching heuristic is dubious, and
maybe we should consider "does not exist in result" to not match any
parent. We already think that "all new" is special ("REV_TREE_NEW" vs
"REV_TREE_DIFFERENT"), so maybe we should think that "all deleted" is also
special ("REV_TREE_DEL")

"git merge -s ours" would do precisely the same thing, wouldn't it?
That has happened to me before, and I noticed that git log does not
show the deletion, but rationalised it as being because I had
explicitly done a "-s ours".

Fixing this would fix that (maybe more common) case too, and show that
the merge commit removed the file.

On Wed, 3 Jun 2009, Sitaram Chamarty wrote:

"git merge -s ours" would do precisely the same thing, wouldn't it?

Yes. It doesn't matter which way it goes - if a file is seen as being 
identical (and "none" very much counts) in one parent, it's not judged 
"interesting" from a merge patch standpoint, or even from a "git log" 
(aka "revision history") standpoint.

That has happened to me before, and I noticed that git log does not
show the deletion, but rationalised it as being because I had
explicitly done a "-s ours".

Fixing this would fix that (maybe more common) case too, and show that
the merge commit removed the file.

The problem is that quite often, a merge that removes a file _is_ the 
correct thing when it was removed in one branch, and a merge that adds a 
file is even more common, and in no way special. We don't show the whole 
diff in a merge, because the whole diff is often nonsensical (ie so 
trivial that showing it all just hides the parts that are actually 
relevant).

So I'll have to think about it a bit more. We clearly don't generate good 
diffs for file deletion/creation in merges, and we should improve on it, 
but it's definitely not a trivial issue either.

		Linus

This simplifies the logic of rev_compare_tree() by removing a special 
case. 

It does so by turning the special case of finding a diff to be "all new 
files" into a more generic case of "all new" vs "all removed" vs "mixed 
changes", so now the code is actually more powerful and more generic, and 
the added symmetry actually makes it simpler too.

This makes no changes to any existing behavior, but apart from the 
simplification it does make it possible to some day care about whether all 
changes were just deletions if we want to. Which we may well want to for 
merge handling.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

This is just a cleanup while I'm looking at the code. There's two things 
that are relevant to merges - the "TREESAME" logic that determines whether 
we should simplify the merge away and pick just one parent, and the actual 
diff creation logic that then creates diffs of the merges that we don't 
simplify away.

The two are totally independent, and this patch just cleans up the helper 
function that we use for the commit simplification logic. 

The only half-way subtle part here (and it really isn't that subtle) is 
that "REV_TREE_NEW | REV_TREE_OLD == REV_TREE_DIFFERENT", which makes 
sense and just simplifies the logic in general. It used to be that mixing 
REV_TREE_NEW state with REV_TREE_DIFFERENT was complicated. Now it's 
trivial, thanks to REV_TREE_OLD that is currently otherwise unused (we 
treat it the same way as REV_TREE_DIFFERENT, which is what that case used 
to result in).

There's an unrelated cleanup which is to just move the "can't happen" 
special case code of a missing commit tree up to the same point as the 
"can't happen" missing parent tree.

On Tue, 2 Jun 2009, Linus Torvalds wrote:

Now, I admit that in this case the matching heuristic is dubious, and 
maybe we should consider "does not exist in result" to not match any 
parent. We already think that "all new" is special ("REV_TREE_NEW" vs 
"REV_TREE_DIFFERENT"), so maybe we should think that "all deleted" is also 
special ("REV_TREE_DEL")

		Linus

 revision.c |   33 ++++++++++++---------------------
 revision.h |    5 +++--
 2 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/revision.c b/revision.c
index 18b7ebb..bf58448 100644
--- a/revision.c
+++ b/revision.c

@@ -256,10 +256,12 @@ static int everybody_uninteresting(struct commit_list *orig)
 
 /*
  * The goal is to get REV_TREE_NEW as the result only if the
- * diff consists of all '+' (and no other changes), and
- * REV_TREE_DIFFERENT otherwise (of course if the trees are
- * the same we want REV_TREE_SAME).  That means that once we
- * get to REV_TREE_DIFFERENT, we do not have to look any further.
+ * diff consists of all '+' (and no other changes), REV_TREE_OLD
+ * if the whole diff is removal of old data, and otherwise
+ * REV_TREE_DIFFERENT (of course if the trees are the same we
+ * want REV_TREE_SAME).
+ * That means that once we get to REV_TREE_DIFFERENT, we do not
+ * have to look any further.
  */
 static int tree_difference = REV_TREE_SAME;
 

@@ -268,22 +270,9 @@ static void file_add_remove(struct diff_options *options,
 		    const unsigned char *sha1,
 		    const char *fullpath)
 {
-	int diff = REV_TREE_DIFFERENT;
+	int diff = addremove == '+' ? REV_TREE_NEW : REV_TREE_OLD;
 
-	/*
-	 * Is it an add of a new file? It means that the old tree
-	 * didn't have it at all, so we will turn "REV_TREE_SAME" ->
-	 * "REV_TREE_NEW", but leave any "REV_TREE_DIFFERENT" alone
-	 * (and if it already was "REV_TREE_NEW", we'll keep it
-	 * "REV_TREE_NEW" of course).
-	 */
-	if (addremove == '+') {
-		diff = tree_difference;
-		if (diff != REV_TREE_SAME)
-			return;
-		diff = REV_TREE_NEW;
-	}
-	tree_difference = diff;
+	tree_difference |= diff;
 	if (tree_difference == REV_TREE_DIFFERENT)
 		DIFF_OPT_SET(options, HAS_CHANGES);
 }

@@ -305,6 +294,8 @@ static int rev_compare_tree(struct rev_info *revs, struct commit *parent, struct
 
 	if (!t1)
 		return REV_TREE_NEW;
+	if (!t2)
+		return REV_TREE_OLD;
 
 	if (revs->simplify_by_decoration) {
 		/*

@@ -323,8 +314,7 @@ static int rev_compare_tree(struct rev_info *revs, struct commit *parent, struct
 		if (!revs->prune_data)
 			return REV_TREE_SAME;
 	}
-	if (!t2)
-		return REV_TREE_DIFFERENT;
+
 	tree_difference = REV_TREE_SAME;
 	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
 	if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "",

@@ -429,6 +419,7 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
 				p->parents = NULL;
 			}
 		/* fallthrough */
+		case REV_TREE_OLD:
 		case REV_TREE_DIFFERENT:
 			tree_changed = 1;
 			pp = &parent->next;

diff --git a/revision.h b/revision.h
index be39e7d..227164c 100644
--- a/revision.h
+++ b/revision.h

@@ -118,8 +118,9 @@ struct rev_info {
 };
 
 #define REV_TREE_SAME		0
-#define REV_TREE_NEW		1
-#define REV_TREE_DIFFERENT	2
+#define REV_TREE_NEW		1	/* Only new files */
+#define REV_TREE_OLD		2	/* Only files removed */
+#define REV_TREE_DIFFERENT	3	/* Mixed changes */
 
 /* revision.c */
 void read_revisions_from_stdin(struct rev_info *revs);