Cobigen CLI commandlet #273

jdiazgon · 2019-10-11T09:54:43Z

Addressing #59. This PR adds a new commandlet capable of installing the CobiGen CLI inside of the ide.

Ready to be merged.

…mdlet

scripts/src/main/resources/scripts/command/cobigen

markusschuh · 2019-10-11T15:40:34Z

scripts/src/main/resources/scripts/command/cobigen

+      doRunCommand "unzip -q '${COBIGEN_CLI_HOME}/${software_name}.jar' 'cg.sh' -d '${COBIGEN_CLI_HOME}'"
+      doRunCommand "unzip -q '${COBIGEN_CLI_HOME}/${software_name}.jar' 'class-loader-agent.jar' -d '${COBIGEN_CLI_HOME}'"
+    fi
+


The "doIsWindows" is a bit misleading name: It (currently) results in "yes", when the local bash environment is either msys (typically installed by git-for-windows) or cygwin. But both those firewalls need the *.sh files, when cobigen shall be started from inside the environment. In addition to that, the *.bat may also help - but only, when a developer also uses windows-CMD.

It might be easiest to extract just all files in any case.

Yes, when we install maven or other multi-platform tools, we just extract everything.
Cant you just use doInstall function for cobigen as well?

Unfortunately I cannot use doInstall as my .jar extension is not on the list of compatible files for doExtract (that method is called inside doInstall)

Also that doExtract would extract other useless files.

I can support .jar in doExtract in a second. Why do you have "useless" files in your CLI release?
Can you change the way you bundle the release? (we do not have to block this PR now, but on the long run I would prefer cleaning this code as it appears odd to me).
Further the content of this JAR is designed/defined in CobiGen but here you are spreading redundant knowledge about it. What if some day CobiGen decides to rename or replace class-loader-agent.jar? Then this will break and nobody will notice as the CobiGen team is not anymore aware about this dependency in another repo...

hohwille

@jdiazgon thanks for this PR already. I wont approve or merge already as you said. Generally this is looking good but what is confusing me is:

why does devon cobigen not support any other arguments? E.g. for maven you can not only install it via devon mvn setup but also invoke it via devon mvn «args» instead of mvn «args». My expectation of cobigen commandlet would be the same.
why do you distribute cobigen CLI as JAR if it technically is a software bundle containing even shell scripts like cg.sh that need executable permissions? IMHO you should distribute it as *.tgz and set permissions accordingly.

hohwille · 2019-10-14T12:29:18Z

scripts/src/main/resources/scripts/command/cobigen

+      doRunCommand "unzip -q '${COBIGEN_CLI_HOME}/${software_name}.jar' 'cg.sh' -d '${COBIGEN_CLI_HOME}'"
+      doRunCommand "unzip -q '${COBIGEN_CLI_HOME}/${software_name}.jar' 'class-loader-agent.jar' -d '${COBIGEN_CLI_HOME}'"
+    fi
+


Yes, when we install maven or other multi-platform tools, we just extract everything.
Cant you just use doInstall function for cobigen as well?

scripts/src/main/resources/scripts/devon.properties

scripts/src/main/resources/scripts/functions

jdiazgon · 2019-10-16T08:30:53Z

Thanks for the feedback! I will update it as soon as possible.

why do you distribute cobigen CLI as JAR? IMHO you should distribute it as *.tgz and set permissions accordingly

Seems a good idea! I think I will implement it for the next release as we are already on a hurry for the current version and I don't have time for new changes.

…mdlet

maybeec · 2019-10-22T17:52:41Z

I am not sure whether I got it right, but cobigen-cli is a runnable jar, so you cannot just release it as it tar.gz or something like this. @jdiazgon as far as I remember, we simply extract a pom.xml file to dynamically download other dependencies, nothing more right?

jdiazgon · 2019-10-23T07:33:55Z

we simply extract a pom.xml file to dynamically download other dependencies

That was right for the previous version. However, for JDK 11 I had to add a Java agent as the class loading has completely changed. Right now our runnable jar contains that pom.xml and the jar of the Java agent.

maybeec · 2019-10-25T16:43:53Z

@jdiazgon the java11 compatibility is fixed, right? so this could be merged?

jdiazgon · 2019-10-27T20:55:20Z

the java11 compatibility is fixed, right? so this could be merged?

It is fixed but we should not merged yet as the download URL is not the correct one. Please see this.

Also the Travis build is failing with the following error:

In scripts/command/cobigen line 35:
  doEcho "Running: ${cobigen_cmd} ${@}"
                                  ^--^ SC2145: Argument mixes string and array. Use * or separate argument.
