Bug 201551 - Review Request: db4o
Summary: Review Request: db4o
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dominik 'Rathann' Mierzejewski
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-08-07 11:04 UTC by Paul F. Johnson
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-08-09 15:42:15 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Paul F. Johnson 2006-08-07 11:04:50 UTC
Spec URL: http://www.knox.net.nz/~nodoid/db4o.spec
SRPM URL: http://www.knox.net.nz/~nodoid/db4o-5.5-1.src.rpm
Description: 

db4o is an open source object database for .NET/mono and Java (though I've only packaged the mono version!)

Objects are stored natively, eliminating the added complexity and performance drain of conversion to other formats such as SQL. The native support of object-oriented database functionalities, such as db4o's acclaimed Native Queries and object-oriented replication, boost the performance and flexibility gains of object-oriented programming languages such as Java and .NET.

Its is *incredibly* powerful!

This software is like Qt3 in that it has a commercial and GPL version, which may cause problems for RHEL.

The version here is not the stable version, but the developer version (the stable version does not compile with mono-1.1.16-1. This has been reported upstream and also to the mono developers)

It also has the mono hack (though it is commented on). There is no devel package as there is no .pc file

The src rpm also includes the API and tutorials.

Comment 1 Dominik 'Rathann' Mierzejewski 2006-08-07 21:28:13 UTC
OK, I'll review this as soon as I can figure out how to assign this bug to myself.

Comment 2 Dominik 'Rathann' Mierzejewski 2006-08-07 23:16:51 UTC
Here's a formal review:

MUST items OK:
* package meets naming guidelines
* source files match upstream
$ md5sum db4o-5.5-mono.tar.gz
4aea1da3b96a2b92c9ac69ee2ccb9a63  db4o-5.5-mono.tar.gz
* specfile is properly named, is cleanly written and uses macros consistently
* dist tag is present
* build root is correct
* license field matches the actual license
* license is open source-compatible
* latest version is being packaged
* BuildRequires are proper
* rpmlint errors can be ignored
$ rpmlint -i db4o-5.5-1.x86_64.rpm
E: db4o no-binary
The package should be of the noarch architecture because it doesn't contain
any binaries.

E: db4o only-non-binary-in-usr-lib
There are only non binary files in /usr/lib so they should be in /usr/share.

* final provides are sane
mono(Db4oMonoTest) = 0.0.0.0
mono(Db4oTools) = 0.0.0.0
mono(db4o) = 5.5.1.0
db4o = 5.5-1
* no traditional shared libraries are present
* package is not relocatable
* owns the directories it creates
* doesn't own any directories it shouldn't
* no duplicates in %files
* file permissions are appropriate
* %clean is present
* %check is not present
* no scriptlets present
* code, not content
* %docs are not necessary for the proper functioning of the package
* no headers
* no pkgconfig files
* no libtool .la droppings
* no locales
* not a GUI app
* not a web app

MUST items BAD:
* package doesn't meet packaging guidelines
- {_prefix}/%{_lib} vs. %{_libdir}, See 
http://fedoraproject.org/wiki/Packaging/Mono#head-ae42c4b5de20e082855e2f5151542730ef40f15b
http://fedoraproject.org/wiki/Packaging/Mono#head-932fb76878e1f34706c9962336a5f23fcc277af2
- installs shipped precompiled binaries, See
http://fedoraproject.org/wiki/Packaging/Mono#head-1d3454f431ec1c8761eb46582e7f66bbddc1fd08
* source URL returns 404
$ wget http://www.db4o.com/community/db4o-5.5-mono.tar.gz
--00:31:44--  http://www.db4o.com/community/db4o-5.5-mono.tar.gz
           => `db4o-5.5-mono.tar.gz'
Resolving www.db4o.com... 213.203.204.172
Connecting to www.db4o.com|213.203.204.172|:80... connected.
HTTP request sent, awaiting response... 404 Not Found
00:31:44 ERROR 404: Not Found.
* final requires contain duplicates
mono(Db4oTools) = 0.0.0.0
mono(System) = 1.0.5000.0
mono(db4o) = 5.5.1.0
mono(mscorlib) = 1.0.5000.0
mono-core
i.e. mono(System) and mono(mscorlib) are provided by mono-core
* documentation takes up 75% of the installed size, -docs subpackage is necessary

SHOULD items:
* License text included in package (License text not included upstream)

Also, I don't like the use of sed where tr -d would suffice.


Comment 3 Paul F. Johnson 2006-08-08 07:07:54 UTC
E: db4o no-binary
The package should be of the noarch architecture because it doesn't contain
any binaries.

I'll change that.

E: db4o only-non-binary-in-usr-lib
There are only non binary files in /usr/lib so they should be in /usr/share.

Mono packages aren't sane when it comes to this and until mono is fixed, this
one can be ignored

* documentation takes up 75% of the installed size, -docs subpackage is necessary

Not a problem - I did that with XaraLX

i.e. mono(System) and mono(mscorlib) are provided by mono-core
* documentation takes up 75% of the installed size, -docs subpackage is necessary

It needs mono-core as a R and mono-devel as a BR.

* package doesn't meet packaging guidelines
- {_prefix}/%{_lib} vs. %{_libdir}, See 

Permitted for the time being - have a look at the mono packaging guidelines

* Also, I don't like the use of sed where tr -d would suffice.

I'm not sure that's a blocker though

I'll fix the docs and noarch and upload a new spec file. Thanks for the review
up to now.

Comment 4 Dominik 'Rathann' Mierzejewski 2006-08-08 07:49:39 UTC
I don't know which mono packaging guidelines you've been reading, but those I
pointed you to (from the Mono SIG page) say to use %{_libdir} and nothing else.

They also say those rpmlint errors are to be ignored, so I don't understand why
you want to make it a noarch package. Mono packages aren't supposed to be
noarch, according to those guidelines. I've already said that.

About Requires: mono-core - it's not necessary, as I've already said. Please
read what I have written.

Using sed instead of my preferred tr -d isn't a blocker, yes.


Comment 5 Paul F. Johnson 2006-08-08 08:13:50 UTC
"I don't know which mono packaging guidelines you've been reading, but those I
pointed you to (from the Mono SIG page) say to use %{_libdir} and nothing else."

See what happens when you build on x86_64 and you'll see why %{_prefix}/lib is
permissable. Until such time that mono is fixed (that is, on x86 builds to
/usr/lib and x86_64 builds to /usr/lib64) either upstream or for FC, the only
100% guaranteed method for others to be able to pick it up is to statically
define where things go. This has been accepted in the short term (have a look at
every other mono package registered to me and you'll see exactly the same hack
is applied)

"They also say those rpmlint errors are to be ignored, so I don't understand why
you want to make it a noarch package. Mono packages aren't supposed to be
noarch, according to those guidelines. I've already said that."

Normally on a mono package, the executable is in /usr/lib/<package_name> with a
symlink back to %{_bindir} and so rpmlint moans. What has happened here is I was
in a rush to get out to get to work and have misread what you've said.

Requires : mono-core, point taken.

Comment 6 Paul F. Johnson 2006-08-08 08:26:37 UTC
Spec URL: http://www.knox.net.nz/~nodoid/db4o.spec

Addresses #2

Not uploaded a new src.rpm - there is no difference other than the spec file.

Comment 7 Dominik 'Rathann' Mierzejewski 2006-08-08 11:16:06 UTC
Ok, upon further reading I see that you're right about %{_libdir}, though the
guidelines say it is a temporary solution.

However, you still haven't addressed my concerns about installing a precompiled
binary. In particular, /usr/lib/mono/gac/db4o/*/db4o.dll (installed) is binary
identical to db4o-5.5/dll/db4o.dll (shipped). The guidelines
http://fedoraproject.org/wiki/Packaging/Mono#head-1d3454f431ec1c8761eb46582e7f66bbddc1fd08
say explicitly that "All packages must build from source".

And I'd like you to name the docs package db4o-doc, not db4o-docs, as this
naming seems to be more common among extras packages.

Comment 8 Paul F. Johnson 2006-08-08 11:31:01 UTC
This package is built from the source. It's simple enough to fix this problem,
just %exclude it.

That said, I have unearthed a slightly more sinister problem...

Spec URL: http://www.knox.net.nz/~nodoid/db4o.spec

Fixes the problems in #7 though

Comment 9 Paul F. Johnson 2006-08-08 12:08:05 UTC
Spec URL: http://www.knox.net.nz/~nodoid/db4o.spec
SRPM URL: http://www.knox.net.nz/~nodoid/db4o-5.5-5.src.rpm

I've removed the gacutil from the package as it's broken and added a devel (with
.pc file)

This now means that 

mcs foo.cs -pkg:db4o can now be used in much the same way as gtk-sharp needs to
be used

The tarball includes the .pc file

Comment 10 Dominik 'Rathann' Mierzejewski 2006-08-08 21:33:22 UTC
Looks almost perfect, except for these new rpmlint warnings:

W: db4o-devel no-documentation
The package contains no documentation (README, doc, etc).
You have to include documentation files.

It can be ignored.

W: db4o-doc non-standard-group Development/Documentation
The value of the Group tag in the package is not valid.  Valid groups are:
[...]"Documentation"[...]

I think you should call it just that. :)

Oh, and you can also add
%define debug_package %{nil}
so that the empty debug package isn't generated. Something like that is found in
other mono packages, too, and follows the packaging guidelines:
http://fedoraproject.org/wiki/Packaging/Debuginfo#head-29573c4f61c3a4698b2d07c7e73cfa194785f257

Running the built Db4oMonoTest.exe seems fine, too.

Fix the group tag, disable debuginfo generation, and it's APPROVED.

Good work.

Comment 11 Paul F. Johnson 2006-08-09 11:50:13 UTC
Spec URL: http://www.knox.net.nz/~nodoid/db4o.spec
SRPM URL: http://www.knox.net.nz/~nodoid/db4o-5.5-7.src.rpm

Fixes #10

I've even added R pkg-config to the devel requires!

Comment 12 Paul Howarth 2006-08-09 12:05:12 UTC
(In reply to comment #11)
> I've even added R pkg-config to the devel requires!

Unfortunately the pkg-config program comes in a package called pkgconfig so you
need an R: pkgconfig rather than an R: pkg-config


Comment 13 Dominik 'Rathann' Mierzejewski 2006-08-09 12:07:54 UTC
There's a typo now:
%define debug_packahe %{nil}

Also, please put Requires: pkgconfig in a separate line.


Comment 14 Paul F. Johnson 2006-08-09 12:30:46 UTC
Spec URL: http://www.knox.net.nz/~nodoid/db4o.spec

Fixes the above

Is there a reason for having the pkgconfig on another line? From what I can see,
there is nothing in the docs about doing it that way (and I certainly haven't on
other packages)

Comment 15 Dominik 'Rathann' Mierzejewski 2006-08-09 15:03:33 UTC
Having each explicit dependency in a separate line makes it easier to see what
is required. I, for one, had some trouble finding your addition at first glance.

That said, it's not a blocker, so the current version is APPROVED.


Comment 16 Paul F. Johnson 2006-08-09 15:42:15 UTC
Thanks for such a wonderful first review (on your part). Keep up the good work
and I'll see you on IRC :-D

Comment 17 Paul Howarth 2006-08-09 15:50:25 UTC
The package imported into cvs still has the wrong pkg-config dependency for the
-devel package, which will make this package uninstallable.


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