Bug 183438

Summary: Review Request: idioskopos (C++ Introspection Library)
Product: [Fedora] Fedora Reporter: Rick L Vinyard Jr <rvinyard>
Component: Package ReviewAssignee: Thorsten Leemhuis (ignored mailbox) <bugzilla-sink>
Status: CLOSED DUPLICATE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideKeywords: FutureFeature
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-07-15 18:25:17 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 201449    

Description Rick L Vinyard Jr 2006-03-01 02:48:12 UTC
Need sponsor.

Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/idioskopos.spec
SRPM Name or Url: http://miskatonic.cs.nmsu.edu/pub/fedora/4/srpms/idioskopos-0.1.7-1.src.rpm

Description:
idioskopos (Greek: idio- inward, within, private; -skopos look, aim, target)
is a C++ library that simplifies (hopefully) the addition of object reflection
and introspection.

Comment 1 Ralf Corsepius 2006-03-03 09:31:28 UTC
This package's configure script is broken:
...
checking whether stripping libraries is possible... yes
checking bits/concurrence.h usability... yes
checking bits/concurrence.h presence... yes
checking for bits/concurrence.h... yes
checking ext/mt_allocator.h usability... yes
checking ext/mt_allocator.h presence... yes
checking for ext/mt_allocator.h... yes
checking tr1/boost_shared_ptr.h usability... no
checking tr1/boost_shared_ptr.h presence... yes
configure: WARNING: tr1/boost_shared_ptr.h: present but cannot be compiled
configure: WARNING: tr1/boost_shared_ptr.h:     check for missing prerequisite
headers?
configure: WARNING: tr1/boost_shared_ptr.h: see the Autoconf documentation
configure: WARNING: tr1/boost_shared_ptr.h:     section "Present But Cannot Be
Compiled"
configure: WARNING: tr1/boost_shared_ptr.h: proceeding with the preprocessor's
result
configure: WARNING: tr1/boost_shared_ptr.h: in the future, the compiler will
take precedence
configure: WARNING:     ## ------------------------------------------ ##
configure: WARNING:     ## Report this to the AC_PACKAGE_NAME lists.  ##
configure: WARNING:     ## ------------------------------------------ ##
checking for tr1/boost_shared_ptr.h... yes
checking for i686-redhat-linux-gnu-pkg-config... no
checking for pkg-config... /usr/bin/pkg-config
checking pkg-config is at least version 0.9.0... yes
checking for PROJECT... yes
configure: creating ./config.status

Apart of this, the docs' packaging is broken:
rpm -qlp idioskopos-devel-0.1.7-1.i386.rpm | \
grep -E '/usr/share/doc/.*7$'
/usr/share/doc/idioskopos-devel-0.1.7
/usr/share/doc/idioskopos-docs-0.1.7


Comment 2 Rick L Vinyard Jr 2006-03-03 18:28:03 UTC
Thanks for the feedback.

With the configure script, using the shared pointer from tr1 required a little
more voodoo that AC_CHECK_HEADER normally requires. It's fixed now.

On the docs packaging, I wasn't sure if I needed a separate package or not.
automake is the one that actually installs the docs into that directory. Anyway,
I went ahead and added a documentation subpackge which is probably better in the
longrun anyway.

The updated spec and srpm can be found here:
Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/idioskopos.spec
SRPM Name or Url:
http://miskatonic.cs.nmsu.edu/pub/fedora/4/srpms/idioskopos-0.1.8-1.src.rpm


Comment 3 Ralf Corsepius 2006-03-04 05:22:24 UTC
Though this package is basically OK, some details are causing me headaches:

1) cat usr/lib/pkgconfig/idioskopos-0.1.pc
...
Description: @PROJECT_DESCRIPTION@
...

2) IMO, the docs belong into *-devel and not into a separate package, installing
its files into /usr/share/doc/<package>-doc-%version.
If you really want to keep the docs as a separate package, then they either
should be installed into a separate shared directory (e.g.
/usr/share/<package>/docs or similar) or be folded into the *-devel package.
I definitely prefer seeing them inside of *-devel's
/usr/share/doc/<package>-%version directory.


Not relevant for Fedora, but more a personal note to you as upstream maintainer:
The configure script could use a major overhaul.

Comment 4 Rick L Vinyard Jr 2006-03-04 09:34:48 UTC
1) Ahhh, I forgot to change that one in the .pc.in file. Fixed now.

2) I like the 'feel' of /usr/share/doc/<package>-%version a lot better. It's in
the new version.

configure script: It's amazing how you overlook something like that until
someone mentions it. It was waaaaay overdue and reworked now.

Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/idioskopos.spec
SRPM Name or Url:
http://miskatonic.cs.nmsu.edu/pub/fedora/4/srpms/idioskopos-0.1.9-1.src.rpm

Comment 5 Rick L Vinyard Jr 2006-03-05 21:34:10 UTC
A few minor tweaks, including cleanup of the Source tag and the %setup section.

Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/idioskopos.spec

