New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AesonException when downloading nightly since 2017-08-13 #3345

Closed
YoEight opened this Issue Aug 14, 2017 · 19 comments

Comments

Projects
None yet
8 participants
@YoEight

YoEight commented Aug 14, 2017

Greetings,

when stack build I got that output:

Downloaded nightly-2017-08-14 build plan.    
AesonException "Error in $.packages.cassava.constraints.flags['bytestring--lt-0_10_4']: Invalid flag name: \"bytestring--lt-0_10_4\""

I got this since nightly-2017-08-13.

It works if I switch to nightly-2017-08-12.

stack version: 1.5.1

snoyberg added a commit that referenced this issue Aug 14, 2017

@hvr

This comment has been minimized.

hvr commented Aug 14, 2017

Sounds like Stack reinvents its own parsing...

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 14, 2017

@hvr Perhaps you could help by providing a reference to the Cabal specification that states the rules for flag name parsing? I was unable to find such a document. I'm also certain that, in the spirit of assistance, you'd be happy to change the flag name in cassava in such a way that conforms to the current parser in Stack, as obviously that will have no negative impact on your workflow.

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 14, 2017

@YoEight Herbert is correct, Stack implements its own parsing of flag names (and a few other fields). I've pushed a commit that seems to bring the parsing in line with Cabal's parsing, though it's just a guess based on looking at the code as I have not found any official flag name spec.

Thank you for the report, we'll get this fixed at the Stackage level, I've already pinged the curator on duty right now.

@YoEight

This comment has been minimized.

YoEight commented Aug 14, 2017

@snoyberg @hvr Thanks for the quick fix 👍

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 14, 2017

Just to be clear: you still won't be able to use those snapshots unless you use the commit I pushed to Stack (which isn't even on master yet). But I'm sure Herbert will be happy to help with a quick release to Hackage so that you'll be able to get the latest cassava in an upcoming nightly.

@YoEight

This comment has been minimized.

YoEight commented Aug 14, 2017

Thanks. This isn't a big issue for me. I was trying to fix my stack nightly resolver build for a lib I maintain. I'm fine with a 2 days old nightly.

@hvr

This comment has been minimized.

hvr commented Aug 14, 2017

@snoyberg You'll surely understand I don't see any point to ineffectively workaround non-compliance bugs in Stack/Stackage, when the reference implementation in Cabal/Hackage works just fine, and would actually have a negative impact on Cabal/Hackage (that's all I'm gonna say here; everything beyond that would require a lengthy blogpost to elaborate). I do agree though, that the specification of valid flag names isn't spelled out in the docs, it's currently basically the regexp [A-Za-z0-9_-]+ with case-insensitive equality. I'll try to remedy that asap. (And fwiw, I'm considering disallowing leading -s)

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 14, 2017

No, I surely do not understand. Can you explain why you are unwilling to make a trivial tweak which will have zero negative impact on cabal users and a positive impact for Stack users?

snoyberg added a commit to commercialhaskell/stackage that referenced this issue Aug 14, 2017

snoyberg added a commit that referenced this issue Aug 14, 2017

Merge pull request #3349 from commercialhaskell/3345-fix-flag-parser
Modify flag parser to allow sequential dashes #3345
@chrisdone

This comment has been minimized.

Member

chrisdone commented Aug 14, 2017

Stack just had a better parser, which handled the separators as actual separators (- and _), naturally disallowing consecutive occurrences. The fact that Cabal's parser didn't bother to validate that properly doesn't really legitimize your use of it here, if anything it shows that Stack flagged up a bug with Cabal's flag parsing. Seeing as you refuse to fix this weirdness, we've lobotomized the parser appropriately.

Package names are also validated,

$ stack new foo--bar
Expected valid package name, but got: foo--bar
Package names consist of one or more alphanumeric words separated by hyphens.
To avoid ambiguity with version numbers, each of these words must contain at least one letter.


