Bug 437570

Summary: KDiff3 3-way merge injects newlines and strips trailing newlines
Product: [Applications] kdiff3 Reporter: nyanpasu64 <nyanpasu64>
Component: applicationAssignee: michael <reeves.87>
Status: RESOLVED FIXED    
Severity: normal CC: emaste, groot, guneyozsan
Priority: NOR    
Version: 1.9.2   
Target Milestone: ---   
Platform: Arch Linux   
OS: Linux   
Latest Commit: Version Fixed In: 1.9.3
Attachments: Three files that when kdiff3 tries to merge, it produces an incorrect result.

Description nyanpasu64 2021-05-23 18:24:28 UTC
Created attachment 138712 [details]
Three files that when kdiff3 tries to merge, it produces an incorrect result.

SUMMARY
When merging complex files and resolving conflicts by hand, KDiff3 1.9.0+ saves an incorrect output file with extraneous newlines inserted, and the trailing newline removed.

STEPS TO REPRODUCE
1. Download and extract the attached "kdiff3 merge error.zip".
2. kdiff3 RtAudio_BASE_452948.h RtAudio_LOCAL_452948.h RtAudio_REMOTE_452948.h -o out.h (or open these 3 files in the GUI).
3. You'll get a merge conflict. Click (optionally double-click) closeStream on the left and press Ctrl+Y. That should fix the conflict. Then save the file.
4. To check the results, run diff RtAudio_LOCAL_452948.h out.h (or kdiff3).


OBSERVED RESULT
Comparing out.h to the local file, there are extraneous newlines introduced within the file, as well as the final newline (present in all 3 input files) being stripped from the written file. (I think this is not caused by inconsistent line endings, and that all 3 files contain LF only.)

The first extraneous newline is introduced after the line containing `RtAudioFormat nativeFormats{};`. After that line, the original KDiff3 (3-way merge) window's output pane says "B <No src line>", but upon saving, writes out an empty line anyway. The second extraneous newline is introduced after the line containing `RtAudioFormat nativeFormats{};`, with the same "B <No src line>" behavior that writes a newline anyway.

If you navigate to either diff, deselect B, and reselect B, then the "B <No src line>" line disappears from the output panel, and the extraneous newline is no longer present when you save.

EXPECTED RESULT
Comparing out.h to the local file, kdiff3 should output exactly 2 added lines of text and no newline changes. And kdiff3 1.8.5 and prior do exactly that.

SOFTWARE/OS VERSIONS

Tested on KDiff3 1.9.0 on Windows 10 x64, and KDiff3 1.9.0 and 1.9.2 on Arch Linux x64. I get the same results on both.

Another person tested KDiff3 1.9.2 on Windows 10 x64, and got only the missing newline on the end, no injected newlines in the middle.

I tried bisecting KDiff3's commit history, it seems 1.9.0 has this bug, `git merge-base 1.9.2 master` pops up a "Severe Internal Error" dialog when trying to merge, and `git merge-base 1.8.5 master` does not have this bug, as does 1.8.5.

ADDITIONAL INFORMATION

When trying to bisect this bug, I've noticed that the Git commit log of KDiff3 has divergent feature branches for 1.8.5 and 1.9.2 and master. My efforts to bisect each of these branches were hampered by occasional commits that failed to compile, and stretches of history that popped up a "Severe Internal Error" dialog instead of merging. Was there a major long-term refactor between 1.8.5 and 1.9.0 that resulted in non-functional intermediate states, and introduced subtle functional issues like this bug?
Comment 1 Ed Maste 2021-06-14 17:56:08 UTC
I encountered this on FreeBSD as well and submitted https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256606 there
Comment 2 groot 2021-06-14 21:03:43 UTC
I'm chasing this downstream, where Ed reported a much smaller reproducer.

