Bug 195647 - Review Request: redland
Review Request: redland
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Package Reviews List
:
Depends On: 195645
Blocks:
  Show dependency treegraph
 
Reported: 2006-06-16 07:31 EDT by Thomas Vander Stichele
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-07-12 15:03:03 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
Specfile for 1.0.5 (5.72 KB, text/plain)
2007-06-27 23:10 EDT, Kevin Kofler
no flags Details

  None (edit)
Description Thomas Vander Stichele 2006-06-16 07:31:17 EDT
Spec URL: https://apestaart.org/thomas/trac/browser/pkg/fedora.extras/redland/redland.spec
SRPM URL: http://thomas.apestaart.org/download/pkg/fedora-5-i386-extras/redland-1.0.4-1.fc5/redland-1.0.4-1.fc5.src.rpm
Description:

Redland is a library that provides a high-level interface for RDF
(Resource Description Framework) implemented in an object-based API.
It is modular and supports different RDF/XML parsers, storage
mechanisms and other elements. Redland is designed for applications
developers to provide RDF support in their applications as well as
for RDF developers to experiment with the technology.
Comment 1 Thomas Vander Stichele 2006-06-17 15:24:02 EDT
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
Comment 2 Kevin Fenzi 2006-10-01 12:20:43 EDT
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. 
Comment 3 Kevin Fenzi 2006-10-01 12:25:43 EDT
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. 

Comment 4 Kevin Fenzi 2007-01-27 22:37:40 EST
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... :)
Comment 5 Thomas Vander Stichele 2007-02-03 17:07:59 EST
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.
Comment 7 Kevin Fenzi 2007-02-09 01:29:47 EST
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.
Comment 8 Thomas Vander Stichele 2007-02-09 07:40:48 EST
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
Comment 9 Ralf Corsepius 2007-02-09 09:01:05 EST
(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.
Comment 10 Kevin Fenzi 2007-02-11 15:41:19 EST
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.  
Comment 11 Thomas Vander Stichele 2007-02-12 05:03:19 EST
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
Comment 12 Ralf Corsepius 2007-02-12 07:54:36 EST
(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".
Comment 13 Kevin Fenzi 2007-02-12 12:35:14 EST
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... 
Comment 14 Ralf Corsepius 2007-02-12 12:59:20 EST
(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.
Comment 15 Kevin Fenzi 2007-02-12 13:34:27 EST
(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. ;) 
Comment 16 Kevin Fenzi 2007-02-28 22:21:36 EST
Hey Thomas. Any news from upstream about this package?
Comment 17 Kevin Fenzi 2007-05-13 14:34:53 EDT
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?
Comment 18 Kevin Kofler 2007-05-22 16:29:14 EDT
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.
Comment 19 Kevin Kofler 2007-05-22 17:26:51 EDT
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.
Comment 20 Kevin Fenzi 2007-05-23 17:48:53 EDT
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?
Comment 21 Kevin Kofler 2007-06-06 15:15:09 EDT
Thomas Vander Stichele: Ping?
Shall we consider this review stalled?
Comment 22 Thomas Vander Stichele 2007-06-08 13:29:44 EDT
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.
Comment 23 Kevin Fenzi 2007-06-11 12:19:49 EDT
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!
Comment 24 Kevin Kofler 2007-06-27 22:51:29 EDT
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.
Comment 25 Kevin Kofler 2007-06-27 23:10:26 EDT
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?
Comment 26 Kevin Fenzi 2007-07-03 16:24:55 EDT
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. 
Comment 27 Kevin Kofler 2007-07-06 16:42:44 EDT
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?
Comment 28 Kevin Kofler 2007-07-12 12:12:12 EDT
New Package CVS Request
=======================
Package Name: redland
Short Description: Redland RDF Application Framework
Owners: thomas@apestaart.org,Kevin@tigcc.ticalc.org
Branches: FC-6 F-7
InitialCC:

Thomas Vander Stichele (the package submitter) agreed to comaintainership.
Comment 29 Kevin Fenzi 2007-07-12 13:01:36 EDT
cvs done.
Comment 30 Kevin Kofler 2007-07-12 15:03:03 EDT
Built for Fedora 6, 7 and 8, filed in Bodhi (requested direct push to stable) 
for F7.
Comment 31 Mamoru TASAKA 2007-07-13 06:00:27 EDT
(Please check the contents of .pc file on the review.
 I filed this as bug 248106 )
Comment 32 Kevin Kofler 2007-07-17 01:59:55 EDT
Package Change Request
======================
Package Name: redland
Updated Fedora Owners: thomas@apestaart.org,kevin@tigcc.ticalc.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.
Comment 33 Kevin Fenzi 2007-07-17 02:11:40 EDT
cvs done.

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