Due to review of rasqal, I've made some additional changes to this package.

Latst src.rpm:
http://thomas.apestaart.org/download/pkg/fedora-5-i386-extras/redland-1.0.4-2.fc5/redland-1.0.4-2.fc5.src.rpm

This has been sitting in the queue for a while... 
I would be happy to review it (and redland-bindings). 

Look for a full review in a bit. 

This review depends on #195645, which appears stalled out on the issue of
 %makeinstall use. This package also uses %makeinstall. 

Adding 195645 as a dependency here. 

Hey Thomas. Now that rasqal is in, we can move forward on this review. 

1.0.5 appears to be out, could you upgrade to that and make any other changes
you would like and I will do a review... :)

well, 1.0.5 relies on raptor 1.4.13.  afaict it's still 1.4.9 in extras, so I
can't yet update.

Will fix the other issue though.

http://thomas.apestaart.org/download/pkg/fedora-5-i386-extras/redland-1.0.4-3.fc5/

Sorry for the delay in reviewing this... 

OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
See below - License
See below - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
3ee58cbf5486c97ef3bc0c4368a344cc  redland-1.0.4.tar.gz
3ee58cbf5486c97ef3bc0c4368a344cc  redland-1.0.4.tar.gz.1
OK - Package compiles and builds on at least one arch.
OK - BuildRequires correct
OK - Spec has needed ldconfig in post and postun
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Headers/static libs in -devel subpackage.
OK - .pc files in -devel subpackage.
OK - .so files in -devel subpackage.
OK - -devel package Requires: %{name} = %{version}-%{release}
OK - .la files are removed.
OK - Package doesn't own any directories other packages own.
See below - No rpmlint output.
OK - final provides and requires are sane:

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
OK - Should have subpackages require base package with fully versioned depend.
OK - Should have dist tag
See below - Should package latest version

Issues:

1. You have the License as: "LGPL or Apache License 2.0", but it
apparently also is optionally licensed under the GPL,
so you should add that option. 

2. -devel package has a .pc file, so shouldn't it
Requires: pkgconfig

3. rpmlint our little friend says:
W: redland invalid-license LGPL or Apache License 2.0
W: redland invalid-license LGPL or Apache License 2.0
W: redland-debuginfo invalid-license LGPL or Apache License 2.0
W: redland-devel invalid-license LGPL or Apache License 2.0

Can be ignored I think.

4. You aren't packaging the newest version, but thats due to
the version of rasqal available. Is there a bug filed for updating
that?

5. The package dumps 18 files into /usr/include/ Perhaps it could be
changed to put them in a /usr/include/redland/ instead? (And then
the .pc file would need to be adjusted to include the right -I/usr/include/redland.

Thanks for reviewing!

Wrt. your issues:

1) Any LGPL package is always allowed to be used under the GPL - this is a
standard "feature" of the LGPL.  As such I don't think it's necessary to add it
to the license field, since I don't see any other LGPL package doing that.  What
do you think ?

2) yep, will add that

4) will file that bug now.

5) IMO this is up to upstream if this should be changed.  I don't necessarily
feel it should - the headers seem to be namespaced with rdf_ - but in any case I
don't think packagers should make changes like this if they are not strictly
necessary because it creates problems for developers.  What do you think ?

I will push a new package when we resolve 1) and 5)

Thanks
Thomas

(In reply to comment #8)
 
> 5) IMO this is up to upstream if this should be changed.
Well, system integration is the task of an rpm's maintainer ;)

> I don't necessarily
> feel it should - the headers seem to be namespaced with rdf_ - but in any case I
> don't think packagers should make changes like this if they are not strictly
> necessary because it creates problems for developers.  What do you think ?
IMO: Move them into a subdir.

In reply to comment #8: 

1. True. I guess it's not a big deal either way if you list it or not. 

2. ok. 

4. ok. Might mention in this review just for completeness. 

5. Well, since the package uses pkgconfig it's pretty easy to move things and
have anything using the devel package pick up the correct include path. You are 
correct however in that it would be good for upstream to make this change. 
Is upstream responsive? Would they take a change like that? Can you ask them?

Some of the headers have a rdf_ namespace, but at least 2 do not.  

