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

Link to SYSTEM_JAVA_HOME on windows #40806

Merged
merged 2 commits into from
Apr 4, 2019
Merged

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Apr 3, 2019

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

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
@rjernst rjernst added >test Issues or PRs that are addressing/adding tests :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts v8.0.0 labels Apr 3, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@rjernst rjernst requested a review from pgomulka April 3, 2019 17:51
@rjernst
Copy link
Member Author

rjernst commented Apr 3, 2019

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).

Copy link
Contributor

@pgomulka pgomulka left a 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' ");
Copy link
Contributor

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

@rjernst
Copy link
Member Author

rjernst commented Apr 3, 2019

run windows tests from command line as they might not be include in package sample.

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.

@rjernst rjernst merged commit 153c1eb into elastic:master Apr 4, 2019
@rjernst rjernst deleted the distro_tests5 branch April 4, 2019 06:01
pgomulka pushed a commit to pgomulka/elasticsearch that referenced this pull request Apr 4, 2019
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
pgomulka pushed a commit to pgomulka/elasticsearch that referenced this pull request Apr 4, 2019
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
pgomulka pushed a commit to pgomulka/elasticsearch that referenced this pull request Apr 4, 2019
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
pgomulka added a commit that referenced this pull request Apr 4, 2019
backports :
Bat scripts to work with JAVA_HOME with parentheses (#39712)
Link to SYSTEM_JAVA_HOME on windows (#40806)
pgomulka added a commit that referenced this pull request Apr 4, 2019
backports :
Migrate systemd packaging tests from bats to java (#39954)
Bat scripts to work with JAVA_HOME with parentheses (#39712)
Link to SYSTEM_JAVA_HOME on windows (#40806)
pgomulka added a commit that referenced this pull request Apr 5, 2019
…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)
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
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
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Delivery Meta label for Delivery team >test Issues or PRs that are addressing/adding tests v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate and fix test53JavaHomeWithSpecialCharacters failures
5 participants