Date: Fri, 30 Sep 2005 12:02:53 -0600 From: Greg Lewis <glewis@eyesbeyond.com> To: Panagiotis Astithas <past@ebs.gr> Cc: freebsd-java@freebsd.org Subject: Re: [patch] Install a desktop icon for the jdk15 Control Panel Message-ID: <20050930180253.GC10169@misty.eyesbeyond.com> In-Reply-To: <433B9826.10607@ebs.gr> References: <433B067C.2020500@ebs.gr> <20050928222659.GA21920@misty.eyesbeyond.com> <433B9826.10607@ebs.gr>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Panagiotis,
On Thu, Sep 29, 2005 at 10:30:46AM +0300, Panagiotis Astithas wrote:
> Greg Lewis wrote:
> >Hi Panagiotis,
> >
> >On Thu, Sep 29, 2005 at 12:09:16AM +0300, Panagiotis Astithas wrote:
> >
> >>I cooked up this patch to fix something that has been annoying me for a
> >>long time. When we install the jdk we don't fix the provided
> >>sun_java.desktop file, or install it in a suitable place for desktop
> >>environments to pick up. This patch is for jdk15 and creates the
> >>necessary menu entry for Gnome and KDE (although I haven't tested on the
> >>latter). It should be trivial to make the same changes for jdk14, by
> >>using MINIMAL instead of WITHOUT_WEB.
> >
> >
> >Thanks working this up. Can you please send-pr it? I'm worried that
> >it will get lost otherwise since I'm currently tracking some other patches
> >as well.
>
> Certainly.
Thanks!
> >It would be good if someone could test your changes against KDE as well.
> >
> >I have some minor comments on the patch.
> >
> >
> >>--- /usr/ports/java/jdk15/Makefile Tue Sep 20 23:00:08 2005
> >>+++ jdk15/Makefile Wed Sep 28 23:57:17 2005
> >>@@ -43,6 +43,7 @@
> >>MAKE_ENV+= BROWSER=mozilla
> >>.endif
> >>USE_ICONV= yes
> >>+USE_GNOME= desktopfileutils
> >
> >
> >Does this pull in any extra dependencies other than desktop-file-utils?
> >the jdk is already pretty fat with dependencies, and I particularly don't
> >want to pull in a bunch of KDE and/or Gnome dependencies.
>
> I don't think so, I used it as a convenience, since desktopfileutils
> itself contains a 'USE_GNOME=glib20' line. So I figured, if you have
> that, you had glib20 at least. I guess a tester with KDE will let us
> know for sure.
Ok, good. I think glib20 is an acceptable extra dependency :).
> >>MAKE_ENV+= ALT_MOZILLA_HEADERS_PATH="${X11BASE}/include"
> >>.endif
> >>
> >>@@ -118,6 +119,9 @@
> >>
> >>PLIST_FILES= jdk${JDK_VERSION}/jre/.systemPrefs/.system.lock \
> >> jdk${JDK_VERSION}/jre/.systemPrefs/.systemRootModFile
> >>+.if !defined(WITHOUT_WEB)
> >>+PLIST_FILES+= share/applications/sun_java15.desktop
> >>+.endif
> >>PLIST_DIRS= jdk${JDK_VERSION}/jre/.systemPrefs
> >>
> >>.if (${ARCH} == amd64)
> >>@@ -284,9 +288,20 @@
> >> ${MKDIR} ${PREFIX}/jdk${JDK_VERSION}
> >> cd ${JDKIMAGEDIR} && ${FIND} . \
> >> | ${CPIO} -pdmu -R ${LIBOWN}:${LIBGRP} ${PREFIX}/jdk${JDK_VERSION}
> >>+.if !defined(WITHOUT_WEB)
> >>+ @${SED} -e
> >>'s#Exec=INSTALL_DIR/JRE_NAME_VERSION/bin/ControlPanel#Exec=${PREFIX}/jdk${JDK_VERSION}/bin/ControlPanel#' \
> >>+ -e
> >>'s#Icon=INSTALL_DIR/JRE_NAME_VERSION/plugin/desktop/sun_java.png#Icon=${PREFIX}/jdk${JDK_VERSION}/jre/plugin/desktop/sun_java.png#' \
> >>+ < ${JDKIMAGEDIR}/jre/plugin/desktop/sun_java.desktop \
> >>+ > ${JDKIMAGEDIR}/jre/plugin/desktop/sun_java.desktop.tmp
> >>+ ${INSTALL_DATA}
> >>${JDKIMAGEDIR}/jre/plugin/desktop/sun_java.desktop.tmp
> >>${PREFIX}/share/applications/sun_java15.desktop
> >>+.endif
> >
> >
> >It feels like maybe it would be more appropriate to have a patch for the
> >source file and then use REINPLACE_CMD on it in post-patch and simply
> >install the icon here.
>
> Sure, if you like it better than this.
Thanks. I just think its more the standard way to do things.
> >> @${ECHO_MSG} "@unexec ${LOCALBASE}/bin/unregistervm
> >> ${PREFIX}/jdk${JDK_VERSION}/bin/java" >> ${TMPPLIST}
> >> @${FIND} -s ${JDKIMAGEDIR} -not -type d | \
> >> ${SED} -ne 's#^${JDKIMAGEDIR}#jdk${JDK_VERSION}#p' >> ${TMPPLIST}
> >>+.if !defined(WITHOUT_WEB)
> >>+ @-update-desktop-database
> >
> >
> >You may want to consider using ${LOCALBASE}/bin/update-desktop-database
> >here. Also, it seems more appropriate to do this in the post-install step.
>
> It seems that this particular invocation of update-desktop-database is
> very popular. Try running the following into any ports category you can
> think of:
>
> grep update-desktop-database /usr/ports/[any category]/*/Makefile
Fair enough then, probably no need to put ${LOCALBASE} in.
> Initially I had it in the post-install step, but I thought an extra
> ifdef there, would be considered uglier than this :-)
My feeling on it is that an operation like this is a post-install
operation rather than an install operation since it would appear to
be updating an external "database" of some sort. If updating that
database is important, then the operation should actually be done
in the pkg-install script (or installing a package will mean that
the database isn't updated).
However, you mention that this step is done in Makefiles for other
ports. That leads me to believe that either these ports are all broken
in terms of being installed as a package (not inconceivable ;) or this
operation doesn't actually do anything important. I think we need
to determine which it is so we can do the right thing here.
Thanks for working on this!
--
Greg Lewis Email : glewis@eyesbeyond.com
Eyes Beyond Web : http://www.eyesbeyond.com
Information Technology FreeBSD : glewis@FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050930180253.GC10169>