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?
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".
+1 on making that warning an error.
@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.
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"]))
Apparently here is the offending code https://github.com/haskell/cabal/blob/538f1269d70b3344f5e0b784f863e9d6caa7967f/Cabal/Distribution/PackageDescription/Parsec.hs#L105, errors report as warnings for some reason. Apparently this code has been moved around by now 5931289bdccb18046578d0fd5acd8109e49fccb9.
Still a bug by my book, but hey try to convince me otherwise.
To elaborate on that, from what I understand is happening here:
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!
And this is not even old code, a6777f378580122ed0bfe642ac00f98f68810589, @phadej is copy & paste now an acceptable software development method?
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.
Most helpful comment
Sorry, @sol, but you are not in a position to tell me how to do software or particularly
Cabaldevelopment. I had my reasons to do that. In particular to wrap up completely rewritten parser for2.2release. 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.2release, so I didn't want to make any changes toCabalAPI, I'm still not sure what the API should look like.If those are the thanks for that work, than I'm just speechless.