-
Notifications
You must be signed in to change notification settings - Fork 26
Add oieserver launcher script #119
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
base: main
Are you sure you want to change the base?
Add oieserver launcher script #119
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. I don't think I've been in the codebase long enough to "request changes", but i am suggesting some in the review comments.
This script is the main launcher for the Open Integration Engine (OIE) server. It prepares the Java environment and executes the server launcher JAR file. The script automatically finds a compatible Java runtime (version 17+ by default) by searching for a valid executable in the following priority order: 1. The OIE_JAVA_PATH environment variable. 2. The -java-cmd directive in the oieserver.vmoptions file or included .vmoptions files. Must specify the path to the 'java' executable. This is the preferred way to declare the desired version for running the server and can be overridden by OIE_JAVA_PATH. Can be a relative path from the location of this script. 3. The JAVA_HOME environment variable. 4. The 'java' command available in the system's PATH. It also parses the 'oieserver.vmoptions' file to configure JVM options, system properties (-D...), and classpath modifications. Signed-off-by: Tony Germano <tony@germano.name> Co-authored-by: Mitch Gaffigan <mitch.gaffigan@comcast.net> Issue: OpenIntegrationEngine#2
bc22c01
to
601fba6
Compare
Issue: OpenIntegrationEngine#2 Pull-request: OpenIntegrationEngine#119 Signed-off-by: Tony Germano <tony@germano.name>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to fix the java check to handle 1.8.x (for a better error message)
It won't change the error message, because it only reports that the given path does not meet the minimum requirement, not which version, if any, was found. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on M2 with Java17.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can agree to disagree. I think the version should be echoed when there's a problem, but will approve anyway.
@pacmano1 the script reported this error when I first ran with JAVA_HOME set to 1.8:
Any other detail you would prefer in addition to this?. |
It's fine |
Signed-off-by: Tony Germano <tony@germano.name>
Signed-off-by: Tony Germano <tony@germano.name>
Signed-off-by: Tony Germano <tony@germano.name>
Signed-off-by: Tony Germano <tony@germano.name>
Signed-off-by: Tony Germano <tony@germano.name>
See mgaffigan@a6cd2ef for some suggested changes. Documentation edited to be available to Other suggestion would be to default based on the registry on windows. I don't know whether JAVA_HOME is set by most installers. $registryJavaHome = `
(Get-ItemProperty -LiteralPath 'HKLM:\SOFTWARE\JavaSoft\Java Development Kit' -ErrorAction SilentlyContinue -RegView Default).JavaHome -or `
(Get-ItemProperty -LiteralPath 'HKLM:\SOFTWARE\JavaSoft\Java Development Kit' -ErrorAction SilentlyContinue -RegView 32Bit).JavaHome -or `
(Get-ItemProperty -LiteralPath 'HKLM:\SOFTWARE\JavaSoft\Java Runtime Environment' -ErrorAction SilentlyContinue -RegView Default).JavaHome -or `
(Get-ItemProperty -LiteralPath 'HKLM:\SOFTWARE\JavaSoft\Java Runtime Environment' -ErrorAction SilentlyContinue -RegView 32Bit).JavaHome |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments and suggested edits commit in mgaffigan@a6cd2ef.
Most are nits or concision. Only significant issue I see in the powershell rev is that the args passed to java should be a splat (@ not $) - not an implicit array to string, since that will not handle quotes.
This matches what the bash script does, and allows the function to work when a path to a java command omits the `.exe`. Signed-off-by: Tony Germano <tony@germano.name>
In env expansion both versions will leave ${UNSET_VAR} untouched to match the behavior of install4j. Previously unset variables were converted to the empty string. incorporated several suggestions to make the powershell script more idiomatic.. Signed-off-by: Tony Germano <tony@germano.name>
This version of the script is a modified version of Mitch Gaffigan's script, which in turn was a bash port of my clojure app from #61.
This script is the main launcher for the Open Integration Engine (OIE) server. It prepares the Java environment and executes the server launcher JAR file.
The script automatically finds a compatible Java runtime (version 17+ by default) by searching for a valid executable in the following priority order:
It also parses the 'oieserver.vmoptions' file to configure JVM options, system properties (-D...), and classpath modifications.
Issue: #2