SOLR-17697 Use picocli instead of commons-cli

changelog/unreleased/SOLR-17697-picocli.yml

@@ -0,0 +1,8 @@
+title: Add picocli as a dependency to enable migration of CLI tools away from commons-cli
+type: other
+authors:
+  - name: Jan Høydahl
+    url: https://home.apache.org/phonebook.html?uid=janhoy
+links:
+  - name: SOLR-17697
+    url: https://issues.apache.org/jira/browse/SOLR-17697

gradle/java/javac.gradle

@@ -60,7 +60,8 @@ allprojects {
           "-Xlint:text-blocks",
           "-proc:none",  // proc:none was added because of LOG4J2-1925 / JDK-8186647
           "-Xlint:removal",
-          "--should-stop=ifError=FLOW" // error-prone 2.41
+          "--should-stop=ifError=FLOW", // error-prone 2.41
+          "-Aproject=${project.group}/${project.name}"
       ]
 
       if (propertyOrDefault("javac.failOnWarnings", true).toBoolean()) {

gradle/libs.versions.toml

@@ -186,6 +186,7 @@ ow2-asm = "9.8"
 owasp-dependencycheck = "12.1.3"
 # @keep for version alignment
 perfmark = "0.27.0"
+picocli = "4.7.6"
 prometheus-metrics = "1.1.0"
 prometheus-simpleclient = "0.16.0"
 quicktheories = "0.26"
@@ -500,6 +501,8 @@ ow2-asm-commons = { module = "org.ow2.asm:asm-commons", version.ref = "ow2-asm"
 ow2-asm-tree = { module = "org.ow2.asm:asm-tree", version.ref = "ow2-asm" }
 # @keep transitive dependency for version alignment
 perfmark-api = { module = "io.perfmark:perfmark-api", version.ref = "perfmark" }
+picocli = { module = "info.picocli:picocli", version.ref = "picocli" }
+picocli-codegen = { module = "info.picocli:picocli-codegen", version.ref = "picocli" }
 prometheus-metrics-expositionformats = { module = "io.prometheus:prometheus-metrics-exposition-formats", version.ref = "prometheus-metrics" }
 prometheus-metrics-model = { module = "io.prometheus:prometheus-metrics-model", version.ref = "prometheus-metrics" }
 prometheus-simpleclient = { module = "io.prometheus:simpleclient", version.ref = "prometheus-simpleclient" }

solr/api/gradle.lockfile

@@ -37,6 +37,7 @@ com.tdunning:t-digest:3.3=jarValidation,testRuntimeClasspath
 commons-cli:commons-cli:1.10.0=jarValidation,testRuntimeClasspath
 commons-codec:commons-codec:1.19.0=jarValidation,testRuntimeClasspath
 commons-io:commons-io:2.20.0=jarValidation,testCompileClasspath,testRuntimeClasspath
+info.picocli:picocli:4.7.6=jarValidation,testRuntimeClasspath
 io.dropwizard.metrics:metrics-annotation:4.2.26=jarValidation,testRuntimeClasspath
 io.dropwizard.metrics:metrics-core:4.2.26=jarValidation,testRuntimeClasspath
 io.dropwizard.metrics:metrics-jetty12-ee10:4.2.26=jarValidation,testRuntimeClasspath

solr/benchmark/gradle.lockfile

@@ -33,6 +33,7 @@ com.tdunning:t-digest:3.3=jarValidation,runtimeClasspath,testRuntimeClasspath
 commons-cli:commons-cli:1.10.0=jarValidation,runtimeClasspath,testRuntimeClasspath
 commons-codec:commons-codec:1.19.0=jarValidation,runtimeClasspath,testRuntimeClasspath
 commons-io:commons-io:2.20.0=compileClasspath,jarValidation,runtimeClasspath,testCompileClasspath,testRuntimeClasspath
+info.picocli:picocli:4.7.6=jarValidation,runtimeClasspath,testRuntimeClasspath
 io.dropwizard.metrics:metrics-annotation:4.2.26=jarValidation,runtimeClasspath,testRuntimeClasspath
 io.dropwizard.metrics:metrics-core:4.2.26=compileClasspath,jarValidation,runtimeClasspath,testCompileClasspath,testRuntimeClasspath
 io.dropwizard.metrics:metrics-jetty12-ee10:4.2.26=jarValidation,runtimeClasspath,testRuntimeClasspath

solr/bin/solr

@@ -812,7 +812,11 @@ if [ $# -gt 0 ]; then
             shift 2
         ;;
         -h|--help)
-            print_usage "$SCRIPT_CMD"
+            if [[ "${SOLR_PICOCLI:-}" == "true" ]]; then
+                run_tool "$SCRIPT_CMD" --help
+            else
+                print_usage "$SCRIPT_CMD"
+            fi
             exit 0
         ;;
         -y|--no-prompt)

solr/bin/solr.cmd

@@ -414,8 +414,22 @@ IF "%1"=="--all" goto set_stop_all
 :parse_general_args
 
 REM Print usage of command in case help option included
-IF "%1"=="--help" goto usage
-IF "%1"=="-h" goto usage
+IF "%1"=="--help" goto check_picocli_help
+IF "%1"=="-h" goto check_picocli_help
+goto after_help_check
+
+:check_picocli_help
+IF "%SOLR_PICOCLI%"=="true" goto run_picocli_help
+goto usage
+
+:run_picocli_help
+"%JAVA%" %SOLR_SSL_OPTS% %AUTHC_OPTS% %SOLR_ZK_CREDS_AND_ACLS% %SOLR_TOOL_OPTS% -Dsolr.install.dir="%SOLR_TIP%" ^
+  -Dlog4j.configurationFile="file:///%DEFAULT_SERVER_DIR%\resources\log4j2-console.xml" ^
+  -classpath "%SOLR_TIP%\lib\*;%DEFAULT_SERVER_DIR%\solr-webapp\webapp\WEB-INF\lib\*;%DEFAULT_SERVER_DIR%\lib\ext\*" ^
+  org.apache.solr.cli.SolrCLI %SCRIPT_CMD% --help
+goto done
+
+:after_help_check
 
 REM other args supported by all special commands
 IF "%1"=="-p" goto set_port

solr/core/build.gradle

@@ -98,6 +98,8 @@ dependencies {
   implementation libs.commonscodec.commonscodec
 
   implementation libs.commonscli.commonscli
+  implementation libs.picocli
+  annotationProcessor libs.picocli.codegen
 
   implementation libs.locationtech.spatial4j
 

solr/core/gradle.lockfile

@@ -37,6 +37,8 @@ com.tdunning:t-digest:3.3=compileClasspath,jarValidation,runtimeClasspath,runtim
 commons-cli:commons-cli:1.10.0=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
 commons-codec:commons-codec:1.19.0=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
 commons-io:commons-io:2.20.0=apiHelper,compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
+info.picocli:picocli-codegen:4.7.6=annotationProcessor
+info.picocli:picocli:4.7.6=annotationProcessor,compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
 io.dropwizard.metrics:metrics-annotation:4.2.26=jarValidation,testRuntimeClasspath
 io.dropwizard.metrics:metrics-core:4.2.26=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
 io.dropwizard.metrics:metrics-jetty12-ee10:4.2.26=jarValidation,testRuntimeClasspath
@@ -187,4 +189,4 @@ org.slf4j:jcl-over-slf4j:2.0.17=apiHelper,jarValidation,runtimeClasspath,runtime
 org.slf4j:slf4j-api:2.0.17=apiHelper,compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
 org.xerial.snappy:snappy-java:1.1.10.8=jarValidation,runtimeClasspath,runtimeLibs,testCompileClasspath,testRuntimeClasspath
 ua.net.nlp:morfologik-ukrainian-search:4.9.1=jarValidation,testRuntimeClasspath
-empty=apiHelperTest,missingdoclet,permitAggregatorUse,permitTestAggregatorUse,permitTestUnusedDeclared,permitTestUsedUndeclared,permitUnusedDeclared,permitUsedUndeclared,signatures
+empty=apiHelperTest,missingdoclet,permitAggregatorUse,permitTestAggregatorUse,permitTestUnusedDeclared,permitTestUsedUndeclared,permitUsedUndeclared,signatures

solr/core/src/java/org/apache/solr/cli/ApiTool.java

@@ -117,4 +117,9 @@ public static ModifiableSolrParams getSolrParamsFromUri(URI uri) {
     }
     return paramsMap;
   }
+
+  @Override
+  public int callTool() throws Exception {
+    throw new UnsupportedOperationException("This tool does not yet support PicoCli");
+  }
 }