wrt. 5:
- upstream is relatively responsive, but I only ever sent one patch upstream
- pkg-config is not the only way they provide paths to developer apps, they also
have redland-config
- there is nothing as such wrong with installing headers in /usr/include, it is
merely a style issue.  While I personally much prefer projects that care about
these issues and use subdirectories, and do so in my own projects, it is IMO not
a requirement and not a maintainer's call to make.  I personally get hugely
annoyed when a downstream packager does something to one of my projects that
really should be the maintainer's call.  Changing stuff like this is a cost to
users/developers of the package that gets paid by the upstream maintainer, not
the packager.
- further examples of packages on my system that install headers in /usr/include
directly: gd, gmp, libidn, libjpeg, js, mx, openldap, libodbc, pilot-link,
libtiff, libtermcap, zlib

(In reply to comment #11)
> wrt. 5:

> - there is nothing as such wrong with installing headers in /usr/include, it is
> merely a style issue.
Sorry, this is not a "mere style" issue.

- It affects effectiveness of compilers when searching /usr/include (N
files/dirs more to stat).
- It raises the potential of file conflicts.
- /usr/include is "special"

ONE package isn't much of a problem, but 10's or 100's are.

>  While I personally much prefer projects that care about
> these issues and use subdirectories, and do so in my own projects, it is IMO not
> a requirement and not a maintainer's call to make.  I personally get hugely
> annoyed when a downstream packager does something to one of my projects that
> really should be the maintainer's call.

My position is opposite:
* /usr/include is the "system-include" directory and should largely be reserved
to essential system packages (Many of them are covered by standards, e.g. POSIX).

* In many cases, upstream doesn't care about this because they assume their
package to be installed into a "per package" hierarchy (/opt/<package> or
similar) so they aren't aware about the issues their habits could cause.

* I consider a maintainer who is not able to work around this or unwilling to
address this issue, to be acting a careless and negligent (I am waiting for the
day, somebody installs a file named "stdint.h", or "list" to /usr/include)

>  Changing stuff like this is a cost to
> users/developers of the package that gets paid by the upstream maintainer, not
> the packager.
CPPFLAGS=-I/usr/include/<package>

> - further examples of packages on my system that install headers in /usr/include
> directly: gd, gmp, libidn, libjpeg, js, mx, openldap, libodbc, pilot-link,
> libtiff, libtermcap, zlib
Yes, many of them all in the same boat, but .. many of them are out of control
of Extras.

Some of these packages however have evolved into "essential system libs" (e.g..
zlib) which would justify installing their headers to /usr/include, redland
definitely is not one of these.

Also, you should be aware that FE-policy so far had been, headers can go to
"/usr/include, if a package installs very few headers which are almost
guaranteed never to conflict".

In reply to comment #12: 

I understand your strong feelings in this, and share many of them, however...

>Also, you should be aware that FE-policy so far had been, headers can go to
>"/usr/include, if a package installs very few headers which are almost
>guaranteed never to conflict".

Where is this guideline listed? I know of no such rule... 
Perhaps you should get the package guidelines to include such a blocker rule?

Thomas: Can you mail upstream and ask if they will change this... for now, we 
can just move forward with the include files in /usr/include, and then 
hopefully they will update to use a /usr/include/<package> soon. If it looks
like they will make this change soon, you can change it in your package and then
drop the change when upstream moves to it?

I'll leave this in REVIEW for a few days to let you talk to upstream and see if
they will do this change, and when... 

(In reply to comment #13)
> In reply to comment #12: 

> >Also, you should be aware that FE-policy so far had been, headers can go to
> >"/usr/include, if a package installs very few headers which are almost
> >guaranteed never to conflict".
> 
> Where is this guideline listed? I know of no such rule... 
It's not listed anywhere. It's simply common sense and established "good"
practice by many FE packagers.

> Perhaps you should get the package guidelines to include such a blocker rule?
<sigh> Seems as if we need regulations for anything </sigh>

Please note, that I only commented and expressed my opinion, I did not block
this package review.

(In reply to comment #14):

>> Where is this guideline listed? I know of no such rule... 
>It's not listed anywhere. It's simply common sense and established "good"
>practice by many FE packagers.

Right. 

>> Perhaps you should get the package guidelines to include such a blocker rule?
><sigh> Seems as if we need regulations for anything </sigh>

For things that should block inclusion of a package, I think so... 

>Please note, that I only commented and expressed my opinion, I did not block
>this package review.

Sure, and like I said, I share much of your opinion on this... but I don't think
it's a blocking issue for the package. 

Lets see what Thomas can find out from upstream. Hopefully they will quickly do
the right thing and we will all be happy. ;) 

Hey Thomas. Any news from upstream about this package?

Hey Thomas. I see 1.0.6 is out. 

Does it address the issues above? 
Can you update your package to 1.0.6 and we can see where we stand?

Ping? We need this package for KDE 4 (as a dependency of Soprano which is a 
dependency of Nepomuk), so it would be nice if it could get in sooner rather 
than later.

IMHO, putting the headers in /usr/include:
1. is upstream's call to make,
2. doesn't violate any guidelines,
3. doesn't cause any real problems (see the next paragraph for why) and
4. can be easily changed in an update (if/when upstream changes it).
Therefore, I don't see this as a good reason to block the review.

Moreover, looking at the actual names of the headers, the only ones which don't 
have an rdf_* prefix are librdf.h and redland.h, both of which have practically 
zero chance to conflict with another package (redland being the project name 
and librdf being the name of the shared object it ships), making this a 
strictly academic issue.

Therefore, please don't let this hold back this package. If you really can't 
get over yourself to approve it without this change, I will.

In reply to comment #19: 

If you will read my comment #13, you will see that I was not intending to block
this package based on the include file issue. I simply wanted Thomas to check
with upstream and see if they would be willing to make that change in a timely
manner. 

Thomas? Any word from upstream? 
In any case can you update to 1.0.6?

Thomas Vander Stichele: Ping?
Shall we consider this review stalled?

sorry, back

I discussed it with upstream, they would be willing to take a patch eventually
once it's done.  I will put it on my todo list, even though I don't think it's
super-critical.

meanwhile, will update and test.

ok. If you could post an updated version for me to look over we can get this
thing approved and in so the KDE folks can use it. ;) 
Thanks!

Redland 1.0.6 needs updates to raptor and rasqal first.

Currently in F7:
raptor 1.4.14
rasqal 0.9.12

Needed for redland 1.0.6:
raptor >= 1.4.15
rasqal >= 0.9.14

It's possible to update to 1.0.5, which needs:
raptor >= 1.4.9
rasqal >= 0.9.12
but not to 1.0.6 unless raptor and rasqal are updated first.

Created attachment 158091 [details]
Specfile for 1.0.5

I updated the specfile for 1.0.5: only trivial changes were needed:
- update to 1.0.5 (1.0.6 needs newer raptor and rasqal than available)
- update minimum raptor version

Kevin Fenzi: Can this be considered approved now? We can worry about updating
the whole stack to 1.0.6 later. As far as I can tell, any version will do for
KDE 4 (Soprano doesn't seem to check for any particular version), but we really
need one version in, so let's not delay the review just for the version bump.

Thomas Vander Stichele: Can you please import and build this once approved?

All the blocker issues I see are solved in the 1.0.5 version in comment #25. 
I would still like to see upstream move the include files into a
/usr/include/redland/ dir, but thats not a blocker. 

This package is APPROVED. 
Don't forget to close this review request once it's been imported and built. 

Now that it's approved, what is this package waiting for? :-(
Do you want a comaintainer to do the CVS requests, imports and builds for you?

New Package CVS Request
=======================
Package Name: redland
Short Description: Redland RDF Application Framework
Owners: thomas,Kevin.org
Branches: FC-6 F-7
InitialCC:

Thomas Vander Stichele (the package submitter) agreed to comaintainership.

cvs done.

Built for Fedora 6, 7 and 8, filed in Bodhi (requested direct push to stable) 
for F7.

(Please check the contents of .pc file on the review.
 I filed this as bug 248106 )

Package Change Request
======================
Package Name: redland
Updated Fedora Owners: thomas,kevin.org

Please change my e-mail address to lowercase, I changed it in the Fedora 
Account System to match Bugzilla, which now normalizes all e-mail addresses to 
lowercase.

cvs done.