In scripts/command/cobigen line 41:
    "${cobigen_cmd}" "${@} --verbose"
                      ^--^ SC2145: Argument mixes string and array. Use * or separate argument.

It seems a Linux related error. I will fix it ASAP.

markusschuh · 2019-10-27T23:58:10Z

With respect to

In scripts/command/cobigen line 35:
  doEcho "Running: ${cobigen_cmd} ${@}"
                                  ^--^ SC2145: Argument mixes string and array. Use * or separate argument.
In scripts/command/cobigen line 41:
    "${cobigen_cmd}" "${@} --verbose"
                      ^--^ SC2145: Argument mixes string and array. Use * or separate argument.

The findings of shellcheck (which is run as part of travis) are correct ones. An explanatory URL is available within the travis build details.

I'd say, the correct fixes were to use the following code instead:

  doEcho "Running: ${cobigen_cmd} ${*}"
. . .
    "${cobigen_cmd}" --verbose "${@}"

This is not only true for linux but for bash scripts on any platform.

…cobigenCmdlet

…mdlet

scripts/src/main/resources/scripts/devon.properties

scripts/src/main/resources/scripts/command/cobigen

markusschuh

Everything looks o.k. now to me.
Or more precise: Everything about the shell syntax. There had been other comments on / requests for this PR which I have not reviewed all for being fulfilled or not.

jdiazgon · 2020-01-12T18:01:49Z

Besser spät als nie. This PR is ready to be merged as the CobiGen CLI has been released to public.

maybeec · 2020-01-12T22:12:54Z

@hohwille if this PR is fine for you as well now, please merge it.

jdiazgon · 2020-01-13T09:08:21Z

The link which should download the latest version of the CLI is returning 404:

https://repository.sonatype.org/service/local/artifact/maven/redirect?r=central-proxy&g=com.devonfw.cobigen&a=cli&v=LATEST

We guess that Maven Central needs to refresh its metadata and it will take some hours to be available. However, I released yesterday and it should have been enough time. IMHO, if in 3 hours it is not yet working, we should go for a URL that works like:

https://repository.sonatype.org/service/local/artifact/maven/redirect?r=central-proxy&g=com.devonfw.cobigen&a=cli&v=1.0.1

jdiazgon · 2020-01-13T12:41:53Z

I had to change the "LATEST" URL as it was not working yet. PR ready to be merged.

maybeec · 2020-01-14T06:11:47Z

@jdiazgon, the LATEST link is working now.

This reverts commit d8a17d4 which changed the download URL.

hohwille

Hey all. To me this is good enough to be merged. I still have some comments I would like to have reworked/improved in the future. Further I would like to kindly ask you to update the PR comments.
Thanks for your effort on this and all CobiGen. 👍

hohwille · 2020-01-14T09:01:46Z

scripts/src/main/resources/scripts/command/cobigen

+      doRunCommand "unzip -q '${COBIGEN_CLI_HOME}/${software_name}.jar' 'cg.sh' -d '${COBIGEN_CLI_HOME}'"
+      doRunCommand "unzip -q '${COBIGEN_CLI_HOME}/${software_name}.jar' 'class-loader-agent.jar' -d '${COBIGEN_CLI_HOME}'"
+    fi
+


I can support .jar in doExtract in a second. Why do you have "useless" files in your CLI release?
Can you change the way you bundle the release? (we do not have to block this PR now, but on the long run I would prefer cleaning this code as it appears odd to me).
Further the content of this JAR is designed/defined in CobiGen but here you are spreading redundant knowledge about it. What if some day CobiGen decides to rename or replace class-loader-agent.jar? Then this will break and nobody will notice as the CobiGen team is not anymore aware about this dependency in another repo...

hohwille · 2020-01-14T09:12:12Z

Not entirely complete. See:
#59 (comment)

jdiazgon added 4 commits October 2, 2019 17:58

[WIP] CobiGen cmdlet

df93e92

[WIP] Everything ready, just change download URL

9437765

JDK 9 compatible

85426c9

Merge branch 'master' of https://github.com/devonfw/ide into cobigenC…

cc45c7e

…mdlet

jdiazgon requested review from maybeec and hohwille October 11, 2019 09:54

jdiazgon commented Oct 11, 2019

View reviewed changes

scripts/src/main/resources/scripts/command/cobigen Outdated Show resolved Hide resolved

markusschuh reviewed Oct 11, 2019

View reviewed changes

hohwille requested changes Oct 14, 2019

View reviewed changes

