Bug 183439 - (papyrus) Review Request: papyrus (Canvas drawing library based on cairo/cairomm)
Review Request: papyrus (Canvas drawing library based on cairo/cairomm)
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul F. Johnson
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-02-28 21:48 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-08-26 00:41:15 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Rick L Vinyard Jr 2006-02-28 21:48:39 EST
Need sponsor.

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.5-1.src.rpm

Description:
Papyrus is a C++ canvas library similar in scope
and function to the Gnome canvas but designed to render on a
Cairo surface.
Comment 1 Rick L Vinyard Jr 2006-02-28 21:49:51 EST
Note: This will only build on FC5, and building depends on two libraries that
are themselves pending review.
Comment 2 Rick L Vinyard Jr 2006-03-03 13:37:01 EST
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.
Comment 3 Rick L Vinyard Jr 2006-03-05 16:35:11 EST
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
Comment 4 Rick L Vinyard Jr 2006-04-09 19:26:44 EDT
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
Comment 5 Michael A. Peters 2006-04-28 04:18:59 EDT
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
Comment 6 Rick L Vinyard Jr 2006-05-01 22:30:01 EDT
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.
Comment 7 Rick L Vinyard Jr 2006-05-07 18:43:37 EDT
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
Comment 8 John Mahowald 2006-05-21 13:41:39 EDT
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.

Comment 9 Rick L Vinyard Jr 2006-05-21 14:50:58 EDT
> 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?
Comment 10 Rick L Vinyard Jr 2006-07-21 13:41:47 EDT
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
Comment 11 Paul F. Johnson 2006-07-29 18:13:16 EDT
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?



Comment 12 Rick L Vinyard Jr 2006-07-29 18:52:54 EDT
> 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.
Comment 13 Paul F. Johnson 2006-07-29 18:57:29 EDT
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.
Comment 14 Rick L Vinyard Jr 2006-07-29 19:19:09 EDT
> > 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.
Comment 15 Rick L Vinyard Jr 2006-07-29 19:24:36 EDT
> 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.
Comment 16 Paul F. Johnson 2006-07-29 19:27:11 EDT
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'
Comment 17 Rick L Vinyard Jr 2006-07-30 18:18:42 EDT
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.
Comment 18 Rick L Vinyard Jr 2006-07-30 18:39:55 EDT
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@cs.nmsu.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
Comment 19 Paul Howarth 2006-07-31 07:35:00 EDT
(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@cs.nmsu.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.
Comment 20 Paul F. Johnson 2006-07-31 07:43:55 EDT
It certainly does look like an x86 problem. I've getting the same results on my
laptop as I do at home.
Comment 21 Paul F. Johnson 2006-08-01 06:40:16 EDT
Tested on two other x86 machines, failed to build in mock with the same error
Comment 22 Rick L Vinyard Jr 2006-08-02 00:03:53 EDT
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.
Comment 23 Paul F. Johnson 2006-08-05 13:54:10 EDT
Has the updated version of cairomm been added to FE yet? It's still not linking
here.
Comment 24 Rick L Vinyard Jr 2006-08-06 20:33:07 EDT
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.

Comment 25 Rick L Vinyard Jr 2006-08-06 23:19:42 EDT
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@cs.nmsu.edu> - 0.2.3-1
- New release
- Added m4 to BuildRequires
Comment 26 Paul F. Johnson 2006-08-10 17:07:21 EDT
%{_datadir}/papyrus-0.2.3/demo/

Who owns %{_datadir}/papyrus-0.2.3?

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

and all should be happy :-)
Comment 27 Paul F. Johnson 2006-08-10 17:38:05 EDT
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
Comment 28 Rick L Vinyard Jr 2006-08-15 23:05:11 EDT
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
Comment 29 Rick L Vinyard Jr 2006-08-20 14:52:31 EDT
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
Comment 30 Paul F. Johnson 2006-08-20 15:48:51 EDT
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
Comment 31 Paul F. Johnson 2006-08-23 06:10:36 EDT
Anything happening with this bug?
Comment 32 Rick L Vinyard Jr 2006-08-23 19:08:42 EDT
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.

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