You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@groovy.apache.org by Keegan Witt <ke...@gmail.com> on 2018/05/06 18:35:16 UTC

Groovy 2.5.0-rc-2 breaks Gant

FYI 2.5.0-rc-2 breaks Gant.  Specifically, it's caused by changing
groovy.util.CliBuilder to use Picocli

java.lang.reflect.InvocationTargetException
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at
org.codehaus.groovy.tools.GroovyStarter.rootLoader(GroovyStarter.java:114)
        at
org.codehaus.groovy.tools.GroovyStarter.main(GroovyStarter.java:136)
Caused by: groovy.lang.ReadOnlyPropertyException: Cannot set readonly
property: parser for class: groovy.util.CliBuilder
        at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:2746)
        at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:3782)
        at groovy.lang.MetaClassImpl.setProperties(MetaClassImpl.java:1759)
        at
org.codehaus.groovy.runtime.callsite.ConstructorSite$NoParamSite.callConstructor(ConstructorSite.java:125)
        at
org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:59)
        at
org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:238)
        at
org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:250)
        at gant.Gant.processArgs(Gant.groovy:463)
        at gant.Gant$processArgs.call(Unknown Source)
        at
org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:47)
        at
org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:116)
        at
org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:128)
        at gant.Gant.main(Gant.groovy:668)
        ... 6 more

The line in Gant is
def cli = new CliBuilder(usage: 'gant [option]* [target]*', parser: new
GnuParser())

Was this breakage intentional?  I think a lot of stuff will break with
parser not being able to be set anymore.

-Keegan

Re: Groovy 2.5.0-rc-3 fails to break Gant [was Groovy 2.5.0-rc-2 breaks Gant]

Posted by Remko Popma <re...@gmail.com>.
Glad to hear that.
Remko

On Wed, May 23, 2018 at 1:55 Russel Winder <ru...@winder.org.uk> wrote:

> Hi,
>
> All Gant master/HEAD tests pass with Groovy 2.5.0-rc-3.
>
> :-)
>
> --
> Russel.
> ==========================================
> Dr Russel Winder      t: +44 20 7585 2200
> 41 Buckmaster Road    m: +44 7770 465 077
> London SW11 1EN, UK   w: www.russel.org.uk
>

Re: Groovy 2.5.0-rc-3 fails to break Gant [was Groovy 2.5.0-rc-2 breaks Gant]

Posted by Russel Winder <ru...@winder.org.uk>.
Hi,

All Gant master/HEAD tests pass with Groovy 2.5.0-rc-3.

:-)

-- 
Russel.
==========================================
Dr Russel Winder      t: +44 20 7585 2200
41 Buckmaster Road    m: +44 7770 465 077
London SW11 1EN, UK   w: www.russel.org.uk

Re: Groovy 2.5.0-rc-2 breaks Gant

Posted by Russel Winder <ru...@winder.org.uk>.
On Fri, 2018-05-18 at 22:29 +0900, Remko Popma wrote:
[…]
> Just out of interest, will Gant migrate to groovy.cli.commons.CliBuilder or
> groovy.cli.picocli.CliBuilder?

Well on the one hand, Gant is a dead project that no-one cares about. It
hasn't had a release since Codehaus died, mainly because I can't work out how
to get a release into Bintray/Artifactory/Maven Central – mostly because no-
one uses Gant and do not actually care, so I don't work on it (*).

On the other hand I do regularly build it just to see what is broken. Mostly
just to be amazed at how broken it has become. Let's get 2.5.0-rc-3 out there
and I might take a more serious look just for the hell of it.

Gant is based on ancient Groovy and it's build on ancient Gradle, technically
it all needs a significant rewrite, but Gant was a thing of 2006 to 2008 (**)
so well past it's use by date – unless someone know differently.



(*) Much of this also applies to GPars.

(**) Because Gradle. Obviously. Gant lost it's raison d'etre when Hans and
Adam started Gradle in late 2007. And good for Hans he did a great job.

-- 
Russel.
==========================================
Dr Russel Winder      t: +44 20 7585 2200
41 Buckmaster Road    m: +44 7770 465 077
London SW11 1EN, UK   w: www.russel.org.uk

Re: Groovy 2.5.0-rc-2 breaks Gant

Posted by Remko Popma <re...@gmail.com>.
Paul changed `groovy.util.CliBuilder` to delegate to `groovy.cli.commons.CliBuilder` just now.

That should get rid of the reported error. 

groovy.util.CliBuilder is now deprecated. 
Just out of interest, will Gant migrate to groovy.cli.commons.CliBuilder or groovy.cli.picocli.CliBuilder?

Remko

