Bug 261581

Summary:	Review Request: xmlroff - A XSL-FO processor
Product:	[Fedora] Fedora	Reporter:	Ignacio Vazquez-Abrams <ivazqueznet>
Component:	Package Review	Assignee:	Mamoru TASAKA <mtasaka>
Status:	CLOSED RAWHIDE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	fedora-package-review, jsmith.fedora, mtasaka, notting
Target Milestone:	---	Flags:	mtasaka: fedora-review+ kevin: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2007-11-16 12:11:11 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:

Description Ignacio Vazquez-Abrams 2007-08-28 20:10:32 UTC

Spec URL: http://ivazquez.fedorapeople.org/packages/xmlroff/xmlroff.spec
SRPM URL: http://ivazquez.fedorapeople.org/packages/xmlroff/xmlroff-0.5.2-1.fc7.src.rpm
Description: xmlroff is a fast, free, high-quality, multi-platform XSL formatter that aims to excel at DocBook formatting and that integrates easily with other programs and with scripting languages.

Comment 1 Mamoru TASAKA 2007-10-15 17:38:22 UTC

Please check your packages by rpmlint before submitting
to review request.

For 0.5.2-1:

* Dependency between subpackags
  - Please check the dependency between subpackages of this srpm
    - xmlroff should have "libfo = %{version}-%{release}"
    - libfo-devel should have "libfo = %{version}-%{release}".

* Requires
  - Would you explain why libfo-devel should require
    libxslt-devel?

* ldconfig
  - Please call ldconfig for libraries under %_libdir on
    scriptlets.

* Documents
  - The dependency order is that xmlroff requires libfo.
    So if only libfo is installed, no documents (COPYING,
    AUTHORS and so on) are installed, which is not proper.

* Directory ownership
  - Please check the owner of %_datadir/xml .
    On my system,
--------------------------------------------------------
$ rpm -qf /usr/share/xml
xml-common-0.6.3-21.fc8
libglade2-2.6.2-3.fc8
--------------------------------------------------------
    However xmlroff requires neither of these packages.

* pkgconfig .pc file
  - should be installed in -devel package.
  - And the package containing .pc file must have 
    "Requires: pkgconfig".

* Timestamp
  - To keep timestamps, please use
-------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
-------------------------------------------------------
    This method usually works for recent Makefiles.

Comment 2 Mamoru TASAKA 2007-10-24 13:26:40 UTC

ping?

Comment 3 Ignacio Vazquez-Abrams 2007-10-26 07:46:57 UTC

