Ticket #191 (closed enhancement: fixed)

Opened 1 year ago

Last modified 10 months ago

Hackage should check for common QA problems in ghc-options

Reported by: duncan Assigned to:
Priority: normal Milestone: Cabal-1.4
Component: Cabal library Version: 1.2.2.0
Severity: normal Keywords:
Cc: Difficulty: easy (<4 hours)
GHC Version: 6.8.1 Platform:

Description (Last modified by ross@soi.city.ac.uk)

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.

Change History

12/17/07 09:09:05 changed by guest

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

Ian

12/30/07 03:29:06 changed by duncan

  • difficulty changed from normal to easy (<4 hours).

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

01/14/08 12:14:07 changed by duncan

  • owner changed.
  • component changed from Cabal library to HackageDB website.

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.

01/15/08 06:51:28 changed by ross@soi.city.ac.uk

  • description changed.

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.

01/15/08 17:11:24 changed by 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#.

01/20/08 16:46:53 changed by duncan

  • owner changed.
  • difficulty changed from easy (<4 hours) to very easy (<1 hour).
  • component changed from HackageDB website to Cabal library.

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.

01/24/08 07:57:32 changed by duncan

  • platform deleted.
  • milestone set to Cabal-1.4.

01/25/08 06:55:22 changed by 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 :-).

01/28/08 16:54:10 changed by 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.

01/29/08 12:39:03 changed by duncan

  • difficulty changed from very easy (<1 hour) to easy (<4 hours).

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.

02/11/08 08:50:42 changed by duncan

  • status changed from new to closed.
  • resolution set to fixed.

Most of these implemented and a few more.