Bug 481717

Summary: Review Request: ugene - genome analysis suite
Product: [Fedora] Fedora Reporter: Ivan Efremov <iefremov>
Component: Package ReviewAssignee: Mamoru TASAKA <mtasaka>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: 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 Flags
Patch to compile with g++44
none
Patch to compile 1.4.1 with g++ 4.4 none

Description Ivan Efremov 2009-01-27 10:57:40 UTC
Spec URL: http://ugene.unipro.ru/downloads/ugene.spec
SRPM URL: http://ugene.unipro.ru/downloads/ugene-1.3.2-1.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.

Comment 1 Ivan Efremov 2009-01-27 11:01:28 UTC
Note: this is my first package in fedora so I need a sponsorship

Comment 2 Fabian Affolter 2009-01-27 12:14:31 UTC
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

Comment 3 Ivan Efremov 2009-01-28 12:27:01 UTC
(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

Comment 4 Fabian Affolter 2009-01-29 15:46:32 UTC
(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.

Comment 5 Ivan Efremov 2009-01-30 11:03:56 UTC
(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

Comment 6 Ivan Efremov 2009-02-02 08:32:42 UTC
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)?

Comment 7 Mamoru TASAKA 2009-03-20 19:56:09 UTC
Assigning.

Comment 8 Mamoru TASAKA 2009-03-20 19:57:37 UTC
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)

Comment 9 Mamoru TASAKA 2009-04-04 16:38:58 UTC
ping?

Comment 10 Ivan Efremov 2009-04-05 06:38:42 UTC
(In reply to comment #9)
> ping?  

pong.
I'll post ugene 1.4.1 during the next week. 
Thanks for reviewing and comments.

Comment 11 Ivan Efremov 2009-04-06 14:21:47 UTC
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

Comment 12 Mamoru TASAKA 2009-04-07 19:18:29 UTC
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.

Comment 13 Ivan Efremov 2009-04-08 10:28:35 UTC
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

Comment 14 Mamoru TASAKA 2009-04-08 10:48:07 UTC
Please change the EVR (Epoch-Version-Release) of your srpm
every time you modify your srpm to avoid confusion.

Comment 16 Mamoru TASAKA 2009-04-08 17:18:26 UTC
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
------------------------------------------------------------

Comment 17 Ivan Efremov 2009-04-14 08:28:58 UTC
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

Comment 18 Mamoru TASAKA 2009-04-14 08:57:04 UTC
(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?

Comment 19 Ivan Efremov 2009-04-14 09:22:30 UTC
Exactly.

Comment 20 Mamoru TASAKA 2009-04-14 16:07:05 UTC
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.

Comment 21 Ivan Efremov 2009-04-15 07:34:57 UTC
I requested for sponsorship, FAS name iefremov.

Comment 22 Mamoru TASAKA 2009-04-15 15:19:03 UTC
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

Comment 23 Ivan Efremov 2009-04-20 07:38:36 UTC
New Package CVS Request
=======================
Package Name: ugene
Short Description: Integrated bioinformatics toolkit
Owners: iefremov
Branches: F-11 F-10

Comment 24 Kevin Fenzi 2009-04-21 19:45:04 UTC
cvs done.

Comment 25 Mamoru TASAKA 2009-04-30 16:53:41 UTC
ping?

Comment 26 Ivan Efremov 2009-05-04 05:22:08 UTC
I'll update CVS branch during next 2-3 days

Comment 27 Fedora Update System 2009-05-05 07:56:11 UTC
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

Comment 28 Fedora Update System 2009-05-05 07:58:02 UTC
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

Comment 29 Mamoru TASAKA 2009-05-05 08:34:18 UTC
Okay, thanks.

Comment 30 Fedora Update System 2009-05-06 23:24:20 UTC
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.

Comment 31 Fedora Update System 2009-05-09 04:10:08 UTC
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.

Comment 32 Yuliya Algaer 2012-10-11 06:50:14 UTC
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.

Comment 33 Peter Lemenkov 2013-08-07 12:06:32 UTC

*** This bug has been marked as a duplicate of bug 866325 ***