(In reply to comment #1)
> * Requires
>   - Would you explain why libfo-devel should require
>     libxslt-devel?

From libfo-0.5.pc:

> Requires:  glib-2.0 gobject-2.0 gmodule-2.0 libxslt gdk-pixbuf-2.0

http://ivazquez.fedorapeople.org/packages/xmlroff/xmlroff.spec
http://ivazquez.fedorapeople.org/packages/xmlroff/xmlroff-0.5.2-2.src.rpm

Comment 4 Mamoru TASAKA 2007-10-26 08:03:00 UTC

(In reply to comment #3)
> (In reply to comment #1)
> > * Requires
> >   - Would you explain why libfo-devel should require
> >     libxslt-devel?
> 
> From libfo-0.5.pc:
> 
> > Requires:  glib-2.0 gobject-2.0 gmodule-2.0 libxslt gdk-pixbuf-2.0

So would you check if libxslt-devel is _actually_ needed?
From header files it seems this "libxslt" should be "libxml-2.0"
(i.e. Requires: not libxslt-devel but libxml2-devel).

Comment 5 Ignacio Vazquez-Abrams 2007-10-26 08:40:25 UTC

(In reply to comment #4)
> So would you check if libxslt-devel is _actually_ needed?
> From header files it seems this "libxslt" should be "libxml-2.0"
> (i.e. Requires: not libxslt-devel but libxml2-devel).

From libfo/fo-xslt-transformer.c:

> #include <libxslt/xslt.h>
> #include <libxslt/xsltInternals.h>
> #include <libxslt/transform.h>

Comment 6 Mamoru TASAKA 2007-10-26 09:25:37 UTC

(In reply to comment #5)
> (In reply to comment #4)
> > So would you check if libxslt-devel is _actually_ needed?
> > From header files it seems this "libxslt" should be "libxml-2.0"
> > (i.e. Requires: not libxslt-devel but libxml2-devel).
> 
> From libfo/fo-xslt-transformer.c:
> 
> > #include <libxslt/xslt.h>
> > #include <libxslt/xsltInternals.h>
> > #include <libxslt/transform.h>

It is for BuildRequires, not for Requires for -devel package.

Comment 7 Ignacio Vazquez-Abrams 2007-10-28 08:12:31 UTC

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > So would you check if libxslt-devel is _actually_ needed?
> > > From header files it seems this "libxslt" should be "libxml-2.0"
> > > (i.e. Requires: not libxslt-devel but libxml2-devel).
> > 
> > From libfo/fo-xslt-transformer.c:
> > 
> > > #include <libxslt/xslt.h>
> > > #include <libxslt/xsltInternals.h>
> > > #include <libxslt/transform.h>
> 
> It is for BuildRequires, not for Requires for -devel package.

No, another package BuildRequires libfo-devel, which then in turn Requires
libxslt-devel so that the package can build.

Comment 8 Mamoru TASAKA 2007-10-28 08:30:06 UTC

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > (In reply to comment #4)
> > > > So would you check if libxslt-devel is _actually_ needed?
> > > > From header files it seems this "libxslt" should be "libxml-2.0"
> > > > (i.e. Requires: not libxslt-devel but libxml2-devel).
> > > 
> > > From 
:
> > > 
> > > > #include <libxslt/xslt.h>
> > > > #include <libxslt/xsltInternals.h>
> > > > #include <libxslt/transform.h>
> > 
> > It is for BuildRequires, not for Requires for -devel package.
> 
> No, another package BuildRequires libfo-devel, which then in turn Requires
> libxslt-devel so that the package can build.

in turn Requires s|libxslt-devel|libxml2-devel|

All symbols which can be used from external libraries/binaries
are written in installed header files.

Comment 9 Ignacio Vazquez-Abrams 2007-11-03 07:08:54 UTC

I see what you did there...

http://ivazquez.fedorapeople.org/packages/xmlroff/xmlroff.spec
http://ivazquez.fedorapeople.org/packages/xmlroff/xmlroff-0.5.2-3.src.rpm

Comment 10 Mamoru TASAKA 2007-11-03 12:34:13 UTC

For 0.5.2-3:

* Dependency for libfo-devel:
  - Well, however libfo-devel needs libxml2-devel, which cannot
    be removed.

    I came to think that because to require libxslt-devel also
    require libxml2-devel and extra dependency for libxslt-devel
    is small, we can leave libxslt-devel dependency

    So would you please remove xmlroff-0.5.2-nolibxslt.patch and
    have libfo-devel require libxslt-devel again? as you did
    before? Sorry...

Comment 11 Ignacio Vazquez-Abrams 2007-11-09 07:42:55 UTC

I'll do you one better.

http://ivazquez.fedorapeople.org/packages/xmlroff/xmlroff.spec
http://ivazquez.fedorapeople.org/packages/xmlroff/xmlroff-0.5.2-4.src.rpm

Comment 12 Mamoru TASAKA 2007-11-09 15:12:44 UTC

Okay. 

------------------------------------------------------------------
    This package (xmlroff) is APPROVED by me
------------------------------------------------------------------

Comment 13 Ignacio Vazquez-Abrams 2007-11-14 13:44:43 UTC

New Package CVS Request
=======================
Package Name: xmlroff
Short Description: A XSL-FO processor
Owners: ivazquez
Branches: F-7 F-8
Cvsextras Commits: yes

Comment 14 Kevin Fenzi 2007-11-15 00:22:43 UTC

cvs done.

Comment 15 Mamoru TASAKA 2007-11-16 06:47:05 UTC

Please close this bug when rebuild and request on bodhi are done.

Comment 16 Ignacio Vazquez-Abrams 2007-11-16 12:11:11 UTC

Built on F-7, F-8, and rawhide, and pushed.