[Bug 1243379] Review Request: Tinyxpath - Small footprint XPath syntax decoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1243379



--- Comment #1 from František Dvořák <valtri@xxxxxxxxxx> ---
It's funny opencity unbundled the tinyxml from tinyxpath and bundle it itself.
:-) Good catch.

Issues found:

1) License file

Currently it is used the link created automatically by automake, which points
to GPL license file. But the license is zlib and source code doesn't contain
text license.

libpng/zlib license doesn't require separate license text file distributed with
the sources/binaries, so it is not show-stopper for Fedora packaging and you
can just remove the wrong link.

The proper way is to ask upstream to include the license text:

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text


2) AUTHORS

This file can be included in the package documentation.


3) shared library versioning

See the link:
http://fedoraproject.org/wiki/Packaging:Guidelines#Downstream_.so_name_versioning

The soname should be something like libxpath.so.0.1.


4) libxpath.so* files are in both packages (tinyxpath, tinyxpath-devel), the
*.so file (the link) should be in -devel, the other files in the main package.

Something like:

%files
%{_libdir}/lib%{name}.so.0
%{_libdir}/lib%{name}.so.0.*

%files devel
%{_libdir}/lib%{name}.so


5) tinyxpath binary is more like test and example code, it is not needed to
include it in the tinyxpath package

By the way you can add %check section and use tinyxpath binary there. :-) It
always returns zero exit code, but the out.htm file will contain some <em> html
tags in case of errors. So the section could look like this?:

%check
./tinyxpath
grep -q '<em>' out.htm && false

The problem is there is one error. It is probably not important error, but it
could be reported upstream (and ignored in the %check).


6) fedora-review complains about requirement of -devel on the base package:

[ ]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     tinyxpath-devel

There is just missing the %{?_isa}.


7) 'Requires: tinyxml' not needed, dependencies on the libraries are generated
automagically by rpmbuild scripts.

See http://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires .


8) fedora-review complains about mixing of %{buildroot} and $RPM_BUILD_ROOT

See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros


9) cosmetic (you can ignore): I would move the %post and %postun sections
before the %file section. This is just cosmetic and I should probably not even
mention it here. But because of atypical placing my first impression was the
%post/%postun sections are not there. :-)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]