Date: Thu, 29 Sep 2005 10:30:46 +0300 From: Panagiotis Astithas <past@ebs.gr> To: Greg Lewis <glewis@eyesbeyond.com> Cc: freebsd-java@freebsd.org Subject: Re: [patch] Install a desktop icon for the jdk15 Control Panel Message-ID: <433B9826.10607@ebs.gr> In-Reply-To: <20050928222659.GA21920@misty.eyesbeyond.com> References: <433B067C.2020500@ebs.gr> <20050928222659.GA21920@misty.eyesbeyond.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
> 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.
>> 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.
>> @${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
Initially I had it in the post-install step, but I thought an extra
ifdef there, would be considered uglier than this :-)
>>+ @${ECHO_MSG} '@exec ${PREFIX}/bin/update-desktop-database > /dev/null || /usr/bin/true' >> ${TMPPLIST}
>
>
> This should use LOCALBASE rather than PREFIX.
OK.
>>+.endif
>> .if defined(WITH_DEBUG)
>> cd ${JDKIMAGEDIR_G} && ${FIND} . \
>> | ${CPIO} -pdmu -R ${LIBOWN}:${LIBGRP} ${PREFIX}/jdk${JDK_VERSION}
>>@@ -296,6 +311,9 @@
>> @${FIND} -s -d ${PREFIX}/jdk${JDK_VERSION} -type d | \
>> ${SED} -ne 's#^${PREFIX}/#@dirrm #p' >> ${TMPPLIST}
>> @${ECHO_MSG} "@exec ${LOCALBASE}/bin/registervm ${PREFIX}/jdk${JDK_VERSION}/bin/java # FREEBSD-JDK${JDK_VERSION}" >> ${TMPPLIST}
>>+.if !defined(WITHOUT_WEB)
>>+ @${ECHO_MSG} '@unexec ${PREFIX}/bin/update-desktop-database > /dev/null || /usr/bin/true' >> ${TMPPLIST}
>
>
> Ditto with PREFIX -> LOCALBASE.
OK.
>>+.endif
>>
>> # XXX: put unregistervm into install script ?
>> post-install:
>
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?433B9826.10607>