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 194612

Summary:	Review Request: pstoedit
Product:	[Fedora] Fedora	Reporter:	Denis Leroy <denis>
Component:	Package Review	Assignee:	Patrice Dumas <pertusus>
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-06-15 17:55:37 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, 175257

Description Denis Leroy 2006-06-14 12:33:30 UTC

Spec URL: http://www.poolshark.org/src/pstoedit.spec
SRPM URL: http://www.poolshark.org/src/pstoedit-3.44-1.src.rpm
Description: Translates PostScript and PDF graphics into other vector formats

Pstoedit converts PostScript and PDF files to various vector graphic
formats. The resulting files can be edited or imported into various
drawing packages. Pstoedit comes with a large set of integrated format
drivers.

Misc notes :

- The main goal of having pstoedit in Extras is for the benefit of inkscape. See bug 175257.

- You'll notice the call to make doesn't use the _smp_flags macros. That's intentional, pstoedit's autoconf setup doesn't like parallel builds. Bug filed upstream.

Comment 1 Patrice Dumas 2006-06-14 15:14:51 UTC

It would be nice for xfig too. 

Not a a blocker but did you consider packaging plotutils, since it
is especially advertized in the readme that it gives much more
output formats.

I am seriously considering packaging ming, and then you'll be able
to enable swf support but it may be approved before that.

From my reading of configure.ac, it seems that you miss 
Buildrequires: gd-devel
gd-devel requires libpng-devel and zlib-devel, but I think that it
makes sense to keep the BR on libpng-devel.

There is a BR (and a Requires) on ghostscript missing (for gs).

Comment 2 Patrice Dumas 2006-06-14 15:24:19 UTC

pstoedit overwrite CXXFLAGS. This should be reported upstream
and in the meantime, the best solution seems to me to be a patch
for
configure
that removes the line 22593

(line 398 in configure.ac for upstream)

Comment 3 Denis Leroy 2006-06-14 23:28:56 UTC

Spec URL: http://www.poolshark.org/src/pstoedit.spec
SRPM URL: http://www.poolshark.org/src/pstoedit-3.44-2.src.rpm

Thanks for catching the missing Req and BRs. I filed the configure.ac bug upstream.

Yes, I was aware of the optional dependency on plotutils, but my main goal was
to close bug 175257 since the ability to import postscript files into inkscape
(and xfig) was requested by multiple people. I would recommend that we go on
with the review of pstoedit without plotutils support for now, and build a new
release later when plotutils is available (I'll work on it). If you submit a
package for ming, I'll review it.

Comment 4 Patrice Dumas 2006-06-15 07:23:06 UTC

There are .so files in /usr/lib/pstoedit/. Are these dlopened
'internal' libraries?

Comment 5 Denis Leroy 2006-06-15 10:47:26 UTC

Right. I had initially put them in the devel package, but that doesn't work.
They're not automatically linked into the pstoedit executable, but rather loaded
internally by pstoedit init code (see src/dynload.cpp).

Comment 6 Patrice Dumas 2006-06-15 14:47:50 UTC

Indeed they seem to qualify as internal dlopened libraries. It is 
somehow strange that upstream don't simply link them, it would be
cleaner, but it isn't really problematic. That was the last issue
I found, so now for the formal review:

* rpmlint is silent
* follow naming guidelines
* licence is GPL and included
* source match upstream 13f24cb070da3f6af82ed84f4e53f049
* build on FC5
* buildrequires seem right, although I haven't built in mock
* ldconfig is run
* creates the directory it owns, except /usr/share/aclocal/. Not a
  big deal, in my opinion, as there are other packages that do that.
  I am not sure but it seems that there were some discussions about
  that, but I can't recall the result. I don't consider that a blocker
  but you may want to raise the issue on the extras list
* things in -devel are right. There are .so in the main package, 
  but these are dlopened libraries

APPROVED

Comment 7 Patrice Dumas 2006-06-15 15:21:41 UTC

(In reply to comment #3)

> release later when plotutils is available (I'll work on it). If you submit a
> package for ming, I'll review it.

I'm not in a hurry to package ming since it evolves rapidly and
is a bit beta. As long as nobody ask explicitly for it I'll 
refrain...

Comment 8 Denis Leroy 2006-06-15 15:50:11 UTC

Patrice, many thanks for your review.

> buildrequires seem right, although I haven't built in mock
I did. Builds cleanly for FC-4, FC-5 and devel.