Bug 436356

I'll volunteer to review this

- Generic Guidelines -

- MUST: rpmlint must be run on every package. The output should be posted in the
review. i.e. NONE for the .src.rpm or noarch.rpm -> PASS
- The package must be named according to the Package Naming Guidelines, same as
upstream, which is a sane name + the openoffice.org- prefix required by the
draft OOo extensions naming scheme -> PASS
- MUST: The spec file name must match the base package %{name} -> PASS
- MUST: The package must meet the  Packaging Guidelines -> PASS
- MUST: The package must be licensed with a Fedora approved license and meet the
 Licensing Guidelines. -> source is in GPLv2+ -> PASS
- MUST: The License field in the package spec file must match the actual
license. -> FAIL, close, but the source I believe is GPLv2+, not just GPLv2.
Easy fix
- MUST: If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s) for the
package must be included in %doc -> PASS
- MUST: The spec file must be written in American English. -> PASS
- MUST: The spec file for the package MUST be legible -> PASS
- MUST: The sources used to build the package must match the upstream source, as
provided in the spec -> PASS
- MUST: The package must successfully compile and build into binary rpms on at
least one supported architecture. -> PASS
- MUST: If the package does not successfully compile... -> PASS
- MUST: All build dependencies must be listed in BuildRequires -> PASS
- MUST: The spec file MUST handle locales properly. -> PASS
- MUST: Every binary RPM package ... -> PASS
- MUST: If the package is designed to be relocatable -> PASS
- MUST: A package must own all directories that it creates... -> PASS
- MUST: A package must not contain any duplicate files -> PASS
- MUST: Permissions on files must be set properly. -> PASS
- MUST: Each package must have a %clean section -> PASS
- MUST: Each package must consistently use macros -> PASS
- MUST: The package must contain code, or permissable content -> PASS
- MUST: Large documentation files should go in a -doc subpackage -> PASS
- MUST: If a package includes something as %doc, it must not affect the runtime
of the application. -> PASS
- MUST: Header files must be in a -devel package. -> PASS
- MUST: Static libraries must be in a -static package. -> PASS
- MUST: Packages containing pkgconfig(.pc) files must... -> PASS
- MUST: If a package contains library files...  -> PASS
- MUST: In the vast majority of cases, devel packages... -> PASS
- MUST: Packages must NOT contain any .la libtool archives -> PASS
- MUST: Packages containing GUI applications must... -> PASS
- MUST: Packages must not own files or directories already owned by other
packages. -> PASS
- MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}
-> PASS
- MUST: All filenames in rpm packages must be valid UTF-8. -> PASS

"Shoulds" looks good too.

- Draft OOo Extensions Guidelines -
- Must have a %postun of 'unopkg list --shared > /dev/null 2>&1' -> PASS
- Should be both installed unpacked and then registered with 'unopkg --link' -> PASS
Unpacked Extensions Must be installed in a dir called NAME.oxt, NAME.uno.pkg or
NAME.zip -> PASS
- An extension should normally just be able to just Require: an appropriate
openoffice.org component e.g. openoffice.org-core -> PASS
- extensions Must be named openoffice.org-FOO -> PASS
- The license Must be acceptable, and the package Must be build-able from
source. -> PASS (code is in StarBasic, so *is* source rather than built from source)

a) so, change the license from GPLv2 to GPLv2+ and we're good. 
b) Also (my fault) I originally had some rules about
-env:JFW_PLUGIN_DO_NOT_CHECK_ACCESSIBILITY' and unopkg, but decided to just fix
it in unopkg instead so that's dropped now and can be removed from the package.

I attach a replacement .spec with the above changes. 

Created attachment 297904 [details]
spec with little change

So, I'll assume you'll be ok with those changes, and so I this this set to
passed on that proviso of getting "+" added to the license.

A small side note is that there is are a number of bugs in unopkg in F-7 and
F-8, so I strongly advise not building this for earlier versions of fedora until
I have those fixed. I'll announce in fedora-devel-list when I've got it fixed.

I've dropped the -env arg, but I don't see any evidence that it is licensed as
GPLv2+.  License statements I see are:

COPYING.txt:
  extendedPDF is free software; you can redistribute it and/or
modify it under the terms of version 2 of the GNU General Public
License as published by the Free Software Foundation.

install.sh:
# extendedPDF is free software; you can redistribute it and/or
# modify it under the terms of version 2 of the GNU General Public
# License as published by the Free Software Foundation.

install.bat:
rem extendedPDF is free software; you can redistribute it and/or
rem modify it under the terms of version 2 of the GNU General Public
rem License as published by the Free Software Foundation.

No reference to "or later" in any of these.  Is this a problem?

http://www.cora.nwra.com/~orion/fedora/openoffice.org-extendedPDF.spec
http://www.cora.nwra.com/~orion/fedora/openoffice.org-extendedPDF-1.4-2.fc8.src.rpm

* Thu Mar 13 2008 Orion Poplawski <orion.com> 1.4-2
- Drop -env arg to unopkg

"or later" is in the header of all the actual source files, i.e the .xba etc
files in the unzipped .zip
e.g. from extendedPDF/bmk.xba

' "extendedPDF"
' OpenOffice.org  macro to produce PDF files with bookmarks.
' Copyright (C) 2003-2006 3BView Limited
' Email: martin.brown
' Website: http://www.3bview.com
' See module extendedPDF for details of other contributors.
'
' 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; either version 2 of the License, or
' (at your option) any later version.

