[Bug 1333531] Review Request: opa-ff - OPA Fast Fabric Tools

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

 



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



--- Comment #12 from Michal Schmidt <mschmidt@xxxxxxxxxx> ---
So I checked out the tag, looking for a spec file.
$ find . -name '*.spec'
./Dsap/dsap.spec
./opasadb/opasadb.spec
./OpenIb_Host/mpi-apps.spec

What are these spec files? Are they distinct packages that we need to add to
Fedora (and RHEL) too?

Where is the spec file for this review?
I found a template opa-ff.spec.in. And there is a script to fill it in. Let's
run it:
$ ./update_opa-ff.spec.sh opa-ff.spec.in opa.spec

That did not work. opa.spec is 100% identical to opa-ff.spec.in, so it's not a
syntactically valid spec file ("error: line 13: Unknown tag: __RPM_DEBUG").
Looking into update_opa-ff.spec.sh... I see it assumes I'm using either RHEL or
SLES. I am using neither, so let's hack it:

diff --git a/update_opa-ff.spec.sh b/update_opa-ff.spec.sh
index d1da2a2ef6..8cc3a594f7 100755
--- a/update_opa-ff.spec.sh
+++ b/update_opa-ff.spec.sh
@@ -37,7 +37,7 @@ then
        exit 1
 fi

-id=$(grep ^ID= /etc/os-release | cut -f2 -d\")
+id=rhel  #$(grep ^ID= /etc/os-release | cut -f2 -d\")
 versionid=$(grep ^VERSION_ID= /etc/os-release | cut -f2 -d\")

 from=$1

...and try again:
$ ./update_opa-ff.spec.sh opa-ff.spec.in opa.spec

This is the result:
Spec URL: https://michich.fedorapeople.org/opa-ff/opa.spec

I see you added ff_install.sh and the spec file is now shorter. Thank you!

Unfortunately it still uses generated file lists. In mentioned that as an issue
in comments #3, #5, and #8. I don't know if you missed those comments, or you
simply insist on the use of file lists. Please comment.

There are several items that go against the Packaging Guidelines. What's worse
is that some of them were already fixed in the previously reviewed spec file
from comment #7 and now they're back, so we're regressing.

> Summary: Intel Omni-Path basic tools and libraries for fabric managment.
Typo. "managment" -> "management"
There should be no dot at the end of the Summary.
See http://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections

> Group: System Environment/Libraries
In comment #3 I wrote the Group tag was unnecessary. Since then the Fedora
packaging guidelines got updated. The wording is now stronger:
    The [...] Group: tag, [...] SHOULD NOT be used. 
(http://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections)
Please remove the Group tags.

> License: GPLv2/BSD
The correct syntax is:
License: GPLv2 or BSD
See
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Dual_Licensing_Scenarios

> Url: http://www.intel.com/
Repeating from comment #3:
The URL should point to a page that's more relevant to the package. For
instance, opa-ff's page on github.
The previously reviewed spec file had:
Url: https://github.com/01org/opa-ff

> Source: opa.tgz
The previously reviewed spec file included a comment on how to reproduce the
tarball.
The comment is now gone!

> %description
> This package contains the tools necessary to manage an Intel(R)
> Omni-Path Architecture fabric.
The "(R)" symbol must not be used in %description.
See
http://fedoraproject.org/wiki/Packaging:Guidelines#Trademarks_in_Summary_or_Description

> Requires: opa-basic-tools
Explicit Requires between subpackages of the same source package should be
fully
versioned and arch-specific:
Requires: opa-basic-tools%{?_isa} = %{version}-%{release}
See http://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

> Requires: atlas
What is the reason for requiring atlas explicitly? Does the software dlopen()
lib[st]atlas.so.3? I did not find anything by grepping.

> %build
> if [ -d OpenIb_Host ]
As a packager, you should know whether the tarball contains this directory or
not. No such conditional should be necessary. Just a minor issue.

> %preun fastfabric
> cd /usr/lib/opa/src/mpi_apps >/dev/null 2>&1
> make -k clean >/dev/null 2>&1 || : # suppress all errors and return codes from the make clean.
Could you explain the purpose of this scriptlet? And perhaps more generally,
the intended purpose of the /usr/lib/opa/src/mpi_apps directory?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




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