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

Make Groovy an "Optional" dependency! #232

Closed
aadrian opened this issue Nov 9, 2017 · 13 comments
Closed

Make Groovy an "Optional" dependency! #232

aadrian opened this issue Nov 9, 2017 · 13 comments

Comments

@aadrian
Copy link

aadrian commented Nov 9, 2017

Please make Groovy an optional dependency.

In 2.0.2 it seems to be a "required" one.
One possibility is by using:

plugins {
    id 'nebula.optional-base' version '3.2.0' // mvn style optional dependencies
}

Thank you.

@remkop
Copy link
Owner

remkop commented Nov 9, 2017

Groovy is required to compile the PicocliBaseScript and other classes in the picocli.groovy package.

I don’t think it’s optional.

Why do you want to make it optional?

@aadrian
Copy link
Author

aadrian commented Nov 9, 2017

Why do you want to make it optional?

If it's used in non-groovy projects, no reason to include (and download) it.

I don’t think it’s optional.

Isn't it optional for the Java part?

@remkop
Copy link
Owner

remkop commented Nov 9, 2017

Ok. I see what you mean now.
It makes sense.

@remkop remkop added this to the 2.0.3 milestone Nov 10, 2017
@remkop
Copy link
Owner

remkop commented Nov 10, 2017

Looking at this Gradle blog, compileOnly is a nice way to do this.

    dependencies {
        compileOnly 'org.codehaus.groovy:groovy-all:2.4.10'
        testCompile 'junit:junit:4.12',
                    'org.hamcrest:hamcrest-core:1.3',
                    'org.fusesource.jansi:jansi:1.15',
                    'org.codehaus.groovy:groovy-all:2.4.10',
                    'org.apache.ivy:ivy:2.4.0' // for Intelli/J
        ivy         'org.apache.ivy:ivy:2.4.0' // for Gradle
    }

@aadrian
Copy link
Author

aadrian commented Nov 10, 2017

@remkop that's the equivalent of Maven 'provided' - a little different from 'optional'

@remkop
Copy link
Owner

remkop commented Nov 10, 2017

I think this is okay because the doc says:

Compile-only dependencies address a number of use cases, including:
•Dependencies required at compile time but never required at runtime, such as source-only annotations or annotation processors;
•Dependencies required at compile time but required at runtime only when using certain features, a.k.a. optional dependencies;
•Dependencies whose API is required at compile time but whose implementation is to be provided by a consuming library, application or runtime environment.

The second bullet point applies in our case I think.

@aadrian
Copy link
Author

aadrian commented Nov 10, 2017

@remkop I don't think it's compileOnly since it's required at runtime too, if that part of the code is used, so it's optional: one needs it if one wants that part of the code to be used, otherwise not, independent how it was compiled.

The main problem arises from the fact that there's only one JAR, where there are 2 "separate" functionality units (one for Java and one for Groovy).

optional is still not officially supported (compileOnly/provided also took years until it was integrated :) ), that's why the plug-in I mention to be able to easily generate a correct Maven POM file.

@remkop
Copy link
Owner

remkop commented Nov 10, 2017

POM for picocli-2.0.2 has this dependencies section:

  ...
  <dependencies>
    <dependency>
      <groupId>org.codehaus.groovy</groupId>
      <artifactId>groovy-all</artifactId>
      <version>2.4.10</version>
      <scope>runtime</scope>
    </dependency>
  </dependencies>

We both agree that this is not desirable.

With the compileOnly configuration in my comment above, a POM is generated that does not have any dependencies. The POM simply does not contain a dependencies section, similar to picocli versions preceding 2.0.

I believe this is acceptable: only Groovy scripts would use the picocli.groovy classes. When running a Groovy script, the groovy-all dependency is provided by Groovy.

I have tried to see what the result would be with the 'nebula-optional-base' plugin, but I have been unable to get it to work... Getting this error:

> Failed to notify project evaluation listener.
   > Cannot configure the 'publishing' extension after it has been accessed.

If we did get this to work, I believe (correct me if I'm wrong) that this would result in a POM like this:

  ...
  <dependencies>
    <dependency>
      <groupId>org.codehaus.groovy</groupId>
      <artifactId>groovy-all</artifactId>
      <version>2.4.10</version>
      <scope>runtime</scope>
      <optional>true</optional>
    </dependency>
  </dependencies>

Personally I would not mind the above but not having a dependencies section at all should also be fine, given that the picocli.groovy classes are only used by Groovy scripts. What do you think?

@aadrian
Copy link
Author

aadrian commented Nov 10, 2017

@remkop I achieved the <optional>true</optional> with that plug-in, e.g. for this project:
See the pom here: https://search.maven.org/#artifactdetails%7Ccom.miragesql%7Cmiragesql%7C2.0.0%7Cjar

and the build.gradle producing it:
https://github.com/mirage-sql/mirage/blob/2.0.0/build.gradle

Your Gradle error seems to be related with the order of applying plug-ins.

Independently how it's generated however, IMO the POM should contain <optional>true</optional> .

@remkop
Copy link
Owner

remkop commented Nov 10, 2017

Thanks for the links! I’ll give it another try but if I can’t make it work I might just go with the compileOnly/provided scope.

Do you think you’ll be any issue with the provided scope? The picocli.groovy classes will only ever be used by Groovy scripts, and we can be sure that the required dependencies are available. I can’t think of any problem with this...

@aadrian
Copy link
Author

aadrian commented Nov 10, 2017

Do you think you’ll be any issue with the provided scope?

Not really, but it would be perfect with the optional flag in POM :) .

@aadrian
Copy link
Author

aadrian commented Nov 10, 2017

and this one too: gradle/gradle#867 needs voting, in order for Gradle to be able to do out-of-the-box what Maven already did since years :) .

@remkop
Copy link
Owner

remkop commented Nov 10, 2017

I voted for the gradle/gradle#876 issue, but for the picocli-2.0.3 release I went with the compileOnly/provided scope. I could perhaps make the nebula-optional-base plugin work but I want to spend my time on other things...

Most important is ensuring that picocli no longer has a required runtime dependency on groovy, and this has been achieved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants