[Bug 307891] Review Request: libvpd - C++ library for system vpd access

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: libvpd - C++ library for system vpd access


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


mtasaka at ioa.s.u-tokyo.ac.jp changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mtasaka at ioa.s.u-tokyo.ac.jp




------- Additional Comments From mtasaka at ioa.s.u-tokyo.ac.jp  2007-12-16 10:57 EST -------
For 1.4.1-1:

* SourceURL
  - http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
    returns 404 (not found).

* BuildRequires
  - Would you explain why "BuildRequires: libtool" is needed?

* Provides
  - "Provides: libvpd" must be removed.
  - And I don't think adding "Provides: libvpd_cxx" is proper.

* pkgconfig .pc file inclusion
  (Please refer to
   http://fedoraproject.org/wiki/Packaging/Guidelines
   http://fedoraproject.org/wiki/Packaging/ReviewGuidelines )
  - Packages containing pkgconfig(.pc) files must 
    'Requires: pkgconfig' (for directory ownership and usability).

* pkgconfig .pc file itself

  - For example, %_libdir/pkgconfig/libvpd-1.pc contains the
    line:
--------------------------------------------------------------
Cflags: -I${includedir}/libvpd-1 -I${libdir}/libvpd-1/include
--------------------------------------------------------------
    However, I don't think -I${libdir}/libvpd-1/include is needed
    as no files are installed under this directory.

  - And this .pc file also contains
--------------------------------------------------------------
Libs: -L${libdir} -lpthread -ldb -llibvpd-1
--------------------------------------------------------------
    "-ldb" means that libvpd-devel should have "Requires: db4-devel".
    Also, %_include/libvpd-1/vpddbenv.h contains
--------------------------------------------------------------
    26  
    27  #include <stdlib.h>
    28  #include <string.h>
    29  #include <db.h>
    30  
--------------------------------------------------------------

* Timestamps
  - Please use
---------------------------------------------------------------
%{__make} install DESTDIR=$RPM_BUILD_ROOT INSTALL="%{__install} -p"
---------------------------------------------------------------
    to keep timestamps on installed files.
    While this sometimes does not work, this method usually works
    for recent Makefiles.

* %exclude
---------------------------------------------------------------
%files
....
%exclude %{_libdir}/*.la
.....
%files devel
.....
%exclude %{_libdir}/*.la
---------------------------------------------------------------
   - Rather simply remove libtool .la file at %install like:
---------------------------------------------------------------
%install
......
%{__rm} -f $RPM_BUILD_ROOT%{_libdir}/*.la
---------------------------------------------------------------

* Macros in %changelog
  - If you try "rpm -q --changelog libvpd", you will see:
---------------------------------------------------------------
* Fri Nov 16 2007 Eric Munson <ebmunson at us.ibm.com> - 1.4.1-1
- Removing INSTALL from docs and docs from -devel package
- Fixing Makfile.am so libraries have the .so extension
- Using 
  CFLAGS="${CFLAGS:--O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables}" ; export CFLAGS ; 
  CXXFLAGS="${CXXFLAGS:--O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables}" ; export CXXFLAGS ; 
  FFLAGS="${FFLAGS:--O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables}" ; export FFLAGS ; 
  for i in $(find . -name config.guess -o -name config.sub) ; do 
           [ -f /usr/lib/rpm/redhat/$(basename $i) ] && /bin/rm -f $i && /bin/cp
-fv /usr/lib/rpm/redhat/$(basename $i) $i ; 
  done ; 
  ./configure --build=i386-redhat-linux-gnu --host=i386-redhat-linux-gnu \
        --target=i386-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=/usr/com \
        --mandir=/usr/share/man \
        --infodir=/usr/share/info, /usr/bin/make, and /bin/rm calls
- Changing source URL
---------------------------------------------------------------
  Here macros are expanded. Suppress macros expanding by using %%, like
---------------------------------------------------------------
* Fri Nov 16 2007 Eric Munson <ebmunson at us.ibm.com> - 1.4.1-1
- Removing INSTALL from docs and docs from -devel package
- Fixing Makfile.am so libraries have the .so extension
- Using %%configure, %%{__make}, and %%{__rm} calls
- Changing source URL
---------------------------------------------------------------

  ! By the way, you can find this type of errors using rpmlint
    (in rpmlint package) like:
---------------------------------------------------------------
$ rpmlint *rpm
libvpd.i386: E: useless-explicit-provides libvpd
libvpd.src:16: W: unversioned-explicit-provides libvpd_cxx
libvpd.src:16: W: unversioned-explicit-provides libvpd
libvpd.src:67: W: macro-in-%changelog configure
libvpd-devel.i386: W: no-documentation
---------------------------------------------------------------


-- 
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, or are watching someone who is.