Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: libixp - Stand-alone client/server 9P library
https://bugzilla.redhat.com/show_bug.cgi?id=530617
Summary: Review Request: libixp - Stand-alone client/server 9P library Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: cassmodiah@fedoraproject.org QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://cassmodiah.fedorapeople.org/libixp/libixp.spec
SRPM URL: http://cassmodiah.fedorapeople.org/libixp/libixp-0.5-1.fc11.src.rpm
Description: libixp is a stand-alone client/server 9P library. libixp's server api is heavily based on that of Plan 9's lib9p.
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=530617
Peter Lemenkov lemenkov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mapleoin@fedoraproject.org
--- Comment #1 from Peter Lemenkov lemenkov@gmail.com 2009-10-29 10:14:49 EDT --- *** Bug 454025 has been marked as a duplicate of this bug. ***
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=530617
Michael Schwendt mschwendt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mschwendt@gmail.com
--- Comment #2 from Michael Schwendt mschwendt@gmail.com 2009-11-10 17:10:25 EDT ---
%{_libdir}/*.a
You build a static library only. You put it into the -devel package with a virtual -static package name.
Then why do you run /sbin/ldconfig in post/postun scriptlets?
%package -n ixpc Summary: Plan9 file protocol client Group: Applications/System Requires: %{name}-static = %{version}-%{release}
Why does a client binary require the static library package?
make %{?_smp_mflags} CC="%{__cc} -c %{optflags}"
This doesn't make the package adhere to the optflags guidelines. The project's internal CFLAGS override some of the optflags. It would be more clean if you could patch mk/gcc.mk and append $RPM_OPT_FLAGS (or %optflags) to $CFLAGS.
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=530617
Jochen Schmitt jochen@herr-schmitt.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |jochen@herr-schmitt.de Flag| |fedora-review?
--- Comment #3 from Jochen Schmitt jochen@herr-schmitt.de 2009-11-19 11:27:16 EDT --- Please work out the comments of mschwendt and upload the fixed release of your package.
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=530617
--- Comment #4 from Simon Wesp cassmodiah@fedoraproject.org 2009-11-21 03:44:22 EDT --- (In reply to comment #2)
Then why do you run /sbin/ldconfig in post/postun scriptlets?
the ldconfig scriptlets are in the lib* template. i forgot to remove them!
Why does a client binary require the static library package?
Doesn't make sense, because they are built static in the binary. sorry
This doesn't make the package adhere to the optflags guidelines. The project's internal CFLAGS override some of the optflags. It would be more clean if you could patch mk/gcc.mk and append $RPM_OPT_FLAGS (or %optflags) to $CFLAGS.
mh, I don't understand that
-current- CC="gcc -c -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" CFLAGS="-I$(echo .:../include:/usr/local/include:/usr/include|sed 's/:/ -I/g') -D_XOPEN_SOURCE=600 -std=c99 -pedantic -pipe -fno-strict-aliasing -Wall -Wimplicit -Wmissing-prototypes -Wno-comment -Wno-missing-braces -Wno-parentheses -Wno-sign-compare -Wno-switch -Wpointer-arith -Wreturn-type -Wstrict-prototypes -Wtrigraphs -g -O1 -fno-builtin -fno-inline -fno-omit-frame-pointer -fno-optimize-sibling-calls -fno-unroll-loops -DIXPlint -O0 -DVERSION="0.5" -D_XOPEN_SOURCE=600" ../util/compile ixpc.o ixpc.c
-new?- CC="gcc -c -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" CFLAGS="-I$(echo .:../include:/usr/local/include:/usr/include|sed 's/:/ -I/g') -D_XOPEN_SOURCE=600 -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 -std=c99 -pedantic -pipe -fno-strict-aliasing -Wall -Wimplicit -Wmissing-prototypes -Wno-comment -Wno-missing-braces -Wno-parentheses -Wno-sign-compare -Wno-switch -Wpointer-arith -Wreturn-type -Wstrict-prototypes -Wtrigraphs -g -O1 -fno-builtin -fno-inline -fno-omit-frame-pointer -fno-optimize-sibling-calls -fno-unroll-loops -DIXPlint -O0 -DVERSION="0.5" -D_XOPEN_SOURCE=600" ../util/compile ixpc.o ixpc.c
I know these outputs are different, but i printed it and tallied it. in the new is not a flag more or less then in the current and reverse. Or not? The order of the flags is important, or? In the new? output i sorted it fedora_optflags and upstream_flags. You said that upstream_flags override some fedora_optflags, so it should be upstream_flags and fedora_optflags, the secound mentoined which conflicts with the first mentoined will override the first mentoined, or? (This sounds logical to me)
-upstream and fedora instead of fedora and upstream- CC="gcc -c -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" CFLAGS="-I$(echo .:../include:/usr/local/include:/usr/include|sed 's/:/ -I/g') -D_XOPEN_SOURCE=600 -std=c99 -pedantic -pipe -fno-strict-aliasing -Wall -Wimplicit -Wmissing-prototypes -Wno-comment -Wno-missing-braces -Wno-parentheses -Wno-sign-compare -Wno-switch -Wpointer-arith -Wreturn-type -Wstrict-prototypes -Wtrigraphs -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 -g -O1 -fno-builtin -fno-inline -fno-omit-frame-pointer -fno-optimize-sibling-calls -fno-unroll-loops -DIXPlint -O0 -DVERSION="0.5" -D_XOPEN_SOURCE=600" ../util/compile ixpc.o ixpc.c
This output is equal to the current-output... I'm confused, sorry!
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=530617
--- Comment #5 from Jochen Schmitt jochen@herr-schmitt.de 2009-11-25 16:15:45 EDT --- I suggest you should modified the mk/gcc.mk file, so that $RPM_OPT_FLAGS will be added to CFLAGS.
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=530617
--- Comment #6 from Simon Wesp cassmodiah@fedoraproject.org 2009-11-27 14:26:45 EDT --- http://cassmodiah.fedorapeople.org/libixp/libixp-0.5-2.fc12.src.rpm
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=530617
Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |jochen@herr-schmitt.de
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=530617
--- Comment #7 from Jochen Schmitt jochen@herr-schmitt.de 2009-11-29 15:28:57 EDT --- Good: + Basename of the SPEC file matches with package name. + Package fullfill naming guildelines + Consistently usage of rpm macros + URL tag shows on proper project homepage + Package contains most recent version of the software + Could download upstream tarball via spectool -g + Package tar ball matches with upstream (md5sum: 2a394310c209605ba54ecf5eab518bff) + License tag states MIT and LPL as valid oSS licenses + Package conatins verbain copy of the license text + Package contains subpackages + Subpackage has proper Requies to main package + Package has proper definition of BuildRoot + Package use smp_mflags + Buildroot will be cleaned at the beginning fo %clean and %install + Local build works fine + Rpmlint is silent on source rpm
Bad: - License tag should be MIT and LPL and Public Domain, because I have found a source files which is declared as public domain oh the head of the source file - Package doesn't create a so file of the library. - Devel package shouldn't contains a static library - libixp package missing
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=530617
--- Comment #8 from Simon Wesp cassmodiah@fedoraproject.org 2009-12-11 16:06:23 EDT --- http://cassmodiah.fedorapeople.org/libixp/libixp.spec http://cassmodiah.fedorapeople.org/libixp/libixp-0.5-3.fc12.src.rpm
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=530617
--- Comment #9 from Michael Schwendt mschwendt@gmail.com 2009-12-14 08:00:31 EDT --- This doesn't fix the issues since now you've added further problems. Probably it's necessary to expand a bit:
%package devel Provides: %{name}-static = %{version}-%{release}
This combination, a -devel package providing a virtual -static package, is dangerous, because it is special-case 2 on: http://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Librarie... It requires special attention during review and later updates.
$ rpmls -p libixp-devel-0.5-3.fc12.i686.rpm | grep -v shar -rw-r--r-- /usr/include/ixp.h -rw-r--r-- /usr/include/ixp_srvutil.h
Package doesn't contain any static library. Raises the question whether a) the header files contain enough code fragments that would be inserted when compiling them, or b) whether a library for these API headers is missing?
The answer is yes to b). After disabling the static build, the shared library for this API is missing, because it is misplaced in the "ixpc" subpackage:
$ rpmls -p ixpc-0.5-3.fc12.i686.rpm | head -3 -rwxr-xr-x /usr/bin/ixpc -rwxr-xr-x /usr/lib/libixp.so -rwxr-xr-x /usr/lib/libixp_pthread.so
This needs work. libixp-devel alone is useless. No library to link with. Notice how /usr/bin/ixpc and examples are linked with -lixp.
rpmlint also prints new warnings when running it on the built rpms: ixpc.i686: W: no-soname /usr/lib/libixp_pthread.so ixpc.i686: W: no-soname /usr/lib/libixp.so
No versioned SONAME, no SONAME at all, automatic RPM dependencies will be less helpful => pray that libixp API and ABI will stay stable.
$ rpm -qpR ixpc-0.5-3.fc12.i686.rpm | grep ixp libixp.so
* A shared library linked with executables, such as /usr/bin/ixpc, will benefit from adding the /sbin/ldconfig scriptlets (which were inappropriate with only the static lib).
* The CFLAGS still look strange. See build.log. /usr/local/include at beginning of search paths. %optflags at the beginning, too, with a multitude of -O1, -O0 and other optimisations overriding them more than once.
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=530617
--- Comment #10 from Simon Wesp cassmodiah@fedoraproject.org 2010-06-01 04:33:29 EDT --- I still haven't understand why I must not build this package as a static lib.
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=530617
Simon Wesp cassmodiah@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |WONTFIX Flag|fedora-review? |
--- Comment #11 from Simon Wesp cassmodiah@fedoraproject.org 2010-06-08 06:07:09 EDT --- closed, because wmii 3.9 will ship an own copy of the ixp lib. https://fedoraproject.org/wiki/Packaging:Guidelines#Duplication_of_system_li... It makes more sense to use the shipped version instead. Thank you for your comments!
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla