[Bug 773485] Review Request: ibutils - InfiniBand fabric management utilities

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

 



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

--- Comment #12 from Jon Stanley <jstanley@xxxxxxxx> ---
(In reply to comment #11)
> Executive summary:
> - The static library libibdmcom.a is in both -devel and -static

Oops. Just missed deleting it :)

> - Add a comment to the spec file explaining the dual license

Done

> - There is no desktop file for this package; should there be?

Not sure if that makes much sense here, but I can add one if required.

> - COPYING should be in -libs instead of the main package, since -libs can be
> installed without the main package

Done.

> - Is swig really needed at runtime?  (It's a Requires).  Just checking here;
> I don't know.

I'm not sure either. I'll drop it for now and see if it still works. (the good
part is ibmgtsim doesn't require IB hardware)

> - Is autoconf really a BR?  It doesn't appear to be used.

Dropped.

> - See the last item (about perl) in the MUST section below.

Is that really necessary? I can see it being for perl modules, but we're just
packaging some perl scripts here.

> - fedora-review seems to be grumpy that you used Source: instead of
> Source0:; I don't care.

Heh, I'll make our automated overlords happy :)

> - I see a few instances of the string "1.5.7" in the spec file.  Should
> those be changed to %{version}?

The only places that I see that is in %files - I think that adding macros there 
will save very little maintenance effort and just obfuscate the spec.

> - The undefined weak symbols and unnecessary linkage noted in comment 6 are
> still there.  You can get rid of the unnecessary linkage by doing this after
> %configure:
> 
> # Workaround libtool reordering -Wl,--as-needed after all the libraries.
> sed -e 's|^LTCC="gcc"|LTCC="gcc -Wl,--as-needed"|' \
>     -e 's|^CC="g++"|CC="g++ -Wl,--as-needed"|' \
>     -i ibdm/libtool ibis/libtool ibmgtsim/libtool

Done.
> 
> The undefined weak symbols can be eliminated by doing this before %configure:
> 
> sed -i "s/^libibmscli_la_LIBADD =/& -lpthread/" ibmgtsim/src/Makefile.in

Done.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



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