Thanks for looking at this Christian. On 12/29/2009 03:54 AM, Christian Faulhammer wrote: > Looks fine so far. What puzzled me is the documentation of the SLOT > variable. What is the motivation to do so?
It would be possible for two different versions of a Control to be installed
and work with gdesklets-core. An id string is calculated based on the
interface to the Control, and this string is used by the Desklet to reference
the Control. So if two versions of a Control have different interfaces,
Desklet "A" could use the older version and Desklet "B" could use the newer
version. The core program would allow it and I'd like to provide the support
for doing this in portage. This is why obz and I originally chose to install
to a directory identified by the Control name and its id string (i.e.
"/usr/lib/gdesklets/Controls/ImageSlideShow_cdu4rwmfeuauyw2wwuaczc3lr-2").
> * Sometimes you give a default on undefined ROOT variable, sometimes
> not. Please make it consistent for cosmetic reasons.
Okay. I guess since the PMS specifies that it "must be non-empty and end in a
trailing slash", I will remove the default on undefined.
> * addwrite "${ROOT}/root/.gnome2": Is this unconditionally necessary?
> Or could a "boolean" in the ebuild be set to activate it?
Unfortunately, it seems to be unconditionally necessary. Bug #128289 [1]
still seems to apply upstream (see comment #18). I have not looked into glib
source code since this issue was originally "fixed" [2].
> * DISPLAY variable export could be done with the assignment. Or is the
> export always needed?
Done. I would like this to go away, but haven't touched the
gdesklets-control-getid script in a long time. The easy way was to export
DISPLAY.
> * Is the file name LICENSE always used for the license or is COPYING
> for example also possible?
I believe it will always be LICENSE if packaged according to upstream [3]. I
believe it is not allowed to be something else in order to be uploaded to the
main site, but as the link is down atm I can't check it.
> * einfo "Installing Control ${CTRL_DIRNAME}": Is not mirrored in the
> desklet branch of the if clause.
True. Originally it was mirrored, but some Desklets in the tree (I'm thinking
of x11-plugins/desklet-ftb right now) contain multiple Desklets (I can't
remember if this is allowed any more with the requirements specified in [3],
but I'm open to suggestions). I thought it was a good compromise to have the
einfo lines in pkg_postinst. The difference between the two is that multiple
Controls cannot be packaged together (both under the new upstream packaging
requirements and for this eclass to work).
Patch to the original revision is attached.
--
Joe
[1] http://bugs.gentoo.org/show_bug.cgi?id=128289#c18
[2] http://bugs.gentoo.org/show_bug.cgi?id=126890
[3] http://gdesklets.de/index.php?q=node/2
--- gdesklets-20091101.eclass 2009-12-16 22:06:05.000000000 -0500
+++ gdesklets-20090103.eclass 2010-01-03 15:52:53.000000000 -0500
@@ -67,7 +67,7 @@
IUSE=""
RDEPEND=">=gnome-extra/gdesklets-core-0.36.1-r3"
-GDESKLETS_INST_DIR="${ROOT:-/}usr/$(get_libdir)/gdesklets"
+GDESKLETS_INST_DIR="${ROOT}usr/$(get_libdir)/gdesklets"
# @FUNCTION: gdesklets_src_install
# @DESCRIPTION:
@@ -88,8 +88,7 @@
# Check to see if DISPLAY is set for the
# gdesklets-control-getid script to run without error
- [ -z "${DISPLAY}" ] && DISPLAY=""
- export DISPLAY
+ [ -z "${DISPLAY}" ] && export DISPLAY=""
debug-print-section docs_install
signature.asc
Description: OpenPGP digital signature
