SOLR-16824: Adopt Linux Command line tool pattern of -- for long option commands

[epugh]

https://issues.apache.org/jira/browse/SOLR-16824

Description

This moves us towards using --help style instead of the -h and -help mishmash that we have. This also lines up with how Commons CLI wants to work as well.

Solution

Update the various OptionBuilders, update the bats tests, and the Ref Guide.

Tests

Running bats

Checklist

Please review the following and check all that apply:

• [x] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• [x] I have created a Jira issue and added the issue ID to my pull request title.
• [x] I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
• [x] I have developed this patch against the main branch.
• [x] I have run ./gradlew check.
• [x] I have added tests for my changes.
• [x] I have added documentation for the Reference Guide

[epugh]

@mkhludnev before I commit this to main only, are you able to give this a test on Windows?

[mkhludnev]

Hi, @epugh. I started to explore this branch. Here we go:

\dev-slim>bin\solr
...
Pass -help after any COMMAND to see command-specific usage information,
  such as:    solr start -help or solr stop -help

I suppose we should have --help option here ^.

But the first option I tried, stumbled me.

dev-slim>bin\solr start -h

ERROR: Hostname is required

Usage: solr start ....

I dont' think we have a chance to resolve the clash between host and help.

\dev-slim>bin\solr start --help

Invalid command-line option: --help

Usage: solr start ...

dev-slim>bin\solr start -help

Usage: solr start [-f] ...

I seems like one dash is working.

[epugh]

I think this branch has definitly had some bit rot, and there is a lot of work to update it.... Thanks for looking @mkhludnev at this branch, glad to get some input on this!

[github-actions[bot]]

This PR had no visible activity in the past 60 days, labeling it as stale. Any new activity will remove the stale label. To attract more reviewers, please tag someone or notify the [email protected] mailing list. Thank you for your contribution!

[epugh]

I updated this PR (thanks StaleBot for bugging me) from Main, and I think have to redo some changes...

[epugh]

@mkhludnev could you take a look again now that I've updated the PR and pushed up a fix for the host and help discrepancies?

Also, if anyone has windows experience, I think the method "set_host" may need tweaking now that host has a -- instead of a - seperator...

[epugh]

Before calling this done, make sure to check out https://github.com/apache/solr/pull/1768/commits/395eaab7199f5f531f6a4121f4e946456ff5c5ad and make sure all the reverted changes (when bringing this PR up to date) are reapplied!

[epugh]

Pending a good run off all the tests, I think I'm done and would love some review. This is to go into Solr X and not be backported to Solr 9...

[epugh]

Thanks @janhoy for starting to look. And yeah, this is 10x only change. Trying to manage two sets of old and new parameters I think would just make this impossible ;-). I'll leave this open for a few more days to give folks time to look at it more.

[epugh]

Is it possible to back port the non-conflicting ones to 9.x? And also log deprecation warnings on anything that's going away?

I was sort of hoping not too... I guess that I could backport places where we ADD something, like a new --long-option. My thought was to document in the upgrade notes more about the changes, but not try to navigate the changes to the shell and command scirpts and the java code...

What's your temperature on this? Worth discussing at the community meetup and getting more input?

[madrob]

My ideal world is everywhere that we remained -x to -y we add -y to 9.x and log a warning on usage of -x. Similar to what gradle does.

But I can appreciate that that’s a ton of work, so maybe we can do that with just the common ones? zkHost at least?

[epugh]

My ideal world is everywhere that we remained -x to -y we add -y to 9.x and log a warning on usage of -x. Similar to what gradle does.

But I can appreciate that that’s a ton of work, so maybe we can do that with just the common ones? zkHost at least?

I will reach out to the commons-cli community see if there are any suggestions. I don't know if we can -z and -zkHost as short parameters for the same option. And if we just start logging in 9.x "Heads up -zkHost will be --zk-host in 10x, but you can't use it today", then that might be annonying.

Once on 10, if you attempt to use -zkHost from muscle memory, you will get an error immediately until you swap to -z or --zk-host.

[janhoy]

I have been thinking about adding a deprecation-feature to commons-cli, such that you could construct an Option:

Option o = Option.builder("n").longOpt("new-long").deprecatedOpt("oldShort").deprecatedLongOpt("oldLong")
...
String myVal = cli.getOptionValue("n")

It would work so that if -n or --new-long are not provided, it will fall back to checking -oldShort and --oldLong, and print a deprecation warning if using deprecated options. Perhaps we could help implement it and use the new version before the 10.0 release..

