-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Link to SYSTEM_JAVA_HOME on windows #40806
Conversation
We don't always have java home defined in packaging tests, as we want to use the bundled jdk most of the time. This commit fixes the java home with special characters test to link to SYSTEM_JAVA_HOME on windows. closes elastic#40797
Pinging @elastic/es-core-infra |
Note I only put 8.0 on here as it looks like the backports of #39712 have not yet been merged (and should incorporate this fix once it is in). |
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.
Probably worthy to run windows tests from command line as they might not be include in package sample.
If the fix in cleanup works then this lgtm
@@ -218,7 +218,7 @@ public void test53JavaHomeWithSpecialCharacters() throws Exception { | |||
|
|||
} finally { | |||
//clean up sym link | |||
sh.run("cmd /c del /F /Q 'C:\\Program Files (x86)\\java' "); | |||
sh.run("cmd /c rmdir 'C:\\Program Files (x86)\\java' "); |
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.
Hopefully it will pass, but I think the problem is the path with spaces (apostrophes didn't help) . Note that del command should pass when run on windows.
If it still fails I would use fileutils.rm
The packaging label triggers a full run of packaging tests, which include windows VMs. I will double check the CI output to ensure windows was run. |
We don't always have java home defined in packaging tests, as we want to use the bundled jdk most of the time. This commit fixes the java home with special characters test to link to SYSTEM_JAVA_HOME on windows. closes elastic#40797
We don't always have java home defined in packaging tests, as we want to use the bundled jdk most of the time. This commit fixes the java home with special characters test to link to SYSTEM_JAVA_HOME on windows. closes elastic#40797
We don't always have java home defined in packaging tests, as we want to use the bundled jdk most of the time. This commit fixes the java home with special characters test to link to SYSTEM_JAVA_HOME on windows. closes elastic#40797
…40768) the elasticsearch.bat and elasticsearch-env.bat won't work if JAVA contains parentheses. This seems to be the limitation of FOR /F IN (command) DO syntax. The JAVA variable present in a command contains a path to a binary to start elasticsearch (with spaces & parens). We can workaround the problem of spaces and parentheses in this path by referring this variable with a CALL command. Note that executing binaries with CALL is an undocumented behaviour (but works) closes #38578 closes #38624 closes #33405 closes #30606 backports: * Bat scripts to work with JAVA_HOME with parentheses(#39712) * Link to SYSTEM_JAVA_HOME on windows (#40806)
We don't always have java home defined in packaging tests, as we want to use the bundled jdk most of the time. This commit fixes the java home with special characters test to link to SYSTEM_JAVA_HOME on windows. closes elastic#40797
We don't always have java home defined in packaging tests, as we want to
use the bundled jdk most of the time. This commit fixes the java home
with special characters test to link to SYSTEM_JAVA_HOME on windows.
closes #40797