Bug 183439 (papyrus)

Note: This will only build on FC5, and building depends on two libraries that
are themselves pending review.

Note to reviewer:

The configure script has a problem with the tr1 shared pointer header and needs
the same AC_CHECK_HEADER voodoo that idioskopos needed
(http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=183438).

There's a new Sketchpad in this library, so I'll push a new release later today
that will have the new code plus the AC_CHECK_HEADER voodoo.

A new release and a few minor tweaks, including cleanup of the Source tag and
the %setup section.

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

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

New release and changes reflecting cairomm 0.6.0.

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

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

Are you in need of a sponsor?
I don't see the e-mail that opened this bug in the owners.list file.

If you are in need of a sponsor, you should add bug #177841 the the
Bug 183439 blocks field

Michael (Peters),

At the time I originally posted this review bug I did need a sponsor, but since 
then the cairomm package (which the papyrus canvas library needed as a 
dependency) has now gone through the process and Michael (Schwendt) sponsored 
it.

At this point, I suppose all this needs is a review of the other dependency 
(idioskopos) and this canvas library.

Fortunately, I think both are in good shape since all the packages I've 
submitted for review use the same autoconf spec template as cairomm did; so 
feedback on one resulted in improvements to all.

New release, and the dependency on Idioskopos is now removed.

With this release and cairomm now in FE, all dependencies are met in FE-5.

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

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

rpmlint of papyrus:
W: papyrus incoherent-version-in-changelog 0.1.10-1 0.1.11-1.fc6

You're one behind in the changelogs.

Sources do not match:

3d0c75f05409da34a8766e7ddbc9df0f  papyrus-0.1.11.tar.bz2 (download)
17886301eb027ff4d727c9d27b38570f  papyrus-0.1.11.tar.bz2 (srpm)

The srpm appears to have a more recent ChangeLog.

While you're at it change the Source to point to download.sourceforge.net to
automatically choose a mirror.

> While you're at it change the Source to point to download.sourceforge.net to
> automatically choose a mirror.

wget http://download.sourceforge.net/libpapyrus/papyrus-0.1.11.tar.bz2
just hangs.

wget http://download.sourceforge.net/libpapyrus/papyrus-0.1.11.tar.bz2?download
succeeds.

Is the latter syntax ok?

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

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

Summary of changes:
- New release
- Added papyrus-demo and papyrusmm-demo to devel package
- Remove rm -rf /usr/bin
- Changed mv of docs to cp of docs (allows for consistent staged builds)
- Changed sourceforge download location to download.sourceforge.net

Group:            System Environment/Libraries

Would this be not better suited to Development/Libraries? System
Environment/Libs are really for the likes of glibc et al.

Need R: cairomm

%package devel
Requires:         gtkmm24-devel >= 2.8.0 cairomm-devel >= 0.6.0 pkgconfig
doxygen graphviz

Once installed, do rpm -qa --requires <package-name>. This will give you the
requires for main and devel packages to run correctly. I seriously doubt it will
need anything past cairomm-devel

%install

%{__cp} -arv docs/reference .
%{__cp} -arv docs/gallery .

Should these not be included in the %docs part of %files? If they're not, where
are they supposed to be getting copied to? Are the permissions being kept correctly?

%files
%{_libdir}/lib*.so.*

Niggle. How many libraries are being installed? If it's one, just expand it.
This makes reading the spec for us poor reviewers much simpler. If it's a pile
which can be globbed together, then %{_libdir}/libpap*.so.* is also okay.

