Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatic rt.jar detection is likely incorrect on macOS for pre-7 versions of the JDK #1062

Closed
RyanGlScott opened this issue Feb 5, 2021 · 4 comments · Fixed by #1063
Closed
Labels
subsystem: crucible-jvm Issues related to Java verification with crucible-jvm

Comments

@RyanGlScott
Copy link
Contributor

I recently came across this note in find-java-rt-jar.sh:

# Find the Java core classes JAR, 'rt.jar' (called 'classes.jar' on OS
# X for JDKs prior to 1.7).

This came as a surprise to me, as I was under the impression that the JDK consistently named this file rt.jar. If this is the case, the logic I implemented for automatically detecting rt.jar in #1030 is incorrect when combining macOS with JDK 6 and earlier. We should verify that this is the case and add a corresponding code path for classes.jar.

@RyanGlScott RyanGlScott added the subsystem: crucible-jvm Issues related to Java verification with crucible-jvm label Feb 5, 2021
@RyanGlScott
Copy link
Contributor Author

This would also affect the crucible-jvm tool in the crucible repo, where I implemented similar code in GaloisInc/crucible#636.

@atomb
Copy link
Contributor

atomb commented Feb 5, 2021

Given how old JDK 6 is at this point, I think it might be reasonable to simply state that we only work with versions 7 and greater. Alternatively, we could identify the whole set of (JDK version, OS, rt.jar location) triples for historical versions. Or this could be the reason to keep SAW_JDK_JAR, as a way to allow the tools to function with old JDK versions.

@RyanGlScott
Copy link
Contributor Author

Valid points. It's perhaps a bit heavy handed to error outright if you use an old JDK, but we could document the fact that old JDKs might not adhere to modern conventions, so you may have to manually specify some locations (be it with -j, SAW_JDK_JAR, or otherwise) to compensate.

@atomb
Copy link
Contributor

atomb commented Feb 5, 2021

Valid points. It's perhaps a bit heavy handed to error outright if you use an old JDK, but we could document the fact that old JDKs might not adhere to modern conventions, so you may have to manually specify some locations (be it with -j, SAW_JDK_JAR, or otherwise) to compensate.

Yeah, something like that sounds exactly right.

RyanGlScott added a commit that referenced this issue Feb 5, 2021
* Remove the use of the `find-java-rt-jar.sh` script in CI, as this is no
  longer necessary with the advent of `--java-bin-dirs`/searching the `PATH`
  for Java. This fixes #1061.

* On a related note, it turns out that SAW's approach to detecting where
  `rt.jar` lives likely doesn't work on pre-7 JDKs on macOS. Given how
  ancient these versions of Java anymore, let's just document this infelicity
  in the manual and describe a workaround for those brave enough to try this.
  Fixes #1062.
RyanGlScott added a commit that referenced this issue Feb 8, 2021
* Remove the use of the `find-java-rt-jar.sh` script in CI, as this is no
  longer necessary with the advent of `--java-bin-dirs`/searching the `PATH`
  for Java. This fixes #1061.

* Now that `find-java-rt-jar.sh` is gone, there is no longer any need for the
  `.github/PropertiesTest.java` utility, nor is there any need for the
  `find_java` bash function that leverages this. This patch remove both of them
  as well.

* On a related note, it turns out that SAW's approach to detecting where
  `rt.jar` lives likely doesn't work on pre-7 JDKs on macOS. Given how
  ancient these versions of Java anymore, let's just document this infelicity
  in the manual and describe a workaround for those brave enough to try this.
  Fixes #1062.
RyanGlScott added a commit that referenced this issue Feb 11, 2021
* Remove the use of the `find-java-rt-jar.sh` script in CI, as this is no
  longer necessary with the advent of `--java-bin-dirs`/searching the `PATH`
  for Java. This fixes #1061.

* Now that `find-java-rt-jar.sh` is gone, there is no longer any need for the
  `.github/PropertiesTest.java` utility, nor is there any need for the
  `find_java` bash function that leverages this. This patch remove both of them
  as well.

* On a related note, it turns out that SAW's approach to detecting where
  `rt.jar` lives likely doesn't work on pre-7 JDKs on macOS. Given how
  ancient these versions of Java anymore, let's just document this infelicity
  in the manual and describe a workaround for those brave enough to try this.
  Fixes #1062.
@mergify mergify bot closed this as completed in #1063 Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
subsystem: crucible-jvm Issues related to Java verification with crucible-jvm
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants