groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Remko Popma <remko.po...@gmail.com>
Subject Re: Groovy 2.5.0-rc-2 breaks Gant
Date Tue, 08 May 2018 04:27:03 GMT
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 <paulk@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 <keeganwitt@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 <remko.popma@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 <remko.popma@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 <keeganwitt@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
>>>>>
>>>>
>>>
>

Mime
View raw message