[Bug 486804] Review Request: libferrisloki - customized build of Loki library from Modern C++ Design for libferris

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

 



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@xxxxxxxxxxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mtasaka@xxxxxxxxxxxxxxxxxxx




--- Comment #10 from Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxxxx>  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.

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@xxxxxxxxxx
http://www.redhat.com/mailman/listinfo/fedora-package-review

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