%files devel
%{_bindir}/*-demo

I'd strongly recommend naming this

%doc ChangeLog reference gallery

This is confusing me. You've already copied reference and gallary somewhere -
why have you got this?

> Would this be not better suited to Development/Libraries?

I'm not sure. Most of the other libraries, similar to papyrus (gtkmm24, 
libglademm26, libxml++, libgnomemm, et. al.) are under System Environment/
Libraries.

Maybe they're behind and need to be changed?

> Need R: cairomm

The cairomm requires gets picked up by rpmbuild as a library requires.

> I seriously doubt it will need anything past cairomm-devel

The papyrusgtk stuff will need gtkmm, and theres no dependency chain between 
cairomm and gtkmm. pkgconfig is there since the autoconf configure requires it 
to find gtkmm and cairomm.

> %{__cp} -arv docs/reference .
> %{__cp} -arv docs/gallery .
> 
> Should these not be included in the %docs part of %files?

> %doc ChangeLog reference gallery
>
> This is confusing me. You've already copied reference and gallary somewhere -
> why have you got this?

This is done to move them into position so they appear in a better position for 
the rpm. In the dist they're in:
   docs/www/reference/
If I include them in the rpm without cp'ing they'd install as:
   /usr/share/doc/papyrus-0.2.0-devel/docs/www/reference/
Doing it this way they install as:
   /usr/share/doc/papyrus-0.2.0-devel/reference/

> %files
> %{_libdir}/lib*.so.*

> %files devel
> %{_bindir}/*-demo

The main reason I've done these is because I'm upstream on several libraries 
and I've tried to generalize the templates a bit to make them maintainable by 
autoconf (autoconf actually generates the specs).

I know it's a little general, but it allows me to apply one correction to all 
the specs related to the libraries.

BR = required to build
R = required to run (once installed)

Will a package really need pkgconfig to run once it's installed?

As to the %{_bindir}, I'd still rather have the name. I have no problem with a
generic spec file as such, but remember, others read these spec files to see how
best write things.

Can you place a comment in the spec file which says why you're doing the cp
please? It will help.

> > Would this be not better suited to Development/Libraries?
> 
> I'm not sure. Most of the other libraries, similar to papyrus (gtkmm24, 
> libglademm26, libxml++, libgnomemm, et. al.) are under System Environment/
> Libraries.

I remember now... System Environment/Libraries is for the runtime stuff. 
Development/Libraries are for the devel packages.

> Will a package really need pkgconfig to run once it's installed?

Ahhh, I see. It's in the Requires for the devel package. I'll remove it and the 
doxygen and graphviz depends. Now that the docs are in the dist, doxygen and 
graphviz aren't even build requires anymore.

> Can you place a comment in the spec file which says why you're doing the cp
> please? It will help.

Good idea.

rpmlint shows no problems anywhere

mock fails to build (x86)

mkdir .libs
g++ (...) .libs/simple_main.o simple.o (...)
/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../libcairomm-1.0.so: undefined
reference to `cairo_ps_surface_set_dpi'

Unfortunately it succeeds on x86_64...

$ mock papyrus-0.2.1-2.src.rpm
init
clean
prep
This may take a while
setup
build
ending
done
Results and/or logs in: /var/lib/mock/fedora-5-x86_64-core/result
$

Perhaps we can get someone else with an x86 to confirm. It looks like the 
problem is a cairo/cairomm disconnect.

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

SRPM Name or Url:
http://miskatonic.cs.nmsu.edu/pub/fedora/5/srpms/papyrus-0.2.1-2.src.rpm

* Sat Jul 29 2006 Rick L Vinyard Jr <rvinyard.edu> - 0.2.1-2
- Changed make to %%{__make}
- Changed %%{name} to autoconf subst that puts specific name in devel requires
- Added comment regarding why cp occurs for docs
- Removed doxygen, graphviz and pkgconfig from devel requires
- Added package name to globs in so libs, .pc and demos

(In reply to comment #17)
> Unfortunately it succeeds on x86_64...
> 
> $ mock papyrus-0.2.1-2.src.rpm
> init
> clean
> prep
> This may take a while
> setup
> build
> ending
> done
> Results and/or logs in: /var/lib/mock/fedora-5-x86_64-core/result
> $
> 
> Perhaps we can get someone else with an x86 to confirm. It looks like the 
> problem is a cairo/cairomm disconnect.

I can reproduce this, and I think your diagnsis may be correct.

(In reply to comment #18)
> Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/papyrus.spec
> 
> SRPM Name or Url:
> http://miskatonic.cs.nmsu.edu/pub/fedora/5/srpms/papyrus-0.2.1-2.src.rpm
> 
> * Sat Jul 29 2006 Rick L Vinyard Jr <rvinyard.edu> - 0.2.1-2
> - Changed make to %%{__make}
> - Changed %%{name} to autoconf subst that puts specific name in devel requires
> - Added comment regarding why cp occurs for docs
> - Removed doxygen, graphviz and pkgconfig from devel requires
> - Added package name to globs in so libs, .pc and demos

Removing pkgconfig as a dependency of the devel package is wrong; it includes a
.pc file and should therefore require pkgconfig.

It certainly does look like an x86 problem. I've getting the same results on my
laptop as I do at home.

Tested on two other x86 machines, failed to build in mock with the same error

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

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

New spec: new release (adds checks for boost smart pointers if tr1 isn't found)
and adds pkgconfig to requires of devel package.

Also, cairomm is rebuilt against cairo, and pending release. Hopefully that will
take care of the linkage issue. If it doesn't, I'll talk with the cairomm devs.

Has the updated version of cairomm been added to FE yet? It's still not linking
here.

Yes. And it does look like the PostScript cairo surface is no longer being
built for Fedora's cairo. That doesn't have any bearing on papyrus, but it did
have an impact on cairomm that at the time built the PS surfaces because they
were present in Fedora's cairo.

$ less /var/lib/mock/fedora-5-i386-core/result/
build.log
mockconfig.log
papyrus-0.2.2-1.fc5.i386.rpm
papyrus-0.2.2-1.fc5.src.rpm
papyrus-debuginfo-0.2.2-1.fc5.i386.rpm
papyrus-devel-0.2.2-1.fc5.i386.rpm
root.log
$

And from root.log:

 cairo                   i386       1.0.4-1          updates-released  317 k
 cairo-devel             i386       1.0.4-1          updates-released  100 k
 cairomm                 i386       0.6.0-2.fc5      extras             36 k
 cairomm-devel           i386       0.6.0-2.fc5      extras            175 k

The 0.6.0-2 is the rebuilt cairomm.

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

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

* Sun Aug  6 2006 Rick L Vinyard Jr <rvinyard.edu> - 0.2.3-1
- New release
- Added m4 to BuildRequires

%{_datadir}/papyrus-0.2.3/demo/

Who owns %{_datadir}/papyrus-0.2.3?

Change to %{_datadir}/%{name}-%{version}/

and all should be happy :-)

Builds fine in mock, but rpmlint on the main installed package is coming up with
a pile of warnings along the lines of 

W: papyrus undefined-non-weak-symbol /usr/lib/libpapyrusgtk.so.0.0.0
_ZTIN7Papyrus6MarkerE

The others are clean

rpm -qa --requires is not showing any missing Rs

Spec Name or Url: http://miskatonic.dyn-o-saur.com/pub/papyrus.spec

SRPM Name or Url:
http://miskatonic.dyn-o-saur.com/pub/fedora/5/srpms/papyrus-0.3.0-1.src.rpm

I think this one addresses everything from the previous comments, plus a few 
more:
- New release fixes some problems with the missing demo files
- Changed ownership of the demo directories
- Added papyrus to the libtool LIBADD automake line should resolve the symbol 
warnings

Updated spec and SRPM locations (the University was having trouble with DNS
records at the time):

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

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

rpmlint is clean, mock is clean

Time for a review I suppose...

Upstream version is the same as this package
No ownership problems
Documentation included
Software does what it says it does
Consistent use of macros
pkgconfig included with R for devel
No problems with the devel package
No dupes found in the installed rpms
No smp_mflags problems

I think this is good to go

APPROVED

Anything happening with this bug?

Yup. There was a new release of cairomm that was just pushed into FC6 with a
stable API and a few minor changes. In particular Cairo::clear_path() became
Cairo::begin_new_path() so I was waiting to do the builds until I could have FC6
and FC5 at the same time.

I just got FC6 on my laptop last night, so I'm hoping to do some final tests
tonight or tomorrow night.