commons-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Russel Winder <russel.win...@concertant.com>
Subject Re: Release of commons-cli?
Date Thu, 29 May 2008 07:39:14 GMT
Paul,

On Wed, 2008-05-28 at 21:27 +0100, Paul Cager wrote:

> I have done some work on CLI-137 as part of the Debian packaging. I 
> think it is quite a severe bug, as it does not allow you to handle (for 
> example) multiple "-D" arguments (as found in Maven or ant).

Exactly.  For the Groovy and Gant projects, this is not so much severe
as a complete blocker.  We upgraded 1.0 -> 1.1, made changes for the
hasArg(s) problems and still nothing about multiple options worked.
Worse all the non option parameters were absorbed into the options
processing.

> I've prepared a patch for it for the Debian build (although I do not 
> necessarily think that's the best way to fix it), and amended the JUnit 
> tests to demonstrate the problem.
> 
> I sent an email to the -dev list a few weeks ago, but I think it might 
> have gone missing. Here it is:

I certainly saw the email and was hoping there would be a response.  I
appreciate Hen is trying his best to keep Commons CLI ticking over, but
it really needs someone with commit access to take ownership of the
Commons CLI package.

Step 1 for me is to give the 1.x branch a Maven 2 build.  I will take a
Bazaar branch and see if dropping the POM from trunk in and editing
suitably does the trick.

> I've fallen over the CLI bug described in issue 137, and I'm wondering
> if I can help provide a patch for it? I'd appreciate someone just
> checking my logic in case I'm missing something obvious. I'm using
> release 1.1, by the way.

I'm afraid I haven't checked your ideas myself yet.  I'll try and get to
it tomorrow.
  
> As far as I can see it is simply not possible to use commons-cli to
> parse repeating options, such as occur in ant:
> 
>       ant -Dproperty1=value1 -Dproperty2=value2 compile
> 
> Cli *does* have a junit test for ant-type options, but it unfortunately
> doesn't catch the error because it tests the command line:
> 
>       ant -Dproperty1=value1 -Dproperty2=value2 -projecthelp
> 
> I attach a patch for the junit test (Cli-junit.patch) that will provoke 
> the error, if you want to try it yourself.
> 
> 
> _Here's what I think is happening:_
> 
> The unit test (and the work-around suggested in issue 137) creates the
> "D" option using "hasArgs()" [note the 's' on the end of the method
> name]. Using hasArgs() for the "D" option causes cli to assume that all
> arguments following the "-D" (until it encounters another option) belong
> to the "-D" option. I've probably not explained that clearly so here's
> an example:
> 
>       ant -Dproperty1=value1 -Dproperty2=value2 compile
> 
> will treat "compile" as though it was "-Dcompile". getArgs() will return
> 0 elements, rather than one ("compile").

Exactly.  The semantics behind the option processing is just broken.  I
agree that it can be seen that the 1.0 meaning of hasArg(s) is wrong
compared to the 1.1 meaning but using 1.0 there is a way of processing
multiple options, whereas with 1.1 there appears to be no workaround for
the seriously fundamental bug.

> If instead you construct the "D" option using hasArg() [singular] then
> it will *not* interpret the second "-D" as an option. getArgs() will
> return 2 elements: "property2=value2" and "compile". getOptionValues()
> will return only the first -D value.

You experience and my experience are identical I think.  Which is of
course as it should be since there is a clear and obvious bug :-)

Clearly there need to be more unit tests in the codebase, and in
particular tests that show that the 1.1 release is actually broken.
Once I have a Bazaar branch of the codebase I will be able to experiment
more to make proper test cases.

> This problem first came to light when Debian upgraded CLI from 1.0 to 
> 1.1, and caused Debian's Maven package to fail (the standard Maven 
> package uses cli 1.0 of course). As part of the Debian packaging I have 
> created a quick-and-dirty patch which allows Maven to work (see 
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=469082 for the Debian 
> bug report, and I have attached the patch (MultiOptions.patch). I would 
> guess that a proper fix would be rather more elegant than my patch.

On the other hand if your patch makes this work, then it is infinitely
better that the current situation which is that 1.1. is unusable.
-- 
Russel.
====================================================
Dr Russel Winder                 Partner

Concertant LLP                   t: +44 20 7585 2200, +44 20 7193 9203
41 Buckmaster Road,              f: +44 8700 516 084
London SW11 1EN, UK.             m: +44 7770 465 077

Mime
View raw message