Bug 481717
Summary: | Review Request: ugene - genome analysis suite | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Ivan Efremov <iefremov> | ||||||
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> | ||||||
Status: | CLOSED DUPLICATE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | low | ||||||||
Version: | rawhide | CC: | fedora-package-review, lemenkov, lukasim, mail, maurizio.antillon, notting, yalgaer | ||||||
Target Milestone: | --- | Flags: | mtasaka:
fedora-review+
kevin: fedora-cvs+ |
||||||
Target Release: | --- | ||||||||
Hardware: | All | ||||||||
OS: | Linux | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | 1.4.1-3.fc11 | Doc Type: | Bug Fix | ||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2009-05-05 08:34:18 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: | |||||||||
Attachments: |
|
Description
Ivan Efremov
2009-01-27 10:57:40 UTC
Note: this is my first package in fedora so I need a sponsorship Some quick comments on your spec file - Please place the changelog section at the end of the file. It's not wrong just unusual. - Please add the dist tag to Release -> 'Release: 1%{?dist}' - You don't need to define macros at the beginning of the file. 'Name:' and 'Version:' are already macros. But you must use macros in the %files section https://fedoraproject.org/wiki/PackageMaintainers/CreatingPackageHowTo#Macros - Please preserve the time stamps, use ' make install DESTDIR=%{buildroot} INSTALL="install -p" ' if possible instead of ' make install INSTALL_ROOT=%{buildroot} ' - This application has a GUI. You need to install the .desktop file. https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files - COPYRIGHT and LICENSE must be added as %doc in the %files section. Maybe adding the sample files (out of data/samples in the source) is also a good idea. - There are 'unowned directories' in your %files section. https://fedoraproject.org/wiki/Packaging/UnownedDirectories (In reply to comment #2) > - Please place the changelog section at the end of the file. It's not wrong > just unusual. done > - Please add the dist tag to Release -> 'Release: 1%{?dist}' done > - You don't need to define macros at the beginning of the file. 'Name:' and > 'Version:' are already macros. But you must use macros in the %files section > https://fedoraproject.org/wiki/PackageMaintainers/CreatingPackageHowTo#Macros fixed > - Please preserve the time stamps, use ' make install DESTDIR=%{buildroot} > INSTALL="install -p" ' if possible instead of ' make install > INSTALL_ROOT=%{buildroot} ' Since Makefile is generated by qmake it has some specific features: 1) it uses INSTALL_ROOT instead of DESTDIR 2) it uses INSTALL_FILE, INSTALL_PROGRAM etc. instead of INSTALL 3) these INSTALL_* variables already have '-p' option so, there is no problem with time stapms > - This application has a GUI. You need to install the .desktop file. > https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files desktop file is part of the upstream tarball and is installed during 'make install' > - COPYRIGHT and LICENSE must be added as %doc in the %files section. Maybe > adding the sample files (out of data/samples in the source) is also a good > idea. done. Samples are installed into /usr/share/ugene/data during 'make install' > - There are 'unowned directories' in your %files section. > https://fedoraproject.org/wiki/Packaging/UnownedDirectories fixed New links: SPEC: http://ugene.unipro.ru/downloads/ugene.spec SRPM: http://ugene.unipro.ru/downloads/ugene-1.3.2-1.fc9.src.rpm (In reply to comment #3) > (In reply to comment #2) > > - Please place the changelog section at the end of the file. It's not wrong > > just unusual. > > done Again 'changelog', if you make any changes on the spec file, you must bump the release of the package. More details at https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs . > > > - This application has a GUI. You need to install the .desktop file. > > https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files > > desktop file is part of the upstream tarball and is installed during 'make > install' If 'make install' is taking care of the .desktop file installation, then 'validate' is the right way if the categories match the 'Desktop Entry Specification'. https://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage Without any further investigations I think that 'Requires: qt' is not necessary, for me it looks like that rpm will pick this automatically. (In reply to comment #3) > Again 'changelog', if you make any changes on the spec file, you must bump the > release of the package. More details at > https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs . fixed > If 'make install' is taking care of the .desktop file installation, then > 'validate' is the right way if the categories match the 'Desktop Entry > Specification'. done > > Without any further investigations I think that 'Requires: qt' is not > necessary, for me it looks like that rpm will pick this automatically. It is necessaty since RPM won't pick versions automatically - ugene needs particular qt version. New links: SPEC: http://ugene.unipro.ru/downloads/ugene.spec SRPM: http://ugene.unipro.ru/downloads/ugene-1.3.2-2.fc9.src.rpm In fact, there is also a serious question about using macros in '%files' section. UGENE build system is based on makefiles generated with qmake. The point is that qmake has not command line options for specifying different installation paths for different files (something like --bindir, --libdir etc.) - these paths are preconfigured in a configuration file (src/ugene_globals.pri). As long as default rpm macros values are identical to preconfigured paths everything goes OK, but when they are not - rpm build will fail. In fact, the main problem will be with %{_libdir} since on 64bit platforms it may be changed from /usr/lib to /usr/lib64. Is there any workaround for this problem (e.g. using hardoced libdir path)? Assigning. Created attachment 336109 [details] Patch to compile with g++44 Some notes * Latest - The latest version is 1.4.0 (and it seems 1.4.1 will be released soon) * License tag - src/libs_3rdparty/qtbindings_core/src/qtscriptconcurrent.h is under GPLv2, which renders libqtbindings_core.so to be GPLv2. So the license tag should be "GPLv2+ and GPLv2". Also please write some comments about this on the spec file, see: https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios * Explicit version dependencies - We now request comments on the spec file when you write explicit version dependencies for rpms which are automatically pulled in by the automated detection of library dependencies by rpmbuild itself, see: https://fedoraproject.org/wiki/PackagingDrafts/ExplicitRequires (this is still under "Drafts" category, but actually this is already approved by FESCo) * Multiple Source entry - There are 2 Source entry. Use full URL one: https://fedoraproject.org/wiki/Packaging/SourceURL * 64 bit issue (In reply to comment #6) > As long as default rpm macros values are identical to preconfigured paths > everything goes OK, but when they are not - rpm build will fail. In fact, the > main problem will be with %{_libdir} since on 64bit platforms it may be changed > from /usr/lib to /usr/lib64. > > Is there any workaround for this problem (e.g. using hardoced libdir path)? - For this package ----------------------------------------------------------------- sed -i -e '/UGENE_INSTALL_DIR/s|/lib/|/%{_lib}/|' src/ugene_globals.pri ----------------------------------------------------------------- works. * g++44 compilation issue - The attached patch is needed to build this package with g++ 4.4 (please check if this is already fixed in the latest version or not) * Functionality - By the way: ----------------------------------------------------------------- $ ugene Translation not found: transl_ja_JP Translation not found: transl_en No translations found, exiting ----------------------------------------------------------------- ugene won't launch on my system (note that I am using GNOME) ping? (In reply to comment #9) > ping? pong. I'll post ugene 1.4.1 during the next week. Thanks for reviewing and comments. 1) update to v 1.4.1 2) linencse tag fixed 3) version dependecies are necessary - comment added 4) extra source entry removed 5) I fixed 64-bit issue within ugene build system - no workarounds needed now 6) gcc 4.4 compilation issues added to the upstream 7) it's a quite serious issue - please provide a build log (compilation step may be skipped, essential steps are qmake'ing and installing) New links: SPEC: http://ugene.unipro.ru/downloads/ugene.spec SRPM: http://ugene.unipro.ru/downloads/ugene-1.4.1-1.fc10.src.rpm Created attachment 338581 [details] Patch to compile 1.4.1 with g++ 4.4 Some notes for 1.4.1-1: * Licensing - Now also files under src/plugins_3rdparty/script_debuger/src/qtscriptdebug/ are under GPLv2 (not under GPLv2+). (The license tag is still okay with "GPLv2+ and GPLv2") * build failure - Actually your srpm does not build. ! On Fedora Qt4 "qmake" is renamed to %_bindir/qmake-qt4 (or you can use %_qt4_bindir macro, which is /usr/lib/qt4/bin/qmake on i586) and Qt4 "lrelease" is renamed to %_bindir/lrelease-qt4. So you have to modify src/qtranslate.cmd, otherwise as I said before trying to launch ugene causes: ------------------------------------------------------- Translation not found: transl_ja_JP Translation not found: transl_en No translations found, exiting -------------------------------------------------------- Currently (after changing "qmake" to "qmake-qt4") build.log says: -------------------------------------------------------- 42 + qmake-qt4 -r INSTALL_BINDIR=/usr/bin INSTALL_LIBDIR=/usr/lib INSTALL_DATADIR=/usr/share INSTALL_MANDIR=/usr/share/man 43 ./qtranslate.cmd: line 1: lrelease: command not found 44 ./qtranslate.cmd: line 2: lrelease: command not found 45 cp: cannot stat `_debug/transl_en.qm': No such file or directory 46 cp: cannot stat `_debug/transl_ru.qm': No such file or directory -------------------------------------------------------- ! And the attached patch is needed for g++ 4.4 . * Desktop file - Installed desktop file must be treated by "desktop-file-install" or so: https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files * Use of glob in %files - %files entry -------------------------------------------------------- %files %{_libdir}/* -------------------------------------------------------- is not acceptable - Because find-debuginfo.sh installs files for debugging into /usr/lib/debug (on i586) and %_libdir/* contains this directory. So some debuginfo files are also installed in ugene binary rpms, not only ugene-debuginfo rpm. This must be changed to %{_libdir}/ugene or so. 1) license issue fixed 2) qmake and lrelease issues fixed 3) g++ 4.4 issue fixed 4) according to the document we must not use 'desktop-file-install'. Instead I use 'desktop-file-validate'. https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files 5) libdir issue fixed New links: SPEC: http://ugene.unipro.ru/downloads/ugene.spec SRPM: http://ugene.unipro.ru/downloads/ugene-1.4.1-1.fc10.src.rpm Please change the EVR (Epoch-Version-Release) of your srpm every time you modify your srpm to avoid confusion. New links: SPEC: http://ugene.unipro.ru/downloads/ugene.spec SRPM: http://ugene.unipro.ru/downloads/ugene-1.4.1-2.fc10.src.rpm Well, * For source tarball: --------------------------------------------------------- 4577346 2009-03-25 18:22 ugene-1.4.1.tar.gz 4582420 2009-03-21 09:13 ugene-1.4.1-1.fc10.src/ugene-1.4.1.tar.gz 4583899 2009-03-23 02:02 ugene-1.4.1-2.fc10.src/ugene-1.4.1.tar.gz --------------------------------------------------------- - The tarball included in your srpm differs from what I can download from the URL written in your spec file. You must use the contain the tarball the same as what is provided on the URL * Some miscs: - %{_bindir}/qmake-qt4 can just be qmake-qt4. - Please also add some comments in %changelog when you modify your spec file. Then: ------------------------------------------------------------- NOTE: Before being sponsored: This package will be accepted with another few work. But before I accept this package, someone (I am a candidate) must sponsor you. Once you are sponsored, you have the right to review other submitters' review requests and approve the packages formally. For this reason, the person who want to be sponsored (like you) are required to "show that you have an understanding of the process and of the packaging guidelines" as is described on : http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored Usually there are two ways to show this. A. submit other review requests with enough quality. B. Do a "pre-review" of other person's review request (at the time you are not sponsored, you cannot do a formal review) When you have submitted a new review request or have pre-reviewed other person's review request, please write the bug number on this bug report so that I can check your comments or review request. Fedora package collection review requests which are waiting for someone to review can be checked on my wiki page: http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets (Check "No one is reviewing") Review guidelines are described mainly on: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ------------------------------------------------------------ 1) I updated the tarball (just added gcc 4.4 fixes, no version bumping) 2) i reviewed the following: bug 492927 https://bugzilla.redhat.com/show_bug.cgi?id=492927 (In reply to comment #17) > 1) I updated the tarball (just added gcc 4.4 fixes, no version bumping) Does this mean that you are one of the upstream developers? Exactly. Well, your pre-review seems good for initial comments. Now I approve this package. ------------------------------------------------------ This package (ugene) is APPROVED by mtasaka ------------------------------------------------------ Please follow the procedure written on: http://fedoraproject.org/wiki/PackageMaintainers/Join from "Get a Fedora Account". After you request for sponsorship a mail will be sent to sponsor members automatically (which is invisible for you) which notifies that you need a sponsor. After that, please also write on this bug for confirmation that you requested for sponsorship and your FAS (Fedora Account System) name. Then I will sponsor you. If you want to import this package into Fedora 9/10/11, you also have to look at http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT (after once you rebuilt this package on koji Fedora rebuilding system). If you have questions, please ask me. I requested for sponsorship, FAS name iefremov. Okay, now I am sponsoring you. Please follow "Join" wiki again. Note: F-11 development freeze already came, so now on CVS admin procedure [1] devel branch points to F-12. If you want to import this package into F-12, F-11, and F-10, for example, please write "F-11 F-10" as "Branches" in "New Package CVS Request". [1] http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure New Package CVS Request ======================= Package Name: ugene Short Description: Integrated bioinformatics toolkit Owners: iefremov Branches: F-11 F-10 cvs done. ping? I'll update CVS branch during next 2-3 days ugene-1.4.1-3.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/ugene-1.4.1-3.fc11 ugene-1.4.1-2.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/ugene-1.4.1-2.fc10 Okay, thanks. ugene-1.4.1-2.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. ugene-1.4.1-3.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report. Please (re)review and readd ugene package to the collection. Spec URL: http://ugene.unipro.ru/downloads/ugene.spec SRPM URL: http://ugene.unipro.ru/downloads/ugene-1.11.2-1.fc17.src.rpm Description: Review is needed for UGENE - complex bioinformatics toolkit based on Qt. Unipro UGENE is a cross-platform visual environment for DNA and protein sequence analysis. UGENE integrates the most important bioinformatics computational algorithms and provides an easy-to-use GUI for performing complex analysis of the genomic data. One of the main features of UGENE is a designer for custom bioinformatics workflows. *** This bug has been marked as a duplicate of bug 866325 *** |