[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

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

 



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



--- Comment #26 from Michal Schmidt <mschmidt@xxxxxxxxxx> ---
> Release: 0

>From what you wrote in comment #19 I can now understand where this "0" comes
from. However, for Fedora packaging, the initial value for Release should be
"1". A Release tag leading with "0" would indicate you're packaging a
pre-release snapshot of 10.1.
Also, it must contain "%{?dist}".
See
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning

> # The source to make this rpm was created at Intel from a private git repo.
> # The source code is mirrored at: https://github.com/01org/opa-psm2/

If it's a true mirror, then it must be possible to create the tarball from
the public repo on github. Why mention a private repo?

> Source0: https://github.com/01org/opa-psm2/releases/10_1/%{name}-%{version}-%{release}.tar.gz

The Source0 URL expands to
https://github.com/01org/opa-psm2/releases/10_1/hfi1-psm-10.1-0.tar.gz ,
which does not exist (HTTP error 404).

>From my previous comments it can be deduced that it cannot be valid for
"%{release}" to appear in the Source URL of a Fedora package.

In your role as an upstream developer, why don't you make a properly
versioned tarball your primary deliverable? Forget about RPM packaging for now.
When you tag a release in git as "10.1", you should generate a tarball from it
called hfi1-psm-10.1.tar.gz (or .bz2, or .xz) and publish it.

Then a Fedora packager (it might be yourself, but in general it can be
another person) can point to your tarball in the Source0 tag of the spec file.

> %build
> %{__make}

I already wrote in comment #2 this needs to respect %optflags.
http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

> %files devel
> %defattr(-,root,root,-)

%defattr is redundant.

> /usr/lib64/libpsm2.so

Should use %{_libdir} here too.

> # The following files were part of the devel-noship and moved to devel:

I don't think anyone would care. The comment can be removed.

> %{_includedir}/hfi1diag/ptl_ips/ipserror.h
> %{_includedir}/hfi1diag/linux-x86_64/bit_ops.h
> %{_includedir}/hfi1diag/linux-x86_64/sysdep.h
> %{_includedir}/hfi1diag/opa_udebug.h
> %{_includedir}/hfi1diag/opa_debug.h
> %{_includedir}/hfi1diag/opa_intf.h
> %{_includedir}/hfi1diag/opa_user.h
> %{_includedir}/hfi1diag/opa_service.h
> %{_includedir}/hfi1diag/opa_common.h
> %{_includedir}/hfi1diag/opa_byteorder.h
> %{_includedir}/hfi1diag/hfi1_deprecated.h

Your package must own the directories %{_includedir}/hfi1diag,
%{_includedir}/hfi1diag/linux-x86_64/, %{_includedir}/hfi1diag/ptl_ips/.
You may just own the whole subtree with a single line:
 %{_includedir}/hfi1diag

> /etc/modprobe.d/hfi1-psm-compat.conf

I already commented about shipping this under /usr/lib.

-- 
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
http://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx




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