[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 #27 from paul.j.reger@xxxxxxxxx ---
(In reply to Michal Schmidt from comment #26)
> > 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

To be clear, 10.1, is actually, currently in a pre-release state.  Still, I can
make the initial value for the Release: tag as 1.  And, I can also add %{?dist}
to it too.  See my next attachment. 

> > # 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?

The code in github is not a true mirror.  I will correct the comment in my next
push.

> > 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).

Whoops.  Sorry.  I will fix this in my next push as well.  The URL lacked a
/download/ after releases.  The correct URL is:

https://github.com/01org/opa-psm2/releases/download/10_1/hfi1-psm-10.1-0.tar.gz

> 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.

Why?

> 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.

I do not believe that would work for our internal needs.  We essentially have a
continuous integration testing scheme where every change in our entire product
creates a new product from probably millions of lines of code. Hundreds of
these CI builds are created, and are comprised of lots of packages all
consisting of 10.1 version, but also qualified with a unique release number.

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

Sorry, I missed your comment.  This fix will go in my next push.

> > %files devel
> > %defattr(-,root,root,-)
> 
> %defattr is redundant.

I will lose the %defattr in my next push.

> > /usr/lib64/libpsm2.so
> 
> Should use %{_libdir} here too.

Will fix in my next push.

> > # 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.

Will fix in my next push.

> > %{_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

Will fix in my next push.

> > /etc/modprobe.d/hfi1-psm-compat.conf
> 
> I already commented about shipping this under /usr/lib.

Yes, we are trying to fix this in a general way.  This will undoubtedly not be
fixed in my next push.  Let us have some time to fix this one.

-- 
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]