From owner-freebsd-java@FreeBSD.ORG Thu Sep 29 07:31:24 2005 Return-Path: X-Original-To: freebsd-java@freebsd.org Delivered-To: freebsd-java@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0844F16A451 for ; Thu, 29 Sep 2005 07:31:24 +0000 (GMT) (envelope-from past@ebs.gr) Received: from fly.ebs.gr (fly.ebs.gr [62.103.84.177]) by mx1.FreeBSD.org (Postfix) with ESMTP id E539543D5A for ; Thu, 29 Sep 2005 07:31:20 +0000 (GMT) (envelope-from past@ebs.gr) Received: from ebs.gr (root@hal.ebs.gr [10.1.1.2]) by fly.ebs.gr (8.12.9p1/8.12.9) with ESMTP id j8T7Ut9V065266; Thu, 29 Sep 2005 10:30:56 +0300 (EEST) (envelope-from past@ebs.gr) Received: from [10.1.1.158] (pc158.ebs.gr [10.1.1.158]) by ebs.gr (8.13.3/8.12.11) with ESMTP id j8T7V4wm073046; Thu, 29 Sep 2005 10:31:06 +0300 (EEST) (envelope-from past@ebs.gr) Message-ID: <433B9826.10607@ebs.gr> Date: Thu, 29 Sep 2005 10:30:46 +0300 From: Panagiotis Astithas Organization: EBS Ltd. User-Agent: Mozilla Thunderbird 1.0.6 (X11/20050830) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Greg Lewis References: <433B067C.2020500@ebs.gr> <20050928222659.GA21920@misty.eyesbeyond.com> In-Reply-To: <20050928222659.GA21920@misty.eyesbeyond.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-java@freebsd.org Subject: Re: [patch] Install a desktop icon for the jdk15 Control Panel X-BeenThere: freebsd-java@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting Java to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Sep 2005 07:31:24 -0000 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: > >