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 <keeganw...@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 <remko.po...@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 <keeganw...@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 <remko.po...@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 <keeganw...@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.po...@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.po...@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 <keeganw...@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 >>>>>>>> >>>>>> >>>> >>> >>
