Skip to content

SOLR-18512: Port ConfigTool to picocli - #5039

Open
serhiy-bzhezytskyy wants to merge 5 commits into
apache:mainfrom
serhiy-bzhezytskyy:SOLR-18512-picocli-config
Open

serhiy-bzhezytskyy wants to merge 5 commits into
apache:mainfrom
serhiy-bzhezytskyy:SOLR-18512-picocli-config

Conversation

@serhiy-bzhezytskyy

Copy link
Copy Markdown
Contributor

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

Description

Adds the picocli code path for bin/solr config, next to the commons-cli one.

Solution

ConfigTool gets its annotated options and callTool() and is registered in SolrCLI; the commons-cli path is unchanged. --value has no short -v here, since picocli does not allow -v to be both --verbose and --value. Written with Claude Code.

Tests

The tool had no tests. ConfigToolTest sets and unsets a property through the Config API and checks the overlay, and ConfigToolPicocliTest runs it through picocli. check -x test is clean (rat skipped).

Checklist

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide
  • I have added a changelog entry for my change

Adds the picocli code path next to the commons-cli one, with the first tests for the tool (run under both parsers) and the generated reference page.
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests cat:cli labels Oct 6, 2026
picocli now validates set-property, unset-property, set-user-property and unset-user-property itself, so an unknown action is a usage error instead of a Config API failure. The commons-cli path is unchanged, and the tests now also cover the user-property actions.
@@ -0,0 +1,9 @@
# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc

title: The `config` command is now available in the experimental picocli command line interface.

@janhoy janhoy Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generic changelog comment, see other PR (add author/link to the main picocli yml file)

// Long-only: "-v" is ToolBase's --verbose, and picocli rejects a duplicate short name. Under
// commons-cli the later-added VALUE_OPTION wins, so there "-v" still means --value.
@picocli.CommandLine.Option(
names = "--value",

@janhoy janhoy Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The functional change from commons-cli regarding -v now not being supported perhaps justifies a small note in major-changes-in-solr-10.adoc as a breaking change. But that is perhaps better filed in the Solr version where we flip SOLR_PICOCLI to default to true?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added to the Known Limitations on cli/index.adoc; the upgrade note can come with the release that flips SOLR_PICOCLI.

bin/solr config -c x --property p -v 10 works with commons-cli, where -v is --value, but picocli keeps -v for --verbose, so the changelog entry now names the difference.
No picocli feature has been released yet, so a new command is a detail of the SOLR-17697 entry. Its author and JIRA are added there instead of a separate entry.
The shared changelog entry does not carry per-command differences, so the picocli page that lists them says that config's --value has no -v short form.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cat:cli documentation Improvements or additions to documentation tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants