Bug 517849 - Review Request: mpiwrappers - Environment module wrappers for MPI packages in RHEL
Summary: Review Request: mpiwrappers - Environment module wrappers for MPI packages in...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rafael Aquini
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-08-17 13:38 UTC by Susi Lehtola
Modified: 2011-01-24 21:07 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2011-01-24 21:07:56 UTC
Type: ---
Embargoed:
aquini: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Susi Lehtola 2009-08-17 13:38:33 UTC
Spec URL:
http://theory.physics.helsinki.fi/~jzlehtol/rpms/mpiwrappers.spec

SRPM URL:
http://theory.physics.helsinki.fi/~jzlehtol/rpms/mpiwrappers-1-1.fc11.src.rpm

Description:
This package provides a compatibility layer with the use of environment modules
for the MPI compilers in RHEL to ease packaging of MPI software in Fedora EPEL.

**

See
http://www.fedoraproject.org/wiki/PackagingDrafts/MPI#EPEL

Comment 1 Susi Lehtola 2009-08-17 13:43:00 UTC
CC Doug, you might want to take a look at this.

**

I'm not totally sure if support for mvapich{,2} should be added, too. As they aren't available on all architectures building packages against them would require the use of conditionals in spec files using them. Besides, I don't think rpm on RHEL 4 and 5 can't cope with subpackages that don't have the same build arch as the main package.

Comment 2 Rafael Aquini 2010-08-20 01:27:39 UTC
It's been almost a year with no progress; This review should be closed soon if there is no response, shouldn't it?

Comment 3 Susi Lehtola 2010-08-20 08:10:04 UTC
No Rafael, it's the other way around - only if there has been no reply from the submitter.

Comment 4 Rafael Aquini 2010-08-20 11:59:42 UTC
Jussi,

This is a janitorial work on Fedora Package Review queues, 
-- http://fedoraproject.org/PackageReviewStatus/ -- in order to identify and close stalled reviews. 

I'm just following this policy:
http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews

Please, consider trying a swap review request to get this work reviewed
http://fedoraproject.org/wiki/Package_Review_Process#Contributor

Regards

Comment 5 Susi Lehtola 2010-08-20 12:16:12 UTC
Well, obviously I'm responsive. Furthermore, the reviewer part only applies to someone who has assigned her/himself to a review and then went missing, so that s/he can be taken off the review and the review bug be returned to an available state.

A stalled review is something where the submitter is not responding. Instead of closing down active bugs to shorten the queue, you can actually do something helpful by doing reviews yourself...

Comment 6 Mamoru TASAKA 2010-08-20 12:36:58 UTC
Rafael, you can review this bug if you want.

Comment 7 Rafael Aquini 2010-08-20 12:41:46 UTC
Ok, 

so I'm assigning this review to myself, and soon I'll be posting a formal review to your package.

Regards

Comment 8 Rafael Aquini 2010-08-21 14:47:53 UTC
Jussi,

Please consider the following review:

Good:
* Spec file naming follows package naming
* Spec is legible and American English
* No locale files
* No shared libraries
* No bundled libraries
* Not relocatable
* Default permissions are set
* Macros used consistently 
* Package is code
* No large documentation
* No header files
* No static libraries
* Not a GUI application
* Does not own files or directories from other packages
* All filenames are utf8

NEEDSWORK
[1] rpmlint complaints:
   mpiwrappers.src: W: no-url-tag
   mpiwrappers.src:83: W: macro-in-comment %{buildroot}
   mpiwrappers.src: W: no-cleaning-of-buildroot %clean
   mpiwrappers.src:30: W: mixed-use-of-spaces-and-tabs (spaces: line 30, tab: line 9)
   mpiwrappers.x86_64: W: no-url-tag
   mpiwrappers.x86_64: E: no-binary
   mpiwrappers.x86_64: W: no-documentation
   mpiwrappers-lam.x86_64: E: devel-dependency lam-devel
   mpiwrappers-lam.x86_64: W: no-url-tag
   mpiwrappers-lam.x86_64: W: no-documentation
   mpiwrappers-lam.x86_64: W: non-conffile-in-etc /etc/modulefiles/lam-x86_64
   mpiwrappers-lam.x86_64: W: non-conffile-in-etc /etc/rpm/macros.lam-x86_64
   mpiwrappers-openmpi.x86_64: E: devel-dependency openmpi-devel
   mpiwrappers-openmpi.x86_64: W: no-url-tag
   mpiwrappers-openmpi.x86_64: W: no-documentation
   mpiwrappers-openmpi.x86_64: W: non-conffile-in-etc /etc/rpm/macros.openmpi-x86_64
   mpiwrappers-openmpi.x86_64: W: non-conffile-in-etc /etc/modulefiles/openmpi-x86_64
   4 packages and 0 specfiles checked; 3 errors, 14 warnings.

