Bug 183438
Summary: | Review Request: idioskopos (C++ Introspection Library) | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Rick L Vinyard Jr <rvinyard> |
Component: | Package Review | Assignee: | 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: | rawhide | Keywords: | 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
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 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 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. 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 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 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 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 Now you've kicked out the docs entirely. Why this? 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 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/ 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 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 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 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 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. *** This bug has been marked as a duplicate of 215883 *** |