SOLR-17697: Implement picocli for package command / PackageTool by jaykay12 · Pull Request #4739 · apache/solr

jaykay12 · 2026-08-15T10:45:18Z

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

Description

Implementing PicoCLI for the PackageTool

Solution

Used Prompt provided in the Jira & got initial migration done by Cursor.

Majoryly, common code which was being used in the commons-cli call path, which could be used in picocli as well, is being refactored to private functions.

#4739 (comment)

Tests

Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.

Checklist

Please review the following and check all that apply:

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

jaykay12 · 2026-08-15T13:18:45Z

This PR supports the PicoCLI migration: #3254

@epugh @janhoy please review once u have time.

Copilot

Pull request overview

Migrates PackageTool to picocli while retaining the legacy Commons CLI path.

Changes:

Adds picocli options, execution, and connection resolution.
Runs existing package tests through both CLI paths.
Registers and documents the package command.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`solr/core/src/java/org/apache/solr/cli/PackageTool.java`	Implements picocli support.
`solr/core/src/java/org/apache/solr/cli/SolrCLI.java`	Registers the package subcommand.
`solr/core/src/test/org/apache/solr/cli/PackageToolTest.java`	Generalizes tests across invocation paths.
`solr/core/src/test/org/apache/solr/cli/PackageToolPicocliTest.java`	Exercises the picocli path.
`solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-package.adoc`	Adds generated CLI documentation.
`solr/solr-ref-guide/modules/deployment-guide/deployment-nav.adoc`	Adds package documentation navigation.

Suppressed comments (1)

solr/core/src/java/org/apache/solr/cli/PackageTool.java:164

This drops the legacy -p spelling for package parameters. The existing package usage text still advertises -p (line 423), and the test comment identifies it as a deprecated value that must continue to work; picocli will reject it unless it is declared explicitly.

      names = {"--param"},

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

janhoy

This is a complex tool, congratulations on picking it 😉

Main comment is that we should use Picoclis sub commands to drive the command param/options parsing, help output and auto-generated docs. It's not a goal to preserve 1:1 help output from commons-cli.

I used Claude Code to help me find discrepancies and author these review comments, so bear over with it being detailed some places.

janhoy · 2026-08-22T10:05:02Z