[2] Package name must follow Naming Guidelines, which states the name should match the upstream tarball / project name;

[3] Source within SRPM must match the upstream source;


Best regards

Comment 9 Susi Lehtola 2010-08-21 15:19:40 UTC
(In reply to comment #8)
> NEEDSWORK
> [1] rpmlint complaints:
>    mpiwrappers.src: W: no-url-tag

Because this is a Fedora-specific wrapper package heavily relying on Fedora specifics.

>    mpiwrappers.src:83: W: macro-in-comment %{buildroot}
>    mpiwrappers.src: W: no-cleaning-of-buildroot %clean

Hmm, these are the same bug. Fixed. I wonder what was my reason. Fixed in 1-2.

>    mpiwrappers.src:30: W: mixed-use-of-spaces-and-tabs (spaces: line 30, tab:
> line 9)

Fixed in 1-2.

>    mpiwrappers.x86_64: E: no-binary

Still, this is an architecture specific package due to file ownership (%{_libdir}). I would add architecture dependent requires, but they're not provided in EPEL 4 and 5 since RPM is too old.

>    mpiwrappers.x86_64: W: no-documentation

None exists.

>    mpiwrappers-lam.x86_64: E: devel-dependency lam-devel

The package is a wrapper, so this is kind of obligatory to have.

>    mpiwrappers-lam.x86_64: W: non-conffile-in-etc /etc/modulefiles/lam-x86_64
>    mpiwrappers-lam.x86_64: W: non-conffile-in-etc /etc/rpm/macros.lam-x86_64

These are not config files.

> [2] Package name must follow Naming Guidelines, which states the name should
> match the upstream tarball / project name;

There is no upstream project, since this is a Fedora specific package. See for instance kde-filesystem.
 
> [3] Source within SRPM must match the upstream source;

Not applicable.


The rest were duplicates of these.

Comment 10 Rafael Aquini 2010-08-24 23:51:24 UTC
Jussi,

The "we are upstream" exception was removed a while ago, what makes this package fall into a grey area, opened to interpretation. 
https://fedoraproject.org/w/index.php?title=Packaging%3ASourceURL&diff=119507&oldid=96909 

As I was not willing to have my mistakes messing with your work, I did some homework in order to have a better understanding of this particular case. Also, I had a talk to my sponsor who have helped me to reach the following understanding:
[1] this is a trivial but atypical package -- what makes difficult its review;
[2] its nature is only useful inside Fedora -- demanding a hosted project, just to fit the guidelines, it would be an unnecessary burden;

So, not only do I ended up agreeing with most of your considerations in Comment 9 but I'm approving this package.

Please, just remember to fix the following warnings, before import this package:
>   mpiwrappers.src:83: W: macro-in-comment %{buildroot}
>   mpiwrappers.src: W: no-cleaning-of-buildroot %clean
>   mpiwrappers.src:30: W: mixed-use-of-spaces-and-tabs (spaces: line 30, tab:
line 9)

Best regards,


APPROVED

Comment 11 Susi Lehtola 2010-08-25 16:00:41 UTC
Thanks a lot for the review!

New Package SCM Request
=======================
Package Name: mpiwrappers
Short Description: Environment module wrappers for MPI packages in RHEL
Owners: jussilehtola
Branches: EL-4 EL-5
InitialCC:

Comment 12 Kevin Fenzi 2010-08-25 17:30:24 UTC
Git done (by process-git-requests).

Comment 13 Fedora Update System 2010-10-09 15:35:21 UTC
mpiwrappers-1-3.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/mpiwrappers-1-3.el5

Comment 14 Fedora Update System 2010-10-09 15:35:29 UTC
mpiwrappers-1-3.el4 has been submitted as an update for Fedora EPEL 4.
https://admin.fedoraproject.org/updates/mpiwrappers-1-3.el4

Comment 15 Fedora Update System 2010-10-10 18:01:33 UTC
mpiwrappers-1-3.el5 has been pushed to the Fedora EPEL 5 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 mpiwrappers'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/mpiwrappers-1-3.el5


Note You need to log in before you can comment on or make changes to this bug.