Opened 7 years ago

Closed 7 years ago

#17279 closed defect (fixed)

Fix java version detection

Reported by: jdemeyer Owned by:
Priority: blocker Milestone: sage-6.4
Component: interfaces Keywords:
Cc: fbissey, vbraun, gutow, kcrisman, strogdon, jdemeyer Merged in:
Authors: François Bissey, Jonathan Gutow Reviewers: Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: 4af9827 (Commits, GitHub, GitLab) Commit: 4af9827113f738838953b67e5786bba0739d06e9
Dependencies: Stopgaps:

Status badges

Description

Since the Java requirements have gone up, the version detection in src/sage/interfaces/jmoldata.py should be fixed.

Change History (27)

comment:1 Changed 7 years ago by jdemeyer

  • Cc fbissey vbraun gutow kcrisman added

comment:2 follow-up: Changed 7 years ago by vbraun

Or jmol should be compiled to run on older JREs if possible ;-)

comment:3 in reply to: ↑ 2 Changed 7 years ago by fbissey

Replying to vbraun:

Or jmol should be compiled to run on older JREs if possible ;-)

Whichever but we have to agree quickly. Do we want to require java 1.7 as a minimum? Is there a good reason to ditch java 1.6?

comment:4 Changed 7 years ago by fbissey

Actually the current code in sage check for version as far back as 1.5 and would be unhappy with 1.8.

        import re
        java_version = re.search("version.*[1][.][567]", version)
        return java_version is not None

so some fix will be in order anyway.

Last edited 7 years ago by fbissey (previous) (diff)

comment:5 follow-up: Changed 7 years ago by gutow

Good, the version checking is still in there. This needs to be changed to not accept anything less than java 7. I think Jmol will work with java 8, but have not done any testing.

I recommend "java_version = re.search("version.*[1].[78]", version)" and if we run into problems with version 8 JVMs we back up.

comment:6 Changed 7 years ago by vbraun

Seems like they intentionally require 1.7+, so we should just require that.

comment:7 Changed 7 years ago by strogdon

  • Cc strogdon added

comment:8 in reply to: ↑ 5 ; follow-up: Changed 7 years ago by kcrisman

  • Cc jeroen added

Good, the version checking is still in there. This needs to be changed to not accept anything less than java 7. I think Jmol will work with java 8, but have not done any testing.

Jonathan, do we need Java 1.7 to view jsmol things, though? Also, that means the wiki for jmol is pretty out of date.

Seems like they intentionally require 1.7+, so we should just require that.

