wallet-tool: add picocli-codegen annotationProcessor dependency

[msgilligan]

This does not seem to be necessary for non-GraalVM builds, but does not hurt (and I suspect it improves startup speed) and is necessary for the GraalVM nativeCompile build which is coming in a separate PR.

[msgilligan]

@schildbach The test failure here also looks like one of the intermittent ones.

[schildbach]

Hmmm, the documentation at https://github.com/remkop/picocli/tree/main/picocli-codegen#213-gradle states that there are different paths to be used, depending on if Gradle is above or below version 4.6. Can we reflect that?

[msgilligan]

Hmmm, the documentation at https://github.com/remkop/picocli/tree/main/picocli-codegen#213-gradle states that there are different paths to be used, depending on if Gradle is above or below version 4.6. Can we reflect that?

There are (at least) three ways we can address this issue:

1. In settings.gradle don't enable the wallettool subproject unless Gradle is 4.10 or later (same as wallettemplate)
2. In wallettool/build.gradle only enable the annotationProcessor dependency when the prerequisites for the GraalVM task are met (i.e. minGraalVMGradleVersion which is currently 7.3, but could probably be as low as 6.7 or perhaps even lower if we don't use the "javaToolchains" feature) This would mean not merging this PR and make enabling annotationProcessor part of PR wallet-tool: add graalvm nativeCompile support to build.gradle #2285)
3. Add a separate conditional in the dependencies block to switch between annotationProcessor and compileOnly depending upon whether Gradle is 4.6+ or not)

I think I have a slight preference for (1), but that would preclude building wallet-tool in the strict Debian-guidelines case until there's a newer Gradle in that environment. So if that's important (2) or (3) would also be fine with me.

(I'll also need to resolve merge conflicts)

[schildbach]

I'd prefer 2️⃣ or 3️⃣, for the reason you mentioned.

Not sure which of the two is less intrusive. In general, I don't really like conditionals (and code-ish constructs) in build.gradle files, so I'd prefer if it doesn't distract too much.

[schildbach]

In case you're wondering, I was trying to find an alternative way to reference from your points list, without inadvertently referencing GitHub issues/PRs.

[msgilligan]

an alternative way to reference from your points list, without inadvertently referencing GitHub issues/PRs.

(1), (2), (3) is a pure ascii way of doing it. (Edited above.) But it's a cool use of emojis if you have them easily available.

[msgilligan]

I'd prefer 2️⃣ or 3️⃣, for the reason you mentioned.

Not sure which of the two is less intrusive.

(2) has fewer conditionals, but will result in the benefits of annotation processing not being available in all cases. I'll make a stab at (3) and we'll see how it looks.

In general, I don't really like conditionals (and code-ish constructs) in build.gradle files, so I'd prefer if it doesn't distract too much.

Well, one way to minimize conditionals is by requiring a recent minimum version of Gradle 😏, but given that we're in this mode of "progressively-enchanced" Gradle builds it means we're stuck with conditionals. (I'm collecting some thoughts on a new approach to this that I will bring up soon in the context of updating our unit tests.)

[msgilligan]

@schildbach How does 77e2f10 look?

[msgilligan]

ec12ad7 adds support for generating manpage documentation in both HTML and man format.

To build the documentation use gradle bitcoinj-wallettool:asciidoctor. The results are in:

• wallettool/build/generated-picocli-docs/wallet-tool.adoc (asciidoc source)
• wallettool/build/docs/html5/wallet-tool.html (HTML version)
• wallettool/build/docs/manpage/wallet-tool.1 (man format)

[schildbach]

Both commits look good to me. Is the asciidoctor naming of the Gradle task commit practise? I'd expect something like generate-manual or just man or doc.

[schildbach]

I just tried it in action. Gradle 4.10.2 works fine. However the Debian Gradle 4.4.1 yields this error:

A problem occurred evaluating project ':bitcoinj-wallettool'.
> Could not get unknown property 'annotationProcessor' for configuration container of type org.gradle.api.internal.artifacts.configurations.DefaultConfigurationContainer.

[schildbach]

Guess I have to continue working on the command line opions metadata, as the generated manuals kind of expose the fact that the migration from the text file isn't really finished…

[msgilligan]

Is the asciidoctor naming of the Gradle task commit practise?

Yes. It's the default provided by the Gradle Asciidoctor Plugin.

I'd expect something like generate-manual or just man or doc.

We could easily add an additional task with a name like man or doc and have that task depend on asciidoctor.

[msgilligan]

I just tried it in action. Gradle 4.10.2 works fine. However the Debian Gradle 4.4.1 yields … error

I somewhat blindly copied the generateManpageAsciiDoc and asciidoctor tasks from a picocli Gradle example and fixed one issue that occurred on newer Gradle versions.

I just pushed d7996eb which should hopefully fix your issue. (It's not currently easy for me to test on Debian Gradle 4.4.1)

[schildbach]

Merged.

[msgilligan]

Guess I have to continue working on the command line opions metadata, as the generated manuals kind of expose the fact that the migration from the text file isn't really finished…

@schildbach Have you thought about or looked into this at all? I'm about to take a quick look and see what can be done...

[msgilligan]

Guess I have to continue working on the command line opions metadata, as the generated manuals kind of expose the fact that the migration from the text file isn't really finished…

@schildbach Have you thought about or looked into this at all? I'm about to take a quick look and see what can be done...

I took a quick look into *picocli support for subcommands and think we should refactor walletooland take advantage of it. I opened Issue #2509 to discuss this along with the bigger picture of refactoring and overall UI improvements.