Bug 215883 - (idioskopos) Review Request: idioskopos - C++ Introspection Library
Review Request: idioskopos - C++ Introspection Library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
: 183438 (view as bug list)
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-11-16 00:36 EST by Rick L Vinyard Jr
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-12-07 21:32:51 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Mock build log of idioskopos 0.3.2-1 (50.12 KB, text/plain)
2006-12-01 22:59 EST, Mamoru TASAKA
no flags Details

  None (edit)
Description Rick L Vinyard Jr 2006-11-16 00:36:13 EST
Spec URL: http://miskatonic.cs.nmsu.edu/pub/idioskopos.spec
SRPM URL: http://miskatonic.cs.nmsu.edu/pub/idioskopos-0.2.1-1.src.rpm

Description:

Idioskopos is a C++ introspection library. The next release of the conexus and conexusmm libraries (both in FE already) will depend on this library.
Comment 1 Jason Tibbitts 2006-11-16 10:26:38 EST
*** Bug 183438 has been marked as a duplicate of this bug. ***
Comment 2 Rick L Vinyard Jr 2006-11-19 21:20:23 EST
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
Comment 3 Mamoru TASAKA 2006-12-01 09:35:33 EST
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
---------------------------------------------------------
Comment 4 Rick L Vinyard Jr 2006-12-01 11:34:44 EST
> 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

Comment 5 Mamoru TASAKA 2006-12-01 22:59:28 EST
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.
Comment 6 Rick L Vinyard Jr 2006-12-03 12:57:05 EST
> 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
Comment 7 Mamoru TASAKA 2006-12-04 08:23:21 EST
(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.
Comment 8 Rick L Vinyard Jr 2006-12-04 20:37:37 EST
(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.

Comment 9 Mamoru TASAKA 2006-12-05 00:35:34 EST
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)
Comment 10 Mamoru TASAKA 2006-12-05 09:16:28 EST
Okay.

-------------------------------------------
  This package (idioskopos) is APPROVED by me.

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