Bug 86860

Summary: [patch] Align java/eclipse invocation with javavmwrapper
Product: Ports & Packages Reporter: Panagiotis Astithas <past>
Component: Individual Port(s)Assignee: Herve Quiroz <hq>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
eclipse.patch none

Description Panagiotis Astithas 2005-10-03 09:30:13 UTC 

The eclipse startup script that launches the Eclipse IDE has a
hardcoded default JAVA_HOME directory to that of java/jdk14. This can
be overridden by the user's setting of JAVA_HOME. However, this is not
the recommended way to override the default jdk. Both javavmwrapper
(a dependency of the jdk ports) and bsd.java.mk recommend the use of
variables like JAVA_VERSION, JAVA_PREFERRED_PORTS, etc.

One known case where the current script fails to override the default
jdk, is when one has both jdk14 and jdk15 and tries to select jdk15
using JAVA_PREFERRED_PORTS. The attached patch make the default
JAVA_HOME directory correspond to javavmwrapper's symlinks, in
/usr/local/bin.

How-To-Repeat: 

Install java/jdk14, java/jdk15 and eclipse.
Set JAVA_PREFERRED_PORTS to JAVA_PORT_NATIVE_BSDJAVA_JDK_1_5.
Try to launch eclipse. 
Verify in Help->About->Configuration Details that it starts with jdk14.
Comment 1 Herve Quiroz freebsd_committer freebsd_triage 2005-10-03 11:12:59 UTC 
Responsible Changed
From-To: freebsd-ports-bugs->freebsd-eclipse

Over to maintainer.
Comment 2 Panagiotis Astithas 2005-10-04 22:13:00 UTC 
I have included this patch in the eclipse update in PR ports/86900.
Comment 3 Norikatsu Shigemura freebsd_committer freebsd_triage 2005-10-09 15:56:08 UTC 
State Changed
From-To: open->closed

Committed, thanks!
Comment 4 Hervé Quiroz 2005-10-10 16:00:27 UTC 
Hi,

I am a bit late on this topic (PR already closed) but I just noticed a
flaw with this change.

javavmwrapper picks a JDK depending on several variables -- see
javavm(1). With this change it is possible that a not suitable JDK gets
picked up. Here is a patch that should fix this (I didn't test it
though).


Index: files/eclipse.in
===================================================================
RCS file: /home/ncvs/ports/java/eclipse/files/eclipse.in,v
retrieving revision 1.5
diff -u -r1.5 eclipse.in
--- files/eclipse.in	9 Oct 2005 14:54:21 -0000	1.5
+++ files/eclipse.in	10 Oct 2005 14:54:13 -0000
@@ -46,4 +46,4 @@
 	exit 1
 fi
 
-PATH=${JAVA_HOME}/bin:$PATH exec "${ECLIPSE_HOME}/eclipse" $@
+JAVA_VERSION="%%JAVA_VERSION%%" JAVA_OS="%%JAVA_OS%%" PATH=${JAVA_HOME}/bin:$PATH exec "${ECLIPSE_HOME}/eclipse" $@
Comment 5 Panagiotis Astithas 2005-10-10 17:02:52 UTC 
Herve Quiroz wrote:
> Hi,
> 
> I am a bit late on this topic (PR already closed) but I just noticed a
> flaw with this change.
> 
> javavmwrapper picks a JDK depending on several variables -- see
> javavm(1). With this change it is possible that a not suitable JDK gets
> picked up. Here is a patch that should fix this (I didn't test it
> though).
> 
> 
> Index: files/eclipse.in
> ===================================================================
> RCS file: /home/ncvs/ports/java/eclipse/files/eclipse.in,v
> retrieving revision 1.5
> diff -u -r1.5 eclipse.in
> --- files/eclipse.in	9 Oct 2005 14:54:21 -0000	1.5
> +++ files/eclipse.in	10 Oct 2005 14:54:13 -0000
> @@ -46,4 +46,4 @@
>  	exit 1
>  fi
>  
> -PATH=${JAVA_HOME}/bin:$PATH exec "${ECLIPSE_HOME}/eclipse" $@
> +JAVA_VERSION="%%JAVA_VERSION%%" JAVA_OS="%%JAVA_OS%%" PATH=${JAVA_HOME}/bin:$PATH exec "${ECLIPSE_HOME}/eclipse" $@

Why do you think this is necessary? What the eclipse executable does is 
invoke the java executable it finds in the PATH. Since we have made sure 
the javavmwrapper's symlink will get chosen as the default, it will 
honour the JAVA_VERSION and JAVA_OS variables, right? Am I missing 
something here?

