Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=542765 --- Comment #7 from Christian Krause <chkr@xxxxxxxxxxx> 2010-04-05 12:15:10 EDT --- Thanks for the new package, here is now the full review: * rpmlint: OK rpmlint RPMS/i686/libghemical-* SRPMS/libghemical-2.99.1-12.fc13.src.rpm SPECS/libghemical.spec libghemical.i686: W: shared-lib-calls-exit /usr/lib/libghemical.so.5.0.0 exit@xxxxxxxxx libghemical-devel.i686: W: no-documentation 4 packages and 1 specfiles checked; 0 errors, 2 warnings. According to rpmlint's help the usage of exit in libraries is discouraged since the calling application can not handle the error. However, in this case it seems to be a design decisions of the upstream evelopers. This will not block the review. But depending on your involvement with upstream it would probably be worth to ask them for the reasons and/or explain them why its discouraged. The no-documentation for the -devel package is also a false positive, it seems that there is no API documentation available. * naming: OK - name matches upstream - spec file name matches package name * sources: OK - md5sum: d2dae2d7d786d3cba335cb29d85033ea libghemical-2.99.1.tar.gz - sources matches upstream - Source0 tag ok - spectool -g works * License: TODO - License in spec file matches the actual license (of the sources) - License GPLv2+ acceptable - However, for most of the data files the license is not 100% clear. A few of the data files use the following license: "The files in this directory were downloaded from: http://www.amber.ucsf.edu/amber/dbase.html At the download page there was the following copyright notice: 'As has always been the case, the parameter information in the above file is in the public domain, and may be redistributed or used in other programs. Any such use should include proper citations, and any changes in the parameters should be prominently noted.'" - Please can you ask upstream for a statement about the licenses of all of the data files? * spec file written in English and legible: TODO Please can you enhance the description of the main package a little bit to include some information what is the purpose of "libghemical"? * compilation: TODO - supports parallel build - RPM_OPT_FLAGS are correctly used - builds in koji: F14 - However, in the %build section the LIBS variable generated by %configure is fully overwritten by the custom LIBS make parameter. If you call "rpmlint libghemical" on a system having the library installed, there will be some warnings: ---- libghemical.i686: W: undefined-non-weak-symbol /usr/lib/libghemical.so.5.0.0 dlopen libghemical.i686: W: undefined-non-weak-symbol /usr/lib/libghemical.so.5.0.0 lm7_set_plots_orbital_index libghemical.i686: W: undefined-non-weak-symbol /usr/lib/libghemical.so.5.0.0 getorb_ ... ---- The warnings show that the library uses symbols but does not link the appropriate libraris. Additionally there are warnings like: ---- libghemical.i686: W: unused-direct-shlib-dependency /usr/lib/libghemical.so.5.0.0 /usr/lib/libSCmbptr12.so.7 libghemical.i686: W: unused-direct-shlib-dependency /usr/lib/libghemical.so.5.0.0 /usr/lib/libSCoint3.so.7 ... ---- These warnings show that a couple of the manually added libraries are not needed on the other hand (this is not that critical as the previous warnings). Sure, I don't know upstream's intention, but IMHO it should be rather done like this: 1. don't overwrite the LIBS in the spec file 2. ensure, that libghemical is linked against all libraries from which libghemical uses any symbols - this should be changed probably in the Makefile.am's of the package - finally there should be no undefined non-weak symbols Some more information can be found here: https://fedoraproject.org/wiki/Features/ChangeInImplicitDSOLinking Additionally the Libs variable in the pkg-config file looks rather strange. In general Libs should only contain the library itself. * BuildRequires: TODO It looks like that the following BuildRequirements are not needed: BuildRequires: gcc-gfortran BuildRequires: openbabel-devel >= 2.2 * locales handling: OK * ldconfig in %post and %postun: OK * package owns all directories that it creates: OK * %files section: TODO (minor) Please add a "/" at the line %{_datadir}/%{name} in the %files section to explicit state that this is a directory. * no files listed twice in %files: OK * file permissions: OK - %defattr used - actual permissions in packages OK * %clean section: OK * macro usage: TODO (minor) please use %{_includedir}/%{name} for consistency * code vs. content: TODO This package contains content (the various data files). Let's wait for upstream's feedback regarding the Licenses and ask then Fedora Legal... * main package should not contain development related parts: OK * large documentation into subpackage: OK (n/a) * header files in -devel subpackage: OK * static libraries in -static package: OK (n/a) * *.so link in -devel package: OK * - devel package requires base package using fully versioned dependency: OK * packages must not contain *.la files: OK * GUI applications must provide *.desktop file: OK (n/a) * packages must not own files/dirs already owned by other packages: OK * rm -rf $RPM_BUILD_ROOT at the beginning of %install: OK * all filenames UTF-8: OK * functional test: OK - ghemical can be compiled and linked against libghemical and works without any problems * debuginfo sub-package: OK - non-empty -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review