Bug 454025 - Review Request: libixp - stand-alone client/server 9P library including ixpc client
Review Request: libixp - stand-alone client/server 9P library including ixpc ...
Status: CLOSED DUPLICATE of bug 530617
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2008-07-03 17:16 EDT by Ionuț Arțăriși
Modified: 2010-01-14 17:19 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-09-24 12:47:43 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Ionuț Arțăriși 2008-07-03 17:16:21 EDT
Spec URL: http://mapleoin.fedorapeople.org/libixp.spec
SRPM URL: http://mapleoin.fedorapeople.org/libixp-0.4-1.fc9.src.rpm
Description: 
libixp is a stand-alone client/server 9P library including ixpc client. It
consists of less than 2000 lines of code (including ixpc).

libixp's server API is based heavily on that of Plan 9's lib9p, and the two
libraries export virtually identical data structures. There are a few notable
differences between the two, however.

This is my first package and I seem to be in need of a sponsor.
Comment 1 Mamoru TASAKA 2008-07-17 13:26:47 EDT
Hello:

* Please support parallel make when possible:
  https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make
  If not possible, write a comment in the spec file that this package does not
support
  parallel make

* build log like:
-------------------------------------------------------------
+ make
MAKE all libixp/
HEADER include/ixp_fcall.h from fcall.h.nounion
CC libixp/client.o
CC libixp/convert.o
CC libixp/error.o
CC libixp/intmap.o
-------------------------------------------------------------
  is not useful. We cannot verify if Fedora specific compiler flags are
  correctly honored on this package from this build log:
  https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
  And actually currently this compiler flags (you can check this by
  $ rpm --eval %optflags) are not correctly honored. Please make build log more
  verbose (you can do this by removing ".SILENT" from makefiles by for example:
------------------------------------------------------------------------------
find . -type f | xargs grep -l '.SILENT' | xargs sed -i.silent -e 's|\.SILENT||'
------------------------------------------------------------------------------
  )

* Please use macros properly. /usr must be %{_prefix}, /etc must be %{_sysconfdir}
  https://fedoraproject.org/wiki/Packaging/RPMMacros

* When packaging static archives follow:
  https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries

* This package does not build on x86_64 (I cannot show x86_64 build log because
currently
  koji is very slow...)
  static archives are installed under /usr/lib even on x86_64 (which is wrong
for 64 bits
  machine) where spec files expects that static archives should be installed under
  %_libdir (on 64 bits machine this is /usr/lib64).

* Usually -devel package must have "Requires: %{name} = %{version}-%{release}"
  (please check:
   https://fedoraproject.org/wiki/Packaging/ReviewGuidelines )
Comment 2 Mamoru TASAKA 2008-07-17 13:32:49 EDT
build log is:

http://koji.fedoraproject.org/koji/taskinfo?taskID=720466
Comment 3 Mamoru TASAKA 2008-08-03 12:47:11 EDT
ping?
Comment 4 Mamoru TASAKA 2008-08-09 12:04:59 EDT
ping again?
Comment 5 Ionuț Arțăriși 2008-08-10 06:03:46 EDT
Thank you for the review!

Sorry for not answering all this time and thanks for continuing to ping. I had given up on the package because I got stuck. 
I managed to add a few of the stuff you proposed, but not others. I uploaded the new spec and src.rpms, though they still have errors:
http://mapleoin.fedorapeople.org/libixp-0.4-2.fc9.i386.rpm
http://mapleoin.fedorapeople.org/libixp.spec

I'm not sure if the libraries should be in the -devel part or if i should replace that with a -static section(though from what I read in the Packaging Guidelines, that's strongly discouraged)

I'm afraid I need more help in order to get this specfile right.
Comment 6 Mamoru TASAKA 2008-08-11 09:37:03 EDT
Well, for 0.4-2:

* Summary/%description
  - "Libixp is a " part is redundant for Summary
  - The Summary for -devel subpackage is not proper.
    Usually this is "Development files for foo".
  - Main package and -devel subpackage have the same %description.
    However the purpose of the two packages are different and
    this is not proper. Please modify the %description.

    You can refer to the %description in the skeleton spec
    file created by "$ rpmdev-newspec -t lib foo".

* 64 bit arch issue
  - Well, actually the fix is not easy, as this package uses somewhat
    unique makefiles which don't seem to be based on recent Makefiles.
    As a workaround, I propose:
-------------------------------------------------------
%setup -q
# Make build.log more verbose
find . -type f | xargs grep -l '.SILENT' | xargs sed -i.silent -e 's|\.SILENT||'

# Umm... fixing 64 bits archtecture directory issue cannot be easy done
# by applying a patch, using sed...

grep -rl '/lib' . | xargs sed -i.lib \
	-e 's|/lib\([ /]\)|/%{_lib}\1|' \
	-e 's|/lib$|/%{_lib}|'

-------------------------------------------------------

   ! The above %setup also contains a fix to make build.log more verbose
   !!! Please recheck what I do by the script above.

* Cflags
  - As I wrote in the comment 1, Fedora specific compilation flags are
    not correctly honored:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
    For this package the following works (note: the following method is
    this package specific)
--------------------------------------------------------
%build
make %{?_smp_mflags} \
	CC="%{__cc} -c %optflags"
--------------------------------------------------------

* %files entry v.s. debuginfo rpm issue
  - Currently (after cflags issue is fixed) rpmlint complains:
--------------------------------------------------------
libixp-devel.i386: W: hidden-file-or-dir /usr/lib/debug/.build-id
libixp-devel.i386: W: hidden-file-or-dir /usr/lib/debug/.build-id
libixp-devel.i386: W: dangling-relative-symlink /usr/lib/debug/.build-id/52/fcd345977adcc0861159407db91f2bc489d3e2 ../../../../bin/ixpc
libixp-devel.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/ixpc.debug
libixp-devel.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/ixpc.debug
--------------------------------------------------------

   debuginfo rpms installs files under %_libdir/debug so writing %files
   entry like:
--------------------------------------------------------
%files devel
%defattr(-,root,root,-)
%{_libdir}/*
--------------------------------------------------------
   is wrong because %_libdir/* contains %_libdir/debug, while files under
   this directory must be owned by -debuginfo rpm and not -devel rpm.
   Please don't use "%{_libdir}/*" and replace this with
   "%{_libdir}/libixp*.a", for example.
Comment 7 Mamoru TASAKA 2008-08-23 23:13:19 EDT
ping?
Comment 8 Mamoru TASAKA 2008-09-01 02:58:16 EDT
ping again?
Comment 9 Mamoru TASAKA 2008-09-08 10:09:43 EDT
ping again?
Comment 10 Mamoru TASAKA 2008-09-17 11:30:27 EDT
I will close this bug if no response is received from the reporter within
ONE WEEK.
Comment 11 Mamoru TASAKA 2008-09-24 12:47:43 EDT
Closing.

If someone wants to import this package into Fedora, please submit a new
review request and make this bug as a duplicate of the new one.

Thank you!
Comment 12 Peter Lemenkov 2009-10-29 10:14:49 EDT

*** This bug has been marked as a duplicate of bug 530617 ***

Note You need to log in before you can comment on or make changes to this bug.