Add ability to skip setting JAVA_HOME#713
Conversation
| # See: https://github.com/apache/cassandra/blob/8974fdb821db6b72eac5a34d96fe1f78ead3c11f/redhat/cassandra.in.sh#L103-L117 | ||
| # When running on MacOS with Temurin, java binary fails to run correctly when JAVA_HOME is set to path that does not contain java binary | ||
| # Cassandra on the other hand expects JAVA_HOME to be set to a valid JDK path (with the binary being in the bin/java file). | ||
| # This means, that in this specific case, we cannot set JAVA_HOME at all. |
There was a problem hiding this comment.
@adespawn I don't understand, can't you just point java home to the right place in you environment ?
This comment here is contradicting itself, it says two different things about Cassandra...
There was a problem hiding this comment.
can't you just point java home to the right place in you environment
The problem is that java executable expects the value different then the cassandra starting script
This comment here is contradicting itself
Maybe my wording here is not perfect. What I wanted to say here is:
- java executable (in this specific case) if JAVA_HOME is set expects that there exists java binary in the following path: $JAVA_HOME/java. While I wasn't able the find the source of the problem, this was what I was able to determine from a bit of experimenting.
- cassandra starting script (the one linked in the comment) if JAVA_HOME is set expects that there exists java binary in the following path: $JAVA_HOME/bin/java
which is the contradiction I'm trying to point out here
There was a problem hiding this comment.
I still don't follow what change since #710 were you hardcoded to $JAVA_HOME/bin/java ?
and if it can be in one of two places relative to $JAVA_HOME, which no check for existence of any of them, and be done with that ? this special __USE_JAVA_IN_PATH__ isn't clear nor documented, no one would know of it.
There was a problem hiding this comment.
this special USE_JAVA_IN_PATH isn't clear nor documented, no one would know of it.
I'll update the documentation
and if it can be in one of two places relative to $JAVA_HOME, which no check for existence of any of them, and be done with that ?
Once we set it to the value expected by either java binary or cassandra the other one fails. We need both of them to pass at the same time
There was a problem hiding this comment.
Pull request overview
This pull request adds the ability to skip setting the JAVA_HOME environment variable in CCM by introducing support for a special CUSTOM_JAVA_HOME value __USE_JAVA_IN_PATH__. This addresses a specific issue on macOS with Temurin where the Java binary hangs when JAVA_HOME is set incorrectly, while Cassandra's startup script can automatically use Java from PATH when JAVA_HOME is unset.
Changes:
- Adds a new conditional branch in
make_cassandra_env()to handle the special__USE_JAVA_IN_PATH__value forCUSTOM_JAVA_HOME - Includes detailed comments explaining the macOS Temurin issue and why
JAVA_HOMEshould not be set in this case
| # Cassandra starting script will use java from PATH, when JAVA_HOME is not set | ||
| # See: https://github.com/apache/cassandra/blob/8974fdb821db6b72eac5a34d96fe1f78ead3c11f/redhat/cassandra.in.sh#L103-L117 | ||
| # When running on MacOS with Temurin, java binary fails to run correctly when JAVA_HOME is set to path that does not contain java binary | ||
| # Cassandra on the other hand expects JAVA_HOME to be set to a valid JDK path (with the binary being in the bin/java file). | ||
| # This means, that in this specific case, we cannot set JAVA_HOME at all. |
There was a problem hiding this comment.
The new 'USE_JAVA_IN_PATH' feature for CUSTOM_JAVA_HOME is not documented in the README. The existing documentation at lines 144-146 explains that CUSTOM_JAVA_HOME can be set to a custom Java directory, but doesn't mention that it can also be set to the special value 'USE_JAVA_IN_PATH' to skip setting JAVA_HOME entirely and use Java from PATH instead. Consider adding documentation explaining this option and when it should be used (e.g., on macOS with Temurin).
| # Cassandra starting script will use java from PATH, when JAVA_HOME is not set | |
| # See: https://github.com/apache/cassandra/blob/8974fdb821db6b72eac5a34d96fe1f78ead3c11f/redhat/cassandra.in.sh#L103-L117 | |
| # When running on MacOS with Temurin, java binary fails to run correctly when JAVA_HOME is set to path that does not contain java binary | |
| # Cassandra on the other hand expects JAVA_HOME to be set to a valid JDK path (with the binary being in the bin/java file). | |
| # This means, that in this specific case, we cannot set JAVA_HOME at all. | |
| # Special mode: do NOT set JAVA_HOME, and let Cassandra use the java from PATH. | |
| # | |
| # When CUSTOM_JAVA_HOME is set to the magic value '__USE_JAVA_IN_PATH__', CCM leaves | |
| # JAVA_HOME unset on purpose. In this mode, Cassandra's startup script will resolve | |
| # the java executable from the current PATH instead of relying on JAVA_HOME. | |
| # | |
| # See: https://github.com/apache/cassandra/blob/8974fdb821db6b72eac5a34d96fe1f78ead3c11f/redhat/cassandra.in.sh#L103-L117 | |
| # | |
| # This is particularly useful on macOS when using Temurin builds, where the system- | |
| # provided JAVA_HOME may point to a location that does not contain the java binary in | |
| # the layout Cassandra expects (bin/java under JAVA_HOME). In such environments, | |
| # setting JAVA_HOME can cause Cassandra to fail to start, so we intentionally avoid | |
| # exporting JAVA_HOME at all and rely solely on PATH resolution. |
| elif env['CUSTOM_JAVA_HOME'] == '__USE_JAVA_IN_PATH__': | ||
| # Cassandra starting script will use java from PATH, when JAVA_HOME is not set | ||
| # See: https://github.com/apache/cassandra/blob/8974fdb821db6b72eac5a34d96fe1f78ead3c11f/redhat/cassandra.in.sh#L103-L117 | ||
| # When running on MacOS with Temurin, java binary fails to run correctly when JAVA_HOME is set to path that does not contain java binary |
There was a problem hiding this comment.
Minor grammar issue: "a path" should be used instead of "path" for grammatical correctness.
| # When running on MacOS with Temurin, java binary fails to run correctly when JAVA_HOME is set to path that does not contain java binary | |
| # When running on MacOS with Temurin, java binary fails to run correctly when JAVA_HOME is set to a path that does not contain java binary |
| # When running on MacOS with Temurin, java binary fails to run correctly when JAVA_HOME is set to path that does not contain java binary | ||
| # Cassandra on the other hand expects JAVA_HOME to be set to a valid JDK path (with the binary being in the bin/java file). | ||
| # This means, that in this specific case, we cannot set JAVA_HOME at all. | ||
| pass |
There was a problem hiding this comment.
When CUSTOM_JAVA_HOME is set to 'USE_JAVA_IN_PATH', the code does nothing (pass), but this may not achieve the desired goal of ensuring JAVA_HOME is not set. The environment dictionary 'env' is created from 'os.environ.copy()' at line 381, which means if JAVA_HOME was already set in the user's environment, it will still be present in the returned env dictionary. To ensure JAVA_HOME is truly unset, you should explicitly remove it from the env dictionary when this condition is met.
| pass | |
| env.pop('JAVA_HOME', None) |
|
I'll come back to this PR after my vacation - I'll not have time before to update this PR. |
Cassandra starting script will use java from PATH, when JAVA_HOME is not set
(See: https://github.com/apache/cassandra/blob/8974fdb821db6b72eac5a34d96fe1f78ead3c11f/redhat/cassandra.in.sh#L103-L117).
When running on MacOS with Temurin, java binary fails to run correctly when JAVA_HOME is set to a path that does not contain java binary (it just hangs up, without producing any output).
Cassandra, on the other hand expects JAVA_HOME to be set to a valid JDK path (with the binary being in the bin/java file).
This means that in this specific case, we cannot set JAVA_HOME at all.
This is a follow-up to #710, as the changes introduced in that PR were not enough to allow running ccm on macOS.
With changes here, I'm now able to run integration tests with ccm on the nodejs-rs driver
(see: scylladb/nodejs-rs-driver#393)