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

crucible-jvm: Add --java-bin-dirs #636

Merged
merged 2 commits into from
Feb 3, 2021
Merged

crucible-jvm: Add --java-bin-dirs #636

merged 2 commits into from
Feb 3, 2021

Conversation

RyanGlScott
Copy link
Contributor

This mirrors a similar recent change introduced to SAW in GaloisInc/saw-script#1030.

While I was in town, I updated the two crucible-jvm–related test suites to use the PATH instead of the -j flag. I can't exactly confirm via CI if these changes work (see #634), but they appear to do the right thing locally.

Fixes #633.

Copy link
Contributor

@atomb atomb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, though I think real descriptions for the new (and old!) options in crucible-jvm might be worthwhile. I hope that once #635 is merged, and this is updated to include that, the CI problems will go away.

crucible-jvm/tool/Main.hs Outdated Show resolved Hide resolved
crucible-jvm/tool/Main.hs Outdated Show resolved Hide resolved
This mirrors a similar recent change introduced to SAW in
GaloisInc/saw-script#1030.

While I was in town, I updated the two `crucible-jvm`–related test suites
to use the `PATH` instead of the `-j` flag. I can't exactly confirm via CI
if these changes work (see #634), but they appear to do the right thing
locally.

Fixes #633.
@RyanGlScott RyanGlScott merged commit 950a629 into GaloisInc:master Feb 3, 2021
@RyanGlScott RyanGlScott deleted the T633 branch February 3, 2021 18:25
RyanGlScott added a commit to GaloisInc/saw-script that referenced this pull request Feb 8, 2021
This bumps the `crucible` submodule to include GaloisInc/crucible#638, which
adds basic support for handling JDK 9 or later. JDK 9+ packages its standard
library not in a JAR file, but in a JIMAGE file. For more details on how
`crucible-jvm` handles JIMAGE files, refer to
`Note [Loading classes from JIMAGE files]` in `Lang.JVM.Codebase`.

This fixes #861, although there are still unsolved issues that arise when
using modern JDKs with certain classes, such as `String`. As a result, I have
decided to label support for JDK 9+ as experimental:

* I have updated the SAW documentation to mention these shortcomings.
* I have opened GaloisInc/crucible#641 to track the remaining issues.

Other things:

* GaloisInc/crucible#636 and GaloisInc/crucible#638 upstreamed the code from
  `SAWScript.JavaTools` and `SAWScript.ProcessUtils` into `crucible-jvm`, so
  we can remove these modules in favor of importing `Lang.JVM.JavaTools` and
  `Lang.JVM.ProcessUtils` from `crucible-jvm`.

* I removed the dependency on the `xdg-basedir`, as it was unused. This
  dependency was likely added quite some time ago, and it appears that
  `saw-script` switched over to using XDG-related functionality from the
  `directory` library since then. I opted to use `directory` to find the
  `.cache` directory as well, so I have made that clear in the `.cabal` file.

* The `biJavaCodebase :: Codebase` field of `BuiltinContext` is completely
  unused, which I noticed when making changes to the `Codebase` type. Let's
  just remove it.
RyanGlScott added a commit to GaloisInc/saw-script that referenced this pull request Feb 11, 2021
This bumps the `crucible` submodule to include GaloisInc/crucible#638, which
adds basic support for handling JDK 9 or later. JDK 9+ packages its standard
library not in a JAR file, but in a JIMAGE file. For more details on how
`crucible-jvm` handles JIMAGE files, refer to
`Note [Loading classes from JIMAGE files]` in `Lang.JVM.Codebase`.

This fixes #861, although there are still unsolved issues that arise when
using modern JDKs with certain classes, such as `String`. As a result, I have
decided to label support for JDK 9+ as experimental:

* I have updated the SAW documentation to mention these shortcomings.
* I have opened GaloisInc/crucible#641 to track the remaining issues.

Other things:

* GaloisInc/crucible#636 and GaloisInc/crucible#638 upstreamed the code from
  `SAWScript.JavaTools` and `SAWScript.ProcessUtils` into `crucible-jvm`, so
  we can remove these modules in favor of importing `Lang.JVM.JavaTools` and
  `Lang.JVM.ProcessUtils` from `crucible-jvm`.

* I removed the dependency on the `xdg-basedir`, as it was unused. This
  dependency was likely added quite some time ago, and it appears that
  `saw-script` switched over to using XDG-related functionality from the
  `directory` library since then. I opted to use `directory` to find the
  `.cache` directory as well, so I have made that clear in the `.cabal` file.

* The `biJavaCodebase :: Codebase` field of `BuiltinContext` is completely
  unused, which I noticed when making changes to the `Codebase` type. Let's
  just remove it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

crucible-jvm: Allow specifying Java's location with --java-bin-dirs/PATH
2 participants