Issue 853: Gerrit side-by-side diff is wrong
Status:  Released
Owner: ----
Closed:  May 2011
Reported by di...@google.com, Feb 22, 2011
Affected Version: 2.1.6.1

What steps will reproduce the problem?
1. Apply the patch below to a vanilla checked out v2.6.32 checked out copy (patch -p1)
2. Commit the change and push for code review (against a v2.6.32 based branch)
3. Look at the side-by-side diff view

What is the expected output? What do you see instead?
Should show the proper diff. It's a very simple diff, but instead it shows that the line after the added #endif was added.

Please provide any additional information below.
I tried to further isolate the problem but I was unable to. I tried some other similar diffs and they just work. I also tried to disable all the options from the diff view page (syntax color, etc) and they don't help.

I used this patch to reproduce it:
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -685,6 +685,7 @@ static void enqueue_sleeper(struct cfs_rq *cfs_rq, struct sc
 
 static void check_spread(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
+#if 0
 #ifdef CONFIG_SCHED_DEBUG
        s64 d = se->vruntime - cfs_rq->min_vruntime;
 
@@ -694,6 +695,7 @@ static void check_spread(struct cfs_rq *cfs_rq, struct sched
        if (d > 3*sysctl_sched_latency)
                schedstat_inc(cfs_rq, nr_spread_over);
 #endif
+#endif
 }
 
 static void

Mar 21, 2011
Project Member #1 jaysoff...@gmail.com
Even worse, when the commit is merged, the result is incorrect. (The result matches the off-by-one that's shown.)
May 16, 2011
#2 sop@google.com
(No comment was entered for this change.)
Status: Accepted
Labels: Milestone-2.1.7
May 16, 2011
#3 sop@google.com
(No comment was entered for this change.)
Labels: -Priority-Minor Priority-Critical
May 16, 2011
#4 sop@google.com
This is a bug in upstream JGit. Its reporting the difference incorrectly, which can then later lead to an incorrect merge as jaysoffian points out above. Unfortunately it shows for either the Myers or Histogram algorithms, which leads me to believe the bug might be in the common header/footer reduction logic.

The JGit "version" of this patch is:

diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 37087a7..33edb41 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -685,6 +685,7 @@
 
 static void check_spread(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
+#if 0
 #ifdef CONFIG_SCHED_DEBUG
 	s64 d = se->vruntime - cfs_rq->min_vruntime;
 
@@ -695,6 +696,7 @@
 		schedstat_inc(cfs_rq, nr_spread_over);
 #endif
 }
+}
 
 static void
 place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int initial)

Labels: Component-JGit
May 16, 2011
#5 sop@google.com
This is upstream JGit bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=345956

A potential fix is in http://egit.eclipse.org/r/3435
May 16, 2011
#6 sop@google.com
 Issue 854  has been merged into this issue.
May 19, 2011
#7 sop@google.com
(No comment was entered for this change.)
Status: Submitted
Labels: -Milestone-2.1.7 FixedIn-2.1.7
May 20, 2011
#8 sop@google.com
 Issue 833  has been merged into this issue.
May 20, 2011
Project Member #9 nas...@grainawi.org
 Issue 885  has been merged into this issue.
May 31, 2011
#10 sop@google.com
(No comment was entered for this change.)
Status: Released