Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 215883 (idioskopos)
Summary: | Review Request: idioskopos - C++ Introspection Library | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Rick L Vinyard Jr <rvinyard> | ||||
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> | ||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | ||||||
Target Milestone: | --- | ||||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2006-12-08 02:32:51 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: | 163779 | ||||||
Attachments: |
|
Description
Rick L Vinyard Jr
2006-11-16 05:36:13 UTC
*** Bug 183438 has been marked as a duplicate of this bug. *** New release. Adds libxml++ dependency. Spec URL: http://miskatonic.cs.nmsu.edu/pub/idioskopos.spec SRPM URL: http://miskatonic.cs.nmsu.edu/pub/idioskopos-0.3.0-1.src.rpm Well, first review for this: A. From http://fedoraproject.org/wiki/Packaging/Guidelines : * Licensing - Well, /usr/lib/pkgconfig/idioskopos-1.0.pc reads: ------------------------------------------------------ ## This program is free software; you can redistribute it and/or modify ## ## it under the terms of the GNU General Public License as ## ## published by the Free Software Foundation version 2.1 ## ------------------------------------------------------ So this package is licensed under GPL, not LGPL because GPL is more strict than LGPL... * BuildRequires: - Is m4 required? Mockbuild succeeds without m4 and rpmdiff shows no difference. * Timestamps - Well, -devel package contains a lot of header files so keeping timestamps is highly preferable as * it shows if vendor (like you) have modified the original files * it shows when the files are created So keep timestamps, at least for header files. Usually, -------------------------------------------------------- make INSTALL="install -p" install -------------------------------------------------------- plays the trick. B. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines (okay.) C. Other things I have noticed: * Spec file description ---------------------------------------------------------- %install ........ %{__cp} -ar docs/reference . ........ %doc ChangeLog reference ---------------------------------------------------------- This should be okay with --------------------------------------------------------- %install ........ ....... %doc ChangeLog docs/reference --------------------------------------------------------- > A. From http://fedoraproject.org/wiki/Packaging/Guidelines : > * Licensing Got that fixed. I copied the .pc.in from another project (that I have under the GPL), and missed changing that one. > * BuildRequires: > - Is m4 required? Yes. My configure script needs m4 (it's used to autobuild the Fedora and SuSE .spec files from spec.m4), and m4 isn't on the exceptions list: http://fedoraproject.org/wiki/Extras/FullExceptionList > * Timestamps I hadn't even noticed that. I agree, preserving the timestamps is nice! Changed: %{__make} DESTDIR=%{buildroot} install To: %{__make} DESTDIR=%{buildroot} INSTALL="%{__install} -p" install > C. Other things I have noticed: Yes, that is much better. Thanks for all your help. A new spec and release are at: Spec URL: http://miskatonic.cs.nmsu.edu/pub/idioskopos.spec SRPM URL: http://miskatonic.cs.nmsu.edu/pub/idioskopos-0.3.2-1.src.rpm Created attachment 142656 [details] Mock build log of idioskopos 0.3.2-1 (In reply to comment #4) > > * BuildRequires: > > - Is m4 required? > > Yes. My configure script needs m4 (it's used to autobuild the Fedora and SuSE > .spec files from spec.m4), and m4 isn't on the exceptions list: > http://fedoraproject.org/wiki/Extras/FullExceptionList This is true for people who have to create spec file from spec.m4, however, srpm should include spec file (not spec.m4) and there is no need to create spec file again from spec.m4. So m4 should not needed for this reason. By the way I cannot rebuild 0.3.2-1 by mockbuild under FC-devel i386. Please check the build log attached. > This is true for people who have to create spec file from > spec.m4, however, srpm should include spec file (not spec.m4) > and there is no need to create spec file again from spec.m4. > So m4 should not needed for this reason. But, the specs are built by autoconf when configure is run. I know it's not the best, but I just haven't had time to modify configure.in to take an option of whether to build the specs or not. Right now, they're always built when configure is run. > By the way I cannot rebuild 0.3.2-1 by mockbuild under > FC-devel i386. Please check the build log attached. Looks like it was an overload issue with size_t and unsigned int on i386. It's fixed in the 0.3.3 release. Spec URL: http://miskatonic.cs.nmsu.edu/pub/idioskopos.spec SRPM URL: http://miskatonic.cs.nmsu.edu/pub/idioskopos-0.3.3-1.src.rpm (In reply to comment #6) > But, the specs are built by autoconf when configure is run. It seems not. ------------------------------------------------- checking for tr1/boost_shared_ptr.h... yes checking tr1/array usability... yes checking tr1/array presence... yes checking for tr1/array... 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: line 22376: suse-10.1/idioskopos.spec.in: No such file or directory configure: creating ./config.status config.status: creating fedora-5/idioskopos.spec config.status: creating fedora-6/idioskopos.spec config.status: creating idioskopos-1.0.pc config.status: creating docs/www/site.php config.status: creating Makefile config.status: creating idioskopos/Makefile config.status: creating examples/Makefile ------------------------------------------------- and ... suse-10.1/idioskopos.spec.in is never used. Actually I successfully rebuild this packge without m4 by mockbuild. Anyway, if running configure requires autoconf, it is not correct and should be fixed if so. (In reply to comment #7) > (In reply to comment #6) > > But, the specs are built by autoconf when configure is run. > Sorry about that. I meant to say, the specs are built by m4 when configure is run. This is the section in configure that requires m4: ######################################################################### # build spec.in files ######################################################################### for distro in "fedora-5 FEDORA 5" "fedora-6 FEDORA 6"; do original_params=("$@") set -- $distro mkdir ${1} m4 -D DISTRO=${2} \ -D DISTRO_LIB_GROUP="${2}_${3}_LIB_GROUP" \ -D DISTRO_BUILD_REQUIRES="${2}_${3}_BUILD_REQUIRES" \ -D DISTRO_DEVEL_GROUP="${2}_${3}_DEVEL_GROUP" \ -D DISTRO_DEVEL_REQUIRES="${2}_${3}_DEVEL_REQUIRES" \ spec.m4 >${1}/${PACKAGE_NAME}.spec.in set -- $original_params done And then, later on in configure.in I have the spec.in that was built above in AC_OUTPUT(). > ./configure: line 22376: suse-10.1/idioskopos.spec.in: No such file or directory I thought I removed the suse-10.1 directories for the 0.3.3 release, but I must have done it after I pushed the release. That's the m4 command failing because the 0.3.3 release didn't have the 'mkdir ${1}'. > Actually I successfully rebuild this packge without m4 > by mockbuild. Now that I think about it, the m4 command will probably silently fail without m4 installed. I don't have a problem removing the m4 build-requires. I just want to make sure it's right. Well, for m4 issue, I will leave it as you say. I will check if there is any rest issue in this package later (mockbuild of 0.3.3-1 is okay) Okay. ------------------------------------------- This package (idioskopos) is APPROVED by me. |