hohwille changed the title ~~Cobigen CLI commandlet~~ WIP: Cobigen CLI commandlet Oct 14, 2019

hohwille added this to the release:3.2.0 milestone Oct 14, 2019

jdiazgon added 3 commits October 16, 2019 13:52

Merge branch 'master' of https://github.com/devonfw/ide into cobigenC…

bb89241

…mdlet

Changes requested

f574b50

Remove .jar extension from unzip

517e296

Merge branch 'master' into cobigenCmdlet

15b8dbc

jdiazgon added 3 commits November 8, 2019 10:30

Fixing shell commands

873d890

Merge branch 'cobigenCmdlet' of https://github.com/jdiazgon/ide into …

ad9fda6

…cobigenCmdlet

Merge branch 'master' of https://github.com/devonfw/ide into cobigenC…

9015bc0

…mdlet

markusschuh reviewed Nov 8, 2019

View reviewed changes

jdiazgon mentioned this pull request Nov 8, 2019

Remove deprecated M2_HOME environment variable devonfw/cobigen#1056

Merged

jdiazgon added 2 commits November 8, 2019 14:56

Removing M2_HOME variable

d848015

Fixing shell commands

acd8ba3

markusschuh approved these changes Nov 10, 2019

View reviewed changes

jdiazgon added 2 commits January 12, 2020 17:49

Merge branch 'master' into cobigenCmdlet

7371e46

Correct URL for CobiGen CLI

c10e666

jdiazgon changed the title ~~WIP: Cobigen CLI commandlet~~ Cobigen CLI commandlet Jan 12, 2020

maybeec approved these changes Jan 12, 2020

View reviewed changes

Last version on URL

d8a17d4

Revert "Last version on URL"

0895005

This reverts commit d8a17d4 which changed the download URL.

hohwille approved these changes Jan 14, 2020

View reviewed changes

hohwille merged commit 968e35a into devonfw:master Jan 14, 2020

hohwille mentioned this pull request Jan 14, 2020

Integrate CobiGen CLI as commandlet #59

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cobigen CLI commandlet #273

Cobigen CLI commandlet #273

jdiazgon commented Oct 11, 2019 •

edited

Loading

markusschuh Oct 11, 2019

hohwille Oct 14, 2019

jdiazgon Oct 16, 2019 •

edited

Loading

hohwille Jan 14, 2020

hohwille left a comment

hohwille Oct 14, 2019

jdiazgon commented Oct 16, 2019

maybeec commented Oct 22, 2019

jdiazgon commented Oct 23, 2019 •

edited

Loading

maybeec commented Oct 25, 2019

jdiazgon commented Oct 27, 2019

markusschuh commented Oct 27, 2019

markusschuh left a comment •

edited

Loading

jdiazgon commented Jan 12, 2020

maybeec commented Jan 12, 2020

jdiazgon commented Jan 13, 2020

jdiazgon commented Jan 13, 2020

maybeec commented Jan 14, 2020

hohwille left a comment

hohwille Jan 14, 2020

hohwille commented Jan 14, 2020

Cobigen CLI commandlet #273

Cobigen CLI commandlet #273

Conversation

jdiazgon commented Oct 11, 2019 • edited Loading

markusschuh Oct 11, 2019

Choose a reason for hiding this comment

hohwille Oct 14, 2019

Choose a reason for hiding this comment

jdiazgon Oct 16, 2019 • edited Loading

Choose a reason for hiding this comment

hohwille Jan 14, 2020

Choose a reason for hiding this comment

hohwille left a comment

Choose a reason for hiding this comment

hohwille Oct 14, 2019

Choose a reason for hiding this comment

jdiazgon commented Oct 16, 2019

maybeec commented Oct 22, 2019

jdiazgon commented Oct 23, 2019 • edited Loading

maybeec commented Oct 25, 2019

jdiazgon commented Oct 27, 2019

markusschuh commented Oct 27, 2019

markusschuh left a comment • edited Loading

Choose a reason for hiding this comment

jdiazgon commented Jan 12, 2020

maybeec commented Jan 12, 2020

jdiazgon commented Jan 13, 2020

jdiazgon commented Jan 13, 2020

maybeec commented Jan 14, 2020

hohwille left a comment

Choose a reason for hiding this comment

hohwille Jan 14, 2020

Choose a reason for hiding this comment

hohwille commented Jan 14, 2020

jdiazgon commented Oct 11, 2019 •

edited

Loading

jdiazgon Oct 16, 2019 •

edited

Loading

jdiazgon commented Oct 23, 2019 •

edited

Loading

markusschuh left a comment •

edited

Loading