Bug 670541 (libxc)
Summary: | Review Request: libxc - Library of exchange and correlation functionals to be used in DFT codes | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Susi Lehtola <susi.lehtola> |
Component: | Package Review | Assignee: | Martin Gieseking <martin.gieseking> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, martin.gieseking, notting |
Target Milestone: | --- | Flags: | martin.gieseking:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | libxc-2.1.0-4.el7 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2011-02-02 19:25:23 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 670558 |
Description
Susi Lehtola
2011-01-18 15:58:59 UTC
Hi Jussi, the package looks almost fine as far as I can see. Just a few things I noticed: - %{?dist} is missing in the Release field - the packaged source tarball differs from the upstream one: $ md5sum libxc-1.0.tar.gz* f9702268cf0015ada6a0ac34659971e6 libxc-1.0.tar.gz 4a8ef0c8fccc695e65466ec1a8878280 libxc-1.0.tar.gz.1 - maybe you want to use %{version} in Source0 - according to [1], %{_isa} must be added to Requires: gcc-gfortran The directory %{_fmoddir} seems to be unowned. Unlike stated in [1], gcc-gfortran doesn't seem to own it. But maybe I'm missing something. [1] http://fedoraproject.org/wiki/Packaging:Fortran Good catches. That teaches me (once again) not to use upstream specfiles as a starting point. The gcc-gfortran stuff are bugs in the GCC packaging - the Fortran guidelines were approved, but the GCC guys haven't wanted to implement them. http://theory.physics.helsinki.fi/~jzlehtol/rpms/libxc.spec http://theory.physics.helsinki.fi/~jzlehtol/rpms/libxc-1.0-2.fc14.src.rpm (In reply to comment #2) > The gcc-gfortran stuff are bugs in the GCC packaging - the Fortran guidelines > were approved, but the GCC guys haven't wanted to implement them. Ah OK, thanks for the clarification. What do you think about the following still unaddressed items? - the packaged source tarball differs from the upstream one: $ md5sum libxc-1.0.tar.gz* f9702268cf0015ada6a0ac34659971e6 libxc-1.0.tar.gz 4a8ef0c8fccc695e65466ec1a8878280 libxc-1.0.tar.gz.1 - according to [1], %{_isa} must be added to Requires: gcc-gfortran (In reply to comment #3) > - the packaged source tarball differs from the upstream one: > $ md5sum libxc-1.0.tar.gz* > f9702268cf0015ada6a0ac34659971e6 libxc-1.0.tar.gz > 4a8ef0c8fccc695e65466ec1a8878280 libxc-1.0.tar.gz.1 /me fails. I ran spectool -g but forgot to move the tarball to the SOURCES directory. *Now* it is fixed. > - according to [1], %{_isa} must be added to Requires: gcc-gfortran Yes, I wrote the guideline proposal :) I had some reason not to place the %{_isa}, but according to my test it seems to work fine on current Fedoras. On EPEL, however, it doesn't work since the 32-bit gcc-gfortran is not available. Let's hope I got everything now :D http://theory.physics.helsinki.fi/~jzlehtol/rpms/libxc.spec http://theory.physics.helsinki.fi/~jzlehtol/rpms/libxc-1.0-3.fc14.src.rpm Thanks for the information and the fixes. The package is almost ready now. As you mentioned you want to build it for EPEL too (and as a BuildRoot field and a %clean section are present), you should also clean the buildroot at the beginning of %install. Everything else looks fine. $ rpmlint /var/lib/mock/fedora-14-x86_64/result/*.rpm libxc.src: W: spelling-error Summary(en_US) functionals -> functional, functional s, functionary libxc.src: W: spelling-error %description -l en_US functionals -> functional, functional s, functionary libxc.x86_64: W: spelling-error Summary(en_US) functionals -> functional, functional s, functionary libxc.x86_64: W: spelling-error %description -l en_US functionals -> functional, functional s, functionary libxc.x86_64: W: shared-lib-calls-exit /usr/lib64/libxc.so.0.0.9 exit.5 libxc-devel.x86_64: W: spelling-error %description -l en_US functionals -> functional, functional s, functionary libxc-devel.x86_64: W: no-documentation 4 packages and 0 specfiles checked; 0 errors, 7 warnings. All above warnings can be ignored: - spelling errors are false positive - call of exit() should be fixed upstream, no blocker here - no docs in devel is expected --------------------------------- key: [+] OK [.] OK, not applicable [X] needs work --------------------------------- [+] MUST: The package must be named according to the Package Naming Guidelines. [+] MUST: The spec file name must match the base package %{name}. [+] MUST: The package must meet the Packaging Guidelines. [+] MUST: The package must be licensed with a Fedora approved license. - LGPLv3+ [+] MUST: The License field in the package spec file must match the actual license. [+] MUST: The file containing the text of the license(s) for the package must be included in %doc. [+] MUST: The spec file must be written in American English. [+] MUST: The spec file for the package MUST be legible. [+] MUST: The sources used to build the package must match the upstream source. $ md5sum libxc-1.0.tar.gz* 4a8ef0c8fccc695e65466ec1a8878280 libxc-1.0.tar.gz 4a8ef0c8fccc695e65466ec1a8878280 libxc-1.0.tar.gz.1 [+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture. koji scratch build (f15): http://koji.fedoraproject.org/koji/taskinfo?taskID=2738376 [.] MUST: If the package does not successfully compile, build or work ... [+] MUST: All build dependencies must be listed in BuildRequires. [.] MUST: The spec file MUST handle locales properly. [+] MUST: Packages storing shared library files (not just symlinks) must call ldconfig in %post and %postun. [+] MUST: Packages must NOT bundle copies of system libraries. [.] MUST: If the package is designed to be relocatable, ... [+] MUST: A package must own all directories that it creates. [+] MUST: A Fedora package must not list a file more than once in %files. [+] MUST: Permissions on files must be set properly. [+] MUST: Each package must consistently use macros. [+] MUST: The package must contain code, or permissable content. [.] MUST: Large documentation files must go in a -doc subpackage. [+] MUST: Files in %doc must not affect the runtime of the application. [+] MUST: Header files must be in a -devel package. [.] MUST: Static libraries must be in a -static package. [+] MUST: library files that end in .so (without suffix) must go in a -devel package. [+] MUST: devel packages must require the base package using a fully versioned dependency [+] MUST: Packages must NOT contain any .la libtool archives [.] MUST: Packages containing GUI applications must include a %{name}.desktop file [+] MUST: Packages must not own files or directories already owned by other packages. [X] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}. [+] MUST: All filenames in rpm packages must be valid UTF-8. [.] SHOULD: If the source package does not include license text(s) as a separate file... [+] SHOULD: The reviewer should test that the package builds in mock. [+] SHOULD: The package should compile and build into binary rpms on all supported architectures. [+] SHOULD: The reviewer should test that the package functions as described. [+] SHOULD: If scriptlets are used, those scriptlets must be sane. [.] SHOULD: subpackages other than devel should require the base package using a fully versioned dependency. [+] SHOULD: pkgconfig(.pc) should be placed in a -devel pkg. [+] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself. [.] SHOULD: your package should contain man pages for binaries/scripts. Again, I blame the crappy upstream template I used. The ape specfile (bug #670558) I wrote myself, so at least that should be cleaner. Sorry for making you work this hard, although the missing cleaning of buildroot is not a blocker per se.. http://theory.physics.helsinki.fi/~jzlehtol/rpms/libxc.spec http://theory.physics.helsinki.fi/~jzlehtol/rpms/libxc-1.0-4.fc14.src.rpm (In reply to comment #6) > The ape specfile (bug > #670558) I wrote myself, so at least that should be cleaner. OK, looks good at first glance. If my time permits, I will have a deeper look at it in the next few days. > Sorry for making you work this hard, although the missing cleaning of buildroot > is not a blocker per se.. That's all right. I know I'm a bit nit-picking sometimes even on non-blocking items. :) ---------------- Package APPROVED ---------------- It's good to see that you have your standards :) Thanks for the review! If you have any packages waiting for a reviewer, just holler... New Package SCM Request ======================= Package Name: libxc Short Description: Library of exchange and correlation functionals to be used in DFT codes Owners: jussilehtola Branches: F-13 F-14 EL-5 EL-6 InitialCC: Git done (by process-git-requests). libxc-1.0-4.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/libxc-1.0-4.el5 libxc-1.0-4.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/libxc-1.0-4.fc14 libxc-1.0-4.fc13 has been submitted as an update for Fedora 13. https://admin.fedoraproject.org/updates/libxc-1.0-4.fc13 libxc-1.0-4.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/libxc-1.0-4.el6 libxc-1.0-4.el6 has been pushed to the Fedora EPEL 6 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update libxc'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/libxc-1.0-4.el6 libxc-1.0-4.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report. libxc-1.0-4.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report. libxc-1.0-4.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report. libxc-1.0-4.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report. Package Change Request ====================== Package Name: libxc New Branches: epel7 Owners: jussilehtola Git done (by process-git-requests). libxc-2.1.0-4.el7 has been submitted as an update for Fedora EPEL 7. https://admin.fedoraproject.org/updates/libxc-2.1.0-4.el7 libxc-2.1.0-4.el7 has been pushed to the Fedora EPEL 7 stable repository. |