Allow 2.4-spec files to be parsed. #5401

quasicomputational · 2018-06-27T09:53:03Z

Lexically, they're identical to 2.2, but the semantics of globs have
changed slightly.

Please include the following checklist in your PR:

Patches conform to the coding conventions.
Any changes that could be relevant to users have been recorded in the changelog.
The documentation has been updated, if necessary.
If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

Please also shortly describe how you tested your change. Bonus points for added tests!

cc @phadej. I grepped for CabalSpecV2_2 and put in an analogous CabalSpecV2_4 wherever I found it. Is there anything else that needs doing?

quasicomputational · 2018-06-27T10:41:22Z

Hmm. There's one more wrinkle: D.S.Check has its own, independent cabal-version check. I wonder if it's worth dropping that, considering that the shiny new Parsec parser will outright refuse to parse anything it doesn't know about.

phadej · 2018-06-27T14:24:02Z

Do we have tests (parser-tests) which test that with 2.4 feature-full cabal files are rejected if they have cabal-version: 2.2 or less.

e.g. like
https://github.com/haskell/cabal/blob/4d3febdfd5848f312f579905bcfc89ca6e8be9c9/Cabal/tests/ParserTests/errors/leading-comma.cabal and
https://github.com/haskell/cabal/blob/4d3febdfd5848f312f579905bcfc89ca6e8be9c9/Cabal/tests/ParserTests/errors/leading-comma.errors

or if they are rejected by cabal check then in check-tests?

phadej · 2018-06-27T14:24:35Z

Generally, we need a separate changelog for cabal spec. @hvr probably have thought about that already.

hvr · 2018-06-27T14:27:11Z

@phadej Well, I basically wanted to add a chapter to the cabal user's guide to act as changelog -- but never got to it; we could just start one documenting changes to 2.4, and then work our way backwards when we're bor... motivated enough :-)

phadej · 2018-06-27T14:33:42Z

👍 for additional chapter. I'd really like to have it. Having another source (i.e. non defined by the implementation) would be great, Than we can say whether implementation is buggy and can be simply fixed.

EDIT: having two sources is good anyway, if they agree: good. If they don't, we can at least discuss which one is the correct or better one.

quasicomputational · 2018-06-27T14:37:36Z

AFAICT there have been no changes to the parser itself between 2.2 and 2.4.

And AFAIK the only thing gated on 2.4 is my glob stuff, which is a cabal check thing, yeah. That's tested in a few places:

Cabal/tests/ParserTests/pre-2.4-globstar.cabal and most things under cabal-testsuite/PackageTests/Check are for ensuring cabal check issues warnings
Cabal/tests/UnitTests/Distribution/Simple/Glob.hs tests the actual behaviour switching

So I'll need to add a spec changelog chapter to the user's guide? That's doable.

phadej · 2018-06-27T14:42:17Z

@quasicomputational 👍 for tests.

If you could start the chapter (as a part of separate PR) that would be super great!

As suggested in haskell#5401. [skip ci]

quasicomputational · 2018-06-27T15:03:01Z

OK, I've documented the changes in 2.4 in a new changelog in #5406.

Are there any concerns about the code changes in this PR, or other things I need to do?

As suggested in #5401. [skip ci]

23Skidoo

LGTM modulo minor comments.

23Skidoo · 2018-06-28T17:26:29Z

Cabal/Distribution/CabalSpecVersion.hs

    ]
