Hackage should check for common QA problems in ghc-options

[duncan]

Simple test that many packages have is dodgy ghc-options, or options that would be suitable for development but not release. It should be easy to check for these.

Suggestions:

ghc-options: -Wall -Werror is a very bad combination. It means the package will break silently as soon as the next version of ghc adds a new warning, which generally does happen every major release.

ghc-options: -fasm is unnecessary and breaks on all arches except for x86, x86-64 and ppc.

ghc-options: -O is unnecessary since Cabal does that itself and it prevents people from configuring with the --disable-optimization flag.

ghc-options: -O2 may be necessary in some circumstances but people should not use it routinely for the same reasons as -O and since -O2 take a lot longer to compile most of the time and usually with little benefit.

ghc-options: -ffi or -fffi use extensions: ForeignFunctionInterface?.

Once #190 is implemented then ghc-options: -fvia-C will be unnecessary too.

[guest]

ghc-options: -Wall -Werror would be quite reasonable inside an if flag(devel), though.

Ian

[duncan]

Adding at least some of these should be an easy project for someone.

[duncan]

Hackage should also reject packages that do not specify a build-type:.

Once we implement striping (ticket #88) we should also reject ghc-options: -optl-Wl,-s.

The code should go in the Cabal lib but be used in hackage and cabal-install.

[ross@…]

I think it's a library issue, specifically parsePackageDescription and sanityCheckPackage in Distribution.PackageDescription. If either of these functions fails, the HackageDB upload will fail and show the error. If they produce warnings, the upload succeeds but shows the warnings (as does check).

Currently HackageDB has additional fatal errors for

• version numbers like 00.02 that change when parsed and printed.

and additional warnings for

• missing Category, Description, Maintainer or Synopsis field.
• over-long Synopsis field.
• exposed modules that use unallocated top-level names in the module hierarchy.

But there are quite a few packages in the archive that triggered these warnings.

[duncan]

Yes it is a lib issue. The code to do these checks should be in the library and used in hackagedb and cabal-install.

However I think the interpretation of some of the checks should be different in the hackage context than in a private build context. So something that might be a warning in cabal locally, like a missing build-type should be a fatal error for hackagedb.

Perhaps we should improve sanityCheckPackage or add an extra qaCheckPackage that gives enough info for hackage and cabal to react differently.

That one about version number parsing should be integrated into the cabal lib. It's ticket 194#.

[duncan]

Distribution.PackageDescription.QA now exports

data QANotice = QAWarning String | QAFailure String
qaCheckPackage :: PackageDescription -> IO [QANotice]

However it needs a bit of tweaking before it can be used in hackage as the code currently assumes that the package is unpacked relative to the current directory (eg it checks for license files). The pure checks should be split from the impure ones and the impure should be given the FilePath of the root of the unpacked package so they can check files relative to that.

[duncan]

More QA and sanity checks:

• configure should error out if a configuration gives duplication in the exposed modules, or other modules (I've seen this happen accidentally in the real world resulting in much confusion).
• qa should check that exposed modules does not change with different configurations. This would be a non-fatal warning because the base package does it :-).

[duncan]

Yet another QA check:

• qa should check that if build-type: Configure then the configure file must exist and be in extra-source-files, so that it gets included into the tarball.

This is partly for hackage's qa checks so that it can reject packages produced by older cabal versions that have a missing configure file.

[duncan]

And another...

Ticket #271 has a .cabal file with some nice examples. Not only does it use -O2 with little reason (probably) and -optl-Wl,-s, but it also uses ghc-options: -lcrypt. Grrr.

• -l -L -I flags in ghc-options or cc-options should give a QA error
  • -lfoo should be extra-libraries: foo
  • -Lbar should be extra-lib-dirs: bar
  • -Ibaz should be include-dirs: baz

The other QA bug in the #217 .cabal file is that it puts files in extra-source-files: that get included anyway. I'm not sure if this would be an easy QA check, it should be doable as an error in sdist however if it finds that it wants to create a fileset with duplicate source file paths. We can revisit that one perhaps.

[duncan]

Most of these implemented and a few more.