Ticket #191 (closed enhancement: fixed)

Opened 6 years ago

Last modified 5 years ago

Hackage should check for common QA problems in ghc-options

Reported by: duncan Owned by:
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@…) (diff)

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

Changed 6 years ago by guest

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

Ian

Changed 5 years ago by duncan

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

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

Changed 5 years ago by duncan

  • 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.

Changed 5 years ago by ross@…

  • description modified (diff)

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.

Changed 5 years ago 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#.

Changed 5 years ago by duncan

  • 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.

Changed 5 years ago by duncan

  • platform Linux deleted
  • milestone set to Cabal-1.4

Changed 5 years ago 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 :-).

Changed 5 years ago 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.

Changed 5 years ago 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.

Changed 5 years ago by duncan

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

Most of these implemented and a few more.

Note: See TracTickets for help on using tickets.