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 1190056 (kgraphviewer) - Review Request: kgraphviewer - Graphviz dot graph file viewer
Summary: Review Request: kgraphviewer - Graphviz dot graph file viewer
Keywords:
Status: CLOSED NEXTRELEASE
Alias: kgraphviewer
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: kde-reviews
TreeView+ depends on / blocked
 
Reported: 2015-02-06 08:15 UTC by Lubomir Rintel
Modified: 2015-02-09 14:10 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-02-09 11:35:56 UTC
Type: Bug
Embargoed:
rdieter: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Lubomir Rintel 2015-02-06 08:15:27 UTC
SPEC: http://v3.sk/~lkundrak/SPECS/kgraphviewer.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/kgraphviewer-2.2.0-1.fc22.src.rpm
Mock: http://koji.fedoraproject.org/koji/taskinfo?taskID=8842194

Description:

KGraphViewer is a Graphviz dot graph file viewer.

Comment 1 Rex Dieter 2015-02-06 08:30:23 UTC
naming: ok

sources: ok
043ace59a061a99fff2757a17be4e1d6  kgraphviewer-2.2.0.tar.xz

licensing: ok

macros: ok

scripotlets: NOT ok

1. MUST fix icon ownership and add missing iconcache scriptlets
change
%{_kde4_iconsdir}/hicolor
to
%{_kde4_iconsdir}/hicolor/*/*/*

and add icon scriptlets per:
https://fedoraproject.org/wiki/Packaging:ScriptletSnippets?rd=Packaging/ScriptletSnippets#Icon_Cache

2. main pkg MUST have tighter dependency on libs, replace
Requires: %{name}-libs = %{version}
with
Requires: %{name}-libs%{?_isa} = %{version}-%{release}

3. -devel dependencies are wrong MUST fix, %{_isa} is in the wrong place.
Requires:       kgraphviewer = %{version}-%{release}%{?isa}
Requires:       %{name}-libs = %{version}-%{release}%{?isa}

I'd suggest:
Requires: %{name}-libs%{?_isa} = %{version}-%{release}
(and skip the dependency on the main pkg)


4.  SHOULD improve kde-related dependencies, replace
BuildRequires:  qt-devel
BuildRequires:  kdelibs-devel
Requires: kde-filesystem

with
BuildRequires: kdelibs4-devel
%{?kde_runtime_requires}
(in main pkg)

5.  SHOULD build out-of-src tree, replace
%cmake_kde4 .
make %{?_smp_mflags}
...
make install DESTDIR={%buildroot}

with (something like):
%build
mkdir %{_target_platform}
pushd %{_target_platform}
%{cmake_kde4} ..
popd

make %{?_smp_mflags} -C %{_target_platform}
...
make install/fast DESTDIR=%{buildroot} -C %{_target_platform}

6. SHOULD omit deprecated rpm tags, like:
Group:

Comment 2 Mario Blättermann 2015-02-06 08:56:01 UTC
The following issues from rpmlint should be investigated:

kgraphviewer.x86_64: W: incoherent-version-in-changelog 0.2.0-1 ['2.2.0-1.fc21', '2.2.0-1']
The latest entry in %changelog contains a version identifier that is not
coherent with the epoch:version-release tuple of the package.

But look at the spec file:

%changelog
* Thu Feb 05 2015 Lubomir Rintel <lkundrak> - 0.2.0-1
- Initial packaging

Seems that a %changelog entry in the binary package will be generated which contains a dist tag.


kgraphviewer.x86_64: E: invalid-appdata-file /usr/share/appdata/kgraphviewer.appdata.xml
appdata file is not valid, check with appdata-validate

Indeed, there's something to fix:

$ appdata-validate *a.xml
kgraphviewer.appdata.xml 3 problems detected:
• tag-missing           : <updatecontact> is not present
• style-invalid         : Not enough <p> content before <ul>
• style-invalid         : <li> is too short

The second and third issues are bogus and not relevant for a really valid appdata file, but <updatecontact> should be added. Try to figure out who has written the appdata file and contact him to add his mail address here.


kgraphviewer-devel.x86_64: W: only-non-binary-in-usr-lib
There are only non binary files in /usr/lib so they should be in /usr/share.

Of course, the symbolic link in %{_kde4_libdir}/ is non-binary. But it has to be there anyway.

Comment 3 Kevin Kofler 2015-02-06 09:06:04 UTC
The incoherence in %changelog is not the dist tag (we ignore those in the version match check), but 0.2.0 vs. 2.2.0.

