Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: GooeyCLI - replacement utility scripting system based on PicoCLI. #4065

Closed
wants to merge 9 commits into from

Conversation

Cervator
Copy link
Member

@Cervator Cervator commented Jul 4, 2020

Still a proof of concept, with a limited amount of old functionality converted to showcase the new approach. Relies on a groovyw tweak (MovingBlocks/groovy-wrapper#5), renames the scripts (gw is < 1/3rd of groovyw ! Think of the savings!), and lets PicoCLI worry about all the CLI structuring. Also experimenting with Terasology/Index#7

Contains

Overhaul of our utility scripting. PicoCLI takes away all the awkwardness surrounding argument parsing and such, plus automatically generates usage text, even at every level, a la kubectl, docker, git, etc.

Key part is defining commands and options inside small classes using PicoCLI. A command class can define a link to sub-command classes, which in turn can have their own and so on (gw -> module -> get ..). From anywhere you can mix-in other classes containing simple definitions for options, so you only need to define a given option (like --origin) in one place then just mix it in wherever needed. If something isn't relevant for a given combination just don't link it. Easy!

  • Replaces a ton of messy code - util.groovy is no more, the weird loading of files as text to then turn them into Groovy scripts dynamically is gone, and you can write way cleaner, well, everything.
  • Adding new commands is trivial rather than painful, and it is easy to add them at any level with a single small file. Options / Parameters can be added separately as traits / mix-ins, then plugged in wherever
  • Is technically in Groovy, but remains largely easy Java without a lot of elaborate Groovy'isms. I did try for a bit to get plain Groovy scripts to work (cuts down even more boilerplate code) but it didn't seem worth the hassle / weirdness / quirks in trying to use the setup with our Groovy Wrapper
  • A help / -h / --help command is available at every level, detailing everything automatically. Extra examples can easily be added beyond the auto-generated stuff
  • Working text colors on Windows - previous ANSI coloring only works on Linux/Mac, and leaves control characters in the output on Windows. Some stuff like underlining still doesn't work on Windows, but colors do 🌈
  • Docs for PicoCLI are pretty great, it seems very mature, and also has lots of nice examples. Plus it is apparently embedded within Groovy itself!

Diagram!

PicoCLI

The green stuff can actually be run, everything else is supporting (some sub-commands require additional sub-commands - you can run gw module but will get an error about that being meaningless on its own). Diagram doesn't cover all use cases and not everything listed is done yet.

How to test

  • Check out PR
  • Run gw commands in the root of your workspace

That's it, really! Everything else should just work, no extra steps needed. Not a lot is actually functional yet

  • Try to use the help command in various places. Or nothing at all and see what happens
  • gw module get ModuleName should work normally, you can add -o / --origin to vary where to get modules from. The old gradle.properties global override for different GitHub locations should also work, if set (example: alternativeGithubHome=Nanoware)
    • You should be able to get multiple modules in one go, but recurse and other stuff isn't in yet
  • gw init says some things, including colored output!
  • Try putting the -o flag in different places, using different commands
  • gw module refresh should do the build.gradle update to all modules, demonstrates a sub-command defined specifically within an item type - sort of like a private command to that parent, rather than write it in its own file, when it won't make sense anywhere else

Outstanding before merging

  • Sanity checks for architecture and such (I feel like I code so rarely I'm forgetting all the patterns and probably could have done this better), plus actually confirm if we want to do this - Technology for workspace management and release tooling #4035
  • Finish converting old functionality
  • See if the PicoCLI and jansi jars in the Gradle Wrapper can just be added to the classpath to avoid their @Grabs
  • May well move to a future goal but: extract the utility scripting to its own repo and solely @Grab a built jar from there. In most cases users don't really need the scripts themselves, and I'm sure we could come up with a way to sub in source when you want to edit it, like with other items. Benefit: Use the exact same setup for both Terasology and DestSol (and more?), limit the amount of code in the engine repo, possibly version things better?

Future goals

While not a goal with this PR I'm hoping to build more advanced functionality on top of it, partly via my day job where I'm trying to do something similar in a different use case (enterprise CI/CD / app dev)

  • Add back in command combinations that ask the user for input? Unsure how useful that really is, so rather wait till later
  • Look into hooking the command auto-completion in (even if it might not work on Windows)
  • A promote / release command that'll tag a repo on GitHub. Possibly deal with a GitHub release? Or would that be Jenkins? Technology for workspace management and release tooling #4035 again
  • A fork command that'll create a fork on GitHub of a target item
  • A command that'll create a normal repo remotely on GitHub (not a fork). Name?
  • A pr command that'll create a PR automagically
  • bulk commands to handle large-scale modifications, such as an engine change needing minor tweaks in a dozen or two modules (prep branches, bulk commit, push to GitHub, make / merge PRs if desired, possibly add markup somewhere to tell Jenkins to test them all together)
  • Possibly minor reporting options? Related to open PRs, branches, and such. Jenkins X does some neat things
  • Possibly utility scripting to help manage teams on GitHub, adding new users easily, standardize user lists, make labels the same in different repos,etc
  • Git adapters for other providers (GitLab, etc) - not necessarily very useful for us here but I need it for work anyway, so here eventually we might be able to support modules on other Git providers seamlessly?
  • Some ways later, maybe: use some of this logic remotely via Jenkins and/or ChatOps (use the commands on chat to create repos for new users, release things, detail forks, etc)
  • Possibly use PicoCLI itself inside the game - could it power our in-game console? We already have code overlap there between TS and DS, maybe we could have a game console library both projects could use. Been wondering about this since I started digging around, and recently actually found https://github.com/Rubydesic/MinecraftPicocli - not sure if that's in-game console or server console 🤔
  • Maybe support wiki items or even gh-pages items if we end up using those

@Cervator Cervator added Status: Needs Investigation Requires to be debugged or checked for feasibility, etc. Category: Doc Requests, Issues and Changes targeting javadoc and module documentation Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. WIP labels Jul 4, 2020
// Copyright 2020 The Terasology Foundation
// SPDX-License-Identifier: Apache-2.0

// Grab the Groovy extensions for PicoCLI - in IntelliJ Alt-ENTER on a `@Grab` to register contents for syntax highlighting
Copy link
Member Author

Choose a reason for hiding this comment

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

Ideally lines 4-12 here would disappear if I can use the code from inside the Gradle Wrapper instead

@pollend
Copy link
Member

pollend commented Jul 6, 2020

I tried to run the ./gw command

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.codehaus.groovy.reflection.CachedClass (file:/home/michaelpollind/.gradle/wrapper/dists/gradle-6.4.1-all/13imxtezgn9nwzqt8rgtkunh1/gradle-6.4.1/lib/groovy-all-1.3-2.5.10.jar) to method java.lang.Object.finalize()
WARNING: Please consider reporting this to the maintainers of org.codehaus.groovy.reflection.CachedClass
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
Caught: picocli.CommandLine$InitializationException: class java.lang.Module is not a command: it has no @Command, @Option, @Parameters or @Unmatched annotations
picocli.CommandLine$InitializationException: class java.lang.Module is not a command: it has no @Command, @Option, @Parameters or @Unmatched annotations
        at picocli.CommandLine$Model$CommandReflection.validateCommandSpec(CommandLine.java:10774)
        at picocli.CommandLine$Model$CommandReflection.extractCommandSpec(CommandLine.java:10612)
        at picocli.CommandLine$Model$CommandSpec.forAnnotatedObject(CommandLine.java:5765)
        at picocli.CommandLine.<init>(CommandLine.java:223)
        at picocli.CommandLine.toCommandLine(CommandLine.java:3415)
        at picocli.CommandLine.access$14200(CommandLine.java:145)
        at picocli.CommandLine$Model$CommandReflection.initSubcommands(CommandLine.java:10636)
        at picocli.CommandLine$Model$CommandReflection.extractCommandSpec(CommandLine.java:10599)
        at picocli.CommandLine$Model$CommandSpec.forAnnotatedObject(CommandLine.java:5765)
        at picocli.CommandLine.<init>(CommandLine.java:223)
        at picocli.CommandLine.<init>(CommandLine.java:196)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at GooeyCLI.main(GooeyCLI.groovy:30)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at org.gradle.wrapper.GroovyBootstrapMainStarter.start(GroovyBootstrapMainStarter.java:35)
        at org.gradle.wrapper.WrapperExecutor.execute(WrapperExecutor.java:108)
        at org.gradle.wrapper.GroovyWrapperMain.main(GroovyWrapperMain.java:60)

image

I moved everything into a package and moved the groovy stuff to another folder.

package org.terasology.cli

// Grab the Groovy extensions for PicoCLI - in IntelliJ Alt-ENTER on a `@Grab` to register contents for syntax highlighting
@Grab('info.picocli:picocli-groovy:4.3.2')
// TODO: Actually exists inside the Gradle Wrapper - gradle-6.4.1\lib\groovy-all-1.3-2.5.10.jar\groovyjarjarpicocli\

// TODO: Unsure if this helps or should be included - don't really need this since we execute via Groovy Wrapper anyway
@GrabExclude('org.codehaus.groovy:groovy-all')

// Needed for colors to work on Windows, along with a mode toggle at the start and end of execution in main
@Grab('org.fusesource.jansi:jansi:1.18') // TODO: Exists at 1.17 inside the Gradle Wrapper lib - can use that one?
import org.fusesource.jansi.AnsiConsole

import picocli.CommandLine
import picocli.CommandLine.Command
import picocli.CommandLine.HelpCommand

// If using local groovy files the subcommands section may highlight as bad syntax in IntelliJ - that's OK
@Command(name = "gw",
        synopsisSubcommandLabel = "COMMAND", // Default is [COMMAND] indicating optional, but sub command here is required
        subcommands = [
                HelpCommand.class, // Adds standard help options (help as a subcommand, -h, and --help)
                Module.class,
                Init.class], // Note that these Groovy classes *must* start with a capital letter for some reason
        description = "Utility system for interacting with a Terasology developer workspace")
class GooeyCLI extends BaseCommand {
    static void main(String[] args) {
        AnsiConsole.systemInstall() // enable colors on Windows - TODO: Test on not-so-Windows systems, should those not run this?
        CommandLine cmd = new CommandLine(new GooeyCLI())
...

any way we can get this to work with idea @keturn would have a clue?

@Cervator
Copy link
Member Author

Cervator commented Jul 6, 2020

Thanks @pollend !

I've wondered if setting this up as its own sourceSet in Gradle would add anything, but IntelliJ already prompts to configure a Groovy SDK. Maybe if it was Gradled it would also auto-fetch the @Grab dependencies you can otherwise manually fetch for completion via ALT-ENTER. Could also perhaps be a way to later embed the CLI source if it gets extracted and shoved into a jar, yet you still want to optionally work on it inside an engine workspace in source form.

Not super high priority to find out just yet. Happy just to know it is working for somebody else (after a few tweaks)

Cervator and others added 9 commits July 10, 2020 23:01
Still a proof of concept, with a limited amount of old functionality converted to showcase the new approach. Relies on a groovyw tweak, renames the scripts, and lets PicoCLI worry about all the CLI structuring
* Moves it to a maybe more sensible place in the gradle directory where the wrapper lives, this seems kinda related
* Standardizes the source path so we don't need to explicitly configure it
* Drops the Groovy dependency (in Gradle) down to match what's currently in the Gradle Wrapper (although it would still get fetched and used separately for any sort of GooeyCLI *build* process)
@Cervator
Copy link
Member Author

Cervator commented Jul 11, 2020

Okay, bunch more updates in, including @pollend's initial Gradle setup taken a few steps further. Commands like gw module recurse JoshariasSurvival should now work, including a bug fix vs the current groovyw variant - in JS if you have AdditionalFruits already present in source, which depends on EdibleFlora, the old way would stop at AF and never retrieve EF.

There was a point when the gw script for Linux/Mac/Git Bash/Cygwin etc wasn't setting the classpath right. That should be fixed now. Much better structure too, starting to pull out overly unique/wide logic in big methods and trying smarter approaches. At least that's what my head suggests to me, not entirely sure!

Sneaking in bits of colored text but this is still a draft with a ton of extra logging just to make it easier to check on this. More to come, just sleepy 💤

Edit: Still not actually copying in template files or a build.gradle for modules. They won't actually compile or anything, but they should retrieve correctly.

description = "Utility system for interacting with a Terasology developer workspace")
class GooeyCLI extends BaseCommandType {
static void main(String[] args) {
AnsiConsole.systemInstall() // enable colors on Windows - TODO: Test on not-so-Windows systems, should those not run this?
Copy link
Contributor

Choose a reason for hiding this comment

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

On not windows systems - no matter.
colors works with and without this line

@Cervator
Copy link
Member Author

Closing this to clean up, will recreate in the future if the coding spirits will it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. Category: Doc Requests, Issues and Changes targeting javadoc and module documentation Status: Needs Investigation Requires to be debugged or checked for feasibility, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants