bugGNU Octave - Bugs: bug #53510, [octave forge] (io) wrong java...

 
 

bug #53510: [octave forge] (io) wrong java check

Submitter:  Muhali <muhali>
Submitted:  Thu 29 Mar 2018 03:11:31 PM UTC
   
 
Category:  Octave Package Severity:  4 - Important
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  philipnienhuis
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sat 14 Apr 2018 07:20:05 PM UTC, comment #22: 

Thanks for the heads up.
Pushed (the io repo works again, was clobbered up up for a few days)

Closing again

Philip Nienhuis <philipnienhuis>
Group Member
Sat 14 Apr 2018 12:44:25 PM UTC, comment #21: 

@Philip: I just noticed that you might have pushed the wrong patch. The one from comment #14 (the one that seems to have been pushed to of-io) fails to detect Java 10 and 11 (like Mike pointed out). The one in comment #16 should be better at that.

Re-opening for that reason.

Markus Mützel <mmuetzel>
Group administrator
Fri 06 Apr 2018 09:26:23 PM UTC, comment #20: 
Philip Nienhuis <philipnienhuis>
Group Member
Fri 06 Apr 2018 09:24:22 PM UTC, comment #19: 

Pushed to io repo.
I can't give a URL now as SourceForge is exceptionally slow.

Closing report.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 06 Apr 2018 09:01:51 AM UTC, comment #18: 

I just tested the patch and it works for me as well.
Later today I'll push it to the OF io repo.

As I figured the io package's spreadsheet scripts may need a bit of attention for Java > 8, with Apache POI (and Java 10) I get:

Testing .xls interface POI using file io-test.xlsx...

 1. Initialize arrays.

 2. Insert first empty sheet.
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.apache.poi.util.DocumentHelper (file:/C:/Users/philip/java/poi-ooxml-3.17.jar) to method com.sun.org.apache.xerces.internal.util.SecurityManager.setEntityExpansionLimit(int)
WARNING: Please consider reporting this to the maintainers of org.apache.poi.util.DocumentHelper
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release


... that wasn't there with Java 8 so indeed some check for Java versions may need to stay in _chk_java_sprt_.m.
But that is less urgent as the above looks like an upstream bug and anyway in the end all io package functions seem to work correctly:

 :
Interface:          COM  POI  POI  JXL  OXS  UNO  OCT  OTK  JOD  UNO  OCT  OCT
File type           xls  xls xlsx  xls  xls  xls xlsx  ods  ods  ods  ods gnumeric
Numeric array p.1:   +    +    +    +    +    +    +    +    +    +    +    +
Numeric array p.2:   +    +    +    +    +    +    +    +    +    +    +    +
Numeric array p.3:   +    +    +    +    +    +    +    +    +    +    +    +
Numeric array p.4:   +    +    +    +    +    +    +    +    +    +    +    +
Cellstr array p.1:   +    +    +    +    +    +    +    +    +    +    +    +
Cellstr array p.2:   +    +    +    +    +    +    +    +    +    +    +    +
 ...special chars:   +    +    +    +    +    +    +    +    +    +    +    +
Boolean value    :   +    +    +    +    +    o    +    +    +    o    +    +
Formula read back:   +    +    +    +    +    +    +    +    +    +    +    +
  +  = correct result returned
  o  = partly correct (e.g., double rather than logical)
  -  = erroneous or no result.


Thanks Mike and Markus for the patches.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 03 Apr 2018 01:48:45 AM UTC, comment #17: 

Yes, this patch works for me with either Java 9.0.4 or 11.

Mike Miller <mtmiller>
Group Member
Sat 31 Mar 2018 09:39:29 PM UTC, comment #16: 

Thanks for testing, Mike.
The attached patch should also catch Java 10 and 11. Let's hope they don't invent yet another numbering scheme for Java 12 or before Java 11 is released.

(file #43752)

Markus Mützel <mmuetzel>
Group administrator
Sat 31 Mar 2018 07:43:46 PM UTC, comment #15: 

Markus - your patch does not work for me with OpenJDK 10 or 11, where "java.version" is simply "10" or "11" with no dotted version numbers.

Mike Miller <mtmiller>
Group Member
Sat 31 Mar 2018 01:00:38 PM UTC, comment #14: 

I installed Java 1.5 64bit and tried to use it with a recent Octave 4.5.0+ on Windows. Even after fixing the registry key value (wrong folder for Java 64bit), Octave is still unable to load it correctly anyway (as expected):

octave:1> version -java
ans = no Java available
octave:2> javaclasspath
error: unable to start Java VM in C:\Program Files\Java\jre1.5.0_22\bin\server\jvm.dll
error: called from
    javaclasspath at line 64 column 16


So the version check isn't really needed for the upcoming Octave versions. However, afaics the minimum Java version was still 1.2 for the 4.2s.
I don't know how widespread those "antique" versions of JRE still are. But for current releases of Octave, this check should probably be kept in (and adapted for the new version numbering scheme of the JRE).
The attached patch should work with the old and new numbering schemes.

(file #43742)

Markus Mützel <mmuetzel>
Group administrator
Fri 30 Mar 2018 06:42:35 PM UTC, comment #13: 

Why do you keep mentioning Java 1.4?

I'm not fully convinced that any Java version check is obsolete.
Java 9 brings along some new things with it so that I want to be sure it will work for the io package.
Let's first see if bug #53520 helps (Markus M supplied a fix), then I can test the rest of the io package stuff incl. the XML I/O in Windows.
I'll buy that it'll all works fine on Linux (for which I do care), but there are more OSes around.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 30 Mar 2018 04:27:46 PM UTC, comment #12: 

I don't think you need Java 9 to test this bug, if the solution is to remove the version check entirely. You can test it with whatever Java you have access to that work with Octave on Windows.

The only thing that needs testing is if you want to try installing a Java 1.4 to see whether this check provides any value, by preventing something from breaking later.

This bug definitely does impact GNU/Linux users now who have Java 9.

Mike Miller <mtmiller>
Group Member
Fri 30 Mar 2018 08:02:12 AM UTC, comment #11: 

I've added a dependency on bug #53520, as on Windows I cannot get Java 9 detected so I cannot even proceed fixing the underlying issues with this bug.
(Hopefully I added the dependency in the right way)

I found that the Java check for >= 1.6 dates back from before the octave-forge switch to hg; the reason for the check is in the .svn history but I can't access that anymore.

So on the one hand, being the reason for the check so ancient it looks safe to throw it out, OTOH, taking into account bug #53520, this check may need to be renewed.

So do we need to set status to "postponed" until bug #53520 has been fixed?

On Linux boxes you probably won't see any issues as the Java version Octave was built with usually coincides with the Java version Octave is running with. But that isn't guaranteed.

Philip Nienhuis <philipnienhuis>
Group Member
Thu 29 Mar 2018 10:43:55 PM UTC, comment #10: 

@Mike comment #7:

At first sight, in lieu of my comment #6, I think your patch should be acceptable.
Yet there must have been a reason I've put the version check in, but that is long ago so I need to dig it up and see if it is still required.
Somewhere this Easter weekend I think I can do that.

Maybe a try-catch around the version method can help as well - that did work with other Java methods that had changed and threw NPEs. We'll see.
Oh and I need to install Java 9, I'm still on 8

Philip Nienhuis <philipnienhuis>
Group Member
Thu 29 Mar 2018 10:22:31 PM UTC, comment #9: 

For the record, what I'm going to do (why I put my name up) is check the various Java based interfaces for a reasonable minimum Java version.

Philip Nienhuis <philipnienhuis>
Group Member
Thu 29 Mar 2018 09:30:29 PM UTC, comment #8: 

Ah I forgot about _chk_java_sprt_ - yes that might be patched as suggested- but hopefully it and the rest of the io package still works for (much) older Java versions as well.
If bug reports pop up we can always suggest to update Java. But I think most people do, or at least should, have fairly up-to-date Java anyway because of patched security issues - at least the distro I'm using (Mageia) regularly updates it. Having an outdated Java installed isn't a very sensible thing to do.

(Anyway it's a typical case of too new a Java version :-)
Apparently the version method has changed in recent Java versions.)

Philip Nienhuis <philipnienhuis>
Group Member
Thu 29 Mar 2018 09:23:42 PM UTC, comment #7: 

Philip - I mean are you willing to apply my suggested patch in #file 43716 to drop the check for versions of Java older than 1.6 completely? It is causing breakage for users with Java 9 (and future Java 10, 11, etc).

Mike Miller <mtmiller>
Group Member
Thu 29 Mar 2018 09:16:57 PM UTC, comment #6: 

@Mike(comment #4):
You mean a minimum required Java version for the io package?

In the io package there's only a check on availability of a Java JRE. Version is retrieved but not invoked anywhere.
In the io package several Java dependent "interfaces" (~dependencies) have been implemented. I'd need to check each of them to find out a minimum Java version. But then again, each of those 'interfaces' comes in several versions each with a separate minimum Java version.

So in the end I think I just don't bother.
In my own experience I found that those "interfaces" are fairly robust as regards Java version; if problems arise I expect it to be due to too new a Java version rather than too old.

Most people I get reports from have a fairly recent or the newest io package version; few of them seem to have bothered to install Java and one of the Java-based interfaces at all and rely on the native Octave (OCT) interface.

All of this probably doesn't answer your question directly, but I think / hope it'll be clear enough for you to make a decision.

Philip Nienhuis <philipnienhuis>
Group Member
Thu 29 Mar 2018 06:48:52 PM UTC, comment #5: 

I'm attaching a patch that I've also sent to Debian to remove this failing version check. I think this should be appropriate to apply to the io package here as well.

(file #43716)

Mike Miller <mtmiller>
Group Member
Thu 29 Mar 2018 06:21:54 PM UTC, comment #4: 

Is this version check still needed at all?

Octave now builds its Java interface class files with the '-source 1.6 -target 1.6' options, meaning the bytecode is only compatible with Java 6 and later. I assume that means that if a user has an ancient Java 1.4 or 1.5, none of Octave's Java functions will work now anyway.

Philip - what do you think? Safe to drop the version check entirely?

Mike Miller <mtmiller>
Group Member
Thu 29 Mar 2018 03:39:04 PM UTC, comment #3: 

@Dmitri: yes.
@jwe: probably

So we should check more carefully.

Muhali <muhali>
Thu 29 Mar 2018 03:35:29 PM UTC, comment #2: 

If we make that change unconditionally, will that break the check for older versions?

John W. Eaton <jwe>
Group administrator
Thu 29 Mar 2018 03:35:24 PM UTC, comment #1: 

Currently i have
javaMethod ("getProperty", "java.lang.System", "java.version")
ans = 9.0.4

Would this work with your patch?

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Thu 29 Mar 2018 03:11:31 PM UTC, original submission:  

the private function _chk_java_sprt_ checks the installed java version. Apparently this doesn't work anymore for newer versions of java. A possible patch is this:


--- /usr/local/octave-dev/share/octave/packages/io-2.4.10/private/__chk_java_sprt__.m
+++ /tmp/__chk_java_sprt__.m
@@ -42,7 +42,7 @@
     jver = ...
       char (javaMethod ("getProperty", "java.lang.System", "java.version"));
     cjver = strsplit (jver, ".");
-    if (sscanf (cjver{2}, "%d") < 6)
+    if (sscanf (cjver{1}, "%d") < 6)
       warning ...
         ("\nJava version too old - you need at least Java 6 (v. 1.6.x.x)\n");
       if (dbug)


Muhali <muhali>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #43752:  bug53510_java9+_version_v2.patch added by mmuetzel (2KiB - application/octet-stream)
file #43742:  bug53510_java9+_version.patch added by mmuetzel (1KiB - application/octet-stream)

 

Digest:
   bug dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Updated the item)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by mtmiller
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by muhali (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 18 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-04-14 philipnienhuis StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2018-04-14 mmuetzel StatusFixed In Progress
        Open/ClosedClosed Open
    2018-04-06 philipnienhuis StatusPatch Reviewed Fixed
        Open/ClosedOpen Closed
    2018-04-06 philipnienhuis StatusPatch Submitted Patch Reviewed
    2018-03-31 mmuetzel Attached File- Added bug53510_java9+_version_v2.patch, #43752
        Operating SystemGNU/Linux Any
    2018-03-31 mmuetzel Attached File- Added bug53510_java9+_version.patch, #43742
    2018-03-30 philipnienhuis Dependencies- Depends on bugs #53520
    2018-03-29 philipnienhuis Assigned toNone philipnienhuis
    2018-03-29 mtmiller Attached File- Added drop-version-number-check-for-outdated-versions-of-JRE.patch, #43716
        Severity3 - Normal 4 - Important
        Item GroupNone Unexpected Error or Warning
        StatusNone Patch Submitted
    2018-03-29 mtmiller Carbon-Copy- Added philipnienhuis
    2018-03-29 jwe Summary[octave-forge] (io) wrong java check [octave forge] (io) wrong java check

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code