> On May 18, 2018, at 22:14, Keegan Witt <ke...@gmail.com> wrote:
> 
> So do you want to change this back for RC3?  I know it's late to change this, but arguably it was late to change it in RC2 too.
> 
> -Keegan
> 
> 
>> On Thu, May 10, 2018, 11:08 AM Remko Popma <re...@gmail.com> wrote:
>> I updated the PR to make the signature of `setPosix` accept Boolean instead of boolean and added tests to ensure that nulls are handled correctly. Thanks for pointing that out! 
>> 
>> The picocli version of CliBuilder has tests to verify that Java-like properties such as -Dkey=value are processed correctly in the new version just like in the commons-cli version. (For multiple properties the option can be specified multiple times: -Da=b -Dx=z )
>> 
>> I have not created a dummy setter for `setOptions` for the reason you mentioned:
>> if users use this to configure the options it is probably better to fail with a compiler error than silently ignoring such options.
>> 
>> It's hard to tell how many people would be impacted by this, but if we want 2.5 to be compatible with 2.4 we cannot make picocli the default in 2.5.
>> 
>> 
>> 
>>> On Thu, May 10, 2018 at 8:42 AM, Keegan Witt <ke...@gmail.com> wrote:
>>> Here's all the differences between 2.4 and 2.5
>>> setFormatter(org.apache.commons.cli.HelpFormatter)  (in the PR)
>>> setParser(org.apache.commons.cli.CommandLineParser)   (in the PR)
>>> setOptions(org.apache.commons.cli.Options)
>>> setPosix(java.lang.Boolean)  (changed from boolean to Boolean, also it's deprecated)
>>> I'd guess setFormatter() wouldn't be a very common thing to do (I think it'd only really come up if HelpFormatter were subclassed for some reason), so that one's probably not a big risk.  But I'm not sure setOptions() is safe to dummy up -- folks might be relying on this to configure the options to parse.  This might be a reason not to default to picocli yet.  What do you think?
>>> 
>>> I also looked at how current arguments passed on the commandline could break with the move, and the only thing I noticed was Commons CLI says it can do Java-like properties such as -Djava.awt.headless=true (although I didn't see how you'd actually configure these options unless you jammed all of them into 1 giant option).
>>> 
>>> -Keegan
>>> 
>>> 
>>>> On Tue, May 8, 2018 at 1:06 AM, Paul King <pa...@asert.com.au> wrote:
>>>> Short answer is we can have as many RCs as needed to get it right but we don't really want to be chopping and changing too much. Big changes are supposed to be settling down by now - fate just brought this change late in the game compared to what we'd like. I'd really like to get +ve feedback from a SNAPSHOT with that change before doing another release including it. I can merge in a couple of days time after conference commitments.
>>>> 
>>>> Cheers, Paul.
>>>> 
>>>>> On Tue, May 8, 2018 at 2:27 PM, Remko Popma <re...@gmail.com> wrote:
>>>>> 
>>>>> I realize now that simply removing the `parser` and `formatter` properties was a mistake. While the picocli version of CliBuilder cannot use commons-cli classes I should have anticipated that some applications are setting these properties.
>>>>> 
>>>>> I believe that I have a solution that provides a smoother migration path.  PR 
>>>>> https://github.com/apache/groovy/pull/696 adds dummy setters for the legacy properties. 
>>>>> 
>>>>> This should significantly reduce the disruption for applications like Gant that depend on the ability to modify these commons-cli properties in CliBuilder. 
>>>>> 
>>>>> Would it be possible to have another 2.5-rc release with the above change and see what the community feedback is before we decide to revert the CliBuilder delegation to point to the commons-cli implementation?
>>>>> 
>>>>> Marking the delegation version class as deprecated is probably a good idea regardless of where it points to. 
>>>>> 
>>>>> Remko
>>>>> 
>>>>> 
>>>>>> On Tue, May 8, 2018 at 3:19 Paul King <pa...@asert.com.au> wrote:
>>>>>> 
>>>>>> Both CliBuilder implementations are there via their full package names, so encouraging people to use the non-delegated implementation of their choice is a good option. They will need to get used to such changes in Groovy 3 anyway.
>>>>>> 
>>>>>> For Groovy 3, we might have a way for the compiler to translate from the existing package names to the new ones brought about by JDK9 modules, in which case we'd do that for CliBuilder and we'd want to point it to the picocli package name by then (though if we do the change below we perhaps wouldn't include CliBuilder in the translations list).
>>>>>> 
>>>>>> For 2.5, we don't have the package translation in place and don't intend to. Because of that, the point of the delegation version is to aid in migration.
>>>>>> Maybe it was too big a jump to try to delegate to the Picocli delegation in 2.5. If we find too many problems I think I should revert that change and delegate to the commons cli implementation instead but mark the whole delegation version class as deprecated.
>>>>>> 
>>>>>> Let me know what people think.
>>>>>> 
>>>>>> Cheers, Paul.
>>>>>> 
>>>>>> 
>>>>>>> On Mon, May 7, 2018 at 3:45 PM, Keegan Witt <ke...@gmail.com> wrote:
>>>>>>> I'll take a look.  I'll also see if there are other places that can break.
>>>>>>> 
>>>>>>> If there are several ways it can break, then maybe we should not use a delegate to picocli and instead have folks switch the CliBuilder instance they instantiate.
>>>>>>> 
>>>>>>> -Keegan
>>>>>>> 
>>>>>>>> On Sun, May 6, 2018, 7:56 PM Remko Popma <re...@gmail.com> wrote:
>>>>>>>> I think I found a way to fix this.
>>>>>>>> See https://github.com/apache/groovy/pull/696
>>>>>>>> This PR adds CliBuilder.setParser and CliBuilder.setFormatter methods that ignore the specified value and print a warning to stderr.
>>>>>>>> 
>>>>>>>>> On Sun, May 6, 2018 at 9:14 PM, Remko Popma <re...@gmail.com> wrote:
>>>>>>>>> In 2.5.0-rc-2, groovy.util.CliBuilder delegates to groovy.cli.picocli.CliBuilder. The error is that the `parser` property of this class is no longer writable. 
>>>>>>>>> 
>>>>>>>>> You can resolve this with 2.5.0-rc-2 by either not setting the `parser` property in the CliBuilder constructor or using the groovy.cli.commons.CliBuilder instead. 
>>>>>>>>> 
>>>>>>>>> On the Groovy side I’m not sure what the best way is to make the transition easier.  The picocli version of CliBuilder can not make use of the Commons-CLI parser class.  We could modify CliBuilder to silently ignore the specified parser.  (We’d have to rename the picocli ParserSpec `parser` property in CliBuilder to something else.) 
>>>>>>>>> 
>>>>>>>>> Thoughts?
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>> On Sun, May 6, 2018 at 20:35 Keegan Witt <ke...@gmail.com> wrote:
>>>>>>>>>> FYI 2.5.0-rc-2 breaks Gant.  Specifically, it's caused by changing groovy.util.CliBuilder to use Picocli
>>>>>>>>>> 
>>>>>>>>>> java.lang.reflect.InvocationTargetException
>>>>>>>>>>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>>>>>>>>>>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>>>>>>>>>>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>>>>>>>>>         at java.lang.reflect.Method.invoke(Method.java:498)
>>>>>>>>>>         at org.codehaus.groovy.tools.GroovyStarter.rootLoader(GroovyStarter.java:114)
>>>>>>>>>>         at org.codehaus.groovy.tools.GroovyStarter.main(GroovyStarter.java:136)
>>>>>>>>>> Caused by: groovy.lang.ReadOnlyPropertyException: Cannot set readonly property: parser for class: groovy.util.CliBuilder
>>>>>>>>>>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:2746)
>>>>>>>>>>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:3782)
>>>>>>>>>>         at groovy.lang.MetaClassImpl.setProperties(MetaClassImpl.java:1759)
>>>>>>>>>>         at org.codehaus.groovy.runtime.callsite.ConstructorSite$NoParamSite.callConstructor(ConstructorSite.java:125)
>>>>>>>>>>         at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:59)
>>>>>>>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:238)
>>>>>>>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:250)
>>>>>>>>>>         at gant.Gant.processArgs(Gant.groovy:463)
>>>>>>>>>>         at gant.Gant$processArgs.call(Unknown Source)
>>>>>>>>>>         at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:47)
>>>>>>>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:116)
>>>>>>>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:128)
>>>>>>>>>>         at gant.Gant.main(Gant.groovy:668)
>>>>>>>>>>         ... 6 more
>>>>>>>>>> 
>>>>>>>>>> The line in Gant is
>>>>>>>>>> def cli = new CliBuilder(usage: 'gant [option]* [target]*', parser: new GnuParser())
>>>>>>>>>> 
>>>>>>>>>> Was this breakage intentional?  I think a lot of stuff will break with parser not being able to be set anymore.
>>>>>>>>>> 
>>>>>>>>>> -Keegan
>>>>>>>> 
>>>>>> 
>>>> 
>>> 
>> 

Re: Groovy 2.5.0-rc-2 breaks Gant

Posted by Keegan Witt <ke...@gmail.com>.
So do you want to change this back for RC3?  I know it's late to change
this, but arguably it was late to change it in RC2 too.

-Keegan


On Thu, May 10, 2018, 11:08 AM Remko Popma <re...@gmail.com> wrote:

> I updated the PR to make the signature of `setPosix` accept Boolean
> instead of boolean and added tests to ensure that nulls are handled
> correctly. Thanks for pointing that out!
>
> The picocli version of CliBuilder has tests to verify that Java-like
> properties such as -Dkey=value are processed correctly in the new version
> just like in the commons-cli version. (For multiple properties the option
> can be specified multiple times: -Da=b -Dx=z )
>
> I have not created a dummy setter for `setOptions` for the reason you
> mentioned:
> if users use this to configure the options it is probably better to fail
> with a compiler error than silently ignoring such options.
>
> It's hard to tell how many people would be impacted by this, but if we
> want 2.5 to be compatible with 2.4 we cannot make picocli the default in
> 2.5.
>
>
>
> On Thu, May 10, 2018 at 8:42 AM, Keegan Witt <ke...@gmail.com> wrote:
>
>> Here's all the differences between 2.4 and 2.5
>>
>>    - setFormatter(org.apache.commons.cli.HelpFormatter)  (in the PR)
>>    - setParser(org.apache.commons.cli.CommandLineParser)   (in the PR)
>>    - setOptions(org.apache.commons.cli.Options)
>>    - setPosix(java.lang.Boolean)  (changed from boolean to Boolean, also
>>    it's deprecated)
>>
>> I'd guess setFormatter() wouldn't be a very common thing to do (I think
>> it'd only really come up if HelpFormatter were subclassed for some
>> reason), so that one's probably not a big risk.  But I'm not sure
>> setOptions() is safe to dummy up -- folks might be relying on this to
>> configure the options to parse.  This might be a reason not to default to
>> picocli yet.  What do you think?
>>
>> I also looked at how current arguments passed on the commandline could
>> break with the move, and the only thing I noticed was Commons CLI says it
>> can do Java-like properties such as -Djava.awt.headless=true (although I
>> didn't see how you'd actually configure these options unless you jammed all
>> of them into 1 giant option).
>>
>> -Keegan
>>
>>
>> On Tue, May 8, 2018 at 1:06 AM, Paul King <pa...@asert.com.au> wrote:
>>
>>> Short answer is we can have as many RCs as needed to get it right but we
>>> don't really want to be chopping and changing too much. Big changes are
>>> supposed to be settling down by now - fate just brought this change late in
>>> the game compared to what we'd like. I'd really like to get +ve feedback
>>> from a SNAPSHOT with that change before doing another release including it.
>>> I can merge in a couple of days time after conference commitments.
>>>
>>> Cheers, Paul.
>>>
>>> On Tue, May 8, 2018 at 2:27 PM, Remko Popma <re...@gmail.com>
>>> wrote:
>>>
>>>>
>>>> I realize now that simply removing the `parser` and `formatter`
>>>> properties was a mistake. While the picocli version of CliBuilder cannot
>>>> use commons-cli classes I should have anticipated that some applications
>>>> are setting these properties.
>>>>
>>>> I believe that I have a solution that provides a smoother migration
>>>> path.  PR
>>>> https://github.com/apache/groovy/pull/696 adds dummy setters for the
>>>> legacy properties.
>>>>
>>>> This should significantly reduce the disruption for applications like
>>>> Gant that depend on the ability to modify these commons-cli properties in
>>>> CliBuilder.
>>>>
>>>> Would it be possible to have another 2.5-rc release with the above
>>>> change and see what the community feedback is before we decide to revert
>>>> the CliBuilder delegation to point to the commons-cli implementation?
>>>>
>>>> Marking the delegation version class as deprecated is probably a good
>>>> idea regardless of where it points to.
>>>>
>>>> Remko
>>>>
>>>>
>>>> On Tue, May 8, 2018 at 3:19 Paul King <pa...@asert.com.au> wrote:
>>>>
>>>>>
>>>>> Both CliBuilder implementations are there via their full package
>>>>> names, so encouraging people to use the non-delegated implementation of
>>>>> their choice is a good option. They will need to get used to such changes
>>>>> in Groovy 3 anyway.
>>>>>
>>>>> For Groovy 3, we might have a way for the compiler to translate from
>>>>> the existing package names to the new ones brought about by JDK9 modules,
>>>>> in which case we'd do that for CliBuilder and we'd want to point it to the
>>>>> picocli package name by then (though if we do the change below we perhaps
>>>>> wouldn't include CliBuilder in the translations list).
>>>>>
>>>>> For 2.5, we don't have the package translation in place and don't
>>>>> intend to. Because of that, the point of the delegation version is to aid
>>>>> in migration.
>>>>> Maybe it was too big a jump to try to delegate to the Picocli
>>>>> delegation in 2.5. If we find too many problems I think I should revert
>>>>> that change and delegate to the commons cli implementation instead but mark
>>>>> the whole delegation version class as deprecated.
>>>>>
>>>>> Let me know what people think.
>>>>>
>>>>> Cheers, Paul.
>>>>>
>>>>>
>>>>> On Mon, May 7, 2018 at 3:45 PM, Keegan Witt <ke...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> I'll take a look.  I'll also see if there are other places that can
>>>>>> break.
>>>>>>
>>>>>> If there are several ways it can break, then maybe we should not use
>>>>>> a delegate to picocli and instead have folks switch the CliBuilder instance
>>>>>> they instantiate.
>>>>>>
>>>>>> -Keegan
>>>>>>
>>>>>> On Sun, May 6, 2018, 7:56 PM Remko Popma <re...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> I think I found a way to fix this.
>>>>>>> See https://github.com/apache/groovy/pull/696
>>>>>>> This PR adds CliBuilder.setParser and CliBuilder.setFormatter
>>>>>>> methods that ignore the specified value and print a warning to stderr.
>>>>>>>
>>>>>>> On Sun, May 6, 2018 at 9:14 PM, Remko Popma <re...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> In 2.5.0-rc-2, groovy.util.CliBuilder delegates to
>>>>>>>> groovy.cli.picocli.CliBuilder. The error is that the `parser` property of
>>>>>>>> this class is no longer writable.
>>>>>>>>
>>>>>>>> You can resolve this with 2.5.0-rc-2 by either not setting the
>>>>>>>> `parser` property in the CliBuilder constructor or using the
>>>>>>>> groovy.cli.commons.CliBuilder instead.
>>>>>>>>
>>>>>>>> On the Groovy side I’m not sure what the best way is to make the
>>>>>>>> transition easier.  The picocli version of CliBuilder can not make use of
>>>>>>>> the Commons-CLI parser class.  We could modify CliBuilder to silently
>>>>>>>> ignore the specified parser.  (We’d have to rename the picocli ParserSpec
>>>>>>>> `parser` property in CliBuilder to something else.)
>>>>>>>>
>>>>>>>> Thoughts?
>>>>>>>>
>>>>>>>>
>>>>>>>> On Sun, May 6, 2018 at 20:35 Keegan Witt <ke...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> FYI 2.5.0-rc-2 breaks Gant.  Specifically, it's caused by changing
>>>>>>>>> groovy.util.CliBuilder to use Picocli
>>>>>>>>>
>>>>>>>>> java.lang.reflect.InvocationTargetException
>>>>>>>>>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native
>>>>>>>>> Method)
>>>>>>>>>         at sun.reflect.NativeMethodAccessorImpl.invoke(
>>>>>>>>> NativeMethodAccessorImpl.java:62)
>>>>>>>>>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(
>>>>>>>>> DelegatingMethodAccessorImpl.java:43)
>>>>>>>>>         at java.lang.reflect.Method.invoke(Method.java:498)
>>>>>>>>>         at org.codehaus.groovy.tools.GroovyStarter.rootLoader(
>>>>>>>>> GroovyStarter.java:114)
>>>>>>>>>         at org.codehaus.groovy.tools.GroovyStarter.main(
>>>>>>>>> GroovyStarter.java:136)
>>>>>>>>> Caused by: groovy.lang.ReadOnlyPropertyException: Cannot set
>>>>>>>>> readonly property: parser for class: groovy.util.CliBuilder
>>>>>>>>>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.
>>>>>>>>> java:2746)
>>>>>>>>>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.
>>>>>>>>> java:3782)
>>>>>>>>>         at groovy.lang.MetaClassImpl.setProperties(MetaClassImpl.
>>>>>>>>> java:1759)
>>>>>>>>>         at org.codehaus.groovy.runtime.callsite.ConstructorSite$
>>>>>>>>> NoParamSite.callConstructor(ConstructorSite.java:125)
>>>>>>>>>         at org.codehaus.groovy.runtime.callsite.CallSiteArray.
>>>>>>>>> defaultCallConstructor(CallSiteArray.java:59)
>>>>>>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>>>>>>>>> callConstructor(AbstractCallSite.java:238)
>>>>>>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>>>>>>>>> callConstructor(AbstractCallSite.java:250)
>>>>>>>>>         at gant.Gant.processArgs(Gant.groovy:463)
>>>>>>>>>         at gant.Gant$processArgs.call(Unknown Source)
>>>>>>>>>         at org.codehaus.groovy.runtime.callsite.CallSiteArray.
>>>>>>>>> defaultCall(CallSiteArray.java:47)
>>>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>>>> llsite.AbstractCallSite.call(AbstractCallSite.java:116)
>>>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>>>> llsite.AbstractCallSite.call(AbstractCallSite.java:128)
>>>>>>>>>         at gant.Gant.main(Gant.groovy:668)
>>>>>>>>>         ... 6 more
>>>>>>>>>
>>>>>>>>> The line in Gant is
>>>>>>>>> def cli = new CliBuilder(usage: 'gant [option]* [target]*', parser:
>>>>>>>>> new GnuParser())
>>>>>>>>>
>>>>>>>>> Was this breakage intentional?  I think a lot of stuff will break
>>>>>>>>> with parser not being able to be set anymore.
>>>>>>>>>
>>>>>>>>> -Keegan
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>
>>
>

Re: Groovy 2.5.0-rc-2 breaks Gant

Posted by Remko Popma <re...@gmail.com>.
I updated the PR to make the signature of `setPosix` accept Boolean instead
of boolean and added tests to ensure that nulls are handled correctly.
Thanks for pointing that out!

The picocli version of CliBuilder has tests to verify that Java-like
properties such as -Dkey=value are processed correctly in the new version
just like in the commons-cli version. (For multiple properties the option
can be specified multiple times: -Da=b -Dx=z )

I have not created a dummy setter for `setOptions` for the reason you
mentioned:
if users use this to configure the options it is probably better to fail
with a compiler error than silently ignoring such options.

It's hard to tell how many people would be impacted by this, but if we want
2.5 to be compatible with 2.4 we cannot make picocli the default in 2.5.



On Thu, May 10, 2018 at 8:42 AM, Keegan Witt <ke...@gmail.com> wrote:

> Here's all the differences between 2.4 and 2.5
>
>    - setFormatter(org.apache.commons.cli.HelpFormatter)  (in the PR)
>    - setParser(org.apache.commons.cli.CommandLineParser)   (in the PR)
>    - setOptions(org.apache.commons.cli.Options)
>    - setPosix(java.lang.Boolean)  (changed from boolean to Boolean, also
>    it's deprecated)
>
> I'd guess setFormatter() wouldn't be a very common thing to do (I think
> it'd only really come up if HelpFormatter were subclassed for some
> reason), so that one's probably not a big risk.  But I'm not sure
> setOptions() is safe to dummy up -- folks might be relying on this to
> configure the options to parse.  This might be a reason not to default to
> picocli yet.  What do you think?
>
> I also looked at how current arguments passed on the commandline could
> break with the move, and the only thing I noticed was Commons CLI says it
> can do Java-like properties such as -Djava.awt.headless=true (although I
> didn't see how you'd actually configure these options unless you jammed all
> of them into 1 giant option).
>
> -Keegan
>
>
> On Tue, May 8, 2018 at 1:06 AM, Paul King <pa...@asert.com.au> wrote:
>
>> Short answer is we can have as many RCs as needed to get it right but we
>> don't really want to be chopping and changing too much. Big changes are
>> supposed to be settling down by now - fate just brought this change late in
>> the game compared to what we'd like. I'd really like to get +ve feedback
>> from a SNAPSHOT with that change before doing another release including it.
>> I can merge in a couple of days time after conference commitments.
>>
>> Cheers, Paul.
>>
>> On Tue, May 8, 2018 at 2:27 PM, Remko Popma <re...@gmail.com>
>> wrote:
>>
>>>
>>> I realize now that simply removing the `parser` and `formatter`
>>> properties was a mistake. While the picocli version of CliBuilder cannot
>>> use commons-cli classes I should have anticipated that some applications
>>> are setting these properties.
>>>
>>> I believe that I have a solution that provides a smoother migration
>>> path.  PR
>>> https://github.com/apache/groovy/pull/696 adds dummy setters for the
>>> legacy properties.
>>>
>>> This should significantly reduce the disruption for applications like
>>> Gant that depend on the ability to modify these commons-cli properties in
>>> CliBuilder.
>>>
>>> Would it be possible to have another 2.5-rc release with the above
>>> change and see what the community feedback is before we decide to revert
>>> the CliBuilder delegation to point to the commons-cli implementation?
>>>
>>> Marking the delegation version class as deprecated is probably a good
>>> idea regardless of where it points to.
>>>
>>> Remko
>>>
>>>
>>> On Tue, May 8, 2018 at 3:19 Paul King <pa...@asert.com.au> wrote:
>>>
>>>>
>>>> Both CliBuilder implementations are there via their full package names,
>>>> so encouraging people to use the non-delegated implementation of their
>>>> choice is a good option. They will need to get used to such changes in
>>>> Groovy 3 anyway.
>>>>
>>>> For Groovy 3, we might have a way for the compiler to translate from
>>>> the existing package names to the new ones brought about by JDK9 modules,
>>>> in which case we'd do that for CliBuilder and we'd want to point it to the
>>>> picocli package name by then (though if we do the change below we perhaps
>>>> wouldn't include CliBuilder in the translations list).
>>>>
>>>> For 2.5, we don't have the package translation in place and don't
>>>> intend to. Because of that, the point of the delegation version is to aid
>>>> in migration.
>>>> Maybe it was too big a jump to try to delegate to the Picocli
>>>> delegation in 2.5. If we find too many problems I think I should revert
>>>> that change and delegate to the commons cli implementation instead but mark
>>>> the whole delegation version class as deprecated.
>>>>
>>>> Let me know what people think.
>>>>
>>>> Cheers, Paul.
>>>>
>>>>
>>>> On Mon, May 7, 2018 at 3:45 PM, Keegan Witt <ke...@gmail.com>
>>>> wrote:
>>>>
>>>>> I'll take a look.  I'll also see if there are other places that can
>>>>> break.
>>>>>
>>>>> If there are several ways it can break, then maybe we should not use a
>>>>> delegate to picocli and instead have folks switch the CliBuilder instance
>>>>> they instantiate.
>>>>>
>>>>> -Keegan
>>>>>
>>>>> On Sun, May 6, 2018, 7:56 PM Remko Popma <re...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> I think I found a way to fix this.
>>>>>> See https://github.com/apache/groovy/pull/696
>>>>>> This PR adds CliBuilder.setParser and CliBuilder.setFormatter methods
>>>>>> that ignore the specified value and print a warning to stderr.
>>>>>>
>>>>>> On Sun, May 6, 2018 at 9:14 PM, Remko Popma <re...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> In 2.5.0-rc-2, groovy.util.CliBuilder delegates to
>>>>>>> groovy.cli.picocli.CliBuilder. The error is that the `parser` property of
>>>>>>> this class is no longer writable.
>>>>>>>
>>>>>>> You can resolve this with 2.5.0-rc-2 by either not setting the
>>>>>>> `parser` property in the CliBuilder constructor or using the
>>>>>>> groovy.cli.commons.CliBuilder instead.
>>>>>>>
>>>>>>> On the Groovy side I’m not sure what the best way is to make the
>>>>>>> transition easier.  The picocli version of CliBuilder can not make use of
>>>>>>> the Commons-CLI parser class.  We could modify CliBuilder to silently
>>>>>>> ignore the specified parser.  (We’d have to rename the picocli ParserSpec
>>>>>>> `parser` property in CliBuilder to something else.)
>>>>>>>
>>>>>>> Thoughts?
>>>>>>>
>>>>>>>
>>>>>>> On Sun, May 6, 2018 at 20:35 Keegan Witt <ke...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> FYI 2.5.0-rc-2 breaks Gant.  Specifically, it's caused by changing
>>>>>>>> groovy.util.CliBuilder to use Picocli
>>>>>>>>
>>>>>>>> java.lang.reflect.InvocationTargetException
>>>>>>>>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native
>>>>>>>> Method)
>>>>>>>>         at sun.reflect.NativeMethodAccess
>>>>>>>> orImpl.invoke(NativeMethodAccessorImpl.java:62)
>>>>>>>>         at sun.reflect.DelegatingMethodAc
>>>>>>>> cessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>>>>>>>         at java.lang.reflect.Method.invoke(Method.java:498)
>>>>>>>>         at org.codehaus.groovy.tools.Groo
>>>>>>>> vyStarter.rootLoader(GroovyStarter.java:114)
>>>>>>>>         at org.codehaus.groovy.tools.Groo
>>>>>>>> vyStarter.main(GroovyStarter.java:136)
>>>>>>>> Caused by: groovy.lang.ReadOnlyPropertyException: Cannot set
>>>>>>>> readonly property: parser for class: groovy.util.CliBuilder
>>>>>>>>         at groovy.lang.MetaClassImpl.setP
>>>>>>>> roperty(MetaClassImpl.java:2746)
>>>>>>>>         at groovy.lang.MetaClassImpl.setP
>>>>>>>> roperty(MetaClassImpl.java:3782)
>>>>>>>>         at groovy.lang.MetaClassImpl.setP
>>>>>>>> roperties(MetaClassImpl.java:1759)
>>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>>> llsite.ConstructorSite$NoParamSite.callConstructor(Construct
>>>>>>>> orSite.java:125)
>>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>>> llsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:59)
>>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>>> llsite.AbstractCallSite.callConstructor(AbstractCallSite.java:238)
>>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>>> llsite.AbstractCallSite.callConstructor(AbstractCallSite.java:250)
>>>>>>>>         at gant.Gant.processArgs(Gant.groovy:463)
>>>>>>>>         at gant.Gant$processArgs.call(Unknown Source)
>>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>>> llsite.CallSiteArray.defaultCall(CallSiteArray.java:47)
>>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>>> llsite.AbstractCallSite.call(AbstractCallSite.java:116)
>>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>>> llsite.AbstractCallSite.call(AbstractCallSite.java:128)
>>>>>>>>         at gant.Gant.main(Gant.groovy:668)
>>>>>>>>         ... 6 more
>>>>>>>>
>>>>>>>> The line in Gant is
>>>>>>>> def cli = new CliBuilder(usage: 'gant [option]* [target]*', parser:
>>>>>>>> new GnuParser())
>>>>>>>>
>>>>>>>> Was this breakage intentional?  I think a lot of stuff will break
>>>>>>>> with parser not being able to be set anymore.
>>>>>>>>
>>>>>>>> -Keegan
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>
>

Re: Groovy 2.5.0-rc-2 breaks Gant

Posted by Keegan Witt <ke...@gmail.com>.
 Here's all the differences between 2.4 and 2.5

   - setFormatter(org.apache.commons.cli.HelpFormatter)  (in the PR)
   - setParser(org.apache.commons.cli.CommandLineParser)   (in the PR)
   - setOptions(org.apache.commons.cli.Options)
   - setPosix(java.lang.Boolean)  (changed from boolean to Boolean, also
   it's deprecated)

I'd guess setFormatter() wouldn't be a very common thing to do (I think
it'd only really come up if HelpFormatter were subclassed for some reason),
so that one's probably not a big risk.  But I'm not sure setOptions() is
safe to dummy up -- folks might be relying on this to configure the options
to parse.  This might be a reason not to default to picocli yet.  What do
you think?

I also looked at how current arguments passed on the commandline could
break with the move, and the only thing I noticed was Commons CLI says it
can do Java-like properties such as -Djava.awt.headless=true (although I
didn't see how you'd actually configure these options unless you jammed all
of them into 1 giant option).

-Keegan


On Tue, May 8, 2018 at 1:06 AM, Paul King <pa...@asert.com.au> wrote:

> Short answer is we can have as many RCs as needed to get it right but we
> don't really want to be chopping and changing too much. Big changes are
> supposed to be settling down by now - fate just brought this change late in
> the game compared to what we'd like. I'd really like to get +ve feedback
> from a SNAPSHOT with that change before doing another release including it.
> I can merge in a couple of days time after conference commitments.
>
> Cheers, Paul.
>
> On Tue, May 8, 2018 at 2:27 PM, Remko Popma <re...@gmail.com> wrote:
>
>>
>> I realize now that simply removing the `parser` and `formatter`
>> properties was a mistake. While the picocli version of CliBuilder cannot
>> use commons-cli classes I should have anticipated that some applications
>> are setting these properties.
>>
>> I believe that I have a solution that provides a smoother migration
>> path.  PR
>> https://github.com/apache/groovy/pull/696 adds dummy setters for the
>> legacy properties.
>>
>> This should significantly reduce the disruption for applications like
>> Gant that depend on the ability to modify these commons-cli properties in
>> CliBuilder.
>>
>> Would it be possible to have another 2.5-rc release with the above change
>> and see what the community feedback is before we decide to revert the
>> CliBuilder delegation to point to the commons-cli implementation?
>>
>> Marking the delegation version class as deprecated is probably a good
>> idea regardless of where it points to.
>>
>> Remko
>>
>>
>> On Tue, May 8, 2018 at 3:19 Paul King <pa...@asert.com.au> wrote:
>>
>>>
>>> Both CliBuilder implementations are there via their full package names,
>>> so encouraging people to use the non-delegated implementation of their
>>> choice is a good option. They will need to get used to such changes in
>>> Groovy 3 anyway.
>>>
>>> For Groovy 3, we might have a way for the compiler to translate from the
>>> existing package names to the new ones brought about by JDK9 modules, in
>>> which case we'd do that for CliBuilder and we'd want to point it to the
>>> picocli package name by then (though if we do the change below we perhaps
>>> wouldn't include CliBuilder in the translations list).
>>>
>>> For 2.5, we don't have the package translation in place and don't intend
>>> to. Because of that, the point of the delegation version is to aid in
>>> migration.
>>> Maybe it was too big a jump to try to delegate to the Picocli delegation
>>> in 2.5. If we find too many problems I think I should revert that change
>>> and delegate to the commons cli implementation instead but mark the whole
>>> delegation version class as deprecated.
>>>
>>> Let me know what people think.
>>>
>>> Cheers, Paul.
>>>
>>>
>>> On Mon, May 7, 2018 at 3:45 PM, Keegan Witt <ke...@gmail.com>
>>> wrote:
>>>
>>>> I'll take a look.  I'll also see if there are other places that can
>>>> break.
>>>>
>>>> If there are several ways it can break, then maybe we should not use a
>>>> delegate to picocli and instead have folks switch the CliBuilder instance
>>>> they instantiate.
>>>>
>>>> -Keegan
>>>>
>>>> On Sun, May 6, 2018, 7:56 PM Remko Popma <re...@gmail.com> wrote:
>>>>
>>>>> I think I found a way to fix this.
>>>>> See https://github.com/apache/groovy/pull/696
>>>>> This PR adds CliBuilder.setParser and CliBuilder.setFormatter methods
>>>>> that ignore the specified value and print a warning to stderr.
>>>>>
>>>>> On Sun, May 6, 2018 at 9:14 PM, Remko Popma <re...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> In 2.5.0-rc-2, groovy.util.CliBuilder delegates to
>>>>>> groovy.cli.picocli.CliBuilder. The error is that the `parser` property of
>>>>>> this class is no longer writable.
>>>>>>
>>>>>> You can resolve this with 2.5.0-rc-2 by either not setting the
>>>>>> `parser` property in the CliBuilder constructor or using the
>>>>>> groovy.cli.commons.CliBuilder instead.
>>>>>>
>>>>>> On the Groovy side I’m not sure what the best way is to make the
>>>>>> transition easier.  The picocli version of CliBuilder can not make use of
>>>>>> the Commons-CLI parser class.  We could modify CliBuilder to silently
>>>>>> ignore the specified parser.  (We’d have to rename the picocli ParserSpec
>>>>>> `parser` property in CliBuilder to something else.)
>>>>>>
>>>>>> Thoughts?
>>>>>>
>>>>>>
>>>>>> On Sun, May 6, 2018 at 20:35 Keegan Witt <ke...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> FYI 2.5.0-rc-2 breaks Gant.  Specifically, it's caused by changing
>>>>>>> groovy.util.CliBuilder to use Picocli
>>>>>>>
>>>>>>> java.lang.reflect.InvocationTargetException
>>>>>>>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native
>>>>>>> Method)
>>>>>>>         at sun.reflect.NativeMethodAccess
>>>>>>> orImpl.invoke(NativeMethodAccessorImpl.java:62)
>>>>>>>         at sun.reflect.DelegatingMethodAc
>>>>>>> cessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>>>>>>         at java.lang.reflect.Method.invoke(Method.java:498)
>>>>>>>         at org.codehaus.groovy.tools.Groo
>>>>>>> vyStarter.rootLoader(GroovyStarter.java:114)
>>>>>>>         at org.codehaus.groovy.tools.Groo
>>>>>>> vyStarter.main(GroovyStarter.java:136)
>>>>>>> Caused by: groovy.lang.ReadOnlyPropertyException: Cannot set
>>>>>>> readonly property: parser for class: groovy.util.CliBuilder
>>>>>>>         at groovy.lang.MetaClassImpl.setP
>>>>>>> roperty(MetaClassImpl.java:2746)
>>>>>>>         at groovy.lang.MetaClassImpl.setP
>>>>>>> roperty(MetaClassImpl.java:3782)
>>>>>>>         at groovy.lang.MetaClassImpl.setP
>>>>>>> roperties(MetaClassImpl.java:1759)
>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>> llsite.ConstructorSite$NoParamSite.callConstructor(Construct
>>>>>>> orSite.java:125)
>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>> llsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:59)
>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>> llsite.AbstractCallSite.callConstructor(AbstractCallSite.java:238)
>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>> llsite.AbstractCallSite.callConstructor(AbstractCallSite.java:250)
>>>>>>>         at gant.Gant.processArgs(Gant.groovy:463)
>>>>>>>         at gant.Gant$processArgs.call(Unknown Source)
>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>> llsite.CallSiteArray.defaultCall(CallSiteArray.java:47)
>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>> llsite.AbstractCallSite.call(AbstractCallSite.java:116)
>>>>>>>         at org.codehaus.groovy.runtime.ca
>>>>>>> llsite.AbstractCallSite.call(AbstractCallSite.java:128)
>>>>>>>         at gant.Gant.main(Gant.groovy:668)
>>>>>>>         ... 6 more
>>>>>>>
>>>>>>> The line in Gant is
>>>>>>> def cli = new CliBuilder(usage: 'gant [option]* [target]*', parser:
>>>>>>> new GnuParser())
>>>>>>>
>>>>>>> Was this breakage intentional?  I think a lot of stuff will break
>>>>>>> with parser not being able to be set anymore.
>>>>>>>
>>>>>>> -Keegan
>>>>>>>
>>>>>>
>>>>>
>>>
>

Re: Groovy 2.5.0-rc-2 breaks Gant

Posted by Paul King <pa...@asert.com.au>.
Short answer is we can have as many RCs as needed to get it right but we
don't really want to be chopping and changing too much. Big changes are
supposed to be settling down by now - fate just brought this change late in
the game compared to what we'd like. I'd really like to get +ve feedback
from a SNAPSHOT with that change before doing another release including it.
I can merge in a couple of days time after conference commitments.

Cheers, Paul.

On Tue, May 8, 2018 at 2:27 PM, Remko Popma <re...@gmail.com> wrote:

>
> I realize now that simply removing the `parser` and `formatter` properties
> was a mistake. While the picocli version of CliBuilder cannot use
> commons-cli classes I should have anticipated that some applications are
> setting these properties.
>
> I believe that I have a solution that provides a smoother migration path.
> PR
> https://github.com/apache/groovy/pull/696 adds dummy setters for the
> legacy properties.
>
> This should significantly reduce the disruption for applications like Gant
> that depend on the ability to modify these commons-cli properties in
> CliBuilder.
>
> Would it be possible to have another 2.5-rc release with the above change
> and see what the community feedback is before we decide to revert the
> CliBuilder delegation to point to the commons-cli implementation?
>
> Marking the delegation version class as deprecated is probably a good idea
> regardless of where it points to.
>
> Remko
>
>
> On Tue, May 8, 2018 at 3:19 Paul King <pa...@asert.com.au> wrote:
>
>>
>> Both CliBuilder implementations are there via their full package names,
>> so encouraging people to use the non-delegated implementation of their
>> choice is a good option. They will need to get used to such changes in
>> Groovy 3 anyway.
>>
>> For Groovy 3, we might have a way for the compiler to translate from the
>> existing package names to the new ones brought about by JDK9 modules, in
>> which case we'd do that for CliBuilder and we'd want to point it to the
>> picocli package name by then (though if we do the change below we perhaps
>> wouldn't include CliBuilder in the translations list).
>>
>> For 2.5, we don't have the package translation in place and don't intend
>> to. Because of that, the point of the delegation version is to aid in
>> migration.
>> Maybe it was too big a jump to try to delegate to the Picocli delegation
>> in 2.5. If we find too many problems I think I should revert that change
>> and delegate to the commons cli implementation instead but mark the whole
>> delegation version class as deprecated.
>>
>> Let me know what people think.
>>
>> Cheers, Paul.
>>
>>
>> On Mon, May 7, 2018 at 3:45 PM, Keegan Witt <ke...@gmail.com> wrote:
>>
>>> I'll take a look.  I'll also see if there are other places that can
>>> break.
>>>
>>> If there are several ways it can break, then maybe we should not use a
>>> delegate to picocli and instead have folks switch the CliBuilder instance
>>> they instantiate.
>>>
>>> -Keegan
>>>
>>> On Sun, May 6, 2018, 7:56 PM Remko Popma <re...@gmail.com> wrote:
>>>
>>>> I think I found a way to fix this.
>>>> See https://github.com/apache/groovy/pull/696
>>>> This PR adds CliBuilder.setParser and CliBuilder.setFormatter methods
>>>> that ignore the specified value and print a warning to stderr.
>>>>
>>>> On Sun, May 6, 2018 at 9:14 PM, Remko Popma <re...@gmail.com>
>>>> wrote:
>>>>
>>>>> In 2.5.0-rc-2, groovy.util.CliBuilder delegates to
>>>>> groovy.cli.picocli.CliBuilder. The error is that the `parser` property of
>>>>> this class is no longer writable.
>>>>>
>>>>> You can resolve this with 2.5.0-rc-2 by either not setting the
>>>>> `parser` property in the CliBuilder constructor or using the
>>>>> groovy.cli.commons.CliBuilder instead.
>>>>>
>>>>> On the Groovy side I’m not sure what the best way is to make the
>>>>> transition easier.  The picocli version of CliBuilder can not make use of
>>>>> the Commons-CLI parser class.  We could modify CliBuilder to silently
>>>>> ignore the specified parser.  (We’d have to rename the picocli ParserSpec
>>>>> `parser` property in CliBuilder to something else.)
>>>>>
>>>>> Thoughts?
>>>>>
>>>>>
>>>>> On Sun, May 6, 2018 at 20:35 Keegan Witt <ke...@gmail.com> wrote:
>>>>>
>>>>>> FYI 2.5.0-rc-2 breaks Gant.  Specifically, it's caused by changing
>>>>>> groovy.util.CliBuilder to use Picocli
>>>>>>
>>>>>> java.lang.reflect.InvocationTargetException
>>>>>>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native
>>>>>> Method)
>>>>>>         at sun.reflect.NativeMethodAccessorImpl.invoke(
>>>>>> NativeMethodAccessorImpl.java:62)
>>>>>>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(
>>>>>> DelegatingMethodAccessorImpl.java:43)
>>>>>>         at java.lang.reflect.Method.invoke(Method.java:498)
>>>>>>         at org.codehaus.groovy.tools.GroovyStarter.rootLoader(
>>>>>> GroovyStarter.java:114)
>>>>>>         at org.codehaus.groovy.tools.GroovyStarter.main(
>>>>>> GroovyStarter.java:136)
>>>>>> Caused by: groovy.lang.ReadOnlyPropertyException: Cannot set
>>>>>> readonly property: parser for class: groovy.util.CliBuilder
>>>>>>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.
>>>>>> java:2746)
>>>>>>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.
>>>>>> java:3782)
>>>>>>         at groovy.lang.MetaClassImpl.setProperties(MetaClassImpl.
>>>>>> java:1759)
>>>>>>         at org.codehaus.groovy.runtime.callsite.ConstructorSite$
>>>>>> NoParamSite.callConstructor(ConstructorSite.java:125)
>>>>>>         at org.codehaus.groovy.runtime.callsite.CallSiteArray.
>>>>>> defaultCallConstructor(CallSiteArray.java:59)
>>>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>>>>>> callConstructor(AbstractCallSite.java:238)
>>>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>>>>>> callConstructor(AbstractCallSite.java:250)
>>>>>>         at gant.Gant.processArgs(Gant.groovy:463)
>>>>>>         at gant.Gant$processArgs.call(Unknown Source)
>>>>>>         at org.codehaus.groovy.runtime.callsite.CallSiteArray.
>>>>>> defaultCall(CallSiteArray.java:47)
>>>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>>>>>> call(AbstractCallSite.java:116)
>>>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>>>>>> call(AbstractCallSite.java:128)
>>>>>>         at gant.Gant.main(Gant.groovy:668)
>>>>>>         ... 6 more
>>>>>>
>>>>>> The line in Gant is
>>>>>> def cli = new CliBuilder(usage: 'gant [option]* [target]*', parser:
>>>>>> new GnuParser())
>>>>>>
>>>>>> Was this breakage intentional?  I think a lot of stuff will break
>>>>>> with parser not being able to be set anymore.
>>>>>>
>>>>>> -Keegan
>>>>>>
>>>>>
>>>>
>>

Re: Groovy 2.5.0-rc-2 breaks Gant

Posted by Remko Popma <re...@gmail.com>.
I realize now that simply removing the `parser` and `formatter` properties
was a mistake. While the picocli version of CliBuilder cannot use
commons-cli classes I should have anticipated that some applications are
setting these properties.

I believe that I have a solution that provides a smoother migration path.
PR
https://github.com/apache/groovy/pull/696 adds dummy setters for the legacy
properties.

This should significantly reduce the disruption for applications like Gant
that depend on the ability to modify these commons-cli properties in
CliBuilder.

Would it be possible to have another 2.5-rc release with the above change
and see what the community feedback is before we decide to revert the
CliBuilder delegation to point to the commons-cli implementation?

Marking the delegation version class as deprecated is probably a good idea
regardless of where it points to.

Remko


On Tue, May 8, 2018 at 3:19 Paul King <pa...@asert.com.au> wrote:

>
> Both CliBuilder implementations are there via their full package names, so
> encouraging people to use the non-delegated implementation of their choice
> is a good option. They will need to get used to such changes in Groovy 3
> anyway.
>
> For Groovy 3, we might have a way for the compiler to translate from the
> existing package names to the new ones brought about by JDK9 modules, in
> which case we'd do that for CliBuilder and we'd want to point it to the
> picocli package name by then (though if we do the change below we perhaps
> wouldn't include CliBuilder in the translations list).
>
> For 2.5, we don't have the package translation in place and don't intend
> to. Because of that, the point of the delegation version is to aid in
> migration.
> Maybe it was too big a jump to try to delegate to the Picocli delegation
> in 2.5. If we find too many problems I think I should revert that change
> and delegate to the commons cli implementation instead but mark the whole
> delegation version class as deprecated.
>
> Let me know what people think.
>
> Cheers, Paul.
>
>
> On Mon, May 7, 2018 at 3:45 PM, Keegan Witt <ke...@gmail.com> wrote:
>
>> I'll take a look.  I'll also see if there are other places that can break.
>>
>> If there are several ways it can break, then maybe we should not use a
>> delegate to picocli and instead have folks switch the CliBuilder instance
>> they instantiate.
>>
>> -Keegan
>>
>> On Sun, May 6, 2018, 7:56 PM Remko Popma <re...@gmail.com> wrote:
>>
>>> I think I found a way to fix this.
>>> See https://github.com/apache/groovy/pull/696
>>> This PR adds CliBuilder.setParser and CliBuilder.setFormatter methods
>>> that ignore the specified value and print a warning to stderr.
>>>
>>> On Sun, May 6, 2018 at 9:14 PM, Remko Popma <re...@gmail.com>
>>> wrote:
>>>
>>>> In 2.5.0-rc-2, groovy.util.CliBuilder delegates to
>>>> groovy.cli.picocli.CliBuilder. The error is that the `parser` property of
>>>> this class is no longer writable.
>>>>
>>>> You can resolve this with 2.5.0-rc-2 by either not setting the `parser`
>>>> property in the CliBuilder constructor or using the
>>>> groovy.cli.commons.CliBuilder instead.
>>>>
>>>> On the Groovy side I’m not sure what the best way is to make the
>>>> transition easier.  The picocli version of CliBuilder can not make use of
>>>> the Commons-CLI parser class.  We could modify CliBuilder to silently
>>>> ignore the specified parser.  (We’d have to rename the picocli ParserSpec
>>>> `parser` property in CliBuilder to something else.)
>>>>
>>>> Thoughts?
>>>>
>>>>
>>>> On Sun, May 6, 2018 at 20:35 Keegan Witt <ke...@gmail.com> wrote:
>>>>
>>>>> FYI 2.5.0-rc-2 breaks Gant.  Specifically, it's caused by changing
>>>>> groovy.util.CliBuilder to use Picocli
>>>>>
>>>>> java.lang.reflect.InvocationTargetException
>>>>>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>>>>>         at
>>>>> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>>>>>         at
>>>>> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>>>>         at java.lang.reflect.Method.invoke(Method.java:498)
>>>>>         at
>>>>> org.codehaus.groovy.tools.GroovyStarter.rootLoader(GroovyStarter.java:114)
>>>>>         at
>>>>> org.codehaus.groovy.tools.GroovyStarter.main(GroovyStarter.java:136)
>>>>> Caused by: groovy.lang.ReadOnlyPropertyException: Cannot set readonly
>>>>> property: parser for class: groovy.util.CliBuilder
>>>>>         at
>>>>> groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:2746)
>>>>>         at
>>>>> groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:3782)
>>>>>         at
>>>>> groovy.lang.MetaClassImpl.setProperties(MetaClassImpl.java:1759)
>>>>>         at
>>>>> org.codehaus.groovy.runtime.callsite.ConstructorSite$NoParamSite.callConstructor(ConstructorSite.java:125)
>>>>>         at
>>>>> org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:59)
>>>>>         at
>>>>> org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:238)
>>>>>         at
>>>>> org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:250)
>>>>>         at gant.Gant.processArgs(Gant.groovy:463)
>>>>>         at gant.Gant$processArgs.call(Unknown Source)
>>>>>         at
>>>>> org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:47)
>>>>>         at
>>>>> org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:116)
>>>>>         at
>>>>> org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:128)
>>>>>         at gant.Gant.main(Gant.groovy:668)
>>>>>         ... 6 more
>>>>>
>>>>> The line in Gant is
>>>>> def cli = new CliBuilder(usage: 'gant [option]* [target]*', parser:
>>>>> new GnuParser())
>>>>>
>>>>> Was this breakage intentional?  I think a lot of stuff will break with
>>>>> parser not being able to be set anymore.
>>>>>
>>>>> -Keegan
>>>>>
>>>>
>>>
>

Re: Groovy 2.5.0-rc-2 breaks Gant

Posted by Paul King <pa...@asert.com.au>.
Both CliBuilder implementations are there via their full package names, so
encouraging people to use the non-delegated implementation of their choice
is a good option. They will need to get used to such changes in Groovy 3
anyway.

For Groovy 3, we might have a way for the compiler to translate from the
existing package names to the new ones brought about by JDK9 modules, in
which case we'd do that for CliBuilder and we'd want to point it to the
picocli package name by then (though if we do the change below we perhaps
wouldn't include CliBuilder in the translations list).

For 2.5, we don't have the package translation in place and don't intend
to. Because of that, the point of the delegation version is to aid in
migration.
Maybe it was too big a jump to try to delegate to the Picocli delegation in
2.5. If we find too many problems I think I should revert that change and
delegate to the commons cli implementation instead but mark the whole
delegation version class as deprecated.

Let me know what people think.

Cheers, Paul.


On Mon, May 7, 2018 at 3:45 PM, Keegan Witt <ke...@gmail.com> wrote:

> I'll take a look.  I'll also see if there are other places that can break.
>
> If there are several ways it can break, then maybe we should not use a
> delegate to picocli and instead have folks switch the CliBuilder instance
> they instantiate.
>
> -Keegan
>
> On Sun, May 6, 2018, 7:56 PM Remko Popma <re...@gmail.com> wrote:
>
>> I think I found a way to fix this.
>> See https://github.com/apache/groovy/pull/696
>> This PR adds CliBuilder.setParser and CliBuilder.setFormatter methods
>> that ignore the specified value and print a warning to stderr.
>>
>> On Sun, May 6, 2018 at 9:14 PM, Remko Popma <re...@gmail.com>
>> wrote:
>>
>>> In 2.5.0-rc-2, groovy.util.CliBuilder delegates to
>>> groovy.cli.picocli.CliBuilder. The error is that the `parser` property of
>>> this class is no longer writable.
>>>
>>> You can resolve this with 2.5.0-rc-2 by either not setting the `parser`
>>> property in the CliBuilder constructor or using the
>>> groovy.cli.commons.CliBuilder instead.
>>>
>>> On the Groovy side I’m not sure what the best way is to make the
>>> transition easier.  The picocli version of CliBuilder can not make use of
>>> the Commons-CLI parser class.  We could modify CliBuilder to silently
>>> ignore the specified parser.  (We’d have to rename the picocli ParserSpec
>>> `parser` property in CliBuilder to something else.)
>>>
>>> Thoughts?
>>>
>>>
>>> On Sun, May 6, 2018 at 20:35 Keegan Witt <ke...@gmail.com> wrote:
>>>
>>>> FYI 2.5.0-rc-2 breaks Gant.  Specifically, it's caused by changing
>>>> groovy.util.CliBuilder to use Picocli
>>>>
>>>> java.lang.reflect.InvocationTargetException
>>>>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>>>>         at sun.reflect.NativeMethodAccessorImpl.invoke(
>>>> NativeMethodAccessorImpl.java:62)
>>>>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(
>>>> DelegatingMethodAccessorImpl.java:43)
>>>>         at java.lang.reflect.Method.invoke(Method.java:498)
>>>>         at org.codehaus.groovy.tools.GroovyStarter.rootLoader(
>>>> GroovyStarter.java:114)
>>>>         at org.codehaus.groovy.tools.GroovyStarter.main(
>>>> GroovyStarter.java:136)
>>>> Caused by: groovy.lang.ReadOnlyPropertyException: Cannot set readonly
>>>> property: parser for class: groovy.util.CliBuilder
>>>>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.
>>>> java:2746)
>>>>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.
>>>> java:3782)
>>>>         at groovy.lang.MetaClassImpl.setProperties(MetaClassImpl.
>>>> java:1759)
>>>>         at org.codehaus.groovy.runtime.callsite.ConstructorSite$
>>>> NoParamSite.callConstructor(ConstructorSite.java:125)
>>>>         at org.codehaus.groovy.runtime.callsite.CallSiteArray.
>>>> defaultCallConstructor(CallSiteArray.java:59)
>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>>>> callConstructor(AbstractCallSite.java:238)
>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>>>> callConstructor(AbstractCallSite.java:250)
>>>>         at gant.Gant.processArgs(Gant.groovy:463)
>>>>         at gant.Gant$processArgs.call(Unknown Source)
>>>>         at org.codehaus.groovy.runtime.callsite.CallSiteArray.
>>>> defaultCall(CallSiteArray.java:47)
>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>>>> call(AbstractCallSite.java:116)
>>>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>>>> call(AbstractCallSite.java:128)
>>>>         at gant.Gant.main(Gant.groovy:668)
>>>>         ... 6 more
>>>>
>>>> The line in Gant is
>>>> def cli = new CliBuilder(usage: 'gant [option]* [target]*', parser: new
>>>> GnuParser())
>>>>
>>>> Was this breakage intentional?  I think a lot of stuff will break with
>>>> parser not being able to be set anymore.
>>>>
>>>> -Keegan
>>>>
>>>
>>

Re: Groovy 2.5.0-rc-2 breaks Gant

Posted by Keegan Witt <ke...@gmail.com>.
I'll take a look.  I'll also see if there are other places that can break.

If there are several ways it can break, then maybe we should not use a
delegate to picocli and instead have folks switch the CliBuilder instance
they instantiate.

-Keegan

On Sun, May 6, 2018, 7:56 PM Remko Popma <re...@gmail.com> wrote:

> I think I found a way to fix this.
> See https://github.com/apache/groovy/pull/696
> This PR adds CliBuilder.setParser and CliBuilder.setFormatter methods that
> ignore the specified value and print a warning to stderr.
>
> On Sun, May 6, 2018 at 9:14 PM, Remko Popma <re...@gmail.com> wrote:
>
>> In 2.5.0-rc-2, groovy.util.CliBuilder delegates to
>> groovy.cli.picocli.CliBuilder. The error is that the `parser` property of
>> this class is no longer writable.
>>
>> You can resolve this with 2.5.0-rc-2 by either not setting the `parser`
>> property in the CliBuilder constructor or using the
>> groovy.cli.commons.CliBuilder instead.
>>
>> On the Groovy side I’m not sure what the best way is to make the
>> transition easier.  The picocli version of CliBuilder can not make use of
>> the Commons-CLI parser class.  We could modify CliBuilder to silently
>> ignore the specified parser.  (We’d have to rename the picocli ParserSpec
>> `parser` property in CliBuilder to something else.)
>>
>> Thoughts?
>>
>>
>> On Sun, May 6, 2018 at 20:35 Keegan Witt <ke...@gmail.com> wrote:
>>
>>> FYI 2.5.0-rc-2 breaks Gant.  Specifically, it's caused by changing
>>> groovy.util.CliBuilder to use Picocli
>>>
>>> java.lang.reflect.InvocationTargetException
>>>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>>>         at
>>> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>>>         at
>>> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>>         at java.lang.reflect.Method.invoke(Method.java:498)
>>>         at
>>> org.codehaus.groovy.tools.GroovyStarter.rootLoader(GroovyStarter.java:114)
>>>         at
>>> org.codehaus.groovy.tools.GroovyStarter.main(GroovyStarter.java:136)
>>> Caused by: groovy.lang.ReadOnlyPropertyException: Cannot set readonly
>>> property: parser for class: groovy.util.CliBuilder
>>>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:2746)
>>>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:3782)
>>>         at
>>> groovy.lang.MetaClassImpl.setProperties(MetaClassImpl.java:1759)
>>>         at
>>> org.codehaus.groovy.runtime.callsite.ConstructorSite$NoParamSite.callConstructor(ConstructorSite.java:125)
>>>         at
>>> org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:59)
>>>         at
>>> org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:238)
>>>         at
>>> org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:250)
>>>         at gant.Gant.processArgs(Gant.groovy:463)
>>>         at gant.Gant$processArgs.call(Unknown Source)
>>>         at
>>> org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:47)
>>>         at
>>> org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:116)
>>>         at
>>> org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:128)
>>>         at gant.Gant.main(Gant.groovy:668)
>>>         ... 6 more
>>>
>>> The line in Gant is
>>> def cli = new CliBuilder(usage: 'gant [option]* [target]*', parser: new
>>> GnuParser())
>>>
>>> Was this breakage intentional?  I think a lot of stuff will break with
>>> parser not being able to be set anymore.
>>>
>>> -Keegan
>>>
>>
>

Re: Groovy 2.5.0-rc-2 breaks Gant

Posted by Remko Popma <re...@gmail.com>.
I think I found a way to fix this.
See https://github.com/apache/groovy/pull/696
This PR adds CliBuilder.setParser and CliBuilder.setFormatter methods that
ignore the specified value and print a warning to stderr.

On Sun, May 6, 2018 at 9:14 PM, Remko Popma <re...@gmail.com> wrote:

> In 2.5.0-rc-2, groovy.util.CliBuilder delegates to
> groovy.cli.picocli.CliBuilder. The error is that the `parser` property of
> this class is no longer writable.
>
> You can resolve this with 2.5.0-rc-2 by either not setting the `parser`
> property in the CliBuilder constructor or using the
> groovy.cli.commons.CliBuilder instead.
>
> On the Groovy side I’m not sure what the best way is to make the
> transition easier.  The picocli version of CliBuilder can not make use of
> the Commons-CLI parser class.  We could modify CliBuilder to silently
> ignore the specified parser.  (We’d have to rename the picocli ParserSpec
> `parser` property in CliBuilder to something else.)
>
> Thoughts?
>
>
> On Sun, May 6, 2018 at 20:35 Keegan Witt <ke...@gmail.com> wrote:
>
>> FYI 2.5.0-rc-2 breaks Gant.  Specifically, it's caused by changing
>> groovy.util.CliBuilder to use Picocli
>>
>> java.lang.reflect.InvocationTargetException
>>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>>         at sun.reflect.NativeMethodAccessorImpl.invoke(
>> NativeMethodAccessorImpl.java:62)
>>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(
>> DelegatingMethodAccessorImpl.java:43)
>>         at java.lang.reflect.Method.invoke(Method.java:498)
>>         at org.codehaus.groovy.tools.GroovyStarter.rootLoader(
>> GroovyStarter.java:114)
>>         at org.codehaus.groovy.tools.GroovyStarter.main(
>> GroovyStarter.java:136)
>> Caused by: groovy.lang.ReadOnlyPropertyException: Cannot set readonly
>> property: parser for class: groovy.util.CliBuilder
>>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:2746)
>>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:3782)
>>         at groovy.lang.MetaClassImpl.setProperties(MetaClassImpl.
>> java:1759)
>>         at org.codehaus.groovy.runtime.callsite.ConstructorSite$
>> NoParamSite.callConstructor(ConstructorSite.java:125)
>>         at org.codehaus.groovy.runtime.callsite.CallSiteArray.
>> defaultCallConstructor(CallSiteArray.java:59)
>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>> callConstructor(AbstractCallSite.java:238)
>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>> callConstructor(AbstractCallSite.java:250)
>>         at gant.Gant.processArgs(Gant.groovy:463)
>>         at gant.Gant$processArgs.call(Unknown Source)
>>         at org.codehaus.groovy.runtime.callsite.CallSiteArray.
>> defaultCall(CallSiteArray.java:47)
>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>> call(AbstractCallSite.java:116)
>>         at org.codehaus.groovy.runtime.callsite.AbstractCallSite.
>> call(AbstractCallSite.java:128)
>>         at gant.Gant.main(Gant.groovy:668)
>>         ... 6 more
>>
>> The line in Gant is
>> def cli = new CliBuilder(usage: 'gant [option]* [target]*', parser: new
>> GnuParser())
>>
>> Was this breakage intentional?  I think a lot of stuff will break with
>> parser not being able to be set anymore.
>>
>> -Keegan
>>
>

Re: Groovy 2.5.0-rc-2 breaks Gant

Posted by Remko Popma <re...@gmail.com>.
In 2.5.0-rc-2, groovy.util.CliBuilder delegates to
groovy.cli.picocli.CliBuilder. The error is that the `parser` property of
this class is no longer writable.

You can resolve this with 2.5.0-rc-2 by either not setting the `parser`
property in the CliBuilder constructor or using the
groovy.cli.commons.CliBuilder instead.

On the Groovy side I’m not sure what the best way is to make the transition
easier.  The picocli version of CliBuilder can not make use of the
Commons-CLI parser class.  We could modify CliBuilder to silently ignore
the specified parser.  (We’d have to rename the picocli ParserSpec `parser`
property in CliBuilder to something else.)

Thoughts?


On Sun, May 6, 2018 at 20:35 Keegan Witt <ke...@gmail.com> wrote:

> FYI 2.5.0-rc-2 breaks Gant.  Specifically, it's caused by changing
> groovy.util.CliBuilder to use Picocli
>
> java.lang.reflect.InvocationTargetException
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>         at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>         at java.lang.reflect.Method.invoke(Method.java:498)
>         at
> org.codehaus.groovy.tools.GroovyStarter.rootLoader(GroovyStarter.java:114)
>         at
> org.codehaus.groovy.tools.GroovyStarter.main(GroovyStarter.java:136)
> Caused by: groovy.lang.ReadOnlyPropertyException: Cannot set readonly
> property: parser for class: groovy.util.CliBuilder
>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:2746)
>         at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:3782)
>         at groovy.lang.MetaClassImpl.setProperties(MetaClassImpl.java:1759)
>         at
> org.codehaus.groovy.runtime.callsite.ConstructorSite$NoParamSite.callConstructor(ConstructorSite.java:125)
>         at
> org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:59)
>         at
> org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:238)
>         at
> org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:250)
>         at gant.Gant.processArgs(Gant.groovy:463)
>         at gant.Gant$processArgs.call(Unknown Source)
>         at
> org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:47)
>         at
> org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:116)
>         at
> org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:128)
>         at gant.Gant.main(Gant.groovy:668)
>         ... 6 more
>
> The line in Gant is
> def cli = new CliBuilder(usage: 'gant [option]* [target]*', parser: new
> GnuParser())
>
> Was this breakage intentional?  I think a lot of stuff will break with
> parser not being able to be set anymore.
>
> -Keegan
>