feature: add gooey CLI #4805

pollend · 2021-07-01T14:13:17Z

Here is an attempt to revive the gooey CLI tool. ended up cutting things down and trying to simplify things. I don't really like how the original groovy code tries to use inheritance to generalize an interface between facades, modules and libraries. I think they're diverse enough where this doesn't really make sense. I migrated in a couple of the commands which should cover most of our needs.

Original PR (with larger description): #4065

pollend · 2021-08-10T04:46:42Z

I was looking at this for formatting the diff but this is ok for the moment: https://github.com/grigoriy-orlov/git-stats/blob/master/src/main/java/ru/ares4322/gitstats/GitStatsImpl.java

skaldarnar · 2021-08-10T08:53:17Z

gradle/gooeycli/src/main/groovy/org/terasology/cli/commands/InitCommand.groovy

@@ -30,14 +30,15 @@ class InitCommand extends BaseCommandType implements Runnable {

    void run() {
        println Ansi.AUTO.string("@|bold,green,underline Time to initialize $distro !|@")
-        def targetDistroURL = "https://raw.githubusercontent.com/$githubOrg/Index/master/distros/$distro/gradle.properties"
+        String origin = gitOptions.resolveOrigin()


How does this resolve? We'd run this command in the MovingBlocks/Terasology repository, but the index repo is located in a different org, Terasology/Index...

because it's resolving to Terasology/Index?

.idea/runConfigurations.xml

gradle/gooeycli/src/main/groovy/org/terasology/cli/GooeyCLI.groovy

gradle/gooeycli/build.gradle

settings.gradle

DarkWeird · 2021-08-16T08:49:17Z

Cannot initialize Omega

keturn · 2021-08-16T16:22:29Z

If you've decided to stick with Groovy, you may be interested to know that gradle 7 ships with Groovy 3.0.

(Related: #4653)

pollend · 2021-08-17T01:07:37Z

If you've decided to stick with Groovy, you may be interested to know that gradle 7 ships with Groovy 3.0.

(Related: #4653)

that sounds really nice

DarkWeird · 2021-08-25T06:44:07Z

We should restore lib/meta/facade commands
what else?

pollend · 2021-08-30T03:59:46Z

@DarkWeird were those commands ever properly implemented?

DarkWeird · 2021-08-30T04:36:46Z

@DarkWeird were those commands ever properly implemented?

Don't sure.
Groovyw just clone repos.
Haven't indexes.
And.. i didn't touch meta.

Cervator

So this is definitely some solid work and I very much appreciate it, and am glad to have at least found enough time to go through the PR just about line by line. Nicely done 👍

This feels like it does really well in areas where the work scratches an important itch, and we have several important itches. But it does also lose some things as a consequence and is somewhat incomplete. Additionally while my first stab was a definite draft effort that lacked stuff and included quirks, I think this one has some different quirks, particularly around some terminology. It also really lacks better docs, be it in the javadoc, command descriptions, usage text, and PR description - even having made it through the whole thing I still don't really understand "snapshots", and that's after having seen it get discussed on chat some and thusly having some more clues. It seems like an awesome feature we badly need, it just really needs some more polish and arranging things in a more user friendly fashion :-)

In that sense this reminds me of my own first effort, as both are incomplete. And I even have a second incomplete effort that was a day job fork where I fixed some of the same issues this PR fixes, yet then I lack some things this covers and fixes other things this one doesn't. Ideally I'd find the time to try splicing my work improvements into this and wind up with a complete work we can then add a few more cool useful things to (like DW's tinkering).

One of the main things in my day job setup is that the workspace has an actual config file, which together with state helps figure out a bunch of things. It seems this mostly is a mix of hard coding and the constants class. Goal was to be able to ship something with workspace config ready out of the box, which in a work context might mean forks of a template that different teams customize. That might still fit in here with distros and temporary "snapshot" thing used to share workspace setup ("junctions" ?)

I don't know what sort of timeline I might be looking at, even reviewing this took all weekend and an innumerous amount of picking it up and putting it away again as I have to go pick up and put away tiny humans in RL. But I'm excited about it again at least so - maybe?

Thank you for the work @pollend (and @DarkWeird!) :-) To be continued, hopefully..

gradle/gooeycli/src/main/groovy/org/terasology/cli/GooeyCLI.groovy

gradle/gooeycli/src/main/groovy/org/terasology/cli/commands/module/ExecuteCommand.groovy

gradle/gooeycli/src/main/groovy/org/terasology/cli/commands/module/GetCommand.groovy

Cervator · 2021-08-30T04:27:54Z

...e/gooeycli/src/main/groovy/org/terasology/cli/commands/workspace/snapshot/LoadCommand.groovy

+        }
+        Snapshot snapshot = new Snapshot(file)
+        snapshot.file = new File(Snapshot.SnapshotDirectory,"${snapshot.captured.epochSecond}.snapshot")
+        snapshot.save()


Snapshot save on the load command? 🤔

Load into local snapshot repo(just dir) from 'external' locations

Cervator · 2021-08-30T04:28:51Z

...e/gooeycli/src/main/groovy/org/terasology/cli/commands/workspace/snapshot/OpenCommand.groovy

+
+    @Override
+    void run() {
+        Desktop desktop = Desktop.getDesktop();


Huh? Desktop ?

Yeah.
Opens snapshot in your headed environment like double click at this file.

Don't works at headless :D

@pollend

Cervator · 2021-08-30T04:29:40Z

...ooeycli/src/main/groovy/org/terasology/cli/commands/workspace/snapshot/RestoreCommand.groovy

+import picocli.CommandLine.Parameters
+
+@Command(name = "restore", description = "Restore workspace from snapshot")
+class RestoreCommand implements Runnable {


What is the difference between "load" and "restore" ? The load command even has "restore" in its text

...ooeycli/src/main/groovy/org/terasology/cli/commands/workspace/snapshot/RestoreCommand.groovy

Cervator · 2021-08-30T04:53:36Z

gradle/gooeycli/src/main/groovy/org/terasology/cli/util/Constants.groovy

+
+    public static final File ModuleCacheFile
+
+    public static final String ModuleIndexUrl = 'http://meta.terasology.org/modules/list/latest'


Just as a reminder related to a brief discussion on Slack: while the Meta server still works I'm not sure its metadata is being reliably refreshed. The Index repo is another thing of potential use, maybe especially with the module site starting to update regularly?

Would be nice if we could abstract out how to figure out dependencies from different source - meta, index, or even local (the old approach that downloads modules one by one then grabs the next set of dependencies). Particularly if there are disagreements, since then we know something is wrong.

Cervator

This looks very promising - I haven't tested so not approving just yet, not sure if I'll have enough time for that tonight. Thanks for the updates @DarkWeird 👍

Cervator · 2021-09-04T22:57:54Z

gradle/gooeycli/src/main/groovy/org/terasology/cli/items/MetaModuleIndex.groovy

+import groovy.json.JsonSlurper
+import org.terasology.cli.config.Config
+
+//TODO deps handling


Not really, meta modules are purely meant as raw asset containers and should never have dependencies

Cervator · 2021-09-05T00:44:15Z

gradle/gooeycli/src/main/groovy/org/terasology/cli/commands/module/GetCommand.groovy

 import picocli.CommandLine
 import picocli.CommandLine.Command
-import picocli.CommandLine.Mixin
 import picocli.CommandLine.Option
 import picocli.CommandLine.Parameters

 @Command(name = "get", description = "get module ")
 class GetCommand implements Runnable {


So if I'm looking at things right there are two GetCommand classes, one common one module, and the one selected is simply based on the import statement? I guess that works but it looks scary to me :-)

If dependencies is the only unique way they differ, would it make sense to abstract out a ProvidesDependencies interface or something with a method, and if during the get command setup we see that only then look at the dependency stuff? Then you could in theory support dependencies per type arbitrarily

wonder what you gain by making this so abstract. trying to add additional layers to best fit the situation. just make more sense to implement this as case by case. The commands are where you implement the details. just make sense to implement them cases by case. wonder if some of this abstraction might add additional trouble down the line.

Cervator · 2021-09-05T00:45:13Z

gradle/gooeycli/src/main/groovy/org/terasology/cli/options/GitOptions.groovy

+
+    abstract ItemConfig getConfig()
+
+    @Option(names = ["-origin", "-o"], description = "Github remote Organization/User to target")


If we're doing Linux style should this become --origin ?

pollend · 2021-09-05T01:22:50Z

I don't think we should be relying on http://meta.terasology.org/modules/list/latest can we revert it back to the recursive behavior.

pollend · 2021-09-05T01:59:39Z

This is starting to look a bit overly convoluted too me. It's way to clever for its own good and I think this will be difficult to add onto in the future. I would opt to shave away some of these abstractions and separate the details more between libs,modules, and faces and keep the codebase more functional. with some helper utilities for some common things to share. this is a CLI utility, why does it need to be so complicated and a lot of this complicated logic feels unwarranted. you've tightly coupled libs, meta, module together when the details of each are not that closely bound to each other. sure there is no duplication but that is in exchange for high coupling and complexity.

I have no problem with proceeding with how this is written but I have some concessions with how this is put together at the moment.

I kind of like what @skaldarnar has written: https://github.com/skaldarnar/node-gooey

pollend · 2021-11-07T16:26:29Z

is there anything else?

DarkWeird · 2021-11-10T08:04:28Z

is there anything else?

idk. :(
I haven't clean and full vision.

skaldarnar · 2021-12-19T19:33:20Z

I did have a brief look at this and played around with it for a bit. Here are my thoughts and notes:

incomplete or buggy functionality

on running gw module init iota the following error occured:

In copyInTemplateFiles for module ModuleTestingEnvironment - copying in a build.gradle then next checking for module.txt
Unable to clone ModuleTestingEnvironment, Skipping: Task java.util.concurrent.FutureTask@3a419fde[Not completed, task = java.util.concurrent.Executors$RunnableAdapter@62ca1d89[Wrapped task = org.terasology.cli.commands.items.ModuleCommand$_recurseInner_closure3$_closure5@6626daa8]] rejected from java.util.concurrent.ThreadPoolExecutor@54906aa4[Shutting down, pool size = 1, active threads = 1, queued tasks = 0, completed tasks = 10]

document the new command line interface
- update existing documentation mentioning groovyw
- should we put explicit documentation about this tool somewhere?
- explain how the whole groovyw project setup works?
- not all commands and options have descriptions
remove old groovyw code in a a separate PR
- allows to work on docs in whatever time is needed
- gives some time to keep both variants around to look out for issues
- separate commit to remove the old code

Things I've tested:

init a workspace with gw module init iota
run a command in specific modules with gw module cmd Health Drops Inventory -c='rm build.gradle'
refresh faulty modules with gw module refresh
check the workspace status with gw workspace status
update outdated modules with gw module update

A few other open questions:

what can we do about this annoying "WARNING: An illegal reflective access operation has occurred"?
running a simple gw help command takes 3 seconds
output of workspace status is not completely clear to me, but usability can be improved in follow-ups

tl;dr: I'd like to split this PR up into smaller chunks in get it merged step by step.

pollend · 2021-12-22T16:22:33Z

I did have a brief look at this and played around with it for a bit. Here are my thoughts and notes:

* incomplete or buggy functionality
  
  * on running `gw module init iota` the following error occured:
    ```
    In copyInTemplateFiles for module ModuleTestingEnvironment - copying in a build.gradle then next checking for module.txt
    Unable to clone ModuleTestingEnvironment, Skipping: Task java.util.concurrent.FutureTask@3a419fde[Not completed, task = java.util.concurrent.Executors$RunnableAdapter@62ca1d89[Wrapped task = org.terasology.cli.commands.items.ModuleCommand$_recurseInner_closure3$_closure5@6626daa8]] rejected from java.util.concurrent.ThreadPoolExecutor@54906aa4[Shutting down, pool size = 1, active threads = 1, queued tasks = 0, completed tasks = 10] 
    ```

* document the new command line interface
  
  * update existing documentation mentioning `groovyw`
  * should we put explicit documentation about this tool somewhere?
  * explain how the whole groovyw project setup works?
  * not all commands and options have descriptions

* remove old `groovyw` code in a a separate PR
  
  * allows to work on docs in whatever time is needed
  * gives some time to keep both variants around to look out for issues
  * separate commit to remove the old code

Things I've tested:

* init a workspace with `gw module init iota`

* run a command in specific modules with `gw module cmd Health Drops Inventory -c='rm build.gradle'`

* refresh faulty modules with `gw module refresh`

* check the workspace status with `gw workspace status`

* update outdated modules with `gw module update`

A few other open questions:

* what can we do about this annoying "WARNING: An illegal reflective access operation has occurred"?

* running a simple `gw help` command takes 3 seconds

* output of `workspace status` is not completely clear to me, but usability can be improved in follow-ups

tl;dr: I'd like to split this PR up into smaller chunks in get it merged step by step.

there is no need to split the PR its self contained so it doesn't affect the existing cli tool.

skaldarnar · 2021-12-22T19:42:33Z

[...] so it doesn't affect the existing cli tool.

Our contributor guide asks the user to run

groovyw module init iota

which does not work anymore with this PR. Thus, I don't follow the argument of this being "self-contained" and "not affecting the existing CLI tooling".

In addition, as reported above, the replacement of exactly that comment threw an error when I tried it out.

pollend · 2021-12-22T20:26:46Z

[...] so it doesn't affect the existing cli tool.

Our contributor guide asks the user to run
groovyw module init iota
which does not work anymore with this PR. Thus, I don't follow the argument of this being "self-contained" and "not affecting the existing CLI tooling".

In addition, as reported above, the replacement of exactly that comment threw an error when I tried it out.

I'll roll back all the changes on the removed groovy code. should keep the old cli available, didn't realize I deleted it.

Squashed commit of the following: commit 89b591f Author: Cervator <[email protected]> Date: Fri Jul 10 22:55:01 2020 -0500 feat: working recurse command in the new style, misc cleanup and utility commit 85433d5 Author: Cervator <[email protected]> Date: Fri Jul 10 16:17:53 2020 -0500 chore: rename stuff, add package structure, tweak some comments, more recurse logic commit 53662ac Author: Cervator <[email protected]> Date: Thu Jul 9 23:23:05 2020 -0500 chore: adjust the Gradle-based GooeyCLI setup * 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) commit c5ed6d3 Author: Michael <[email protected]> Date: Mon Jul 6 13:14:35 2020 -0700 correct synopsisSubcommandLabel commit fc46d27 Author: Michael <[email protected]> Date: Mon Jul 6 12:29:17 2020 -0700 add sourceset for cli config commit d9e2e18 Author: Cervator <[email protected]> Date: Sun Jul 5 22:59:08 2020 -0500 chore: fix path on Windows, avoid some warning about logging itself commit 4a9dc64 Author: Michael <[email protected]> Date: Sun Jul 5 20:35:35 2020 -0700 move CLI and added into package commit d21e82a Author: Cervator <[email protected]> Date: Sat Jul 4 15:09:45 2020 -0500 chore: Minor cleanup, plus bring back the module file Git thought was being deleted rather than replaced commit 05ca0f8 Author: Cervator <[email protected]> Date: Fri Jul 3 20:51:36 2020 -0500 feat: GooeyCLI - replacement utility scripting system based on PicoCLI. 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

It provides bash-completion for `zsh` and `bash` also add a lot help for setup it.

* Use `Instant` for date-time of snapshot - one format for all and supports timezones * Use epoch seconds as filename for snapshots - most datetime formats contains illegal symbols for OSs

* reimplement algorithm. gather dependencies before git cloning. * parallel getting at x1.5-x4 faster then sequence getting

…nce) and move snapshot's commands to `gw workspace snapshot`

Co-authored-by: Rasmus Praestholm <[email protected]>

…dule/ExecuteCommand.groovy Co-authored-by: Rasmus Praestholm <[email protected]>

* Provide `Modules` static class for working with modules * extract Git commands for Items ( future update for facades, libs, metas) * Provide reformat groovys files

…snapshot open`

…config

…nknown item handler

This reverts commit 9738982

…s. add unknown item handler" This reverts commit ba4077f

…onflict in hierarchy) make parallel on-demand recursive dep resolving

github-actions bot added the Type: Improvement Request for or addition/enhancement of a feature label Jul 1, 2021

pollend requested a review from keturn August 9, 2021 05:07

pollend marked this pull request as ready for review August 9, 2021 05:09

pollend requested a review from skaldarnar August 10, 2021 04:46

skaldarnar reviewed Aug 10, 2021

View reviewed changes

DarkWeird reviewed Aug 16, 2021

View reviewed changes

.idea/runConfigurations.xml Outdated Show resolved Hide resolved

gradle/gooeycli/src/main/groovy/org/terasology/cli/GooeyCLI.groovy Outdated Show resolved Hide resolved

gradle/gooeycli/build.gradle Outdated Show resolved Hide resolved

settings.gradle Show resolved Hide resolved

Cervator added Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. Category: Doc Requests, Issues and Changes targeting javadoc and module documentation labels Aug 21, 2021

pollend mentioned this pull request Aug 22, 2021

groovyw update fails with "unknown protocol: git" #4503

Open

Cervator requested changes Aug 30, 2021

View reviewed changes

Cervator reviewed Sep 5, 2021

View reviewed changes

pollend requested review from Cervator and skaldarnar September 5, 2021 02:27

pollend force-pushed the feature/gooey-cli branch from 6065983 to d142b19 Compare April 10, 2022 19:48

pollend and others added 29 commits May 11, 2022 21:10

chore: update git commands

4c750d7

chore: start work on snapshot scheme

bc0cfd6

fix: Fixes invalid infer type for dependencies

fb37b28

fix get command

0555e9a

feat: add workspace commands

e1d72e0

feat: add LoadCommand

fa7b5cd

chore: delete runConfigurations.xml

d7e187a

feat: add ./gw completion command. (#4848)

db0b028

It provides bash-completion for `zsh` and `bash` also add a lot help for setup it.

fix: fix problem with Date localization formats

1e28dd5

* Use `Instant` for date-time of snapshot - one format for all and supports timezones * Use epoch seconds as filename for snapshots - most datetime formats contains illegal symbols for OSs

fix: fix remote url repo saving for snapshot

f14955e

feat: Add parallel module get (#4852)

0312567

* reimplement algorithm. gather dependencies before git cloning. * parallel getting at x1.5-x4 faster then sequence getting

feat(gooey-cli): implement command gw workspace status(gfold refere…

a5894f7

…nce) and move snapshot's commands to `gw workspace snapshot`

chore: replace jcenter with mavenCentral()

3f0582a

Update AnsiConsole#systemInstall usage comment

ef3f6db

Co-authored-by: Rasmus Praestholm <[email protected]>

Update gradle/gooeycli/src/main/groovy/org/terasology/cli/commands/mo…

61527ac

…dule/ExecuteCommand.groovy Co-authored-by: Rasmus Praestholm <[email protected]>

chore(gooey-cli): refactor names and descriptions

1c44bf1

refactor(gooey-cli): refactor module workings

4de50ad

* Provide `Modules` static class for working with modules * extract Git commands for Items ( future update for facades, libs, metas) * Provide reformat groovys files

feat(gooey-cli): Add support for headless systems to `./gw workspace …

24a5ff9

…snapshot open`

doc(gooey-cli): extend descriptiom for SnapshotCommand

9b8f666

feat(gooeys-cli): Add meta,lib,facade, refactor subcommand model and …

49ce9e5

…config

feat(gooeys-cli): add remote index for modules and other items. add u…

77b4b57

…nknown item handler

fix(gooeys-cli): switch modules to github repo index

b4d5f03

chore: cleanup formatting

345cf4c

Revert "fix(gooeys-cli): switch modules to github repo index"

de4e516

This reverts commit 9738982

Revert "feat(gooeys-cli): add remote index for modules and other item…

d100662

…s. add unknown item handler" This reverts commit ba4077f

feat(gooey-cli): replace commands with command-methods

13f893f

feat(gooey-cli): makes recurse subcommand for modules (command name c…

c077473

…onflict in hierarchy) make parallel on-demand recursive dep resolving

chore(gooey-cli): remove empty files

2fdfe67

chore: restore old groovy commands

f2f5b41

pollend force-pushed the feature/gooey-cli branch from d142b19 to f2f5b41 Compare May 12, 2022 04:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: add gooey CLI #4805

feature: add gooey CLI #4805

pollend commented Jul 1, 2021 •

edited by Cervator

Loading

pollend commented Aug 10, 2021

skaldarnar Aug 10, 2021

pollend Aug 10, 2021 •

edited

Loading

DarkWeird commented Aug 16, 2021

keturn commented Aug 16, 2021

pollend commented Aug 17, 2021

DarkWeird commented Aug 25, 2021

pollend commented Aug 30, 2021 •

edited

Loading

DarkWeird commented Aug 30, 2021

Cervator left a comment

Cervator Aug 30, 2021

DarkWeird Aug 30, 2021 •

edited

Loading

Cervator Aug 30, 2021

DarkWeird Aug 30, 2021

Cervator Aug 30, 2021

Cervator Aug 30, 2021

Cervator left a comment

Cervator Sep 4, 2021

Cervator Sep 5, 2021

pollend Sep 5, 2021 •

edited

Loading

Cervator Sep 5, 2021

pollend commented Sep 5, 2021 •

edited

Loading

pollend commented Sep 5, 2021 •

edited

Loading

pollend commented Nov 7, 2021

DarkWeird commented Nov 10, 2021

skaldarnar commented Dec 19, 2021

pollend commented Dec 22, 2021

skaldarnar commented Dec 22, 2021

pollend commented Dec 22, 2021 •

edited

Loading


		public static final File ModuleCacheFile

		public static final String ModuleIndexUrl = 'http://meta.terasology.org/modules/list/latest'


		abstract ItemConfig getConfig()

		@Option(names = ["-origin", "-o"], description = "Github remote Organization/User to target")

feature: add gooey CLI #4805

Are you sure you want to change the base?

feature: add gooey CLI #4805

Conversation

pollend commented Jul 1, 2021 • edited by Cervator Loading

pollend commented Aug 10, 2021

Choose a reason for hiding this comment

pollend Aug 10, 2021 • edited Loading

Choose a reason for hiding this comment

DarkWeird commented Aug 16, 2021

keturn commented Aug 16, 2021

pollend commented Aug 17, 2021

DarkWeird commented Aug 25, 2021

pollend commented Aug 30, 2021 • edited Loading

DarkWeird commented Aug 30, 2021

Cervator left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarkWeird Aug 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cervator left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pollend Sep 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pollend commented Sep 5, 2021 • edited Loading

pollend commented Sep 5, 2021 • edited Loading

pollend commented Nov 7, 2021

DarkWeird commented Nov 10, 2021

skaldarnar commented Dec 19, 2021

pollend commented Dec 22, 2021

skaldarnar commented Dec 22, 2021

pollend commented Dec 22, 2021 • edited Loading

pollend commented Jul 1, 2021 •

edited by Cervator

Loading

pollend Aug 10, 2021 •

edited

Loading

pollend commented Aug 30, 2021 •

edited

Loading

DarkWeird Aug 30, 2021 •

edited

Loading

pollend Sep 5, 2021 •

edited

Loading

pollend commented Sep 5, 2021 •

edited

Loading

pollend commented Sep 5, 2021 •

edited

Loading

pollend commented Dec 22, 2021 •

edited

Loading