From a2f43f3e5140331af76aa3d928837b8ce9430de7 Mon Sep 17 00:00:00 2001
From: Oleg Grenrus <oleg.grenrus@iki.fi>
Date: Tue, 24 Sep 2019 10:28:34 +0300
Subject: [PATCH] cabal check: cpp-options allows only -D and -U options
Before:
130113 files processed
7304 have lexer/parser warnings
332 build impossible
9742 build warning
49779 build dist suspicious
38666 build dist suspicious warning
11834 build dist inexcusable
After:
130113 files processed
7304 have lexer/parser warnings
332 build impossible
10063 build warning
49779 build dist suspicious
38666 build dist suspicious warning
11834 build dist inexcusable
i.e. 321 build warnings on all Hackage
Examples:
NO_DEBUG_MODE -- forgotten -D?
-traditional -- doesn't work, nor needed
-fallow-undecidable-instances -- wrong -options?
-fno-exceptions
-Wall
-Werror
--include=include/config.h -- doesn't work
-maes -- cpp is not C++ ?
-mpclmul
-mssse3
---
Cabal/Cabal.cabal | 2 +
Cabal/ChangeLog.md | 2 +
.../Distribution/PackageDescription/Check.hs | 17 ++++---
Cabal/tests/CheckTests.hs | 1 +
Cabal/tests/HackageTests.hs | 44 ++++++++++++-----
.../regressions/assoc-cpp-options.cabal | 48 +++++++++++++++++++
.../regressions/assoc-cpp-options.check | 1 +
7 files changed, 96 insertions(+), 19 deletions(-)
create mode 100644 Cabal/tests/ParserTests/regressions/assoc-cpp-options.cabal
create mode 100644 Cabal/tests/ParserTests/regressions/assoc-cpp-options.check
diff --git a/Cabal/Cabal.cabal b/Cabal/Cabal.cabal
index 73ab549f0f..ef7bafa53a 100644
--- a/Cabal/Cabal.cabal
+++ b/Cabal/Cabal.cabal
@@ -108,6 +108,8 @@ extra-source-files:
tests/ParserTests/regressions/Octree-0.5.cabal
tests/ParserTests/regressions/Octree-0.5.expr
tests/ParserTests/regressions/Octree-0.5.format
+ tests/ParserTests/regressions/assoc-cpp-options.cabal
+ tests/ParserTests/regressions/assoc-cpp-options.check
tests/ParserTests/regressions/bad-glob-syntax.cabal
tests/ParserTests/regressions/bad-glob-syntax.check
tests/ParserTests/regressions/cc-options-with-optimization.cabal
diff --git a/Cabal/ChangeLog.md b/Cabal/ChangeLog.md
index 720a0dd46b..d69cb7a2a0 100644
--- a/Cabal/ChangeLog.md
+++ b/Cabal/ChangeLog.md
@@ -1,4 +1,6 @@
# 3.1.0.0 (current development version)
+ * `cabal check` verifies `cpp-options` more pedantically, allowing only
+ options starting with `-D` and `-U`.
* TODO
----
diff --git a/Cabal/Distribution/PackageDescription/Check.hs b/Cabal/Distribution/PackageDescription/Check.hs
index 0f2d21a277..b4c5c9470a 100644
--- a/Cabal/Distribution/PackageDescription/Check.hs
+++ b/Cabal/Distribution/PackageDescription/Check.hs
@@ -1025,13 +1025,18 @@ checkCLikeOptions label prefix accessor pkg =
checkCCFlags flags = check (any (`elem` flags) all_cLikeOptions)
checkCPPOptions :: PackageDescription -> [PackageCheck]
-checkCPPOptions pkg =
- catMaybes [
- checkAlternatives "cpp-options" "include-dirs"
- [ (flag, dir) | flag@('-':'I':dir) <- all_cppOptions]
+checkCPPOptions pkg = catMaybes
+ [ checkAlternatives "cpp-options" "include-dirs"
+ [ (flag, dir) | flag@('-':'I':dir) <- all_cppOptions ]
+ ]
+ ++
+ [ PackageBuildWarning $ "'cpp-options': " ++ opt ++ " is not portable C-preprocessor flag"
+ | opt <- all_cppOptions
+ -- "-I" is handled above, we allow only -DNEWSTUFF and -UOLDSTUFF
+ , not $ any (`isPrefixOf` opt) ["-D", "-U", "-I" ]
]
- where all_cppOptions = [ opts | bi <- allBuildInfo pkg
- , opts <- cppOptions bi ]
+ where
+ all_cppOptions = [ opts | bi <- allBuildInfo pkg, opts <- cppOptions bi ]
checkAlternatives :: String -> String -> [(String, String)]
-> Maybe PackageCheck
diff --git a/Cabal/tests/CheckTests.hs b/Cabal/tests/CheckTests.hs
index 42768dc482..a0513a45da 100644
--- a/Cabal/tests/CheckTests.hs
+++ b/Cabal/tests/CheckTests.hs
@@ -39,6 +39,7 @@ checkTests = testGroup "regressions"
, checkTest "cxx-options-with-optimization.cabal"
, checkTest "ghc-option-j.cabal"
, checkTest "multiple-libs-2.cabal"
+ , checkTest "assoc-cpp-options.cabal"
]
checkTest :: FilePath -> TestTree
diff --git a/Cabal/tests/HackageTests.hs b/Cabal/tests/HackageTests.hs
index 353b55419c..d77e45efe5 100644
--- a/Cabal/tests/HackageTests.hs
+++ b/Cabal/tests/HackageTests.hs
@@ -70,6 +70,9 @@ import qualified Distribution.Types.GenericPackageDescription.Lens as L
import qualified Distribution.Types.PackageDescription.Lens as L
import qualified Options.Applicative as O
+-- import Distribution.Types.BuildInfo (BuildInfo (cppOptions))
+-- import qualified Distribution.Types.BuildInfo.Lens as L
+
#ifdef MIN_VERSION_tree_diff
import Data.TreeDiff (ediff)
import Data.TreeDiff.Pretty (ansiWlEditExprCompact)
@@ -165,36 +168,50 @@ parseParsecTest fpath bs = do
parseCheckTest :: FilePath -> B.ByteString -> IO CheckResult
parseCheckTest fpath bs = do
- let (_warnings, parsec) = Parsec.runParseResult $
- Parsec.parseGenericPackageDescription bs
+ let (warnings, parsec) = Parsec.runParseResult $
+ Parsec.parseGenericPackageDescription bs
case parsec of
Right gpd -> do
let checks = checkPackage gpd Nothing
+ let w [] = 0
+ w _ = 1
+
+ -- Look into invalid cpp options
+ -- _ <- L.traverseBuildInfos checkCppFlags gpd
+
-- one for file, many checks
- return (CheckResult 1 0 0 0 0 0 <> foldMap toCheckResult checks)
+ return (CheckResult 1 (w warnings) 0 0 0 0 0 <> foldMap toCheckResult checks)
Left (_, errors) -> do
traverse_ (putStrLn . Parsec.showPError fpath) errors
exitFailure
-data CheckResult = CheckResult !Int !Int !Int !Int !Int !Int
+-- checkCppFlags :: BuildInfo -> IO BuildInfo
+-- checkCppFlags bi = do
+-- for_ (cppOptions bi) $ \opt ->
+-- unless (any (`isPrefixOf` opt) ["-D", "-U", "-I"]) $
+-- putStrLn opt
+--
+-- return bi
+
+data CheckResult = CheckResult !Int !Int !Int !Int !Int !Int !Int
instance NFData CheckResult where
rnf !_ = ()
instance Semigroup CheckResult where
- CheckResult n a b c d e <> CheckResult n' a' b' c' d' e' =
- CheckResult (n + n') (a + a') (b + b') (c + c') (d + d') (e + e')
+ CheckResult n w a b c d e <> CheckResult n' w' a' b' c' d' e' =
+ CheckResult (n + n') (w + w') (a + a') (b + b') (c + c') (d + d') (e + e')
instance Monoid CheckResult where
- mempty = CheckResult 0 0 0 0 0 0
+ mempty = CheckResult 0 0 0 0 0 0 0
mappend = (<>)
toCheckResult :: PackageCheck -> CheckResult
-toCheckResult PackageBuildImpossible {} = CheckResult 0 1 0 0 0 0
-toCheckResult PackageBuildWarning {} = CheckResult 0 0 1 0 0 0
-toCheckResult PackageDistSuspicious {} = CheckResult 0 0 0 1 0 0
-toCheckResult PackageDistSuspiciousWarn {} = CheckResult 0 0 0 0 1 0
-toCheckResult PackageDistInexcusable {} = CheckResult 0 0 0 0 0 1
+toCheckResult PackageBuildImpossible {} = CheckResult 0 0 1 0 0 0 0
+toCheckResult PackageBuildWarning {} = CheckResult 0 0 0 1 0 0 0
+toCheckResult PackageDistSuspicious {} = CheckResult 0 0 0 0 1 0 0
+toCheckResult PackageDistSuspiciousWarn {} = CheckResult 0 0 0 0 0 1 0
+toCheckResult PackageDistInexcusable {} = CheckResult 0 0 0 0 0 0 1
-------------------------------------------------------------------------------
-- Roundtrip test
@@ -309,8 +326,9 @@ main = join (O.execParser opts)
checkP = checkA <$> prefixP
checkA pfx = do
- CheckResult n a b c d e <- parseIndex pfx parseCheckTest
+ CheckResult n w a b c d e <- parseIndex pfx parseCheckTest
putStrLn $ show n ++ " files processed"
+ putStrLn $ show w ++ " have lexer/parser warnings"
putStrLn $ show a ++ " build impossible"
putStrLn $ show b ++ " build warning"
putStrLn $ show c ++ " build dist suspicious"
diff --git a/Cabal/tests/ParserTests/regressions/assoc-cpp-options.cabal b/Cabal/tests/ParserTests/regressions/assoc-cpp-options.cabal
new file mode 100644
index 0000000000..1387cde69c
--- /dev/null
+++ b/Cabal/tests/ParserTests/regressions/assoc-cpp-options.cabal
@@ -0,0 +1,48 @@
+cabal-version: 1.12
+name: assoc
+version: 1.1
+license: BSD3
+license-file: LICENSE
+synopsis: swap and assoc: Symmetric and Semigroupy Bifunctors
+category: Data
+description:
+ Provides generalisations of
+ @swap :: (a,b) -> (b,a)@ and
+ @assoc :: ((a,b),c) -> (a,(b,c))@
+ to
+ @Bifunctor@s supporting similar operations (e.g. @Either@, @These@).
+
+author: Oleg Grenrus <oleg.grenrus@iki.fi>
+maintainer: Oleg Grenrus <oleg.grenrus@iki.fi>
+build-type: Simple
+tested-with:
+ GHC ==7.0.4
+ || ==7.2.2
+ || ==7.4.2
+ || ==7.6.3
+ || ==7.8.4
+ || ==7.10.3
+ || ==8.0.2
+ || ==8.2.2
+ || ==8.4.4
+ || ==8.6.5
+ || ==8.8.1
+
+source-repository head
+ type: git
+ location: https://github.com/phadej/assoc.git
+
+library
+ default-language: Haskell2010
+ hs-source-dirs: src
+ build-depends:
+ base >=4.3 && <4.13
+ , bifunctors >=5.5.4 && <5.6
+
+ cpp-options: -traditional
+
+ exposed-modules:
+ Data.Bifunctor.Assoc
+ Data.Bifunctor.Swap
+
+ other-extensions: TypeFamilies
diff --git a/Cabal/tests/ParserTests/regressions/assoc-cpp-options.check b/Cabal/tests/ParserTests/regressions/assoc-cpp-options.check
new file mode 100644
index 0000000000..fd1446fceb
--- /dev/null
+++ b/Cabal/tests/ParserTests/regressions/assoc-cpp-options.check
@@ -0,0 +1 @@
+'cpp-options': -traditional is not portable C-preprocessor flag
--
GitLab