Yup. Wow, that is a bit of a change. Sorry, Jeroen :(

comment:9 in reply to: ↑ 8 ; follow-up: Changed 7 years ago by gutow

Replying to kcrisman:

Jonathan, do we need Java 1.7 to view jsmol things, though? Also, that means the wiki for jmol is pretty out of date.

JSmol and Jmol work without the backend. The backend is presently just used to make the nice static pictures, without it a less pretty Tachyon image is generated. Ultimately, the backend should be used to package the data as well. The present packaging method is not terribly efficient or flexible.

It is quite possible that the wiki is behind. I'll put it on our todo list.

comment:10 in reply to: ↑ 9 ; follow-up: Changed 7 years ago by kcrisman

Jonathan, do we need Java 1.7 to view jsmol things, though? Also, that means the wiki for jmol is pretty out of date.

JSmol and Jmol work without the backend. The backend is presently just used to make the nice static pictures, without it a less pretty Tachyon image is generated. Ultimately, the backend should be used to package the data as well. The present packaging method is not terribly efficient or flexible.

Wait, wait. So is Java 1.7+ necessary to view Jmol applets from the command line? Exactly what is it that needs 1.7? If it's just the static images, then we could fail gracefully in the notebook to Tachyon images when the Java is not recent enough. If I'm not understanding this properly, sorry for the noise.

comment:11 in reply to: ↑ 10 Changed 7 years ago by gutow

Replying to kcrisman:

Wait, wait. So is Java 1.7+ necessary to view Jmol applets from the command line? Exactly what is it that needs 1.7?

Yes, because from the command line you run the Jmol application which is a java application and now requires a recent JVM. A side note. I looked carefully at how we comile Jmol. Interestingly, we actually compile Jmol to be 1.6+ compatible, but it looks like the external gettext libraries now require 1.7+ and we're not about to rewrite those. So people may only run into this problem if they are using translations.

If it's just the static images, then we could fail gracefully in the notebook to Tachyon images when the Java is not recent enough. If I'm not understanding this properly, sorry for the noise.

We already do that except that we were still accepting JVM >1.5, which no longer works. I think we have to accept only 1.7+.

comment:12 Changed 7 years ago by fbissey

OK thanks for all that information it is time to start committing now. There is the spot mentioned above and at least one that I can easily find in the FAQ. I should be done with those in the next 15mn.

comment:13 Changed 7 years ago by fbissey

  • Authors set to François Bissey
  • Branch set to u/fbissey/java-1.7
  • Commit set to 8b145bc1f14c074371ed766b9c2133a7bd9478e7
  • Status changed from new to needs_review

OK minimal change in the code but the documentation clearly needs a dusting. Especially in the wake of jsmol. Someone should review what I wrote and may be come with a paragraph about it.


New commits:

8b145bcNow testing for java 7 or 8. Edited the documentation to reflect these change and refresh it a bit.

comment:14 Changed 7 years ago by gutow

Something has happened to the trac changelog. I get an error about malformed xml when I try git trac checkout.

comment:15 Changed 7 years ago by gutow

OK, I was able to clone the branch...I'll work with that.

comment:16 Changed 7 years ago by gutow

That didn't work either. I'll just hand copy the patch...

comment:17 Changed 7 years ago by fbissey

Please correct my cruft while you are at it: "use at a Java" yuk.

comment:18 Changed 7 years ago by gutow

  • Branch changed from u/fbissey/java-1.7 to u/gutow/ticket/17279
  • Created changed from 11/03/14 09:21:15 to 11/03/14 09:21:15
  • Modified changed from 11/04/14 00:17:42 to 11/04/14 00:17:42

comment:19 Changed 7 years ago by gutow

  • Commit changed from 8b145bc1f14c074371ed766b9c2133a7bd9478e7 to 5eb4a37ae25e8a4dc3863301f9d86527fa7a9553

OK, I think this last commit is a reasonable update to the faq.


New commits:

656c1b2Java version to 1.7 and Jmol/JSmol FAQ update
5eb4a37Merge branch 'u/fbissey/java-1.7' of git://trac.sagemath.org/sage into develop

comment:20 Changed 7 years ago by gutow

No! 'sage -dev push --ticket 17279' only seems to have added the last couple of lines of the rewrite. I'm giving up on the trac/git combination. Here's the corrected text.

The default live 3-D plotting for Sage 6.4+ uses Jmol/JSmol <http://jmol.sourceforge.net>_ for viewing. From the command line the Jmol Java application is used, and for in browser viewing either pure javascript or a Java applet is used. By default in browsers pure javascript is used to avoid the problems with some browsers that do not support java applet plugins (namely Chrome). On each browser worksheet there is a checkbox which must be checked before a 3-D plot is generated if the user wants to use the Java applet (the applet is a little faster with complex plots). The most likely reason for a malfunction is that you do not have a Java Run Time Environment (JRE) installed or you have one older than version 1.7. If things work from the command line another possibility is that your browser does not have the proper plugin to support Java applets (at present, 2014, plugins do not work with most versions of Chrome). Make sure you have installed either the IcedTea? browser plugin (for linux see your package manager), see: IcedTea <http://icedtea.classpath.org/wiki/IcedTea-Web>_, or the Oracle Java plugin see: Java <https://java.com/en/download/help/index_installing.xml>_.

If you are using a Sage server over the web and even javascript rendering does not work, you may have a problem with your browser's javascript engine or have it turned off.

comment:21 Changed 7 years ago by gutow

I'm confused. This commit ​656c12 looks OK, but the final commit only has the last couple of lines. It appears that resolving the conflict between the two updates confused things. Or am I just not understanding?

comment:22 Changed 7 years ago by fbissey

  • Authors changed from François Bissey to François Bissey, Jonathan Gutow

I am sure you did it the recommended way but it is all there. Thank you for rewriting that bit of faq.

comment:23 Changed 7 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman

This is pretty awesome, thanks. I can't test this since I don't have a bad java :-) but presumably Jeroen can and of course the change is quite minimal. In terms of the doc, I would just suggest that

+with complex plots).
+The most likely reason for a malfunction is that you do not have

have a newline in between, because otherwise Sphinx will turn it into one enormous paragraph, I think. Nice work!

comment:24 Changed 7 years ago by jeroen

  • Cc jdemeyer added; jeroen removed

comment:25 Changed 7 years ago by fbissey

  • Branch changed from u/gutow/ticket/17279 to u/fbissey/ticket_17279
  • Commit changed from 5eb4a37ae25e8a4dc3863301f9d86527fa7a9553 to 4af9827113f738838953b67e5786bba0739d06e9
  • Status changed from needs_review to positive_review

Done. Putting this to positive review then.


New commits:

4af9827Incorporate Karl-Dieter's comment about a newline.

comment:26 Changed 7 years ago by kcrisman

Great.

comment:27 Changed 7 years ago by vbraun

  • Branch changed from u/fbissey/ticket_17279 to 4af9827113f738838953b67e5786bba0739d06e9
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.