This is in relation to: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=177063

It would perhaps be simpler to just generate the docs from
/usr/include/subversion-1 to avoid having to run configure and package whole
tarball again; would just need a copy of the doxygen.conf from upstream and the
doxygen invocation from the Makefile.

Thanks for the idea - I quite like it. The whole thing with full svn source RPM
being in both core and extras was weird, but I didn't think of doing this. I'll
rework and report back when done.

I uploaded the new version of the spec file to the same location. Lots of hacks,
but it does work on my Rawhide box.

There is also the source RPM there
(ftp://ftp.rexursive.com/pub/fedora-extras/subversion-api-docs-1.3.0-1.src.rpm)
- only 17kB :-)

Quite an interesting package.  Only a cursory look so far, but the one thing I
noticed is your packaging the files into
%{_docdir}/subversion-%{version}/api-docs.  Why did you choose to put them there
instead of allowing them to make their own %{_docdir} area?

Answer to comment #5:

Because they effectively belong to subversion package, but they cannot be
distributed with Fedora Core because of the size constraints. So, they end up in
the correct place using the Fedora Extras package.

Okay -- I'll do a review tonight.

Good:
* Naming guidelines:  the name of the source is subversion.  This package is
  intended as an addon to the Core subversion package using the Core subversion
  headers (in subversion-devel) to generate its content.  Thus the name looks
  like a subpackage of subversion.
* spec file name matches package name
* OSI approved license (BSD)
* LICENSE text is included in the subversion package which is required by this
  package.  As it's intended as an addon to the Core package extracting its
  content from the actual Core headers this seems acceptable to me.
* Package is legible and in American English
* Builds successfully on FC4 using version 1.2.3 instead of 1.3.0
* No excluded BuildRequires
* All necessary BuildRequires provided.
* No locales
* No shared libraries
* Not relocatable
* Owns all directories it creates
* No duplicate files
* Correct permissions
* %clean's the buildroot
* Uses consistent macros
* Permissible content: API documentation for a packaged library.
* Does not own directories owned by other packages.

Needswork:
* Does the subversion-api-docs-doxygen.conf file come from upstream somewhere?
  Is it in their distributed tarball?  Having a URL to the file would be
  good -- perhaps a link into their subversion repository for the %{version}
  tag?

Needs addressing:
* If we do treat this as a pseudo-subpackage of Core's subversion, then we
  need to make these changes:
  - Requires should be on subversion rather than subversion-devel
  - BuildRequires subversion-devel and the Requires should require the full EVR:
     BuildRequires: subversion-devel = %{version}-%{release}
      Requires: subversion = %{version}-%{release}
  This leads into my concern, however:
  - How are you going to keep this package synced up with Core's subversion?
    To do this right, you need to create a new version whenever Core releases
    an updated subversion package.  Otherwise there will be unresolved
    dependencies between Core and Extras that prevent people from upgrading.
    This isn't a hard thing to do, but it is time critical.  Let's say there's
    a security bug in neon that requires apps to be recompiled against the new
    neon library.  This affects subversion, ethereal, and openoffice.org among
    others.  These packages are recompiled aainst the new library and released.
    But because people have the subversion-api-docs package installed which
    hasn't been upgraded yet, yum refuses to upgrade them and people are left
    vulnerable until we rebuild subversion-api-docs against the new subversion.
  - "Large documentation files should go in a -docs subpackage."  If these
    files were created as a subpackage of the Core subversion package they
    would likely just be included as %doc in a subpackage.  They wouldn't be
    specially installed into the main subversion package's doc directory.
  So it seems much more graceful to install this into it's own documentation
  directory and not have a Requires: on subversion or subversion-devel.  The
  API documentation can get out of sync with the installed subversion, then,
  but it seems preferable to blocking upgrading in case of security releases
  or the like.
  - We would want to include a copy of subversion's LICENSE in this case.

> Does the subversion-api-docs-doxygen.conf file come from upstream somewhere?

Yes. We can link into the correct branch/tag of SVN repository of Subversion.

> Requires should be on subversion rather than subversion-devel

People that use this package are developers that use Subversion APIs. It is
unlikely they will be developing software that uses Subversion APIs without
wanting to have subversion-devel installed. It make sense that this package
pulls in the -devel package.

> BuildRequires subversion-devel and the Requires should require the full EVR

Documentation goes into packagename-%{version} subdirectory. APIs of Subversion
don't change mid major release. The documentation in the newer version/release
may be a bit better (i.e. more accurate) but that's not critical for the API or
functioning of the package. That's why release info isn't necessary.

> How are you going to keep this package synced up with Core's subversion?

Well, that is a good question. AFAIK, FE packages are rebuilt by hand. The
maintaner just needs to update the package when required - a situation that is
no different to many other packages from FE that may depend on FC packages that
get bumped up, therefore creating an upgrade problem. This can be fixed like this:

Requires: subversion-devel >= %{version}

Then we can also own:

%doc %dir %{_docdir}/subversion-%{version}

To make sure this directory gets whacked when the old version of
subversion-api-docs finally gets upgraded.

> We would want to include a copy of subversion's LICENSE in this case.

Sure. Again, we can point to the relevant file in Subversion SVN repository.

I don't think that'll work::
  $ repoquery --requires --repoid=development  subversion-devel
  apr-devel
  apr-util-devel
  subversion = 1.3.0-4.2

So whether we depend on subversion or subversion-devel we're creating a
situation where we cannot update subversion-core because the documentation is
out of sync.

OTOH, not including the release portion of EVR in the package dependencies makes
it less likely that the scenario I mentioned where a neon security flaw is
blocked by out-of-sync subversion/subversion-api-docs could occur but the
example would still be valid for security flaws in subversion itself.

You argue yourself, that exact EVR isn't necessary for the package dependencies
because an upgrade will just add accuracy to what is already there.  My argument
is just an extension of this: it's better that the subversion-api-docs are not
made dependent on subversion and are placed in their own
%{_docdir}/subversion-api-docs directory because it eliminates a potential
security problem in trade for a similar accuracy of the documentation.

In comment #10, did you take into account this (I mentioned it in my reply):

Requires: subversion-devel >= %{version}

With the above, the upgrade of subversion, subversion-devel and all its
dependencies will simply go through as normal, as subversion-devel (the
requirement for subversion-api-docs) will always be greater or equal to that of
subversion-api-docs.

So, unless there is some weird downgrade (e.g. FC starts shipping Subversion
1.2.9 instead of 1.3.0, 1.3.1 etc.), I don't see a problem with that. No
security upgrade should be affected by this package.

Ah -- missed the ">=".  Hmmm... That sounds fine then.  Roll another package
with the URLs filled in, the updated Requires:, and docdir ownership and I'll
take a look.

Sorry about the delay. Just been very busy lately. I'll get to it probably some
time next week.

Hopefully, I addressed to concerns. The spec file and source RPM are at the same
URLs as before:

ftp://ftp.rexursive.com/pub/fedora-extras/subversion-api-docs.spec
ftp://ftp.rexursive.com/pub/fedora-extras/subversion-api-docs-1.3.0-1.src.rpm

8888a30767fe277cd193bee2cfe0af95  subversion-api-docs-1.3.0-1.src.rpm

All issues have been addressed.

ACCEPTED

The one thing I noted is that you should always bump the release when you make a
new version, even while it is in review.  Please remember this for next time.

Thanks Toshio. I'll try to remember the release number rule for next time -
version numbers are cheap after all :-)

Again, sorry about the delay. I'm finding it hard these days to be on the right
machine at the right time to finish this. Hopefully in the next week or so I'll
move to next steps...