SRPM Name or Url:
http://miskatonic.cs.nmsu.edu/pub/fedora/4/srpms/idioskopos-0.1.9-2.src.rpm


Comment 6 Ralf Corsepius 2006-03-06 05:37:02 UTC
The code isn't ready for GCC-4.1:
...
make[3]: Entering directory `/builddir/build/BUILD/idioskopos-0.1.9/examples/simple'
if g++ -DHAVE_CONFIG_H -I. -I. -I../.. -I../../. -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=i386
-mtune=generic -fasynchronous-unwind-tables -MT simple.o -MD -MP -MF
".deps/simple.Tpo" -c -o simple.o simple.cpp; \
then mv -f ".deps/simple.Tpo" ".deps/simple.Po"; else rm -f ".deps/simple.Tpo";
exit 1; fi
simple.cpp:27: error: extra qualification 'Simple::' on member 'Simple'
simple.cpp:32: error: extra qualification 'Simple::' on member 'Simple'
simple.cpp:46: error: extra qualification 'Example::' on member 'Example'
simple.cpp: In function 'int main()':
simple.cpp:73: warning: unused variable 'y'
simple.cpp:75: warning: unused variable 'c'
make[3]: *** [simple.o] Error 1

Comment 7 Rick L Vinyard Jr 2006-03-06 07:03:46 UTC
How embarrassing to have such a simple error in the example code labelled
'simple'; and that after the state that configure.in was in. At least on this
one I can claim that GCC-4.0 passed it, even though good style would have
demanded that it never be there.

Ralf, I never intended for you to end up a code reviewer, but I sincerely
appreciate the items you've pointed out.

Anyway, I've pushed a new release with the fixed example code. I've made a few
changes to the spec (replacing ${RPM_BUILD_ROOT} with %{buildroot}, make with
%{__make}, and rm with %{__rm}). 

I've double and triple checked it to ensure that I didn't introduce any new
errors to ensure that this is the last time you have to look at this package.

Thanks again, and hopefully for the last time, here are the files:

Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/idioskopos.spec

SRPM Name or Url:
http://miskatonic.cs.nmsu.edu/pub/fedora/4/srpms/idioskopos-0.1.10-1.src.rpm

Comment 8 Ralf Corsepius 2006-03-06 07:40:48 UTC
Now you've kicked out the docs entirely. Why this?


Comment 9 Rick L Vinyard Jr 2006-03-08 05:31:44 UTC
It took awhile to hunt this one down, since I didn't actually kick the docs out;
rpmbuild was doing a rm -rf on the directory that automake installed the docs
into, since I was installing them to:
  /usr/share/doc/idioskopos-0.1.10/html/

What if I have automake do the install into:
  /usr/share/doc/idioskopos-docs-0.1.10/

And have the following in the devel subpackage %files section:
  %docdir %{_docdir}/%{name}-docs-%{version}/html
  %{_docdir}/%{name}-docs-%{version}/html


Comment 10 Rick L Vinyard Jr 2006-03-08 06:04:08 UTC
Even better... how about if I do
  mv docs/www/reference/html .

at the end of %install, and then simply
  %doc ChangeLog html

in %files devel?

That way all the docs are put under:
  /usr/share/doc/idioskopos-devel-0.1.10/html/


Comment 11 Matt Domsch 2006-03-08 13:11:28 UTC
I think Rick's Comment #10 is a fair thing to do, based on what I see in other
packages as a short list of examples:
alsa-lib-devel-1.0.10 has files in %doc/doxygen/html/*
libstdc++-devel-4.0.2 has files in %doc/html/*
libusb-devel-0.1.10a has files in %doc/html/*
neon-devel-0.24.7 has files in %doc/html/*

-Matt

Comment 12 Rick L Vinyard Jr 2006-03-10 06:08:09 UTC
Docs are back. I used the approach in comment #10.

Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/idioskopos.spec

SRPM Name or Url:
http://miskatonic.cs.nmsu.edu/pub/fedora/4/srpms/idioskopos-0.1.11-1.src.rpm

Comment 13 Rick L Vinyard Jr 2006-03-27 04:32:49 UTC
Made a few changes to reflect other items brought up in the cairomm review.

Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/idioskopos.spec

SRPM Name or Url:
http://miskatonic.cs.nmsu.edu/pub/fedora/5/srpms/idioskopos-0.1.13-1.src.rpm

Comment 14 Rick L Vinyard Jr 2006-05-21 16:37:59 UTC
New release - 0.2.0

Small changes to spec file to remove doxygen and graphiz build dependencies
since docs are now pre-built.

Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/idioskopos.spec

SRPM Name or Url:
http://miskatonic.cs.nmsu.edu/pub/fedora/5/srpms/idioskopos-0.2.0-1.src.rpm

Comment 15 Rick L Vinyard Jr 2006-07-15 18:25:17 UTC
This library was submitted as a dependency for papyrus and conexus. Neither
depend on this library anymore, so I'm withdrawing this submission. If there is
other interest in this library, I'll resubmit.

Comment 16 Jason Tibbitts 2006-11-16 15:26:33 UTC

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