Bug 724883

Summary: Review Request: biomaj - A workflow engine dedicated to data synchronization and processing
Product: [Fedora] Fedora Reporter: osallou <olivier.sallou>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED DEFERRED QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: akurtako, msuchy, package-review, pingou
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard: NotReady
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-07-21 13:15:17 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: 201449    

Description osallou 2011-07-22 07:32:07 UTC
Spec URL: http://osallou.genouest.org/biomaj/biomaj.spec
SRPM URL: http://osallou.genouest.org/biomaj/biomaj-1.1.0-1.fc15.src.rpm
Description: 
This is a first upload of the Biomaj package on Fedora and I search for a sponsor to do so.

 BioMAJ downloads remote data banks check their status and apply transformation
 workflows, with consistent state,to provide ready-to-use data to biologists and
 bioinformaticians. For example, it can transform original fasta files to blast 
 indexes. It is very flexible and post-processes can be extended very easily.

BioMAJ is already packaged on Debian.

Comment 1 Pierre-YvesChibon 2011-07-22 08:18:05 UTC
I happen to be bioinformaticien, french and sponsor, so let's have a look.

First comments at first glance:
* Please split the lines in your spec. It doesn't fit in my 19" screen. We normally don't make line longer than 80 characters
* Use %global instead of %define: http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define
* Why do you redefine %{name}, %{version} ? (first lines)
* I would prefer to see a direct link to upstream's sources. They want to see who is using their tool, but they won't be able to with the tool in debian or Fedora. So as soon as they accept to have their tool packaged, this "registration" system is rather pointless.
* this is associated with the invalid source0 that rpmlint shows
* your %clean section looks strange to me
* Some comments in %file are not necessary anymore
* on %prep, you might want to use pushd and popd rather then cd ../../....

The full output from rpmlint on all the rpm:
$ rpmlint biomaj-1.1.0-1.fc15.src.rpm 
biomaj.src: W: spelling-error %description -l en_US workflows -> work flows, work-flows, workloads
biomaj.src: W: spelling-error %description -l en_US bioinformaticians -> transformations
biomaj.src: W: spelling-error %description -l en_US fasta -> fasts, fast, pasta
biomaj.src: W: invalid-license BSD with attribution
biomaj.src: W: strange-permission Deb2Fedora.sh 0755L
biomaj.src:24: W: macro-in-comment %{name}
biomaj.src:24: W: macro-in-comment %{name}
biomaj.src:24: W: macro-in-comment %{version}
biomaj.src:74: W: macro-in-comment %__cp
biomaj.src:82: W: macro-in-comment %{nonConfigFiles}
biomaj.src:105: W: macro-in-comment %dir
biomaj.src:106: W: macro-in-comment %dir
biomaj.src:107: W: macro-in-comment %dir
biomaj.src:108: W: macro-in-comment %dir
biomaj.src: W: invalid-url Source0: /biomaj-1.1.0.tar.gz
1 packages and 0 specfiles checked; 0 errors, 15 warnings.

$ rpmlint  /home/pierrey/rpmbuild/RPMS/noarch/biomaj-1.1.0-1.fc15.noarch.rpm
biomaj.noarch: W: spelling-error %description -l en_US bioinformaticians -> transformations
biomaj.noarch: W: spelling-error %description -l en_US fasta -> fasts, fast, pasta
biomaj.noarch: W: invalid-license BSD with attribution
biomaj.noarch: W: hidden-file-or-dir /usr/share/biomaj/lib/vendor/commons-cli2/src/test/data/.hidden.txt
biomaj.noarch: W: log-files-without-logrotate /var/log/biomaj
biomaj.noarch: W: class-path-in-manifest /usr/share/biomaj/lib/biomaj.jar
1 packages and 0 specfiles checked; 0 errors, 6 warnings.

I believe some can be fixed.

Comment 2 osallou 2011-07-22 09:18:53 UTC
I have updated spec and source according to remarks. Files are updated on remote site (same url)
All warning are not removed, cause some are software intrisic or spelling unknown....

For Source, upstream archive is not named according expected format. I cannot really change this, (quite a mess to maintain different source package for different distributions). So I define, as proposed in Fedora policy, the file name. URL to use and script to apply are defined in comments and Source1

Thanks for review

