[Bug 866325] Review Request: ugene - genome analysis suite

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

 



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

--- Comment #9 from Peter Lemenkov <lemenkov@xxxxxxxxx> ---
Sorry for the delay - I was busy with a random real-world proceedings.

For those who'd like to see a full context - this package was included in
Fedora for some time but unfortunately was retired after Yegor Yefremov ceased
his activity within our community (the pity of it!)

This package is (AFAIK) basically a continuation of Yegor's efforts. It is
almost in the same shape that it was when Yegor left it. I see some somewhat
relatively large issues which (if I were more partial towards the Fedora
Guidelines) could be a stopper. I'd like to revive package first and address
these issues (which slept through previous review) second.

So here is my official

REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+ rpmlint is NOT silent but almost all these messages are either a complaints
about non-executable-script / wrong-script-interpreter (false positives
triggered by UGENE file format), incorrect-fsf-address (triggered by the
outdated licensing header in some files), spelling false positives, and
messages triggered by a non-standard layout (usage of %{_libdir}/%{name}).

The latter I suppose should be improved/fixed upstream (I mean by you and your
fellow colleagues) and I really won't insist on this now.

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format
%{name}.spec.
+ The package is licensed with a Fedora approved license and meets the
Licensing Guidelines.
+ The License field in the package spec file matches the actual license.

+/- The file, containing the text of the license(s) for the package, is
included in %doc. Please also include LICENSE.3rd_party for now.

+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, match the upstream source, as provided
in the spec URL.

sulaco ~/rpmbuild/SOURCES: sha256sum ugene-1.11.2.tar.gz*
ba8c25a03eebd60730f5b9b5e08cd0e6d0cc1d5a9f965bc5870a5431c1c24296 
ugene-1.11.2.tar.gz
ba8c25a03eebd60730f5b9b5e08cd0e6d0cc1d5a9f965bc5870a5431c1c24296 
ugene-1.11.2.tar.gz.1
sulaco ~/rpmbuild/SOURCES: 

+ The package successfully compiles and builds into binary rpms on at least one
primary architecture. See koji link above.
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
0 No shared library files in some of the dynamic linker's default paths.

+/- The package shouldn't bundle copies of system libraries (CANNOT actually
but in this very case I wouldn't insist assuming that you will fix/improve
this). See my questions below.

0 The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files
listings.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the
application.
0 No C/C++ header files.

+/- The package contains static libraries - they should be removed. If they are
required for some operations (I really doubt that) they should be stored in a
-static package. I personally advise you to remove them entirely - they do
looks like a leftover.

0 No pkgconfig(.pc) files.

+/- The library files that end in .so (without suffix) should be stored in a
-devel package. Well, I do believe that they should be simply removed (or even
built w/o versioned so-names).

0 No devel sub-package.
+ The package does NOT contain any .la libtool archives.
+ The package includes a %{name}.desktop file, and this file is properly
installed (and validated with desktop-file-validate) in the %install section.
+ The package does not own files or directories already owned by other
packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.


Almost finished. So far I've got the following questions/suggestions/requests:


* (Until we drop all bundled libs) please include LICENSE.3rd_party.
* What "UGENE_EXCLUDE_LIST_ENABLED" switch does? Are there any other
compile-time switches?
* You may drop %defattr line from the %files section - it's no longer needed
(even in EL5)
* Do you have plans to provide builds for EL5 or EL6? If no, then you may
remove %clean section entirely, "rm -rf %{buildroot}" from %install and kill
Buildroot directive as well.
* I really felt sad when I saw a bunch of bundled libraries residing within
your tarball. You really should drop them and compile against system-wide ones.
Is it possible? Did you patch them?
* The libraries - why not to install them into %{_libdir}? You will get rid of
all these LD_LIBRARY_PATH shell magic? I understand - the only application
which uses them is ugene itself, so someone might argue that it should go into
named subdirectory rather than into system-wide directory.
* Remove (or explain why it's impossible) two static libs.

-- 
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



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