Cabal 2.2 fails on package descriptions that earlier versions of Cabal were able to parse

[sol]

Steps to reproduce:

Create a file foo.cabal

name: foo

and in ghci observe

ghci> readGenericPackageDescription silent "foo.cabal"
*** Exception: dieVerbatim: user error (<interactive>: Failed parsing "foo.cabal".

Apparently version is now required to make this parse. At the very least I would hope for a better error message. I mean Failed parsing "foo.cabal".? Come on guys, seriously?

[hvr]

Well, a package description consisting merely of

name: foo

was never truly legitimate to begin with. In the past, the validation for the version: field presence was delayed in cabal check, but we moved that one into the parser recently.

The problem is that you used silent, so you wouldn't notice this (one could argue though that this shouldn't be merely a warning, but a proper error-severity message):

> readGenericPackageDescription maxBound "foo.cabal"
Warning: foo.cabal:0:0: "version" field missing
*** Exception: dieVerbatim: user error (CallStack (from HasCallStack):
  die', called at libraries/Cabal/Cabal/Distribution/Parsec/ParseResult.hs:184:13 in Cabal-2.4.0.0:Distribution.Parsec.ParseResult
  parseString, called at libraries/Cabal/Cabal/Distribution/Parsec/ParseResult.hs:169:5 in Cabal-2.4.0.0:Distribution.Parsec.ParseResult
  readAndParseFile, called at libraries/Cabal/Cabal/Distribution/PackageDescription/Parsec.hs:89:33 in Cabal-2.4.0.0:Distribution.PackageDescription.Parsec
  readGenericPackageDescription, called at <interactive>:3:1 in interactive:Ghci1
<interactive>: Failed parsing "foo.cabal".
)

$ cabal check
Warning: foo.cabal:0:0: "version" field missing
cabal: Failed parsing "./foo.cabal".

[23Skidoo]

+1 on making that warning an error.

[sol]

@hvr thanks for pointing that out. I indeed tried verbose too and somehow missed the warning between all the stack trace output. So we can call this a user error. I think from my side there is a strong selection bias when it comes to Cabal due to all the bullshit that I have seen in the past.

What I still don't get is why this is reported as a warning first and then only later on parsing fails? But I assume you guys can point out that this is yet an other user error.

[hvr]

Btw, if you use runParseResult . parseGenericPackageDescription it is actually reported as error-level message:

> runParseResult . parseGenericPackageDescription <$> Data.ByteString.readFile "foo.cabal"
([],Left (Just (mkVersion [0]),[PError (Position 0 0) "\"version\" field missing"]))

[sol]

Apparently here is the offending code

cabal/Cabal/Distribution/PackageDescription/Parsec.hs

Line 105 in 538f126

traverse_ (warn verbosity . showPError fpath) errors

, errors report as warnings for some reason. Apparently this code has been moved around by now 5931289.

Still a bug by my book, but hey try to convince me otherwise.

[sol]

To elaborate on that, from what I understand is happening here:

1. Parsings fails with a list of error messages
2. We report these error messages as warnings
3. We then fail with a generic error message that does not provide any hints on what went wrong

[sol]

While looking into this I was surprised that my code changes are not picked up by cabal-install. After ruling out all build related issues I had to conclude that this code path is not used by cabal-install. And indeed, yes, awesomely we have multiple copies of that code. Copy & paste, good job!

[sol]

And this is not even old code, a6777f3, @phadej is copy & paste now an acceptable software development method?

[phadej]

Sorry, @sol, but you are not in a position to tell me how to do software or particularly Cabal development. I had my reasons to do that. In particular to wrap up completely rewritten parser for 2.2 release. It has been three years of low-bandwidth yet continuous work. The rewrite has been quite stressful, because it's quite important part of the code base. I'm personally very proud we haven't yet found any major regressions, but I'm not surprised there are small inconveniences here and there.

To my defense (though I don't think I need to defend myself at all) that particular change was done very close to 2.2 release, so I didn't want to make any changes to Cabal API, I'm still not sure what the API should look like.

If those are the thanks for that work, than I'm just speechless.