- 1.8 branch split off master in march 2019; tip of that branch (1.8.5) works correctly.
- 1.9 branch split off master in february 2021; the base of that branch b68a00406 hits a "severe internal error"; 1.9.2 has this problem as described here.
- master has the same problem.
Comment 3 groot 2021-06-14 21:36:16 UTC
925449e88749c05cbb17e7eb6456874766c482d8 is good; after that it's all (er .. I didn't test **every** commit) internal errors along master with this kind of merge until e59cb08b3782c04d8698d48b885680db8a525ec4 is the last internal-error commit. Starting with d0c987f9b793ce2028595cc8df02271c3b2fb9aa on master it doesn't internal-error anymore. I can't pick any resolutions in that commit, but  at least face74301b09ba62f85b79f74fdb9565e79ccf99 (4 commits later) can -- showing the bad merge.
Comment 5 Guney Ozsan 2021-06-14 23:32:14 UTC
I don't have a sample but I use kdiff with Unity 3D files like *.cs or *.asset. Version 1.9.x has this issue for "all" files that have merge conflict and opened Kdiff via git client Fork (arguments: "$REMOTE" -b "$BASE" "$LOCAL" -o "$MERGED"). I downgraded to latest 1.8 and it works fine.
Comment 6 michael 2021-06-21 23:52:26 UTC
There was a major refactor. The some of compile errors however the result changes in the standard c/c++ hearders with clang11++ and gcc11++. Specifically <limits> must now be explicitly included TypeUtils.h. The introduction of the LineRef wrapper class is the main source of the "Data Lose" error. I have determined the that last EOL being dropped is a regression in the initial reading of the file. Not the comparison algorithm. The second part of this bug seems to be a standing issue with the merge algorithm that pre-dates the refactor. That will be fixed by the MR.
Comment 7 michael 2021-06-24 19:48:58 UTC
Git commit cb1801979c87a36e2b0d38f8c1d5780b95cf7d60 by Michael Reeves, on behalf of Adriaan de Groot.
Committed on 24/06/2021 at 19:47.
Pushed by mreeves into branch 'master'.

Do not put line feed between removed lines

If a line is removed in a merge, then there should be no
line feed before it -- that will come from a subsequent
**non**-deleted line. This avoids double-newlines when a
line is removed.

(cherry picked from commit 942325c58a922b0297861ad3c2b1832248b1bf14)
FIXED-IN:1.9.3

M  +4    -1    src/mergeresultwindow.cpp

https://invent.kde.org/sdk/kdiff3/commit/cb1801979c87a36e2b0d38f8c1d5780b95cf7d60
Comment 8 michael 2021-06-24 19:49:06 UTC
Git commit f8cec3c7b954bc182f80c113c914baee90195b0b by Michael Reeves.
Committed on 24/06/2021 at 19:46.
Pushed by mreeves into branch 'master'.

Fix EOL handling in SourceData::FileData::preprocess

This resolves the striping of the trailing EOLs.

M  +42   -12   src/SourceData.cpp
M  +2    -2    src/SourceData.h
M  +67   -12   src/autotests/DiffTest.cpp
M  +2    -0    src/diff.cpp

https://invent.kde.org/sdk/kdiff3/commit/f8cec3c7b954bc182f80c113c914baee90195b0b
Comment 9 michael 2021-06-24 20:08:29 UTC
Git commit 17b02147d5e9d14a518dd4c5c9b63ef330e1c539 by Michael Reeves, on behalf of Adriaan de Groot.
Committed on 24/06/2021 at 20:08.
Pushed by mreeves into branch '1.9'.

Do not put line feed between removed lines

If a line is removed in a merge, then there should be no
line feed before it -- that will come from a subsequent
**non**-deleted line. This avoids double-newlines when a
line is removed.

(cherry picked from commit 942325c58a922b0297861ad3c2b1832248b1bf14)
FIXED-IN:1.9.3
(cherry picked from commit cb1801979c87a36e2b0d38f8c1d5780b95cf7d60)

M  +4    -1    src/mergeresultwindow.cpp

https://invent.kde.org/sdk/kdiff3/commit/17b02147d5e9d14a518dd4c5c9b63ef330e1c539
Comment 10 michael 2021-06-24 20:08:37 UTC
Git commit 4ee5627d28d65e095ce2149f80e6fdc7c6d5047d by Michael Reeves.
Committed on 24/06/2021 at 20:07.
Pushed by mreeves into branch '1.9'.

Fix EOL handling in SourceData::FileData::preprocess

This resolves the striping of the trailing EOLs.

M  +26   -6    src/SourceData.cpp
M  +2    -0    src/diff.cpp

https://invent.kde.org/sdk/kdiff3/commit/4ee5627d28d65e095ce2149f80e6fdc7c6d5047d