Comment 3 Pierre-YvesChibon 2011-07-22 09:42:57 UTC
(In reply to comment #2)
> I have updated spec and source according to remarks. Files are updated on
> remote site (same url)

We normally update the %changelog to reflect the changes made to the spec file.

> For Source, upstream archive is not named according expected format. I cannot
> really change this, (quite a mess to maintain different source package for
> different distributions). So I define, as proposed in Fedora policy, the file
> name. URL to use and script to apply are defined in comments and Source1

Could you elaborate on this ? What is the format ?
You don't have to maintain source package for each distro, just one with the correct installer (ant/pom/setup.py/makefile/install.sh), then it is up to the packager.

Also, looking at the sources quickly, you seem to embed apache-commons-cli instead of using the one on the system. This is not allowed (bundle library).

Comment 4 osallou 2011-07-22 10:05:00 UTC
Hi,
for changelog, I though that changes were made only after first upload.

For source, the source on our server is named biomaj_1.1.0.orig.tar.gz, and unzipped directory is named biomaj_1.1.0.
In SPEC , it expects biomaj-1.1.0 (dash, not underscore): when building rpm it takes the source file, unzip it and goes to biomaj-1.1.0, so it fails. So I have to rename directory to biomaj-1.1.0 using Def2Fedora script.

At least, regarding commons-cli, this is not exact. We embed commons-cli2 which is not packaged (and will never be as project is orphan). Future version will remove this dependency to use commons-cli from system.

Comment 5 Pierre-YvesChibon 2011-07-22 10:12:57 UTC
(In reply to comment #4)
> Hi,
> for changelog, I though that changes were made only after first upload.

All the changes on the spec :)

> For source, the source on our server is named biomaj_1.1.0.orig.tar.gz, and
> unzipped directory is named biomaj_1.1.0.
> In SPEC , it expects biomaj-1.1.0 (dash, not underscore): when building rpm it
> takes the source file, unzip it and goes to biomaj-1.1.0, so it fails. So I
> have to rename directory to biomaj-1.1.0 using Def2Fedora script.

This is not a problem, just use %prep -n %{name}_%{version} and you are done.

> At least, regarding commons-cli, this is not exact. We embed commons-cli2 which
> is not packaged (and will never be as project is orphan). Future version will
> remove this dependency to use commons-cli from system.

I am not sure it is possible/acceptable.

Comment 6 osallou 2011-07-22 10:53:08 UTC
Hi,
I have updated code on web site (with changelog).
I fixed the Source issue help with your command to avoid Deb2Fedora stuff.

regarding commons-cli2, this is what we do on Debian. The apache-commons-cli2 is Apache2 license, and can as such be delivered with the code. It is used as an internal library, with original source code. The library is built with the rest of the code at package built.

Thanks

Comment 7 Pierre-YvesChibon 2011-07-22 11:05:47 UTC
The changelog format is wrong.
You need to increase the release and make a proper entry in the changelog.

Comment 8 osallou 2011-07-22 11:14:16 UTC
updated to release 2 with according src.rpm. (with spec file)

Comment 9 Alexander Kurtakov 2011-07-22 11:16:17 UTC
In Fedora we are more strict about bundling libraries - in short it's forbidden
unless you receive an exception from FPC (Fedora Packaging Committee). See
https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries for details and
https://fedoraproject.org/wiki/Packaging:Treatment_Of_Bundled_Libraries for
details.
Bundling libraries is doing so much harm that it should be really forbidden
without exceptions if you ask me. If you intend to remove the bundled library
and to use the system one please do it sooner.

Comment 10 osallou 2011-07-22 11:36:19 UTC
So I will wait for next release of the software. I cannot simply fix current version to so.
Can I keep this request open in the meanwhile and ping again once new release is available?

Comment 11 Pierre-YvesChibon 2011-07-22 11:44:14 UTC
Of course, no problem :)

In the mean while, normally to become fedora packager we ask a candidate to show their understanding of the guidelines.
The easiest way to do that is to perform some un-official review (since you are not a packager, you cannot approve a package) and eventually submit another review request.

Please mention on this ticket which bugs you have reviewed and if you submit another review request.

Comment 12 Jason Tibbitts 2012-06-29 17:59:04 UTC
So, it's been nearly another year.  Since this still isn't ready for review I'm going to mark it as such; please clear the whiteboard if you provide something you'd like to have reviewed.

Comment 13 Alexander Kurtakov 2014-06-25 08:57:20 UTC
As two more years passed. Is there any planned action or it's dead review?

Comment 14 Miroslav Suchý 2015-07-21 13:15:17 UTC
Closing due long inactivity. Feel free to reopen if you want to continue.