[oe] testing branch 2010-10-29

[Frans Meulenbroeks]

2010/11/1 Koen Kooi <k.kooi at student.utwente.nl>:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 01-11-10 08:49, Frans Meulenbroeks wrote:
>
>> PS: a few comments wrt the libcdio issue:
>>
>> - it would have been nice if this was discussed with the recipe
>> maintainer. This is in accordance with our commit policy [2, 4th
>> block]. I strongly doubt that this has happened (and apologies if this
>> is a faulty assumption).
>
> Since I'm not sure if you're getting at the fix or breakage, I will talk
> about the breakage commit:
>
> I don't tend to discuss things with myself since it creeps people out :)

:-)

The confusion was caused by the fact that this one:
http://cgit.openembedded.org/cgit.cgi/openembedded/commit/?id=92e23e98bdbd1bdacb23f5d2c848159cbab6d057
touches both libcdio.inc and libcddb_1.3.2.bb
In the latter one you removed among others:
-MAINTAINER = "Andreas Frisch <andreas.frisch at multimedia-labs.de>"

Have the changes in that file been discussed with Andreas ?

Also the libcdio 0.82 recipe was added by Khem on sep 30:
http://cgit.openembedded.org/cgit.cgi/openembedded/commit/?id=c5a250437273e98fd66a13cfa6f5088b9ae68a97
because 0.81 was not cross-compile safe, after which Andreas made some
improvements to it.
The MAINTAINERS file also does not list you as maintainer for this
recipe. Guess this is a good case why it is useful to list the
maintainer in the recipe.

Anyway that (and the fact that we are poor on ACK-ing recipes) caused
me to push Andreas' patch (after verifying that it builds).

> I created the cdio recipe myself in 2009 and pushed in januari 2010.
> The funny thing is that all the breakage occured by this:
>
> http://cgit.openembedded.org/cgit.cgi/openembedded/commit/?id=b672b28ba6a29e2715c91fcbbda8b2776b6f1105
>
> That commit has no ACKs so SOBs from me, so I wonder why it went in.

As explained above because you were not identified as the maintainer
as you did not write or push the 0.82 recipe and are not listed as
such in the MAINTAINERS file.

And (unrelated to this issue) I do not see the same prudence you
expect from others when you modify recipes that are maintained by
others.
"Do unto others as you would have them do unto you"

>
>> (btw: a good place would probably be [3] and yeah, I am aware of the
>> debian renaming [4], but I am also aware of [5] which as an example
>> says:
>> FILES_${PN} = "\
>>     ${bindir}/* \
>>     ${sbindir}/* \
>>     ${libexecdir}/* \
>>     ${libdir}/lib*.so.* \
>>     .....
>> which was exactly what was in the original commit [6]:
>> +FILES_${PN} = "${libdir}/lib${PN}${SOLIB}"
>
> But there's no file called liblibcdio.so.x.y, only libcdio.so.x.y. The
> original recipe already had a way to split all the libs automatically.

That code was still there.

+python populate_packages_prepend () {
+ glibdir = bb.data.expand('${libdir}', d)
+ do_split_packages(d, glibdir, '^lib(.*)\.so\..*', 'lib%s',
'gstreamer %s library', extra_depends='', allow_links=True)
+}

Not sure how well it applies with the FILES_${PN} breakage.

On a personal note:
I feel that python functions like this,  clever as they are, do
severely reduce the understandability and maintainability of the
recipe.
Not everybody is a python wiz. I feel recipes should be easy to
understand for the average developer.
Personally I would prefer just iterating the files in a FILES* assignment.
That makes things so much better understandable (and yes if a new file
is added in a newer version of the recipe it will not be added
automagically. However there will be a note to warn that the new file
is not packaged)

>
> Thankfully the incremental angstrom autobuilder caught that issue and
> was fixed shortly after.

Which is good :-)

Frans.

PS: since you are so concerned about libcdio, would you please fix the
dependencies or the configure.
configure --help says
...
  --enable-cddb           include CDDB lookups in cd_info (default enabled)
  --enable-vcd-info      include Video CD Info from libvcd
...

Seems like whatever is build depends on whether libcddb is build before or not.
Andreas fixed that by adding a dependency on libcddb, but you undid
that in http://cgit.openembedded.org/cgit.cgi/openembedded/commit/?id=92e23e98bdbd1bdacb23f5d2c848159cbab6d057

And while you're at it perhaps add in the MAINTAINERS file that you
are the maintainer of this recipe.