Comment 4 Lubomir Rintel 2015-02-06 09:54:13 UTC
(In reply to Rex Dieter from comment #1)
> scripotlets: NOT ok
> 
> 1. MUST fix icon ownership and add missing iconcache scriptlets
> change
> %{_kde4_iconsdir}/hicolor
> to
> %{_kde4_iconsdir}/hicolor/*/*/*
> 
> and add icon scriptlets per:
> https://fedoraproject.org/wiki/Packaging:ScriptletSnippets?rd=Packaging/
> ScriptletSnippets#Icon_Cache

Fixed.

> 2. main pkg MUST have tighter dependency on libs, replace
> Requires: %{name}-libs = %{version}
> with
> Requires: %{name}-libs%{?_isa} = %{version}-%{release}

Fixed.

> 3. -devel dependencies are wrong MUST fix, %{_isa} is in the wrong place.
> Requires:       kgraphviewer = %{version}-%{release}%{?isa}
> Requires:       %{name}-libs = %{version}-%{release}%{?isa}
> 
> I'd suggest:
> Requires: %{name}-libs%{?_isa} = %{version}-%{release}
> (and skip the dependency on the main pkg)

Whoops. Fixed.

> 4.  SHOULD improve kde-related dependencies, replace
> BuildRequires:  qt-devel
> BuildRequires:  kdelibs-devel
> Requires: kde-filesystem
> 
> with
> BuildRequires: kdelibs4-devel
> %{?kde_runtime_requires}
> (in main pkg)

Done.

> 5.  SHOULD build out-of-src tree, replace
> %cmake_kde4 .
> make %{?_smp_mflags}
> ...
> make install DESTDIR={%buildroot}
> 
> with (something like):
> %build
> mkdir %{_target_platform}
> pushd %{_target_platform}
> %{cmake_kde4} ..
> popd
> 
> make %{?_smp_mflags} -C %{_target_platform}
> ...
> make install/fast DESTDIR=%{buildroot} -C %{_target_platform}

Done.

> 6. SHOULD omit deprecated rpm tags, like:
> Group:

I like it there. I'll start removing those the day RPM starts to complain or guidelines forbid it.

(In reply to Mario Blättermann from comment #2)
> The following issues from rpmlint should be investigated:
> 
> kgraphviewer.x86_64: W: incoherent-version-in-changelog 0.2.0-1
> ['2.2.0-1.fc21', '2.2.0-1']
> The latest entry in %changelog contains a version identifier that is not

Fixed.

> kgraphviewer.x86_64: E: invalid-appdata-file
> /usr/share/appdata/kgraphviewer.appdata.xml
> appdata file is not valid, check with appdata-validate
> 
> Indeed, there's something to fix:
> 
> $ appdata-validate *a.xml
> kgraphviewer.appdata.xml 3 problems detected:
> • tag-missing           : <updatecontact> is not present
> • style-invalid         : Not enough <p> content before <ul>
> • style-invalid         : <li> is too short
> 
> The second and third issues are bogus and not relevant for a really valid
> appdata file, but <updatecontact> should be added. Try to figure out who has
> written the appdata file and contact him to add his mail address here.

I notified upstream.

I'm not patching it in package as the relaxed check in %install passes just fine.

SPEC: http://v3.sk/~lkundrak/SPECS/kgraphviewer.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/kgraphviewer-2.2.0-2.fc22.src.rpm

Comment 6 Rex Dieter 2015-02-06 16:15:34 UTC
Looks good, thanks.

APPROVED

Comment 7 Lubomir Rintel 2015-02-07 08:37:12 UTC
Thank you!

New Package SCM Request
=======================
Package Name: kgraphviewer
Short Description: Graphviz dot graph file viewer
Upstream URL: https://projects.kde.org/projects/extragear/graphics/kgraphviewer
Owners: lkundrak
Branches: f20 f21 el6 epel7

Comment 8 Gwyn Ciesla 2015-02-07 20:05:59 UTC
Git done (by process-git-requests).

Comment 9 Lubomir Rintel 2015-02-09 11:35:56 UTC
Imported and built. Thank you!

Comment 10 Fedora Update System 2015-02-09 14:08:07 UTC
kgraphviewer-2.2.0-2.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/kgraphviewer-2.2.0-2.fc21

Comment 11 Fedora Update System 2015-02-09 14:10:12 UTC
kgraphviewer-2.2.0-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/kgraphviewer-2.2.0-2.fc20


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