Regards,

Panagiotis
Comment 6 Hervé Quiroz 2005-10-10 17:16:06 UTC 
On Mon, Oct 10, 2005 at 07:02:52PM +0300, Panagiotis Astithas wrote:
> >-PATH=${JAVA_HOME}/bin:$PATH exec "${ECLIPSE_HOME}/eclipse" $@
> >+JAVA_VERSION="%%JAVA_VERSION%%" JAVA_OS="%%JAVA_OS%%" 
> >PATH=${JAVA_HOME}/bin:$PATH exec "${ECLIPSE_HOME}/eclipse" $@
> 
> Why do you think this is necessary? What the eclipse executable does is 
> invoke the java executable it finds in the PATH. Since we have made sure 
> the javavmwrapper's symlink will get chosen as the default, it will 
> honour the JAVA_VERSION and JAVA_OS variables, right? Am I missing 
> something here?

The port will pick a suitable JDK at *install* time depending on
JAVA_VERSION and JAVA_OS in the Makefile. What your patch is about is
the ability for the eclipse launcher to pick up a suitable JDK at *run*
time. And you patch does this well, that is javavmwrapper will be used
to find a JDK. Anyway, javavmwrapper will find a JDK depending on
JAVA_HOME, JAVA_VERSION, JAVA_OS and JAVA_VENDOR (any of those which are
defined). With my suggested changes, you still allow the launcher to let
javavmwrapper pick a JDK but you enforce the policy of the java/eclipse
port by restricting the choice of the JDK depending on JAVA_VERSION and
JAVA_OS that were specified while installing the port, namely:

  JAVA_VERSION=	1.4+
  JAVA_OS=	native

Without these changes, it is possible that Eclipse is run with a
non-suitable JDK. For example:

$ unset JAVA_HOME
$ unset JAVA_VENDOR
$ unset JAVA_OS
$ export JAVA_PREFERRED_PORTS=JAVA_PORT_NATIVE_BSDJAVA_JDK_1_1
$ eclipse

--> This will have the launcher to pick java/jdk11 which is not a
suitable JDK (as per the JAVA_VERSION variable specified in
java/eclipse/Makefile).

Herve
Comment 7 Hervé Quiroz 2005-10-10 17:23:28 UTC 
Ok, I just realized that the launcher was not executing a 'java' command
but rather the original 'eclipse' executable so the fix may be more
complex than the one I submitted.

I am currently installing eclipse then I will have a closer look at this
issue.

Herve
Comment 8 Hervé Quiroz 2005-10-10 18:06:44 UTC 
On Mon, Oct 10, 2005 at 06:23:28PM +0200, Herve Quiroz wrote:
> Ok, I just realized that the launcher was not executing a 'java' command
> but rather the original 'eclipse' executable so the fix may be more
> complex than the one I submitted.
> 
> I am currently installing eclipse then I will have a closer look at this
> issue.

After a bit of testing, it appears that the scenario I presented in my
other message (JAVA_PREFERRED_PORTS=JAVA_PORT_NATIVE_BSDJAVA_JDK_1_1)
will indeed break things.

And that the fix I submitted does address this problem.

Herve
Comment 9 Panagiotis Astithas 2005-10-10 21:26:23 UTC 
Herve Quiroz wrote:
> On Mon, Oct 10, 2005 at 06:23:28PM +0200, Herve Quiroz wrote:
> 
>>Ok, I just realized that the launcher was not executing a 'java' command
>>but rather the original 'eclipse' executable so the fix may be more
>>complex than the one I submitted.
>>
>>I am currently installing eclipse then I will have a closer look at this
>>issue.
> 
> 
> After a bit of testing, it appears that the scenario I presented in my
> other message (JAVA_PREFERRED_PORTS=JAVA_PORT_NATIVE_BSDJAVA_JDK_1_1)
> will indeed break things.
> 
> And that the fix I submitted does address this problem.

Ah, OK, I hadn't considered such a scenario.
Thanks for the insight.

Panagiotis
Comment 10 Herve Quiroz freebsd_committer freebsd_triage 2005-10-14 18:29:37 UTC 
State Changed
From-To: closed->open

I'll fix this.
Comment 11 Herve Quiroz freebsd_committer freebsd_triage 2005-10-14 18:29:37 UTC 
Responsible Changed
From-To: freebsd-eclipse->hq

I'll fix this.
Comment 12 Herve Quiroz freebsd_committer freebsd_triage 2005-10-18 03:35:10 UTC 
State Changed
From-To: open->closed

I commited the patch.