-
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
Configure TMP for test nodes on Windows #39959
Configure TMP for test nodes on Windows #39959
Conversation
This breaks on windows where TMP dir default to C:\Windows and startup fails with a permission error. I tried to create a tmp dir and pass in `TMP` env, but it lead to a class not found error, and since testclusers is already independent of the calling environment I stopped there.
The new environment is necessary; otherwise we pass through JAVA_HOME and always use that, even when we want to use the bundled jdk. I think we can solve this by passing through whatever env vars windows needs? |
Fix incorrect argument for paths with spaces
Thanks @rjernst I was too quick on that one. I added |
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, one optional suggestion
// Since we configure ant to run with a new environment above, we need to set this to a dir we have access to | ||
File tmpDir = new File(node.baseDir, "tmp") | ||
tmpDir.mkdirs() | ||
env(key: "TMP", value: tmpDir.absolutePath) |
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.
shouldn't we just pass through TMP? this would match the previous behavior where the environment was inherited
This breaks on windows where TMP dir default to C:\Windows and startup fails with a permission error. I tried to create a tmp dir and pass in `TMP` env, but it lead to a class not found error, and since testclusers is already independent of the calling environment I stopped there.
This breaks on windows where TMP dir default to C:\Windows and startup fails with a permission error. I tried to create a tmp dir and pass in `TMP` env, but it lead to a class not found error, and since testclusers is already independent of the calling environment I stopped there.
This breaks on windows where TMP dir default to C:\Windows and startup fails with a permission error. I tried to create a tmp dir and pass in `TMP` env, but it lead to a class not found error, and since testclusers is already independent of the calling environment I stopped there.
After elasticsearch change, ${ES_TMPDIR} needs quotes around it. Relates elastic/elasticsearch#39959
After elasticsearch change, ${ES_TMPDIR} needs quotes around it. Relates elastic/elasticsearch#39959
This reverts commit 97562a8.
On windows TMP dir default to C:\Windows and startup fails with a permission error.
The reason we don't see this more is that typically windows has TMP defined in users environment, but with
newenvironment: true
this is not passed along.Closes #39904.