git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Elijah Newren <newren@gmail.com>, git@vger.kernel.org
Subject: Re: new behaviour in git merge
Date: Thu, 24 Feb 2011 06:52:33 -0500	[thread overview]
Message-ID: <20110224115233.GA31356@sigill.intra.peff.net> (raw)
In-Reply-To: <20110224202454.d3b8668e.sfr@canb.auug.org.au>

[+cc Elijah Newren; this is a bug that bisects to one of your commits.
 The backstory is that during a merge, many unrelated files get
 unnecessarily re-written with the same content. Read on for details.]

On Thu, Feb 24, 2011 at 08:24:54PM +1100, Stephen Rothwell wrote:

> In today's linux-next tree (available at
> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git),
> commit ad11c1e8abca18872c2993b09b1abde418955b6c is just before the merge
> and commit a2c06ee2fe5b48a71e697bae00c6e7195fc016b6 is what was merged.
> 
> I just did this in a copy of my tree:
> 
> 	git reset --hard ad11c1e8abca18872c2993b09b1abde418955b6c
> 	sleep 90
> 	git merge a2c06ee2fe5b48a71e697bae00c6e7195fc016b6
> 
> A comparison of the ls -lR ouput before and after showed a lot of changed
> modification times.
> 
> Several other merges showed the same problem.

Thanks for the reproduction recipe. I was able to bisect using this
script as my test:

-- >8 --
#!/bin/sh

stat() {
  git ls-files -z | xargs --null stat --format='%Y %n' | sort
}

cd linux-next &&
git reset --hard ad11c1e8abca18872c2993b09b1abde418955b6c &&
sleep 3 &&
stat >before &&
git merge a2c06ee2fe5b48a71e697bae00c6e7195fc016b6 &&
stat >after &&
comm -13 before after | cut -d' ' -f2 | sort >stat-diff &&
git diff-tree -r --name-only HEAD HEAD^ >tree-diff &&
comm -23 stat-diff tree-diff >extra-stats &&
if test -s extra-stats; then
  cat extra-stats
  echo FAIL
  exit 1
else
  echo OK
  exit 0
fi
-- 8< --

It bisects to 882fd11 (merge-recursive: Delay content merging for
renames, 2010-09-20). And indeed, looking further at the files that get
modified, they appear to be renames on one side of the merge, but not
touched on the other side. But rather than notice that there is nothing
to be done on them, we seem to update the index and write out the new
entries.

The patch below makes the problem go away in your test case, but also
introduces some test failures in t3509, so I'm sure there is something
else going on. I'm somewhat clueless about the merge code, so I'll defer
to Elijah, who wrote 882fd11, and see what he says.

-Peff

diff --git a/merge-recursive.c b/merge-recursive.c
index 0ca54bd..1dd643f 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1083,7 +1083,8 @@ static int process_renames(struct merge_options *o,
 					b = ren1->pair->two;
 					a = &src_other;
 				}
-				update_stages_and_entry(ren1_dst, ren1->dst_entry, one, a, b, 1);
+				if (hashcmp(one->sha1, b->sha1))
+					update_stages_and_entry(ren1_dst, ren1->dst_entry, one, a, b, 1);
 				if (string_list_has_string(&o->current_directory_set, ren1_dst)) {
 					setup_rename_df_conflict_info(RENAME_NORMAL,
 								      ren1->pair,

  reply	other threads:[~2011-02-24 11:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-24  3:33 new behaviour in git merge Stephen Rothwell
2011-02-24  8:15 ` Jeff King
2011-02-24  9:24   ` Stephen Rothwell
2011-02-24 11:52     ` Jeff King [this message]
2011-02-26 18:34       ` [RFC PATCH 0/2] Fix unnecessary updates of files during merge Elijah Newren
2011-02-26 18:34         ` [RFC PATCH 1/2] t6022: New test checking for unnecessary updates of renamed+modified files Elijah Newren
2011-02-26 18:34         ` [RFC PATCH 2/2] merge-recursive: When we detect we can skip an update, actually skip it Elijah Newren
2011-02-26 18:43         ` [RFC PATCH 0/2] Fix unnecessary updates of files during merge Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110224115233.GA31356@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=sfr@canb.auug.org.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).