Bug 330401 - [PATCH] Badly parsing context diffs
Summary: [PATCH] Badly parsing context diffs
Status: RESOLVED FIXED
Alias: None
Product: kompare
Classification: Applications
Component: parser (show other bugs)
Version: 4.1.3
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Kompare developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-25 17:21 UTC by Pavel Raiskup
Modified: 2014-09-29 07:29 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 4.14.2


Attachments
Testcase. (6.72 KB, patch)
2014-01-25 17:22 UTC, Pavel Raiskup
Details
Fix. (2.75 KB, patch)
2014-01-25 17:22 UTC, Pavel Raiskup
Details
0001-Testcase (4.45 KB, patch)
2014-01-25 18:11 UTC, Pavel Raiskup
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Raiskup 2014-01-25 17:21:38 UTC
Problem: SSIA.   For examples, please look at the test-case in patch 0001.  Fix is in 0002.

The tool http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/tools/git-external-diff;h=59fa36624c7c produces diffs which are not usable (e.g. by git show --ext-diff).

Workaround: `git show --ext-diff | filterdiff --format unified`

Reproducible: Always

Actual Results:  
Fails to parse.
Comment 1 Pavel Raiskup 2014-01-25 17:22:26 UTC
Created attachment 84840 [details]
Testcase.
Comment 2 Pavel Raiskup 2014-01-25 17:22:47 UTC
Created attachment 84841 [details]
Fix.
Comment 3 Pavel Raiskup 2014-01-25 18:11:30 UTC
Created attachment 84842 [details]
0001-Testcase

Removed leftovers in testcase.
Comment 4 Pavel Raiskup 2014-08-11 09:00:37 UTC
Ping?
Comment 5 Kevin Kofler 2014-08-11 10:30:03 UTC
Sorry for the (long) delay. Your patches look right. Do you have KDE git write access? (If yes, you can just push the patches, if not, just tell me and I'll push them.)

And as I see your @redhat.com mailing address: I can also add these to Fedora packaging. I don't have any control over RHEL though.
Comment 6 Pavel Raiskup 2014-08-11 11:37:45 UTC
(In reply to Kevin Kofler from comment #5)
> Your patches look right. Do you have KDE git write access? (If yes, you can
> just push the patches, if not, just tell me and I'll push them.)

Yes, please push them.

> And as I see your @redhat.com mailing address: I can also add these to
> Fedora packaging.  I don't have any control over RHEL though.

Fedora would be really nice, thanks!  Patches will find a way probably to
RHEL8 automatically (or possibly, somebody may request them later for RHEL7,
not my case ATM).
Comment 7 Kevin Kofler 2014-08-15 20:21:18 UTC
Unfortunately, we're right in the middle of the KDE Applications 4.14.0 release, so I'll better wait until after the release to push the fixes.
Comment 8 Christoph Feck 2014-09-14 12:14:10 UTC
Has this patch been committed yet?
Comment 9 Kevin Kofler 2014-09-14 14:30:34 UTC
Oops, darn, and now we're in the 4.14.1 tagging freeze. I'll get this in for 4.14.2. Sorry for that. :-(
Comment 10 Christoph Feck 2014-09-28 15:47:13 UTC
Now would be the right time to commit for 4.14.2 ;)
Comment 11 Kevin Kofler 2014-09-28 17:11:17 UTC
Git commit 7705bc06e3baeb869993702872256505b4518b87 by Kevin Kofler, on behalf of Pavel Raiskup.
Committed on 25/01/2014 at 16:58.
Pushed by kkofler into branch 'KDE/4.14'.

tests: add tests for context diffs

This problem is fixed by the following commit.

M  +92   -0    tests/interactivedifftest.cpp
M  +1    -0    tests/interactivedifftest.h

http://commits.kde.org/libkomparediff2/7705bc06e3baeb869993702872256505b4518b87
Comment 12 Kevin Kofler 2014-09-28 17:11:18 UTC
Git commit 2e2a56b4069deee62525cd4fcc207afc58e19089 by Kevin Kofler, on behalf of Pavel Raiskup.
Committed on 25/01/2014 at 11:10.
Pushed by kkofler into branch 'KDE/4.14'.

contextdiff: parse bigger subset of diffs

E.g. filterdiff tries to put some context info (function name,
namespace) at the line determining changed lines.  Also, it does
not have the ability to use timestamp information, so it
generates diffs without it.
FIXED-IN: 4.14.2

M  +2    -2    diffparser.cpp
M  +5    -5    parserbase.cpp

http://commits.kde.org/libkomparediff2/2e2a56b4069deee62525cd4fcc207afc58e19089
Comment 13 Kevin Kofler 2014-09-28 17:18:51 UTC
I also merged those 2 commits into master and frameworks.

For Fedora, we are currently in the process of getting 4.14.1 into Fedora 20 and 21, we will be importing 4.14.2 as soon as it is released, and so Fedora will get the fix with the 4.14.2 rebase. (I don't think it's worth editing the 4.14.1 update, unless we have other changes to pull in anyway, as that will just slow down the process.) I can backport the fix to Fedora 19 if you think it's worth it.
Comment 14 Pavel Raiskup 2014-09-29 07:29:20 UTC
(In reply to Kevin Kofler from comment #13)
> I also merged those 2 commits into master and frameworks.

Thanks for merging!

> For Fedora, we are currently in the process of getting 4.14.1 into Fedora 20
> and 21, we will be importing 4.14.2 as soon as it is released, and so Fedora
> will get the fix with the 4.14.2 rebase. (I don't think it's worth editing
> the 4.14.1 update, unless we have other changes to pull in anyway, as that
> will just slow down the process.) I can backport the fix to Fedora 19 if you
> think it's worth it.

I Agree here.  Back-porting would be contra-effective, and Fedora 19 is almost
EOL.