solr/core/src/java/org/apache/solr/cli/AssertTool.java

@@ -426,6 +426,11 @@ private static boolean runningSolrIsCloud(String url, String credentials) throws
     }
   }
 
+  @Override
+  public int callTool() throws Exception {
+    throw new UnsupportedOperationException("This tool does not yet support PicoCli");
+  }
+
   public static class AssertionFailureException extends Exception {
     public AssertionFailureException(String message) {
       super(message);

solr/core/src/java/org/apache/solr/cli/AuthTool.java

@@ -452,4 +452,9 @@ public void runImpl(CommandLine cli) throws Exception {
       throw new IllegalStateException("Only type=basicAuth supported at the moment.");
     }
   }
+
+  @Override
+  public int callTool() throws Exception {
+    throw new UnsupportedOperationException("This tool does not yet support PicoCli");
+  }
 }

solr/core/src/java/org/apache/solr/cli/CLIUtils.java

@@ -183,6 +183,7 @@ public static String normalizeSolrUrl(String solrUrl, boolean logUrlFormatWarnin
    * Get the base URL of a live Solr instance from either the --solr-url command-line option or from
    * ZooKeeper.
    */
+  @Deprecated
   public static String normalizeSolrUrl(CommandLine cli) throws Exception {
     String solrUrl = cli.getOptionValue(CommonCLIOptions.SOLR_URL_OPTION);
 

solr/core/src/java/org/apache/solr/cli/CliDefaultValueProvider.java

@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cli;
+
+import org.apache.solr.common.util.EnvUtils;
+import picocli.CommandLine;
+
+/** Provides default values for CLI arguments. */
+public class CliDefaultValueProvider implements CommandLine.IDefaultValueProvider {
+  @Override
+  public String defaultValue(CommandLine.Model.ArgSpec argSpec) throws Exception {
+    return switch (argSpec.paramLabel()) {
+      case "<zkHost>" -> EnvUtils.getProperty("zkHost");
+      case "<solrUrl>" -> EnvUtils.getProperty("solr.url");
+      case "<port>" -> EnvUtils.getProperty("solr.port", "8983");
+      case "<maxWaitSecs>" -> EnvUtils.getProperty("solr.max.wait.seconds", "0");
+      default -> null;
+    };
+  }
+}

solr/core/src/java/org/apache/solr/cli/ClusterTool.java

@@ -97,4 +97,9 @@ public void runImpl(CommandLine cli) throws Exception {
       }
     }
   }
