[Bug 486804] Review Request: libferrisloki - customized build of Loki library from Modern C++ Design for libferris
bugzilla at redhat.com
bugzilla at redhat.com
Wed Sep 2 20:12:33 UTC 2009
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=486804
Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |mtasaka at ioa.s.u-tokyo.ac.jp
--- Comment #10 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> 2009-09-02 16:12:31 EDT ---
Some notes:
* Versioning
- Well, I cannot understand why your spec file has excessively
large release number and I don't think this large release
number is needed.
If this release number comes from some reasons unrelated to
Fedora, please reset this.
- Also, please consider to use %{?_dist} tag.
https://fedoraproject.org/wiki/Packaging/DistTag
* Licensing
- The license tag should simply be "GPLv2+".
src/Extensions.cpp is under GPLv2+, libferrisloki.so uses
.libs/Extensions.o, which renders libferrisloki.so to be under GPLv2+,
so other license tag is useless.
--------------------------------------------------------------------
192 libtool: compile: g++ -DHAVE_CONFIG_H -I. -I.. -I. -I. -I.. -I..
-I/usr/include -I./../include/FerrisLoki -I/usr/local/include
-I/usr/include/sigc++-2.0 -I/usr/lib/sigc++-2.0/include -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i686 -mtune=atom
-fasynchronous-unwind-tables -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
-fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i686
-mtune=atom -fasynchronous-unwind-tables -MT Extensions.lo -MD -MP -MF
.deps/Extensions.Tpo -c Extensions.cpp -fPIC -DPIC -o .libs/Extensions.o
213 libtool: link: g++ -shared -nostdlib
/usr/lib/gcc/i686-redhat-linux/4.4.1/../../../crti.o
/usr/lib/gcc/i686-redhat-linux/4.4.1/crtbeginS.o .libs/Extensions.o
.libs/OrderedStatic.o .libs/SafeFormat.o .libs/Singleton.o .libs/SmallObj.o
.libs/SmartPtr.o .libs/StrongPtr.o -lsigc-2.0
-L/usr/lib/gcc/i686-redhat-linux/4.4.1
-L/usr/lib/gcc/i686-redhat-linux/4.4.1/../../.. -lstdc++ -lm -lc -lgcc_s
/usr/lib/gcc/i686-redhat-linux/4.4.1/crtendS.o
/usr/lib/gcc/i686-redhat-linux/4.4.1/../../../crtn.o -m32 -march=i686
-mtune=atom -Wl,-O1 -Wl,--hash-style=both -Wl,-soname -Wl,libferrisloki.so.3
-o .libs/libferrisloki.so.3.0.0
--------------------------------------------------------------------
* BuildRoot
- BuildRoot currently used in your spec file is not valid
on Fedora:
https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
! Note
On currently supported Fedora version (Fedora 10/11/12), you can
simply BuildRoot line completely.
* URL
- Perhaps http://witme.sourceforge.net/ is better?
* BuildRequires
- BR: gcc-c++ is redundant:
https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2
* About %build
- From build.log:
-------------------------------------------------------------------
44 + ./configure --build=i386-redhat-linux-gnu
--host=i386-redhat-linux-gnu --target=i686-redhat-linux-gnu --program-prefix=
--prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin
--sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include
--libdir=/usr/lib --libexecdir=/usr/libexec --localstatedir=/var
--sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info
--with-sigcxx-2x=yes
45 configure: WARNING: unrecognized options: --with-sigcxx-2x
-------------------------------------------------------------------
is "--with-sigcxx-2x" needed?
- tarball contains include/FerrisLoki/loki/, i.e. this package uses
internal copy of loki, however on Fedora loki is already packaged:
https://admin.fedoraproject.org/pkgdb/packages/name/loki-lib
http://koji.fedoraproject.org/koji/packageinfo?packageID=4740
This package (ferrisloki) should be patched to use system-wide loki-lib
and include/FerrisLoki/loki/ should be removed at %prep.
* %files
- Fedora strongly suggests not to include static archives (.a files)
https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries
- The directory %{_includedir}/FerrisLoki itself is not owned by
any packages:
https://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership
https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Forgetting_to_Include_a_Toplevel_Directory
* pkgconfig file
- ferrisloki.pc says (on i686, rawhide):
---------------------------------------------------
6 Name: ferrisloki
7 Description: Version of standard loki library with extensions.
8 Version: 3.0.2
9 Requires:
10 Libs: -L${libdir} -lferrisloki -lsigc-2.0
11 Cflags: -I${includedir} -I${includedir}/FerrisLoki
-I${includedir}/FerrisLoki/loki -I/usr/include/sigc++-2.0
-I/usr/lib/sigc++-2.0/include
---------------------------------------------------
This should be changed to:
---------------------------------------------------
Name: ferrisloki
Description: Version of standard loki library with extensions.
Version: 3.0.2
Requires: sigc++-2.0
Libs: -L${libdir} -lferrisloki
Cflags: -I${includedir}/FerrisLoki -I${includedir}/FerrisLoki/loki
---------------------------------------------------
* Requires (for -devel subpackage)
- For example FerrisLoki/BoostExtensions.hh contains:
---------------------------------------------------
53 #include <boost/config.hpp>
54 #include <boost/mpl/integral_c.hpp>
55 #include <boost/mpl/integral_c_tag.hpp>
---------------------------------------------------
This package means that ferrisloki-devel should have "Requires:
boost-devel".
* Requires between subpackages
- Usually the dependency between packages rebuilt from the same srpm should
have exact EVR (Epoch-Version-Release).
i.e. ferrislock-devel should have "Requires: %{name} =
%{version}-%{release}",
not just "Requires: %{name} >= %{version}":
https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
More information about the Fedora-package-review
mailing list