bin/elasticsearch: add help, fix endless loop #8729
Conversation
This change adds command line help for all options to the es start script. Both '-h' and '--help' options are accepted. Also, an endless busy loop in the long options parser was fixed: running the script with a long opt parameter w/o value (e.g. "elasticsearch --buuuurrrnn") the long option parser would end up in an endless busy loop. Signed-off-by: Thilo Fromm <github@thilo-fromm.de>
clintongormley
added
review
:Delivery/Packaging
|
LGTM - @spinscale can you take another look please? |
|
I think we should have the same help option for elasticsearch.bat. |
| shift | ||
| continue | ||
| } | ||
| properties="$properties -Des.${1#--}=$2" | ||
| shift 2 |
There was a problem hiding this comment.
cant we just call shift ; shift (which will throw an error as expected if there are no more arguments) instead of shift 2 and the whole if-statement thing?
There was a problem hiding this comment.
shift; shift will silently fail (i.e. the second shift will when only one argument is present), no error will be reported. My rationale for the [ ] statement (and accompanying echo) was to mimic the error message reported by getopt (which is used to parse the short options). So you will get the same error message if you miss a mandatory value for both short and long options.
There was a problem hiding this comment.
more userfriendly is always better. Sounds good! Thx for the explanation
|
left one minor question, maybe we can do it a bit more simple here? Apart from that it looks good. |
|
@dadoonet Let me take a look at this. Time to de-dust my Windows VM I guess :) |
|
@dadoonet I just looked at |
|
I think but might be wrong that What happened if you run for example |
|
Ah, I see, you're right, command line options are passed directly to the Java runtime. The batch file itself however does not seem to take any arguments. Afaict running |
|
But the process exits, right? Still, to me this is not a big concern. As long as we don't document in docs the option |
It does - however, it's the Java process which does not like the command line argument, not the batch file. There's a significant difference between the Linux and Windows start scripts. The Linux script "understands" command line arguments, parses them, and generates a different set of arguments/options for the Elasticsearch Java binary. The Windows script otoh just passes options to the Java binary w/o looking at them. Since the Windows script does not "understand" what it passes to ES it wouldn't make much sense imho to implement command line help / documentation in the batch file. |
|
Agreed. Basically on windows, there are no command line options apart the +1 for merging it. Will do in a few minutes. |
This change adds command line help for all options to the es start script.
Both
-hand--helpoptions are accepted.Also, an endless busy loop in the long options parser was fixed: running the script with a long opt parameter w/o value (e.g.
elasticsearch --buuuurrrnn) the long option parser would end up in an endless busy loop.Fixes #2168
Fixes #7104