[epugh]

One reason I've been feeling a bit daunted on the idea of enhancing commons-cli to support deprecation is because it would mean that we would need to keep/extend the lines of code in bin/solr and bin/solr.cmd that deal with parameter parsing... Although now that I look at it, maybe if we move along more of the parameter passing INTO the java code, then the impact would be less.

[epugh]

I have been thinking about adding a deprecation-feature to commons-cli, such that you could construct an Option:

Option o = Option.builder("n").longOpt("new-long").deprecatedOpt("oldShort").deprecatedLongOpt("oldLong")
...
String myVal = cli.getOptionValue("n")

It would work so that if -n or --new-long are not provided, it will fall back to checking -oldShort and --oldLong, and print a deprecation warning if using deprecated options. Perhaps we could help implement it and use the new version before the 10.0 release..

I have something cooking along these lines! It actually helped me find places where I was using solrUrl instead of solr-url as the parameter name in the CLI!

[epugh]

I have been thinking about adding a deprecation-feature to commons-cli, such that you could construct an Option:

Option o = Option.builder("n").longOpt("new-long").deprecatedOpt("oldShort").deprecatedLongOpt("oldLong")
...
String myVal = cli.getOptionValue("n")

It would work so that if -n or --new-long are not provided, it will fall back to checking -oldShort and --oldLong, and print a deprecation warning if using deprecated options. Perhaps we could help implement it and use the new version before the 10.0 release..

I have something cooking along these lines! It actually helped me find places where I was using solrUrl instead of solr-url as the parameter name in the CLI!

https://issues.apache.org/jira/browse/CLI-329

[epugh]

What do folks think about moving forward with this as is, and then adding the deprecations when commons-cli comes out....

[epugh]

https://github.com/apache/commons-cli/pull/252 replaced what I proposed and was merged. Now waiting on a release!

[epugh]

Waiting for the upcoming commons cli 1.8.0 to come out with nicer handling of deprecated options.

[janhoy]

1.8.0 is out, will merge the renovate PR https://github.com/apache/solr/pull/2481

[epugh]

@janhoy I ran a bit out of steam while waiting for 1.8.0, please do feel free to continue to push this PR along ;-).

[janhoy]

I had a thought now that bin/solr supports a remote Solr cluster with --solr-url. We currently support solr version which prints the version from SolrVersion.java, i.e. the client version. However, if you run the tool towards a remote solr with a different version, you won't see that.

So I wonder if solr --version should be the tool version, but solr version --solr-url http://foo.bar/solr/ should call the backend to ask for its version.

Probably scope creep for this PR though.

[epugh]

I had a thought now that bin/solr supports a remote Solr cluster with --solr-url. We currently support solr version which prints the version from SolrVersion.java, i.e. the client version. However, if you run the tool towards a remote solr with a different version, you won't see that.

So I wonder if solr --version should be the tool version, but solr version --solr-url http://foo.bar/solr/ should call the backend to ask for its version.

Probably scope creep for this PR though.

i love it... https://issues.apache.org/jira/browse/SOLR-17318

[janhoy]

Did a few low hanging ones

[epugh]

thanks for tackling those.. I would love to get more of the string parsing out of the scripts for solr.. I went back and looked at them, and I think that for the various commands not related to "start", "stop", and "status", I can actually slim down a lot what is in the scripts. However, looking at "start", "stop", and "status" they have a lot of environment specific logic that I don't feel confident porting over to Java code ;-).

[epugh]

Hey all.. so, just got all green on the checks! I'm wondering, how many of the parameters do we want to have deprecation for... We have it for solr url and zk host, and they are pretty common. Do we want to add it for all the other parameters? I'm on the fence about it, because each one requires additional code, and they are each relatively hard to test...

[epugh]

chatted with @gerlowskija and the plan is to mark all changed features as Deprecated, not just the solrurl and zkhost, for 9.x, so your scripts don't break in 9.6 to 9.7, but to remove the deprecated option support in 10x....

[epugh]

Okay! I believe that deprecated options are back in, so scripts shouldn't break between 9.6 and 9.7.... I would love another set of eyes and then look to merge to main in the next few days.

[epugh]

@HoustonPutman any tips on how to validate that changes on this PR don't impact solr operator?

[janhoy]

Great review Jason. @epugh do let me know if you need some assistance in implementing the changes.