Allow specifying Java with --java-bin-dirs or PATH #1030
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RyanGlScott
commented
Jan 21, 2021
RyanGlScott
force-pushed
the
wip/T1022
branch
from
d8f6c25 to
41e3126
Compare
Closed
RyanGlScott
added a commit
that referenced
this pull request
Jan 27, 2021
This allows SAW to deal with JDK 9 or later, which packages its standard library not in a JAR file, but in a JIMAGE file. Extracting `.class` files from JIMAGE files proves to be surprisingly tricky, and I've carefully documented the intricacies of doing so in `Note [Loading classes from JIMAGE files]` in `SAWScript.JavaCodebase`. This fixes #861. This depends on #1030 to work. Remaining tasks: * Ideally, the code in `SAWScript.JavaCodebase` would be upstreamed to `crucible-jvm`, where the all-important `Codebase` data type lives. Unfortunately, some parts of SAW (e.g., `java_verify` still rely on the `jvm-verifier` library, which defines a separate `Codebase` type. SAW is in the process of phasing out the use of `jvm-verifier` in favor of `crucible-jvm` (see #993), but until that happens, I needed to introduce some ugly hacks in order to make everything typecheck. In particular, the (hopefully temporary) `SAWScript.JavaCodebase` module defines a shim version of `Codebase` that puts the experimental new things that I added in an `ExperimentalCodebase` constructor, but preserving the ability to use the `jvm-verifier` version of `Codebase` in the `LegacyCodebase` constructor. If JDK 8 or earlier is used, then `LegacyCodebase` is chosen, and if JDK 9 or later is used, then `ExperimentalCodebase` is chosen. * Unfortunately, `java_verify` doesn't work with `ExperimentalCodebase`. Nor would we necessarily want to make this happen, as that would require upstreaming changes to `jvm-verifier`, which we are in the process of phasing out. As a result, this is blocked on #993. * The CI should be updated to test more versions of the JDK than just 8. Other things: * 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.
Most of the action happens in: * The new `SAWScript.JavaTools` module, which exports functionality for locating a Java executable and finding its system properties. * `SAWScript.Options`, which now exposes a new `--java-bin-dirs` flag. (Alternatively, one can use the `PATH`.) The `processEnv` function now performs additional post-processing based on whether `--java-bin-dirs`/`PATH` are set. Fixes #1022, and paves the way to a better user experience for #861. Other things: * I ended up cargo-culting some `process`-related code from `SAWScript.Builtins` for use in `SAWScript.JavaTools`. To avoid blatant code duplication (and because I end up needing the exact same code later in another patch), I factored out this code into the new `SAWScript.ProcessUtils` module. I considered putting it in the existing `SAWScript.Utils` module, but that would have led to import cycles. Sigh.
RyanGlScott
commented
Jan 29, 2021
RyanGlScott
force-pushed
the
wip/T1022
branch
from
41e3126 to
685e8ae
Compare
|
I've rebased and resolved the TODOs. Barring any other review comments, this should be ready to go. |
atomb
approved these changes
Jan 29, 2021
|
I've added the |
This was referenced Feb 2, 2021
Closed
RyanGlScott
added a commit
to GaloisInc/crucible
that referenced
this pull request
Feb 3, 2021
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.
This was referenced Feb 5, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Most of the action happens in:
SAWScript.JavaToolsmodule, which exports functionality for locating a Java executable and finding its system properties.SAWScript.Options, which now exposes a new--java-bin-dirsflag. (Alternatively, one can use thePATH.) TheprocessEnvfunction now performs additional post-processing based on whether--java-bin-dirs/PATHare set.Fixes #1022, and paves the way to #861.