Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 994434 (kompare) - Review Request: kompare - Diff tool
Summary: Review Request: kompare - Diff tool
Keywords:
Status: CLOSED RAWHIDE
Alias: kompare
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: kde-reviews 990444
TreeView+ depends on / blocked
 
Reported: 2013-08-07 09:42 UTC by Jan Grulich
Modified: 2013-08-15 21:25 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-08-15 07:32:33 UTC
Type: ---
Embargoed:
rdieter: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Jan Grulich 2013-08-07 09:42:58 UTC
Spec URL: http://jgrulich.fedorapeople.org/kompare.spec
SRPM URL: http://jgrulich.fedorapeople.org/kompare-4.10.97-1.fc19.src.rpm
Description: Tool to visualize changes between two versions of a file
Fedora Account System Username: jgrulich

Successful build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5788909

This package was previously part of kdesdk. Now it's distributed separately in KDE 4.11.

Comment 1 Christopher Meng 2013-08-07 09:45:08 UTC
Please add virtual provides

Provides:  mergetool

Comment 2 Jan Grulich 2013-08-07 09:55:06 UTC
Provides: mergetool

Spec URL: http://jgrulich.fedorapeople.org/kompare.spec
SRPM URL: http://jgrulich.fedorapeople.org/kompare-4.10.97-1.fc19.src.rpm

Comment 3 Mario Blättermann 2013-08-11 20:51:42 UTC
Just a few initial comments:

Don't use hardcoded paths in the file lists.

/usr/include/kde4/kompare/kompareinterface.h

has to be

 %{_kde4_includedir}/kompare/kompareinterface.h

See https://fedoraproject.org/wiki/SIGs/KDE/Packaging/BestPractices?rd=SIGs/KDE/Packaging/Guidelines#Macros


Obsoletes:      kdesdk-kompare < 4.10.80

doesn't include all versions prior to the current one. Either use

Obsoletes:      kdesdk-kompare <= 4.10.80

or refer to the current package version:

Obsoletes:      kdesdk-kompare < %{version}

Well, in this certain case there won't be a kompare-4.10.97 due to the new release policy, but we would be on the safer side in any case.


It's not useful to repeat the summary in the descriptions. My suggestions:

%description libs
This package contains shared libraries for %{name}.

%description    devel
The %{name}-devel package contains libraries and header files for
developing applications that use %{name}.


Moreover, rpmlint gives me the following error message for your spec file:

kompare.spec:26: W: unversioned-explicit-provides mergetool
The specfile contains an unversioned Provides: token, which will match all
older, equal, and newer versions of the provided thing.  This may cause update
problems and will make versioned dependencies, obsoletions and conflicts on
the provided thing useless -- make the Provides versioned if possible.

Would be be useful here to add the version number, as you already did in the Obsoletes: and Conflicts: tags?

Comment 4 Kevin Kofler 2013-08-11 21:18:56 UTC
> Obsoletes:      kdesdk-kompare < 4.10.80
>
> doesn't include all versions prior to the current one. Either use
>
> Obsoletes:      kdesdk-kompare <= 4.10.80
>
> or refer to the current package version:
>
> Obsoletes:      kdesdk-kompare < %{version}
>
> Well, in this certain case there won't be a kompare-4.10.97 due to the new
> release policy, but we would be on the safer side in any case.

I think it is actually the best practice to not Obsolete more than necessary, and also, Obsoletes with <= is usually a bad idea. (It may work with Version only (as here), but if you try using <= with a Version-Release, you'll have trouble with the disttags.) And < %{version} in Obsoletes is also not that nice, it will unnecessarily Obsolete versions that were never released in the first place.

I'd keep < 4.10.80 for the Obsoletes version here.


> It's not useful to repeat the summary in the descriptions.

Well, that's what we use here in the KDE SIG when we cannot think of a better description. ;-)

Comment 5 Jan Grulich 2013-08-13 08:25:38 UTC
Fixed description and hardcoded path, but I'm not sure what to do with virtual providing of mergetool, because I think it should be unversioned.

Spec URL: http://jgrulich.fedorapeople.org/kompare.spec
SRPM URL: http://jgrulich.fedorapeople.org/kompare-4.10.97-1.fc19.src.rpm

Comment 6 Christopher Meng 2013-08-13 08:26:57 UTC
(In reply to Jan Grulich from comment #5)
> Fixed description and hardcoded path, but I'm not sure what to do with
> virtual providing of mergetool, because I think it should be unversioned.


???Isn't it unversioned now?

Comment 7 Jan Grulich 2013-08-13 08:45:35 UTC
Yes, it is, but Mario Blättermann wrote, that should be versioned and I think it should stay as it is.

Comment 8 Michael Schwendt 2013-08-13 09:54:57 UTC
"mergetool" is a non-versioned capability and not a virtual package that will ever be obsoleted.
https://bugzilla.redhat.com/buglist.cgi?quicksearch=mergetool

Comment 9 Mario Blättermann 2013-08-13 19:17:31 UTC
Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=5812705

$ rpmlint -i -v *
kompare.src: I: checking
kompare.src: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare.src:26: W: unversioned-explicit-provides mergetool
The specfile contains an unversioned Provides: token, which will match all
older, equal, and newer versions of the provided thing.  This may cause update
problems and will make versioned dependencies, obsoletions and conflicts on
the provided thing useless -- make the Provides versioned if possible.

kompare.src: I: checking-url http://download.kde.org/unstable/4.10.97/src/kompare-4.10.97.tar.xz (timeout 10 seconds)
kompare.armv7hl: I: checking
kompare.armv7hl: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare.armv7hl: W: devel-file-in-non-devel-package /usr/lib/libkomparediff2.so
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

kompare.armv7hl: W: gzipped-svg-icon /usr/share/icons/hicolor/scalable/apps/kompare.svgz
Not all desktop environments that support SVG icons support them gzipped
(.svgz).  Install the icon as plain uncompressed SVG.

kompare.armv7hl: W: devel-file-in-non-devel-package /usr/lib/libkomparedialogpages.so
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

kompare.armv7hl: W: no-manual-page-for-binary kompare
Each executable in standard binary directories should have a man page.

kompare.i686: I: checking
kompare.i686: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare.i686: W: devel-file-in-non-devel-package /usr/lib/libkomparediff2.so
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

kompare.i686: W: gzipped-svg-icon /usr/share/icons/hicolor/scalable/apps/kompare.svgz
Not all desktop environments that support SVG icons support them gzipped
(.svgz).  Install the icon as plain uncompressed SVG.

kompare.i686: W: devel-file-in-non-devel-package /usr/lib/libkomparedialogpages.so
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

kompare.i686: W: no-manual-page-for-binary kompare
Each executable in standard binary directories should have a man page.

kompare.x86_64: I: checking
kompare.x86_64: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare.x86_64: W: gzipped-svg-icon /usr/share/icons/hicolor/scalable/apps/kompare.svgz
Not all desktop environments that support SVG icons support them gzipped
(.svgz).  Install the icon as plain uncompressed SVG.

kompare.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libkomparediff2.so
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

kompare.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libkomparedialogpages.so
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

kompare.x86_64: W: no-manual-page-for-binary kompare
Each executable in standard binary directories should have a man page.

kompare-debuginfo.armv7hl: I: checking
kompare-debuginfo.armv7hl: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare-debuginfo.i686: I: checking
kompare-debuginfo.i686: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare-debuginfo.x86_64: I: checking
kompare-debuginfo.x86_64: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare-devel.armv7hl: I: checking
kompare-devel.armv7hl: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare-devel.armv7hl: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

kompare-devel.i686: I: checking
kompare-devel.i686: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare-devel.i686: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

kompare-devel.x86_64: I: checking
kompare-devel.x86_64: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare-devel.x86_64: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

kompare-libs.armv7hl: I: checking
kompare-libs.armv7hl: W: spelling-error Summary(en_US) Runtime -> Run time, Run-time, Rudiment
The value of this tag appears to be misspelled. Please double-check.

kompare-libs.armv7hl: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare-libs.armv7hl: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

kompare-libs.i686: I: checking
kompare-libs.i686: W: spelling-error Summary(en_US) Runtime -> Run time, Run-time, Rudiment
The value of this tag appears to be misspelled. Please double-check.

kompare-libs.i686: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare-libs.i686: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

kompare-libs.x86_64: I: checking
kompare-libs.x86_64: W: spelling-error Summary(en_US) Runtime -> Run time, Run-time, Rudiment
The value of this tag appears to be misspelled. Please double-check.

kompare-libs.x86_64: I: checking-url http://www.kde.org/ (timeout 10 seconds)
kompare-libs.x86_64: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

kompare.spec:26: W: unversioned-explicit-provides mergetool
The specfile contains an unversioned Provides: token, which will match all
older, equal, and newer versions of the provided thing.  This may cause update
problems and will make versioned dependencies, obsoletions and conflicts on
the provided thing useless -- make the Provides versioned if possible.

kompare.spec: I: checking-url http://download.kde.org/unstable/4.10.97/src/kompare-4.10.97.tar.xz (timeout 10 seconds)
13 packages and 1 specfiles checked; 0 errors, 23 warnings.


A lot of the warnings:

"no-documentation"
There's no documentation which you could add. Well, you could move the license file from the main to the libs package, but this doesn't make sense either and would only be for getting rpmlint silent. Keep it as is.

"unversioned-explicit-provides"
Has been discussed already.

"devel-file-in-non-devel-package"
Should be investigated. Due to the wildcards in the -libs filelist, it could happen that unversioned libraries which have associated versioned ones have been landed in the wrong package. Have a look at the file lists:

$ rpm -qpl kompare-libs-4.10.97-1.fc20.x86_64.rpm
/usr/lib64/libkomparedialogpages.so.4
/usr/lib64/libkomparedialogpages.so.4.11.0
/usr/lib64/libkomparediff2.so.4
/usr/lib64/libkomparediff2.so.4.11.0
/usr/lib64/libkompareinterface.so.4
/usr/lib64/libkompareinterface.so.4.11.0

[mariobl@localhost kompare]$ rpm -qpl kompare-4.10.97-1.fc20.x86_64.rpm
/usr/bin/kompare
/usr/lib64/kde4/komparenavtreepart.so
/usr/lib64/kde4/komparepart.so
/usr/lib64/libkomparedialogpages.so
/usr/lib64/libkomparediff2.so
/usr/share/applications/kde4/kompare.desktop
/usr/share/doc/HTML/en/kompare
/usr/share/doc/HTML/en/kompare/common
/usr/share/doc/HTML/en/kompare/dock.png
/usr/share/doc/HTML/en/kompare/index.cache.bz2
/usr/share/doc/HTML/en/kompare/index.docbook
/usr/share/doc/HTML/en/kompare/settings-diff1.png
/usr/share/doc/HTML/en/kompare/settings-diff2.png
/usr/share/doc/HTML/en/kompare/settings-diff3.png
/usr/share/doc/HTML/en/kompare/settings-diff4.png
/usr/share/doc/HTML/en/kompare/settings-view1.png
/usr/share/doc/HTML/en/kompare/settings-view2.png
/usr/share/doc/HTML/en/kompare/undock.png
/usr/share/doc/kompare
/usr/share/doc/kompare/COPYING
/usr/share/doc/kompare/README
/usr/share/icons/hicolor/128x128/apps/kompare.png
/usr/share/icons/hicolor/16x16/apps/kompare.png
/usr/share/icons/hicolor/22x22/apps/kompare.png
/usr/share/icons/hicolor/32x32/apps/kompare.png
/usr/share/icons/hicolor/48x48/apps/kompare.png
/usr/share/icons/hicolor/scalable/apps/kompare.svgz
/usr/share/kde4/apps/kompare
/usr/share/kde4/apps/kompare/komparepartui.rc
/usr/share/kde4/apps/kompare/kompareui.rc
/usr/share/kde4/services/komparenavtreepart.desktop
/usr/share/kde4/services/komparepart.desktop
/usr/share/kde4/servicetypes/komparenavigationpart.desktop
/usr/share/kde4/servicetypes/kompareviewpart.desktop

"gzipped-svg-icon"
"no-manual-page-for-binary"
Can be ignored, also the spelling errors.

Comment 10 Rex Dieter 2013-08-13 22:20:17 UTC
naming: ok

1. better url:
https://projects.kde.org/projects/kde/kdesdk/kompare

2. licensing, main pkg should be:
License: GPLv2+ and GFDL
add
%doc COPYING.DOC

(some parts of libdiff2 and headers are LGPLv2+, but they're combined with GPLv2+, so aggregate is GPLv2+)

3. obsoletes/provides not ok
-libs subpkg missing
Obsoletes: kdesdk-kompare-libs < 4.10.80

macros: ok

4. %files not ok, move these from main to -devel pkg:
%{_kde4_libdir}/libkomparedialogpages.so
%{_kde4_libdir}/libkomparediff2.so
or delete/omit these symlinks from packaging altogether, as there is apparently no api (ie, headers) associated with them (Kevin, correct me if you think this is inaccurate).

Comment 12 Rex Dieter 2013-08-14 12:39:03 UTC
Thanks, looks good.  APPROVED

Comment 13 Jan Grulich 2013-08-14 12:45:34 UTC
New Package SCM Request
=======================
Package Name: kompare
Short Description: Diff tool
Owners: than rdieter kkofler ltinkl jgrulich
Branches: f18 f19
InitialCC:

Comment 14 Gwyn Ciesla 2013-08-14 14:21:07 UTC
Git done (by process-git-requests).

Comment 15 Kevin Kofler 2013-08-15 21:25:48 UTC
The idea is that libkomparediff2 will be used by KDevelop (or actually kdevplatform, IIRC) in the future (it currently bundles a fork of that code), but of course that cannot happen until it starts installing header files. So there will be -devel material in the (near) future, but for now, the symlinks are just useless and can be deleted.


Note You need to log in before you can comment on or make changes to this bug.