Skip to content

Conversation

@jasontedor
Copy link
Member

@jasontedor jasontedor commented Dec 13, 2016

A previous fix for the handling of paths on Windows related to paths
containing multiple spaces introduced a issue where if JAVA_HOME ends
with a backslash, then Elasticsearch will refuse to start. This is not a
critical bug as a workaround exists (remove the trailing backslash), but
should be fixed nevertheless. This commit addresses this situation while
not regressing the previous fix.

Relates #21921

@jasontedor jasontedor added :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >bug review v5.0.3 v5.1.2 v5.2.0 v6.0.0-alpha1 labels Dec 13, 2016
jasontedor referenced this pull request Dec 13, 2016
This commit fixes the handling of spaces in Windows paths. The current
mechanism works fine in a path that contains a single space, but fails
on a path that contains multiple spaces. With this commit, that is no
longer the case.

Relates #21921
A previous fix for the handling of paths on Windows related to paths
containing multiple spaces introduced a issue where if JAVA_HOME ends
with a backslash, then Elasticsearch will refuse to start. This is not a
critical bug as a workaround exists (remove the trailing backslash), but
should be fixed nevertheless. This commit addresses this situation while
not regressing the previous fix.
@jasontedor jasontedor force-pushed the oh-windows-how-do-i-love-thee-let-me-count-the-ways-answer-zero branch from 3c071fb to 647ebb1 Compare December 13, 2016 12:24
@yannlebel
Copy link

When I read this it seems the issue is not closed and included in a release package but when I download the elasticsearch zip from here https://www.elastic.co/downloads/elasticsearch it's already included. Did something went wrong?

This fix in not working for me. I tested the version 5.1.1 on windows 7 (x64) and windows server 2012 R2 (x64) standard and had the same issue every time.

The quote around the %JAVA_HOME% and %JAVA% variables are not placed correctly which prevent the scripts to start the node and to detect the JVM version which always default the service installation to the x86 version.

My JAVA_HOME variable is set with the value: C:\Program Files\Java\jdk1.8.0_111
My installation folder for elasticsearch is: D:\ELK\elasticsearch\

######Result of the elasticsearch.bat execution before the fix:
image

######Result of the elasticsearch-service.bat execution before the fix:
image

To fix the problem I had to do the following changes:

  • In elastisearch.bat
    • Line 82
      • From: %JAVA% %ES_JAVA_OPTS% %ES_PARAMS% -cp "%ES_CLASSPATH%" "org.elasticsearch.bootstrap.Elasticsearch" !newparams!
      • To: "%JAVA%" %ES_JAVA_OPTS% %ES_PARAMS% -cp "%ES_CLASSPATH%" "org.elasticsearch.bootstrap.Elasticsearch" !newparams!
  • In elasticsearch.in.bat
    • Line 4
      • From: set JAVA="%JAVA_HOME%"\bin\java.exe
      • To: set JAVA=%JAVA_HOME%\bin\java.exe
    • Line 8
      • From: IF NOT EXIST %JAVA% (
      • To: IF NOT EXIST "%JAVA%" (
  • In Elasticsearch-service.bat
    • Line 32
      • From: SET JAVA="%JAVA_HOME%"\bin\java.exe
      • To: SET JAVA=%JAVA_HOME%\bin\java.exe
    • Line 36
      • From: IF NOT EXIST %JAVA% (
      • To: IF NOT EXIST "%JAVA%" (
    • Line 55
      • From: %JAVA% -Xmx50M -version > nul 2>&1
      • To: "%JAVA%" -Xmx50M -version > nul 2>&1
    • Line 62
      • From: %JAVA% -Xmx50M -version 2>&1 | "%windir%\System32\find" "64-Bit" >nul:
      • To: "%JAVA%" -Xmx50M -version 2>&1 | "%windir%\System32\find" "64-Bit" >nul:
    • Line 149
      • From: if exist "%JAVA_HOME%"\jre\bin\server\jvm.dll (
      • To: if exist "%JAVA_HOME%\jre\bin\server\jvm.dll" (
    • Line 155
      • From: if exist "%JAVA_HOME%"\bin\server\jvm.dll (
      • To: if exist "%JAVA_HOME%\bin\server\jvm.dll" (
    • Line 161
      • From: if exist "%JAVA_HOME%"\bin\client\jvm.dll (
      • To: if exist "%JAVA_HOME%\bin\client\jvm.dll" (

Here are the files with the fixes:

elasticsearch-service.bat.txt
elasticsearch.bat.txt
elasticsearch.in.bat.txt

Hope it helps.

@jasontedor
Copy link
Member Author

This PR will address that.

@yannlebel
Copy link

yes sorry like I answer on the other PR I miss read the commits all is fine. Thanks again and keep the good work I can't stop loving elasticsearch :)

@jasontedor
Copy link
Member Author

jasontedor commented Dec 14, 2016

sorry

It's me that should be sorry for introducing one bug while fixing another. I am sorry about that.

Thanks again and keep the good work I can't stop loving elasticsearch :)

Music to my ears; you're welcome.

@jasontedor jasontedor removed the v5.0.3 label Dec 16, 2016
@jasontedor jasontedor requested a review from rjernst December 20, 2016 23:58
@rjernst
Copy link
Member

rjernst commented Dec 21, 2016

Windows escaping is so weird. :) LGTM

@jasontedor jasontedor merged commit 5e68b63 into elastic:master Dec 21, 2016
jasontedor added a commit that referenced this pull request Dec 21, 2016
A previous fix for the handling of paths on Windows related to paths
containing multiple spaces introduced a issue where if JAVA_HOME ends
with a backslash, then Elasticsearch will refuse to start. This is not a
critical bug as a workaround exists (remove the trailing backslash), but
should be fixed nevertheless. This commit addresses this situation while
not regressing the previous fix.

Relates #22132
jasontedor added a commit that referenced this pull request Dec 21, 2016
A previous fix for the handling of paths on Windows related to paths
containing multiple spaces introduced a issue where if JAVA_HOME ends
with a backslash, then Elasticsearch will refuse to start. This is not a
critical bug as a workaround exists (remove the trailing backslash), but
should be fixed nevertheless. This commit addresses this situation while
not regressing the previous fix.

Relates #22132
@jasontedor
Copy link
Member Author

Thanks @rjernst.

@jasontedor jasontedor deleted the oh-windows-how-do-i-love-thee-let-me-count-the-ways-answer-zero branch December 21, 2016 02:10
@ChristianGruen
Copy link

Thanks for this fix! I solved the problem locally by rewriting…

set JAVA="%JAVA_HOME%"\bin\java.exe

…to…

set JAVA="%JAVA_HOME%\bin\java.exe"

…but @rjernst’s proposal works fine, and it includes the services batch script.

@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

>bug :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Delivery Meta label for Delivery team v5.1.2 v5.2.0 v6.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants