remkop / picocli

Picocli is a modern framework for building powerful, user-friendly, GraalVM-enabled command line apps with ease. It supports colors, autocompletion, subcommands, and more. In 1 source file so apps can include as source & avoid adding a dependency. Written in Java, usable from Groovy, Kotlin, Scala, etc.
https://picocli.info
Apache License 2.0
4.79k stars 414 forks source link

Cannot use Path option to load a file from Google Cloud Storage #2269

Closed reddaly closed 2 months ago

reddaly commented 2 months ago

Sorry for the quick bug report. Essentially the issue is that Picocli options of type Path do not seem to work with https://github.com/googleapis/java-storage-nio.

@CommandLine.Option(
        names = ["--csv"],
        required = true,
        description = [
            "Path to a CSV to parse.",
        ],
    )
    private lateinit var csv: Path

/// ...

csv.readText() // fails

Path.of(URI.create("gs://blah/blah.csv")).readText() // works
remkop commented 2 months ago

Thank you for raising this @reddaly!

I am not sure I understand the issue from the information exception you provided.

Can you provide a failing test, or otherwise more details on how to reproduce the issue? (What input was provided, what exactly happens when you say "doesn't work"? NullPointerException? Or IOException? Something else?)

reddaly commented 2 months ago

I'll try to get a failing test and more details at some point. For now I'm working around the issue with


class FooCommand { 

@CommandLine.Option(
        names = ["--csv"],
        required = true,
        description = [
            "Path to a CSV to parse.",
        ],
        converter = [PathFlagConverter::class],
    )
    private lateinit var csv: Path

}

/**
 * A PicoCLI converter for parsing a [Path]. Unlike the default parser for the [Path] type, this
 * converter will parse "gs://"-style URLs as [Path] objects using
 * https://github.com/googleapis/java-storage-nio. This is a workaround for
 * https://github.com/remkop/picocli/issues/2269.
 */
class PathFlagConverter : CommandLine.ITypeConverter<Path> {
    override fun convert(value: String): Path =
        if (value.startsWith("gs://")) {
            CloudStorageUri(value).path
        } else {
            Path.of(value)
        }
}

Where CloudStorageUri(value).path is essentially equivalent to Path.of(URI.create(value))

remkop commented 2 months ago

Having a custom converter sounds like a perfect solution actually! đź‘Ť

reddaly commented 2 months ago

It certainly works. It is easy to forget, though. The issue is that it's surprising that a custom converter is needed. Elsewhere Path can be used with these types of URLs without special handling.

remkop commented 2 months ago

If the Path.of(URI.create(String)) invocation works with “normal” local files then perhaps I should change the default type converter for Path to use that invocation.

remkop commented 2 months ago

@reddaly It looks like it is not feasible to make the Path.of(URI.create(String)) invocation the default for picocli's built-in Path type converter. For Windows local file paths, a common path like "C:\\Users\\remko\\hello.txt" cannot be used to construct a URI. Attempting to construct a URI with that path results in a java.nio.file.FileSystemNotFoundException: Provider "C" not installed error.

Therefore, the current Path type converter used by picocli (which returns the result of Paths.get(String) to create a Path object, is correct.

Applications that need to construct a URI may be better off declaring the option as an URI instead of as a Path. For example:

@Command
class MyCommand : Runnable {
    @Option(names = ["--csv"], required = true, description = [ "Path to a CSV to parse." ])
    private lateinit var csv: URI

    override fun run() {
        Path path = Paths.get(csv);
        // do something with the path...
    }
}

However, the above will fail for user input like --csv C:\\something, which may not be desirable.

If you need something that handles both local filesystem locations (like C:\something) as well as URI-based paths like "gs://something", then you will need to do what you did: have a custom Path type converter, or declare the option as a String, and put the logic for converting the user input to a Path object in the business logic.

I like your current solution.