Skip to content

Fix handling of Windows paths containing parentheses#26916

Merged
jasontedor merged 2 commits intoelastic:masterfrom
olcbean:windows_path_parentheses
Oct 10, 2017
Merged

Fix handling of Windows paths containing parentheses#26916
jasontedor merged 2 commits intoelastic:masterfrom
olcbean:windows_path_parentheses

Conversation

@olcbean
Copy link
Copy Markdown
Contributor

@olcbean olcbean commented Oct 6, 2017

If the ES_HOME contains parentheses, the service cannot be installed.

Fixes #26454

@karmi
Copy link
Copy Markdown
Contributor

karmi commented Oct 6, 2017

Hi @olcbean, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

@elasticmachine
Copy link
Copy Markdown
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@olcbean olcbean force-pushed the windows_path_parentheses branch from ce8ed8f to 45ff1db Compare October 6, 2017 16:16
* master: (22 commits)
  Allow only a fixed-size receive predictor (elastic#26165)
  Add Homebrew instructions to getting started
  ingest: Fix bug that prevent date_index_name processor from accepting timestamps specified as a json number
  Scripting: Fix expressions to temporarily support filter scripts (elastic#26824)
  Docs: Add note to contributing docs warning against tool based refactoring (elastic#26936)
  Fix thread context handling of headers overriding (elastic#26068)
  SearchWhileCreatingIndexIT: remove usage of _only_nodes
  update Lucene version for 6.0-RC2 version
  Calculate and cache result when advanceExact is called (elastic#26920)
  Test query builder bwc against previous supported versions instead of just the current version.
  Set minimum_master_nodes on rolling-upgrade test (elastic#26911)
  Return List instead of an array from settings (elastic#26903)
  remove _primary and _replica shard preferences (elastic#26791)
  fixing typo in datehistogram-aggregation.asciidoc (elastic#26924)
  [API] Added the `terminate_after` parameter to the REST spec for "Count" API
  Setup debug logging for qa.full-cluster-restart
  Enable BWC testing against other remotes
  Use LF line endings in Painless generated files (elastic#26822)
  [DOCS] Added info about snapshotting your data before an upgrade.
  Add documentation about disabling `_field_names`. (elastic#26813)
  ...
Copy link
Copy Markdown
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jasontedor jasontedor merged commit c03f0c8 into elastic:master Oct 10, 2017
jasontedor pushed a commit that referenced this pull request Oct 10, 2017
This commit fixes an issue with the handling of paths containing
parentheses on Windows. When such a path is used as a component of
Elasticsearch home, then a later echo statement that is guarded by an if
will fail because the parentheses in the path will be confused with the
parentheses defining the if block. This commit fixes the issue by
protecting this echo statement by wrapping the possibly offending path
in quotes.

Relates #26916
jasontedor pushed a commit that referenced this pull request Oct 10, 2017
This commit fixes an issue with the handling of paths containing
parentheses on Windows. When such a path is used as a component of
Elasticsearch home, then a later echo statement that is guarded by an if
will fail because the parentheses in the path will be confused with the
parentheses defining the if block. This commit fixes the issue by
protecting this echo statement by wrapping the possibly offending path
in quotes.

Relates #26916
jasontedor pushed a commit that referenced this pull request Oct 10, 2017
This commit fixes an issue with the handling of paths containing
parentheses on Windows. When such a path is used as a component of
Elasticsearch home, then a later echo statement that is guarded by an if
will fail because the parentheses in the path will be confused with the
parentheses defining the if block. This commit fixes the issue by
protecting this echo statement by wrapping the possibly offending path
in quotes.

Relates #26916
@jasontedor jasontedor added :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >bug v5.6.4 v6.0.0 v6.1.0 v7.0.0 review labels Oct 10, 2017
@jasontedor
Copy link
Copy Markdown
Member

Thanks @olcbean.

@jasontedor jasontedor removed the review label Oct 10, 2017
@olcbean olcbean deleted the windows_path_parentheses branch October 10, 2017 21:44
@olcbean
Copy link
Copy Markdown
Contributor Author

olcbean commented Oct 14, 2017

@jasontedor thanks for merging this!

There is a tiny difference between 5.6 and 6.0+. I just verified that applying only this fix to 5.6 still results in an error when the service in installed (if the path contains parentheses). I opened another PR #27012 which is only applicable to 5.6 (on top of this one). Can you please have a look at it?

@lcawl lcawl added v6.0.0-rc2 and removed v6.0.0 labels Oct 30, 2017
@lcawl lcawl removed the v6.1.0 label Dec 12, 2017
@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.6.4 v6.0.0-rc2 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

elasticsearch-service.bat that generates an error with install

7 participants