headers
Laszlo Nagy | 11 Sep 2009 11:24
Picon

Re: darcs patch: Fix the version-range parser to allow arbitrary expres...

Hi Malcom,

This patch contains a typo, and does not compile for me. It's easy to
fix and I did. After that run simple test cases against it and it
failed. Here you can find my tests:

  http://sites.google.com/site/rizsotto/patches/cabal-ticket212.darcs

And I also tried to solve this ticket earlier, here is my solution:

  http://sites.google.com/site/rizsotto/patches/cabal-ticket484.darcs

Regards
-- LAca

On Thu, Sep 10, 2009 at 9:15 PM,  <Malcolm.Wallace <at> cs.york.ac.uk> wrote:
> Thu Sep 10 20:09:12 BST 2009  Malcolm.Wallace <at> cs.york.ac.uk
>  * Fix the version-range parser to allow arbitrary expressions over constraints.
>  Previously, only a single conjunction (&&) or disjunction (||) was
>  parseable, despite an internal representation that permits arbitrary
>  combinations.  Now, any sequence of (&&) and (||) combining forms is
>  parsed.  (&&) binds more tightly than (||).
>
> _______________________________________________
> cabal-devel mailing list
> cabal-devel <at> haskell.org
> http://www.haskell.org/mailman/listinfo/cabal-devel
>
>
(Continue reading)

Permalink | Reply |
headers
Malcolm Wallace | 11 Sep 2009 12:52
Picon

Re: darcs patch: Fix the version-range parser to allow arbitrary expres...

Laszlo,

> This patch contains a typo, and does not compile for me.

Yes, I sent it off too quickly.  I attach a revised version of the  
patch, that fixes the typo, improves the parser by allowing  
parentheses in version range expressions, and adds a check that the  
new syntax is accepted only by cabal >= 1.8.

>  http://sites.google.com/site/rizsotto/patches/cabal-ticket212.darcs

I have been having great trouble finding a way to test my patch - e.g.  
darcs cabal-install does not build with darcs Cabal currently.  Since  
you appear to have a testing framework that is not yet incorporated  
into the standard Cabal repo, I would be very grateful if you could  
test the revised patch and let me know of any remaining errors.

> And I also tried to solve this ticket earlier, here is my solution:

Regards,
     Malcolm
Attachment (cabal.email): application/applefile, 71 bytes
Attachment (cabal.email): application/octet-stream, 51 KiB
(Continue reading)

Permalink | Reply |
headers
Malcolm Wallace | 11 Sep 2009 13:22
Picon

Re: darcs patch: Fix the version-range parser to allow arbitrary expres...

> I attach a revised version of the patch, that fixes the typo,  
> improves the parser by allowing parentheses in version range  
> expressions, and adds a check that the new syntax is accepted only  
> by cabal >= 1.8.

Argh! That patch still contains a very obvious bug.  Here is yet  
another revised version.

Regards,
     Malcolm
Attachment (patch.versionranges): application/applefile, 79 bytes
Attachment (patch.versionranges): application/octet-stream, 51 KiB
> I attach a revised version of the patch, that fixes the typo,  
> improves the parser by allowing parentheses in version range  
> expressions, and adds a check that the new syntax is accepted only  
> by cabal >= 1.8.

Argh! That patch still contains a very obvious bug.  Here is yet  
another revised version.

Regards,
     Malcolm
Permalink | Reply |
headers
Duncan Coutts | 17 Sep 2009 18:54
Picon
Picon
Favicon

Re: darcs patch: Fix the version-range parser to allow arbitrary expres...

On Fri, 2009-09-11 at 12:22 +0100, Malcolm Wallace wrote:
> > I attach a revised version of the patch, that fixes the typo,  
> > improves the parser by allowing parentheses in version range  
> > expressions, and adds a check that the new syntax is accepted only  
> > by cabal >= 1.8.
> 
> Argh! That patch still contains a very obvious bug.  Here is yet  
> another revised version.

Applied.

The scope for problems in this bit of the code seems quite high. I fixed
two more issues. :-)

The check on using the new syntax was not quite right. The problem is
how we represent >= in terms of UnionVersionRanges (EarlierVersion v)
(ThisVersion v). For example this dep violated the check: ">= 1 && < 2"

I also fixed the pretty printer to respect operator precedence.

Duncan
Permalink | Reply |
headers
Laszlo Nagy | 11 Sep 2009 13:15
Picon

Re: darcs patch: Fix the version-range parser to allow arbitrary expres...

Hi Malcolm,

My test based on QC2 where I generate random VersionRange values,
format them as a string, parse them back and check are they the same.
So if you modify only the parsing part it will fail. As it is now. (It
might be correct parsing although. :))

Regards,
-- LAca

On Fri, Sep 11, 2009 at 12:52 PM, Malcolm Wallace
<Malcolm.Wallace <at> cs.york.ac.uk> wrote:
> Laszlo,
>
>> This patch contains a typo, and does not compile for me.
>
> Yes, I sent it off too quickly.  I attach a revised version of the patch,
> that fixes the typo, improves the parser by allowing parentheses in version
> range expressions, and adds a check that the new syntax is accepted only by
> cabal >= 1.8.
>
>>  http://sites.google.com/site/rizsotto/patches/cabal-ticket212.darcs
>
> I have been having great trouble finding a way to test my patch - e.g. darcs
> cabal-install does not build with darcs Cabal currently.  Since you appear
> to have a testing framework that is not yet incorporated into the standard
> Cabal repo, I would be very grateful if you could test the revised patch and
> let me know of any remaining errors.
>
>> And I also tried to solve this ticket earlier, here is my solution:
(Continue reading)

Permalink | Reply |

Gmane