Usage: stack new PACKAGE_NAME [--bare] [TEMPLATE_NAME] [-p|--param KEY:VALUE]
                 [DIR] [--solver] [--omit-packages] [--force] [--ignore-subdirs]
                 [--help]
  Create a new project from a template. Run `stack templates' to see available
  templates.

Fortunately this validation is done for package names in Cabal too, as we can see in this user-friendly error message:

$ cabal init --package-name foo--bar
Cannot parse package name: foo--bar
CallStack (from HasCallStack):
error, called at libraries/Cabal/Cabal/Distribution/ReadE.hs:44:24 in
Cabal-1.24.0.0:Distribution.ReadE

So we're safe from @hvr creating package names like cassava--wibble.

@hvr

This comment has been minimized.

hvr commented Aug 14, 2017

@chrisdone Clearly, everyone's entitled to their opinion; however, package name identifiers and flag name identifiers don't use the same syntax, and there's a rationale why package name identifiers are less liberal than flag name identifiers. But your apparent hostility seems to imply that you don't really care to know, so I won't waste both our time to elaborate. (Should I have misread you, and you genuinely want to know, let me know)

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 14, 2017

@hvr I don't see the hostility you're implying @chrisdone showed, he was accurately describing the chain of events here. However, I'm still interested in your response to my question above:

Can you explain why you are unwilling to make a trivial tweak which will have zero negative impact on cabal users and a positive impact for Stack users?

@chrisdone

This comment has been minimized.

Member

chrisdone commented Aug 14, 2017

@hvr I think package names being restricted is a great idea. It makes them predictable, easy to recognize and parse (compatibly) and combine with other suffixes.

From all of Stackage, this is the first time anyone (you) has diverged from the format my parser was validating, I think that's evidence enough of a standard format right there. It's not that I just think it's aesthetically pleasing or something.

@alanz

This comment has been minimized.

Contributor

alanz commented Aug 14, 2017

I think this all points to the need for a proper grammar/spec for cabal files.

@kgadek

This comment has been minimized.

kgadek commented Aug 15, 2017

I've linked the issue I created on fpco/stackage about the same problem — initially I had no idea about root cause, only later traced it to the stack's parser.

I've already pinged the curator on duty right now.

@snoyberg, is there a ticket to trace that? If so, I'll just close my ticket as duplicate.

You'll surely understand I don't see any point to ineffectively workaround non-compliance bugs in Stack/Stackage, (…)

@hvr, please consider that some people do use Stackage/Stack daily. Breaking software happens and it's fine. However, I can't see any valid argument from you not to fix a problem… Please reconsider your decision; note that stack/Stackage is used by quite a bit of people.

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 15, 2017

@kgadek I didn't open an issue, I just checked with the curator team and then pushed commercialhaskell/stackage@4d8e405.

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 15, 2017

This issue is resolved on master, the next nightly will have a workaround until the upcoming Stack release, and it seems that upstream will not be making any accommodations here (nor explain the reasons why not). I don't think there's much else we can do here, so closing the issue. Thanks for the report.

@snoyberg snoyberg closed this Aug 15, 2017

@ajnsit

This comment has been minimized.

ajnsit commented Aug 15, 2017

Not blaming anyone, not judging, nor claiming any authority to preach, but I soo wish the leaders of the Haskell community would start getting along better :(

@hvr

This comment has been minimized.

hvr commented Aug 15, 2017

@ajnsit I agree, it would be nice, but I don't see this happening any time soon (that's all I can say w/o getting off-topic & airing dirty laundry -- please don't ask), sadly. At some point you just have to accept that some people just don't get along with each other, and it's best for everyone involved if they keep their distance rather than forcing them to work together.

@kgadek I totally agree that this problem should be fixed, but the problem isn't at my end (except for -- and here I totally agree with Alan -- bringing up to speed the .cabal specification which has bitrotted since its initial publication of CABAL 1.0) as nothing I do will undo the fact that the package index contains at least one .cabal file that Stack chokes on; it's like toothpaste out of the tube. There's no doubt in my view, that the bugs were clearly at Stackage's & Stack's end. The very first bug was that stackage-curator failed to validate its YAML input and allowed a snapshot to be generated with an identifier which Stack's validation would choke on - I'd expect stackage-curator either to be at least as strict as Stack in its validation, or alternatively (and that's something we failed ourselves recently w/ 2.0) to have better QA to validate snapshots against released Stack versions before publishing them; the second bug was that Stack had made obviously wrong assumption about what is considered a valid flag identifier, the only thing one can argue here is that we're currently in implementation-as-spec land -- that's clearly suboptimal --, but I already mentioned a couple of weeks ago that updating the specification is on the roadmap (for multiple reasons actually, but that's a different story). In any case, this is much ado about nothing, as all it took to workaround this is to rollback the snapshot (NB: even if I had renamed the flag, the earliest time this would have had any effect would have been today's snapshot) and Stack will soon have a bugfix release anyway. Programming errors happen from time to time, and they get fixed; that's how software development works. Can we please move on to another topic already?

tfausak added a commit to tfausak/cassava that referenced this issue Sep 8, 2017

Use more compatible flag name
This flag name was changed without comment in 6e1ff78. It has caused a few problems:

- commercialhaskell/stack#3345
- commercialhaskell/stackage#2755
- commercialhaskell/stackage#2759
- commercialhaskell/stackage#2842
- haskell/cabal#4686
- haskell-hvr#150

Those problems either caused or accelerated some (attempted) changes in Cabal:

- haskell/cabal#4654
- haskell/cabal#4687
- haskell/cabal#4696

Those problems also caused a change in Stack:

- commercialhaskell/stack#3349

In short: Cabal never had any trouble with this. Stack did. The current master version of Stack can handle flags like this, but no released versions of it can. It's not clear when the next version of Stack will be released. In the meantime, no Stack users can use this package. This commit changes the offending flag to something that Stack can handle. By using a flag that Stack can handle, Stack users can once again use this package, and it can return to Stackage. There are no apparent downsides to using a more compatible flag name.

DanBurton added a commit to commercialhaskell/stackage that referenced this issue Dec 8, 2017

DanBurton added a commit to commercialhaskell/stackage that referenced this issue Dec 8, 2017

DanBurton added a commit to commercialhaskell/stackage that referenced this issue Dec 8, 2017

@h4ck3rm1k3

This comment has been minimized.

@commercialhaskell commercialhaskell locked as resolved and limited conversation to collaborators Dec 20, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.