Works for me.

* Thu Mar 13 2008 Orion Poplawski <orion.com> 1.4-2
- Drop -env arg to unopkg
- Change license to GPLv2+

I've pinged upstream to get the various statements in sync, but source rules here.

Thanks for the review!

New Package CVS Request
=======================
Package Name: openoffice.org-extendedPDF
Short Description: Create PDF with hyperlinks, bookmarks and more
Owners: orion
Branches: F-8 F-7
InitialCC: 
Cvsextras Commits: yes

I've requested branches for F-7/8 but won't build until unopkg is fixed in those
releases.  What's the status of EL-5?  Will unopkg ever be supported there?

god only knows, RHEL is a special controlled product, and there are no Red Hat
provided OOo extenions so unless there is a rebase of OOo to 2.4 or explicit
enterprise demand for it, then there might even never be a fully safe to use
EL-5 unopkg. But there will be in RHEL-6

Should this package wait a bit on the formal adoption of the 
http://fedoraproject.org/wiki/PackagingDrafts/OpenOffice.orgExtensions
guidelines? 

According to a recent article (see link off lwn.net), RHEL5.2 is going to have
Openoffice.org 2.3.0 in it... 

RHEL5.2 will have 2.3.0 in it, but it will not have the fixes for unopkg that
are spoken about here as I packaged the RHEL OOo some months before I started
playing around with getting extensions packagable.

I don't think it's neccessary to wait for the formal adoption, we already have
two prior extensions packaged, it would seem unfair to treat this one
differently. Especially as the draft (modified with feedback over that time, I'm
not complaining, it's a good thing) has been a draft for more that a month I
think so it could be a long wait until adoption.

This should NOT have passed review for one simple reason. It installs binaries
(itext.jar) directly from the source zip file. This sort of thing is forbidden
in Fedora:
http://fedoraproject.org/wiki/Packaging/Guidelines?#head-c23c2cd3782be842dc7ab40c35199c07cfbfe347
.

The documentation says that if you want to support PDF security you need to
install either itext.jar or PDFTK and indeed a pre-built itext.jar is provided
in the extendedPDF upstream .zip, but it's not installed by this .spec.

http://www.cora.nwra.com/~orion/fedora/openoffice.org-extendedPDF.spec
http://www.cora.nwra.com/~orion/fedora/openoffice.org-extendedPDF-1.4-3.fc8.src.rpm

* Fri Mar 14 2008 Orion Poplawski <orion.com> 1.4-3
- Remove itext.jar from source

of course "The sources used to build the package must match the upstream source,
as provided in the spec" doesn't really hold now. 

I still don't think there was a problem with the -2 version, the jar wasn't
installed and the jar is itext.jar which is a distributable MPL/LGPL java
library available from http://www.lowagie.com/iText/download.html. 

The same situation arises in apache-ant where the third party
"./lib/xercesImpl.jar" and "./lib/xml-apis.jar" are included in the source
distribution

But itext isn't in Fedora because of Bug 236309: itext contains Sun confidential
code.  The files mentioned in that bug are in the itext.jar in compiled form.

Orion, the only stuff that *really* needs to be stripped from sources are things
that aren't (re)distributable (like patent-encumbered bits), which doesn't apply
in this case.  afaict, ymmv, and all that.

(In reply to comment #15)
> of course "The sources used to build the package must match the upstream source,
> as provided in the spec" doesn't really hold now. 
> 
> I still don't think there was a problem with the -2 version, the jar wasn't
> installed

Forgive me for jumping the gun then, but I couldn't tell that from the %files
list. If it's not installed, then it's OK I guess.

>I don't think it's neccessary to wait for the formal adoption, we already have
>two prior extensions packaged, it would seem unfair to treat this one
>differently. Especially as the draft (modified with feedback over that time, I'm
>not complaining, it's a good thing) has been a draft for more that a month I
>think so it could be a long wait until adoption.

Well, we are blocking the java packages until guidelines are done... and there
are a number of those already in. It would be a bit unfair to allow this in
without approved guidelines while we are blocking all those I would think. 

Fair enough, lets set to blocked on F-GUIDELINES, that seems a reasonable way to
document the status of this for now.

Sounds good. Thanks for your understanding. 
(Clearing the cvs flag here until it's ready for cvs)

So the Extension guidelines and the java guidelines are finalized now, so I
adapted the spec to the new canonical locations and retained the "cleaned" source.

So I believe we're good now again to continue, agreed ?

http://people.redhat.com/caolanm/extendedPDF/openoffice.org-extendedPDF.spec
http://people.redhat.com/caolanm/extendedPDF/openoffice.org-extendedPDF-1.4-4.fc9.src.rpm

I think so. 
Orion: Do you want the cvs request from comment #8? Or does it need any updating
before processing? Is the package in comment #22 good from your side?

(In reply to comment #23)
> I think so. 
> Orion: Do you want the cvs request from comment #8? Or does it need any updating
> before processing? Is the package in comment #22 good from your side?
> 

Original CVS request is good.  New package looks good too.  I'll remove the
Requires EVR from the F-9 package, and add appropriate versions to the F-7/8
packages as noted in the guidelines.

cvs done.