Re: [PATCH] buildscript java version detection fails on Linux/Ubuntu on Picked up JAVA_TOOL_OPTIONS

2015-08-29 Thread Maurice le Rutte 
Since I haven't received any additional feedback, should I push this patch?

Op 27 aug. 2015 15:56 schreef i...@cuhka.com:

 I've adapted the build.gradle to use java -fullversion instead of java  
 -version, parsing the result with a regexp. 

 diff -r 9b5fc7c1e5e6 build.gradle 
 --- a/build.gradle Fri Aug 07 18:35:42 2015 -0700 
 +++ b/build.gradle Thu Aug 27 15:52:11 2015 +0200 
 @@ -719,29 +719,27 @@ 
   if (!file(JAVAH).exists()) throw new Exception(Missing or incorrect  
 path to 'javah': '$JAVAH'. Perhaps bad JDK_HOME? $JDK_HOME) 
   if (!file(JAVADOC).exists()) throw new Exception(Missing or  
 incorrect path to 'javadoc': '$JAVADOC'. Perhaps bad JDK_HOME?  
 $JDK_HOME) 

 - 
 + 
   // Determine the verion of Java in JDK_HOME. It looks like this: 
   // 
 -// $ java -version 
 -// java version 1.7.0_45 
 -// Java(TM) SE Runtime Environment (build 1.7.0_45-b18) 
 -// Java HotSpot(TM) 64-Bit Server VM (build 24.45-b08, mixed mode) 
 -// 
 -// We need to parse the second line 
 -def inStream = new java.io.BufferedReader(new  
 java.io.InputStreamReader(new java.lang.ProcessBuilder(JAVA,  
 -version).start().getErrorStream())); 
 +// $ java -fullversion 
 +// for OpenJDK: openjdk full version 1.8.0_45-internal-b14 
 +// for Oracle JDK: java full version 1.8.0_45-b14 
 + 
 +def inStream = new java.io.BufferedReader(new  
 java.io.InputStreamReader(new java.lang.ProcessBuilder(JAVA,  
 -fullversion).start().getErrorStream())); 
   try { 
 -    if (inStream.readLine() != null) { 
 -    String v = inStream.readLine(); 
 -    if (v != null) { 
 -    int ib = v.indexOf( (build ); 
 -    if (ib != -1) { 
 -    String ver = v.substring(ib + 8, v.size() - 1); 
 - 
 -    defineProperty(jdkRuntimeVersion, ver) 
 -    defineProperty(jdkVersion, 
 jdkRuntimeVersion.split(-)[0]) 
 -    defineProperty(jdkBuildNumber,  
 jdkRuntimeVersion.substring(jdkRuntimeVersion.lastIndexOf(-b) + 2)) 
 -    } 
 -    } 
 +    def v = inStream.readLine(); 
 + 
 +    if (v != null) { 
 + def pattern =  
 java.util.regex.Pattern.compile(^[^\]*\((\\d+(?:\\.\\d+)*)_(\\d+))(?:-\\w+)?(?:-b(\\d+))\\$)
  
 + def matcher = pattern.matcher(v) 
 + 
 + if (matcher.matches()) { 
 +    defineProperty(jdkVersion, matcher.group(1)) 
 +    defineProperty(jdkRuntimeVersion, matcher.group(2)) 
 +    defineProperty(jdkUpdate, matcher.group(3)) 
 +    defineProperty(jdkBuildNumber, matcher.group(4)) 
 + } 
   } 
   } finally { 
   inStream.close();

Re: [PATCH] buildscript java version detection fails on Linux/Ubuntu on Picked up JAVA_TOOL_OPTIONS

2015-08-29 Thread Kevin Rushforth 

Hi Maurice,

No, you cannot simply push this patch. Please refer to the OpenJDK Wiki 
[1] for information on contributing patches to OpenJFX. Additional 
information on Submitting OpenJFX bugs [2] and Code Review policies [3] 
are also on the Wiki. I note that simple patches such as this can be 
accepted without an OCA being signed (although if you want to contribute 
a non-trivial patch you will need to sign the OCA).


Everything starts out with a bug report, which you can submit if you 
like. However, in this case, I am likely to just fix this as part of 
fixing JDK-8133750 [4] since the version detection logic also needs to 
be modified for the new Java version string JEP as described in that bug 
report.


So: Either I can add your information to JDK-8133750 or you can file a 
new bug and I will link them together and fix them at the same time.


-- Kevin

[1] http://openjdk.java.net/contribute/
[2] https://wiki.openjdk.java.net/display/OpenJFX/Submitting+a+Bug+Report
[3] https://wiki.openjdk.java.net/display/OpenJFX/Code+Reviews
[4] https://bugs.openjdk.java.net/browse/JDK-8133750



Maurice le Rutte wrote:

Since I haven't received any additional feedback, should I push this patch?

Op 27 aug. 2015 15:56 schreef i...@cuhka.com:
  
I've adapted the build.gradle to use java -fullversion instead of java  
-version, parsing the result with a regexp. 

diff -r 9b5fc7c1e5e6 build.gradle 
--- a/build.gradle Fri Aug 07 18:35:42 2015 -0700 
+++ b/build.gradle Thu Aug 27 15:52:11 2015 +0200 
@@ -719,29 +719,27 @@ 
  if (!file(JAVAH).exists()) throw new Exception(Missing or incorrect  
path to 'javah': '$JAVAH'. Perhaps bad JDK_HOME? $JDK_HOME) 
  if (!file(JAVADOC).exists()) throw new Exception(Missing or  
incorrect path to 'javadoc': '$JAVADOC'. Perhaps bad JDK_HOME?  
$JDK_HOME) 

- 
+ 
  // Determine the verion of Java in JDK_HOME. It looks like this: 
  // 
-// $ java -version 
-// java version 1.7.0_45 
-// Java(TM) SE Runtime Environment (build 1.7.0_45-b18) 
-// Java HotSpot(TM) 64-Bit Server VM (build 24.45-b08, mixed mode) 
-// 
-// We need to parse the second line 
-def inStream = new java.io.BufferedReader(new  
java.io.InputStreamReader(new java.lang.ProcessBuilder(JAVA,  
-version).start().getErrorStream())); 
+// $ java -fullversion 
+// for OpenJDK: openjdk full version 1.8.0_45-internal-b14 
+// for Oracle JDK: java full version 1.8.0_45-b14 
+ 
+def inStream = new java.io.BufferedReader(new  
java.io.InputStreamReader(new java.lang.ProcessBuilder(JAVA,  
-fullversion).start().getErrorStream())); 
  try { 
-if (inStream.readLine() != null) { 
-String v = inStream.readLine(); 
-if (v != null) { 
-int ib = v.indexOf( (build ); 
-if (ib != -1) { 
-String ver = v.substring(ib + 8, v.size() - 1); 
- 
-defineProperty(jdkRuntimeVersion, ver) 
-defineProperty(jdkVersion, jdkRuntimeVersion.split(-)[0]) 
-defineProperty(jdkBuildNumber,  
jdkRuntimeVersion.substring(jdkRuntimeVersion.lastIndexOf(-b) + 2)) 
-} 
-} 
+def v = inStream.readLine(); 
+ 
+if (v != null) { 
+ def pattern =  
java.util.regex.Pattern.compile(^[^\]*\((\\d+(?:\\.\\d+)*)_(\\d+))(?:-\\w+)?(?:-b(\\d+))\\$) 
+ def matcher = pattern.matcher(v) 
+ 
+ if (matcher.matches()) { 
+defineProperty(jdkVersion, matcher.group(1)) 
+defineProperty(jdkRuntimeVersion, matcher.group(2)) 
+defineProperty(jdkUpdate, matcher.group(3)) 
+defineProperty(jdkBuildNumber, matcher.group(4)) 
+ } 
  } 
  } finally { 
  inStream.close();

Re: [PATCH] buildscript java version detection fails on Linux/Ubuntu on Picked up JAVA_TOOL_OPTIONS

2015-08-29 Thread info 


Citeren Kevin Rushforth kevin.rushfo...@oracle.com:
Either I can add your information to JDK-8133750 or you can file a  
new bug and I will link them together and fix them at the same time.

I'd prefer the former over the latter.

Maurice.

[PATCH] buildscript java version detection fails on Linux/Ubuntu on Picked up JAVA_TOOL_OPTIONS

2015-08-27 Thread info 
I've adapted the build.gradle to use java -fullversion instead of java  
-version, parsing the result with a regexp.


diff -r 9b5fc7c1e5e6 build.gradle
--- a/build.gradle  Fri Aug 07 18:35:42 2015 -0700
+++ b/build.gradle  Thu Aug 27 15:52:11 2015 +0200
@@ -719,29 +719,27 @@
 if (!file(JAVAH).exists()) throw new Exception(Missing or incorrect  
path to 'javah': '$JAVAH'. Perhaps bad JDK_HOME? $JDK_HOME)
 if (!file(JAVADOC).exists()) throw new Exception(Missing or  
incorrect path to 'javadoc': '$JAVADOC'. Perhaps bad JDK_HOME?  
$JDK_HOME)


-
+
 // Determine the verion of Java in JDK_HOME. It looks like this:
 //
-// $ java -version
-// java version 1.7.0_45
-// Java(TM) SE Runtime Environment (build 1.7.0_45-b18)
-// Java HotSpot(TM) 64-Bit Server VM (build 24.45-b08, mixed mode)
-//
-// We need to parse the second line
-def inStream = new java.io.BufferedReader(new  
java.io.InputStreamReader(new java.lang.ProcessBuilder(JAVA,  
-version).start().getErrorStream()));

+// $ java -fullversion
+// for OpenJDK: openjdk full version 1.8.0_45-internal-b14
+// for Oracle JDK: java full version 1.8.0_45-b14
+
+def inStream = new java.io.BufferedReader(new  
java.io.InputStreamReader(new java.lang.ProcessBuilder(JAVA,  
-fullversion).start().getErrorStream()));

 try {
-if (inStream.readLine() != null) {
-String v = inStream.readLine();
-if (v != null) {
-int ib = v.indexOf( (build );
-if (ib != -1) {
-String ver = v.substring(ib + 8, v.size() - 1);
-
-defineProperty(jdkRuntimeVersion, ver)
-defineProperty(jdkVersion, jdkRuntimeVersion.split(-)[0])
-defineProperty(jdkBuildNumber,  
jdkRuntimeVersion.substring(jdkRuntimeVersion.lastIndexOf(-b) + 2))

-}
-}
+def v = inStream.readLine();
+
+if (v != null) {
+	def pattern =  
java.util.regex.Pattern.compile(^[^\]*\((\\d+(?:\\.\\d+)*)_(\\d+))(?:-\\w+)?(?:-b(\\d+))\\$)

+   def matcher = pattern.matcher(v)
+
+   if (matcher.matches()) {
+defineProperty(jdkVersion, matcher.group(1))
+defineProperty(jdkRuntimeVersion, matcher.group(2))
+defineProperty(jdkUpdate, matcher.group(3))
+defineProperty(jdkBuildNumber, matcher.group(4))
+   }
 }
 } finally {
 inStream.close();