Source additional files correctly in elasticsearch-cli #40890
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Since we only source additional sources from the same dir as our cli scripts, resolve the path relevant to $ES_HOME Resolves: elastic#40889
jkakavas
added
>bug
:Delivery/Packaging
|
Pinging @elastic/es-core-infra |
jkakavas
changed the title
jasontedor
reviewed
Apr 5, 2019
| @@ -7,7 +7,7 @@ source "`dirname "$0"`"/elasticsearch-env | |||
| IFS=';' read -r -a additional_sources <<< "$ES_ADDITIONAL_SOURCES" | |||
| for additional_source in "${additional_sources[@]}" | |||
| do | |||
| source "`dirname "$0"`"/$additional_source | |||
| source $ES_HOME/bin/$additional_source | |||
There was a problem hiding this comment.
We probably need quotes here since ES_HOME could have spaces. Can you verify this, and quote if needed.
Also, does Windows have the same problem and need a fix too?
There was a problem hiding this comment.
Also, does Windows have the same problem and need a fix too?
we use:
if defined ES_ADDITIONAL_SOURCES (
for %%a in ("%ES_ADDITIONAL_SOURCES:;=","%") do (
call "%~dp0%%a"
)
)
%~dp0 will always expand to the full path where the batch file is located and not one relative to where the batch script is executed from, so it will not be affected by the cd that happens when we source elasticsearch-env.bat . That said, I'd have nothing against a manual confirmation, I don't have access to a windows machine right now :/
There was a problem hiding this comment.
That looks right to me, thanks for checking.
|
Can we add a packaging test for this? How about one that from not ES_HOME loops through each tool and tries to invoke help on them? That would fail before this change, and succeed after? |
|
@elasticmachine run elasticsearch-ci/2 ( we hit the issue that #40658 hopefully addresses) |
jasontedor
pushed a commit
that referenced
this pull request
Apr 5, 2019
Since we only source additional sources from the same dir as our cli scripts, resolve the path relevant to $ES_HOME
jasontedor
pushed a commit
that referenced
this pull request
Apr 5, 2019
Since we only source additional sources from the same dir as our cli scripts, resolve the path relevant to $ES_HOME
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
Apr 6, 2019
* master: (63 commits) Suppress lease background sync failures if stopping (elastic#40902) [DOCS] Added settings page for ILM. (elastic#40880) [Docs] Remove extraneous text (elastic#40914) Move test classes to test root in Painless (elastic#40873) Fix date index name processor default date_formats (elastic#40915) Source additional files correctly in elasticsearch-cli (elastic#40890) Allow AVX-512 on JDK 11+ (elastic#40828) [Docs] Change example to show col headers (elastic#40822) Update apache httpclient to version 4.5.8 (elastic#40875) Update monitoring-kibana.json (elastic#40899) Introduce Delegating ActionListener Wrappers (elastic#40129) Deprecate old transport settings (elastic#40821) Add Kibana application privileges for monitoring and ml reserved roles (elastic#40651) Use Writeable for TransportReplAction derivatives (elastic#40894) Add test for HTTP and Transport TLS on basic license (elastic#40714) Remove unneded cluster config from test (elastic#40856) Make Fuzziness reject illegal values earlier (elastic#33511) Remove test-only customisation from TransReplAct (elastic#40863) Fix dense/sparse vector limit documentation (elastic#40852) Make -try xlint warning disabled by default. (elastic#40833) ...
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
Apr 6, 2019
* master: (77 commits) Suppress lease background sync failures if stopping (elastic#40902) [DOCS] Added settings page for ILM. (elastic#40880) [Docs] Remove extraneous text (elastic#40914) Move test classes to test root in Painless (elastic#40873) Fix date index name processor default date_formats (elastic#40915) Source additional files correctly in elasticsearch-cli (elastic#40890) Allow AVX-512 on JDK 11+ (elastic#40828) [Docs] Change example to show col headers (elastic#40822) Update apache httpclient to version 4.5.8 (elastic#40875) Update monitoring-kibana.json (elastic#40899) Introduce Delegating ActionListener Wrappers (elastic#40129) Deprecate old transport settings (elastic#40821) Add Kibana application privileges for monitoring and ml reserved roles (elastic#40651) Use Writeable for TransportReplAction derivatives (elastic#40894) Add test for HTTP and Transport TLS on basic license (elastic#40714) Remove unneded cluster config from test (elastic#40856) Make Fuzziness reject illegal values earlier (elastic#33511) Remove test-only customisation from TransReplAct (elastic#40863) Fix dense/sparse vector limit documentation (elastic#40852) Make -try xlint warning disabled by default. (elastic#40833) ...
gurkankaymak
pushed a commit
to gurkankaymak/elasticsearch
that referenced
this pull request
May 27, 2019
Since we only source additional sources from the same dir as our cli scripts, resolve the path relevant to $ES_HOME
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since we only source additional sources from the same dir as our
cli scripts, resolve the path relevant to $ES_HOME
Resolves: #40889