+    }
+    String defaultUrl = CLIUtils.getDefaultSolrUrl();
+    CLIO.err(
+        "Neither --solr-connection, --zk-host or --solr-url parameters, nor SOLR_CONNECTION, ZK_HOST env var provided, so assuming solr url is "


This message promises more than the code delivers: the only fallback actually consulted is EnvUtils.getProperty("zkHost") (line above, and again in resolveZkHost). SOLR_CONNECTION / the solrConnection property is never read, so it works under commons-cli (via CLIUtils.resolveSolrConnectionFromCli) but is silently ignored on the picocli path — while this message claims it was checked.

Worth noting the underlying cause is a framework gap rather than something to solve per-tool: picocli doesn't apply the CliDefaultValueProvider to @ArgGroup members when the group is unmatched, which is exactly why connectionOptions can be null here. That's the still-unchecked "Solve value-fallback to ENV" milestone on #3254.

If you spin this into a separate PR, then add a TODO or NOCOMMIT comment with a link so we know there is a bug here that depends on some other PR.

Took help of cursor to understand this, still not fully sure of the implications here.

AI reply:

Fixed on the picocli path: when the ArgGroup is unmatched we read solr-connection / zkHost from env before the default URL, so the message matches behaviour. The ArgGroup default-provider issue remains #3254.

janhoy · 2026-09-12T14:05:00Z

Heads up that we're about to push a change to main that refactors all the Tools for easier picocli migration. This means eventually the feature branch will have a changed PackageTool which will conflict with this PR. But the changes are largely complimentary, so it will be resolvable.

jaykay12

self-reviewed ✅

@janhoy @epugh can review this PR again?

jaykay12 · 2026-10-03T11:42:00Z

AI Generated High Level changes:

flowchart TB
  subgraph user [User]
    CLI["bin/solr package …"]
  end

  CLI --> SOLRCLI[SolrCLI]
  SOLRCLI --> FLAG{picocli flag?}

  FLAG -->|no: commons-cli| PT_CLI[PackageTool.runImpl]
  PT_CLI --> EXEC[executePackage<br/>logger OFF + resolve via CLIUtils]
  EXEC --> SWITCH[handleCommand switch]
  SWITCH --> HELPERS

  FLAG -->|yes: picocli| PT_PICO[PackageTool parent]
  PT_PICO --> SUB{subcommand}

  SUB --> AddRepo
  SUB --> AddKey
  SUB --> ListInstalled
  SUB --> ListAvailable
  SUB --> ListDeployed
  SUB --> Install
  SUB --> Deploy
  SUB --> Undeploy
  SUB --> Uninstall
  SUB -.->|bare package| USAGE[print usage + notes a/b]

  AddRepo & AddKey & ListInstalled & ListAvailable & ListDeployed & Install & Deploy & Undeploy & Uninstall --> PSC[PackageSubCommand.runWithManagers]
  PSC --> RWM[PackageTool.runWithManagers<br/>logger OFF]
  RWM --> CONN[ConnectionOptions.resolveSolrUrl / resolveZkHost<br/>flags, then SOLR_CONNECTION / ZK_HOST]
  CONN --> HELPERS[shared helpers:<br/>addRepo, install, deploy, …]

  HELPERS --> PM[PackageManager / RepositoryManager]

flowchart LR
  subgraph before [Before this PR]
    B1["one PackageTool"]
    B2["cmd + ARGS catch-all"]
    B3["hand-written switch"]
    B1 --> B2 --> B3
  end

  subgraph after [After]
    A1["PackageTool parent"]
    A2["9 picocli leaves"]
    A3["shared helpers + ConnectionOptions"]
    A1 --> A2 --> A3
  end

  before --> after

Copilot

Copilot review overview

🟡 Changes recommended

Connection resolution can mix clusters, and multi-value --param invocations regress.

Review effort: Balanced
Findings: 1 · 1

Open (2)

Avoid mixing explicit Solr target with environment ZooKeeper · New
Restore multiple assignments per --param option · New

Resolved since last review (2)

The migrated command has no usage-help option, so bin/solr package --help is parsed as an unknown…
The generated example URL is invalid: the source annotation contains…

+    String solrConnectionProp = EnvUtils.getProperty("solr-connection");
+    if (solrConnectionProp != null && !solrConnectionProp.isBlank()) {
+      var connection = CloudSolrClient.CloudSolrClientConnection.parse(solrConnectionProp);
+      if (connection.isZookeeper()) {
+        return solrConnectionProp;
+      }
+    }
+
+    String zkHostProp = EnvUtils.getProperty("zkHost");
+    if (zkHostProp != null && !zkHostProp.isBlank()) {
+      return zkHostProp;
+    }


+  @picocli.CommandLine.Option(
+      names = {"-p", "--param"},
+      paramLabel = "PARAMS",
+      description = "List of parameters to be used with deploy command.")


janhoy · 2026-10-06T12:09:44Z

I had a look and there is still have lots of review comments to make, will try to address them in the coming days.

Meantime, I intend to merge #3254 to main, meaning this PR will automatically also target main branch. Otherwise things should be equal.

janhoy

Can we prefix these sub command class names with Pkg (similar to Zk*) to namespace them and make it obvious what tool they are for.

janhoy · 2026-10-06T13:57:34Z

I did not intend to close the issue, but it was auto closed when the feature branch was deleted. I expected Github to auto move this onto main. I cannot seem to re-target the PR either, so please re-open one targeting main. When doing so, please let it reference https://issues.apache.org/jira/browse/SOLR-18515 which is a new dedicated JIRA for this tool. Again, sorry for the inconvenience.

jaykay12 added 3 commits August 15, 2026 13:45

prompt done

f71a932

minor

f61cdeb

tidying done

86913e5

github-actions Bot added the cat:cli label Aug 15, 2026

generated cli docs

124256c

github-actions Bot added the documentation Improvements or additions to documentation label Aug 15, 2026

epugh mentioned this pull request Aug 15, 2026

SOLR-18443 Refactor CLI Tools to be independent of commons-cli #4695

Merged

tests

4e2e528

github-actions Bot added the tests label Aug 15, 2026

janhoy requested a balanced review from Copilot August 22, 2026 09:21

Copilot started reviewing on behalf of janhoy August 22, 2026 09:21 View session

Copilot AI reviewed Aug 22, 2026

View reviewed changes

Comment thread solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-package.adoc Outdated

Comment thread solr/core/src/java/org/apache/solr/cli/PackageTool.java Outdated

janhoy requested changes Aug 22, 2026

View reviewed changes

jaykay12 added 7 commits August 23, 2026 03:00

fixed url

e9cefa8

help added

5e76d92

subcommmands

ed3602d

tried outs

7098064

changes

65b4187

licenses

60f3592

tidy

c566d3c

janhoy mentioned this pull request Sep 12, 2026

SOLR-17697 Use picocli instead of commons-cli #3254

Merged

22 of 41 tasks

jaykay12 added 3 commits September 17, 2026 22:44

Merge branch 'jira/SOLR-17697-picocli' into SOLR-17697-packagetool

caea330

merge conflict resolved

9056c86

Merge branch 'jira/SOLR-17697-picocli' into SOLR-17697-packagetool

46c2b7a

serhiy-bzhezytskyy mentioned this pull request Sep 22, 2026

SOLR-17697: Implement picocli for assert, cluster, config, api, export, postlogs, run_example, post, stream and the snapshot-* commands #4931

Closed

jaykay12 added 3 commits October 3, 2026 12:52

some changes done

1da6917

subcommands ready

638f097

generated cli docs

1e8bb70

jaykay12 added 7 commits October 3, 2026 14:32

tidy ran

91019f1

minor

4147876

pr comments

e7c3693

minor

670cb97

pr comments

2142ded

added CL

eb59da5

minor

970ee26

jaykay12 commented Oct 3, 2026

View reviewed changes

jaykay12 requested a review from janhoy October 3, 2026 10:39

janhoy requested a balanced review from Copilot October 6, 2026 11:39

Copilot started reviewing on behalf of janhoy October 6, 2026 11:39 View session

Copilot AI reviewed Oct 6, 2026

View reviewed changes

janhoy requested changes Oct 6, 2026

View reviewed changes

janhoy deleted the branch apache:jira/SOLR-17697-picocli October 6, 2026 13:37

janhoy closed this Oct 6, 2026

jaykay12 mentioned this pull request Oct 6, 2026

SOLR-18515 | PackageTool Migration to PicoCLI #5047

Merged

8 tasks done

Conversation

jaykay12 commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Solution

Tests

Checklist

Uh oh!

jaykay12 commented Aug 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

janhoy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

janhoy Aug 22, 2026

Choose a reason for hiding this comment

Uh oh!

jaykay12 Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

janhoy commented Sep 12, 2026

Uh oh!

jaykay12 left a comment

Choose a reason for hiding this comment

Uh oh!

jaykay12 commented Oct 3, 2026

AI Generated High Level changes:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

janhoy commented Oct 6, 2026

Uh oh!

janhoy left a comment

Choose a reason for hiding this comment

Uh oh!

janhoy commented Oct 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jaykay12 commented Aug 15, 2026 •

edited

Loading