+cabalSpecFeatures CabalSpecV2_4  = Set.fromList
+    [ Elif
+    , CommonStanzas


Would it make sense to add Globstar here?

Hmm. We could. I didn't think to because the parser is wholly agnostic to the pattern syntax. AFAICT, this 'feature' concept is only used to toggle bits of D.PD.Parsec on and off, depending on the version.

Could you please implement this then if it's not too much trouble?

23Skidoo · 2018-06-28T17:29:54Z

Cabal/Distribution/PackageDescription/Parsec.hs

    -- if we get too new version, fail right away
    case ver of
-        Just v | v > mkVersion [2,2] -> parseFailure zeroPos
+        Just v | v > mkVersion [2,4] -> parseFailure zeroPos


Hmm, why did this work before? We don't have .cabal files with cabal-version: 2.4 in the test suite?

Apparently not! I only found I needed to do this when I tried to add one in the course of #5403.

Nice, that means that with #5403 merged we will have one.

Lexically, they're identical to 2.2, but the semantics of globs have changed slightly.

quasicomputational · 2018-07-04T11:08:48Z

OK. This has been up for a week, @23Skidoo has looked at it and I've put in the Globstar feature as suggested, so I'm going to go ahead and merge.

23Skidoo · 2018-07-04T16:24:01Z

@quasicomputational

Great, thanks! BTW, was it you who did this:

* [new tag]             1283           -> 1283
* [new tag]             1285           -> 1285
* [new tag]             1311           -> 1311
* [new tag]             1312           -> 1312

?

git show 1285 says

tag 1285
Tagger: EXIA$ <EXIA$@EXIA>
Date:   Sun Jul 1 21:35:43 2018 +0100
commit c718a22fa6aa93916269537b27cde72dbe32d613 (HEAD -> master, tag: 1312, tag: 1311, tag: 1285, tag: 1283)
Author: quasicomputational <[email protected]>

quasicomputational · 2018-07-04T16:49:48Z

Huh. I have no idea what that is or how I might have done that. 1311 and 1312 could plausibly have been me fat-fingering workspace switching somehow, but I can't work out how I'd manage to accidentally tag anything and then push tags (which isn't the default).

23Skidoo · 2018-07-05T21:10:39Z

@Mistuke Apparently it's you who is responsible for pushing these tags:

21:06 < hexagoxel> refold: have you looked at https://api.github.com/repos/haskell/cabal/events ?
21:07 < hexagoxel> refold: if i am reading this stuff correctly, the pusher is account "Mistuke"
21:16 < hexagoxel> {"type": "CreateEvent", "payload": { "ref": "1348", .. }, "actor": { .. "login": "Mistuke", .. }}
21:19 < hexagoxel> (their email would be "[email protected]", according to some more api digging

Can you please stop doing that?

Mistuke · 2018-07-05T21:20:02Z

@23Skidoo well it certainly wasn't intentional, I think I got rid of the flag that was causing this. If not let me know.

quasicomputational added this to the 2.4 milestone Jun 27, 2018

quasicomputational mentioned this pull request Jun 27, 2018

cabal check improvements for globs #5403

Merged

4 tasks

quasicomputational force-pushed the bump-cabal-version-2.4 branch from 7abc22a to 88459b0 Compare June 27, 2018 13:26

quasicomputational added a commit to quasicomputational/cabal that referenced this pull request Jun 27, 2018

Add the start of a changelog for the Cabal file format

565b99f

As suggested in haskell#5401. [skip ci]

quasicomputational mentioned this pull request Jun 27, 2018

Add the start of a changelog for the Cabal file format #5406

Merged

4 tasks

hvr pushed a commit that referenced this pull request Jun 27, 2018

Add the start of a changelog for the Cabal file format

2e78642

As suggested in #5401. [skip ci]

23Skidoo approved these changes Jun 28, 2018

View reviewed changes

quasicomputational added 3 commits June 28, 2018 18:37

Allow 2.4-spec files to be parsed.

4332360

Lexically, they're identical to 2.2, but the semantics of globs have changed slightly.

Bump cabalSpecLatest as well.

a049065

Add a Globstar constructor to CabalFeature.

8160446

quasicomputational force-pushed the bump-cabal-version-2.4 branch from 88459b0 to 8160446 Compare June 28, 2018 18:00

quasicomputational merged commit 285ed5b into haskell:master Jul 4, 2018

Allow 2.4-spec files to be parsed. #5401

Allow 2.4-spec files to be parsed. #5401

Uh oh!

Conversation

quasicomputational commented Jun 27, 2018

Uh oh!

quasicomputational commented Jun 27, 2018

Uh oh!

phadej commented Jun 27, 2018

Uh oh!

phadej commented Jun 27, 2018

Uh oh!

hvr commented Jun 27, 2018

Uh oh!

phadej commented Jun 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quasicomputational commented Jun 27, 2018

Uh oh!

phadej commented Jun 27, 2018

Uh oh!

quasicomputational commented Jun 27, 2018

Uh oh!

23Skidoo left a comment

Choose a reason for hiding this comment

Uh oh!

23Skidoo Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

quasicomputational Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

23Skidoo Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

23Skidoo Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

quasicomputational Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

23Skidoo Jun 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

quasicomputational commented Jul 4, 2018

Uh oh!

23Skidoo commented Jul 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quasicomputational commented Jul 4, 2018

Uh oh!

23Skidoo commented Jul 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mistuke commented Jul 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

phadej commented Jun 27, 2018 •

edited

Loading

23Skidoo Jun 28, 2018 •

edited

Loading

23Skidoo commented Jul 4, 2018 •

edited

Loading

23Skidoo commented Jul 5, 2018 •

edited

Loading