+
+  @Override
+  public int callTool() throws Exception {
+    throw new UnsupportedOperationException("This tool does not yet support PicoCli");
+  }
 }

solr/core/src/java/org/apache/solr/cli/ConfigSetDownloadTool.java

@@ -19,14 +19,19 @@
 import java.lang.invoke.MethodHandles;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
 import org.apache.commons.cli.Options;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /** Supports zk downconfig command in the bin/solr script. */
+@picocli.CommandLine.Command(
+    name = "downconfig",
+    description = "Download a configset from ZooKeeper to the local filesystem.")
 public class ConfigSetDownloadTool extends ToolBase {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
@@ -48,6 +53,14 @@ public class ConfigSetDownloadTool extends ToolBase {
           .desc("Local directory with configs.")
           .get();
 
+  @picocli.CommandLine.Mixin ZkConnectionOptions zkOpts;
+
+  @picocli.CommandLine.Mixin ConfigSetOptions configSetOpts;
+
+  public ConfigSetDownloadTool() {
+    this(new DefaultToolRuntime());
+  }
+
   public ConfigSetDownloadTool(ToolRuntime runtime) {
     super(runtime);
   }
@@ -74,28 +87,50 @@ public String getUsage() {
   @Override
   public void runImpl(CommandLine cli) throws Exception {
     String zkHost = CLIUtils.getZkHost(cli);
-
     String confName = cli.getOptionValue(CONF_NAME_OPTION);
     String confDir = cli.getOptionValue(CONF_DIR_OPTION);
 
     echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");
     try (SolrZkClient zkClient = CLIUtils.getSolrZkClient(cli, zkHost)) {
-      Path configSetPath = Path.of(confDir);
-      // we try to be nice about having the "conf" in the directory, and we create it if it's not
-      // there.
-      if (!configSetPath.endsWith("/conf")) {
-        configSetPath = configSetPath.resolve("conf");
-      }
-      Files.createDirectories(configSetPath);
-      echo(
-          "Downloading configset "
-              + confName
-              + " from ZooKeeper at "
-              + zkHost
-              + " to directory "
-              + configSetPath.toAbsolutePath());
-
-      zkClient.downConfig(confName, configSetPath);
+      doDownconfig(zkClient, zkHost, confName, confDir);
+    } catch (Exception e) {
+      log.error("Could not complete downconfig operation for reason: ", e);
+      throw (e);
+    }
+  }
+
+  private void doDownconfig(SolrZkClient zkClient, String zkHost, String confName, String confDir)
+      throws Exception {
+    Path configSetPath = Path.of(confDir);
+    // we try to be nice about having the "conf" in the directory, and we create it if it's not
+    // there.
+    if (!configSetPath.endsWith("conf")) {
+      configSetPath = configSetPath.resolve("conf");
+    }
+    Files.createDirectories(configSetPath);
+    echo(
+        "Downloading configset "
+            + confName
+            + " from ZooKeeper at "
+            + zkHost
+            + " to directory "
+            + configSetPath.toAbsolutePath());
+
+    zkClient.downConfig(confName, configSetPath);
+  }
+
+  @Override
+  public int callTool() throws Exception {
+    String zkHost = zkOpts.resolveZkHost();
+
+    echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");
+    try (SolrZkClient zkClient =
+        new SolrZkClient.Builder()
+            .withUrl(zkHost)
+            .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
+            .build()) {
+      doDownconfig(zkClient, zkHost, configSetOpts.confName, configSetOpts.confDir);
+      return 0;
     } catch (Exception e) {
       log.error("Could not complete downconfig operation for reason: ", e);
       throw (e);

solr/core/src/java/org/apache/solr/cli/ConfigSetOptions.java

@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cli;
+
+import picocli.CommandLine;
+
+/**
+ * Picocli mixin providing common configset options shared by upconfig and downconfig sub-commands.
+ *
+ * <p>Use {@code @CommandLine.Mixin ConfigSetOptions configSetOpts} in a command class to inherit
+ * these options.
+ */
+public class ConfigSetOptions {
+
+  @CommandLine.Option(
+      names = {"-n", "--conf-name"},
+      description = "Configset name in ZooKeeper.",
+      required = true)
+  public String confName;
+
+  @CommandLine.Option(
+      names = {"-d", "--conf-dir"},
+      description = "Local directory with configs.",
+      required = true,
+      paramLabel = "DIR")
+  public String confDir;
+}