Merge pull request #4958

Improve `build-type` defaulting 

(see 9fb03d7 and #4958 for details)

Author: hvr

Cabal/Distribution/PackageDescription.hs

@@ -18,6 +18,7 @@ module Distribution.PackageDescription (
         PackageDescription(..),
         emptyPackageDescription,
         specVersion,
+        buildType,
         descCabalVersion,
         BuildType(..),
         knownBuildTypes,

Cabal/Distribution/PackageDescription/Check.hs

@@ -448,20 +448,20 @@ checkFields pkg =
            "Package names with the prefix 'z-' are reserved by Cabal and "
         ++ "cannot be used."
 
-  , check (isNothing (buildType pkg)) $
+  , check (isNothing (buildTypeRaw pkg) && specVersion pkg < mkVersion [2,1]) $
       PackageBuildWarning $
            "No 'build-type' specified. If you do not need a custom Setup.hs or "
         ++ "./configure script then use 'build-type: Simple'."
 
   , case buildType pkg of
-      Just (UnknownBuildType unknown) -> Just $
+      UnknownBuildType unknown -> Just $
         PackageBuildWarning $
              quote unknown ++ " is not a known 'build-type'. "
           ++ "The known build types are: "
           ++ commaSep (map display knownBuildTypes)
       _ -> Nothing
 
-  , check (isJust (setupBuildInfo pkg) && buildType pkg /= Just Custom) $
+  , check (isJust (setupBuildInfo pkg) && buildType pkg /= Custom) $
       PackageBuildWarning $
            "Ignoring the 'custom-setup' section because the 'build-type' is "
         ++ "not 'Custom'. Use 'build-type: Custom' if you need to use a "
@@ -1283,7 +1283,7 @@ checkCabalVersion pkg =
 
   , check (specVersion pkg >= mkVersion [1,23]
            && isNothing (setupBuildInfo pkg)
-           && buildType pkg == Just Custom) $
+           && buildType pkg == Custom) $
       PackageBuildWarning $
            "Packages using 'cabal-version: >= 1.23' with 'build-type: Custom' "
         ++ "must use a 'custom-setup' section with a 'setup-depends' field "
@@ -1293,7 +1293,7 @@ checkCabalVersion pkg =
 
   , check (specVersion pkg < mkVersion [1,23]
            && isNothing (setupBuildInfo pkg)
-           && buildType pkg == Just Custom) $
+           && buildType pkg == Custom) $
       PackageDistSuspiciousWarn $
            "From version 1.23 cabal supports specifiying explicit dependencies "
         ++ "for Custom setup scripts. Consider using cabal-version >= 1.23 and "
@@ -1903,7 +1903,7 @@ checkSetupExists :: Monad m => CheckPackageContentOps m
                  -> PackageDescription
                  -> m (Maybe PackageCheck)
 checkSetupExists ops pkg = do
-  let simpleBuild = buildType pkg == Just Simple
+  let simpleBuild = buildType pkg == Simple
   hsexists  <- doesFileExist ops "Setup.hs"
   lhsexists <- doesFileExist ops "Setup.lhs"
   return $ check (not simpleBuild && not hsexists && not lhsexists) $
@@ -1913,13 +1913,14 @@ checkSetupExists ops pkg = do
 checkConfigureExists :: Monad m => CheckPackageContentOps m
                      -> PackageDescription
                      -> m (Maybe PackageCheck)
-checkConfigureExists ops PackageDescription { buildType = Just Configure } = do
-  exists <- doesFileExist ops "configure"
-  return $ check (not exists) $
-    PackageBuildWarning $
-      "The 'build-type' is 'Configure' but there is no 'configure' script. "
-      ++ "You probably need to run 'autoreconf -i' to generate it."
-checkConfigureExists _ _ = return Nothing
+checkConfigureExists ops pd
+  | buildType pd == Configure = do
+      exists <- doesFileExist ops "configure"
+      return $ check (not exists) $
+        PackageBuildWarning $
+          "The 'build-type' is 'Configure' but there is no 'configure' script. "
+          ++ "You probably need to run 'autoreconf -i' to generate it."
+  | otherwise = return Nothing
 
 checkLocalPathsExist :: Monad m => CheckPackageContentOps m
                      -> PackageDescription

Cabal/Distribution/PackageDescription/FieldGrammar.hs

@@ -86,7 +86,7 @@ packageDescriptionFieldGrammar = PackageDescription
     <*> prefixedFields      "x-"                                       L.customFieldsPD
     <*> pure [] -- build-depends
     <*> optionalFieldDefAla "cabal-version" SpecVersion                L.specVersionRaw (Right anyVersion)
-    <*> optionalField       "build-type"                               L.buildType
+    <*> optionalField       "build-type"                               L.buildTypeRaw
     <*> pure Nothing -- custom-setup
     -- components
     <*> pure Nothing  -- lib

Cabal/Distribution/PackageDescription/Parse.hs

@@ -97,7 +97,7 @@ pkgDescrFieldDescrs =
            specVersionRaw         (\v pkg -> pkg{specVersionRaw=v})
  , simpleField "build-type"
            (maybe mempty disp)  (fmap Just parse)
-           buildType              (\t pkg -> pkg{buildType=t})
+           buildTypeRaw           (\t pkg -> pkg{buildTypeRaw=t})
  , simpleField "license"
            disp                   parseLicenseQ
            license                (\l pkg -> pkg{license=l})

Cabal/Distribution/Types/PackageDescription.hs

@@ -30,6 +30,7 @@ module Distribution.Types.PackageDescription (
     PackageDescription(..),
     specVersion,
     descCabalVersion,
+    buildType,
     emptyPackageDescription,
     hasPublicLib,
     hasLibs,
@@ -128,7 +129,11 @@ data PackageDescription
         -- only ranges of the form @>= v@ make sense. We are in the process of
         -- transitioning to specifying just a single version, not a range.
         specVersionRaw :: Either Version VersionRange,
-        buildType      :: Maybe BuildType,
+        -- | The original @build-type@ value as parsed from the
+        -- @.cabal@ file without defaulting. See also 'buildType'.
+        --
+        -- @since 2.2
+        buildTypeRaw   :: Maybe BuildType,
         setupBuildInfo :: Maybe SetupBuildInfo,
         -- components
         library        :: Maybe Library,
@@ -177,6 +182,31 @@ descCabalVersion pkg = case specVersionRaw pkg of
   Right versionRange -> versionRange
 {-# DEPRECATED descCabalVersion "Use specVersion instead" #-}
 
+-- | The effective @build-type@ after applying defaulting rules.
+--
+-- The original @build-type@ value parsed is stored in the
+-- 'buildTypeRaw' field.  However, the @build-type@ field is optional
+-- and can therefore be empty in which case we need to compute the
+-- /effective/ @build-type@. This function implements the following
+-- defaulting rules:
+--
+--  * For @cabal-version:2.0@ and below, default to the @Custom@
+--    build-type unconditionally.
+--
+--  * Otherwise, if a @custom-setup@ stanza is defined, default to
+--    the @Custom@ build-type; else default to @Simple@ build-type.
+--
+-- @since 2.2
+buildType :: PackageDescription -> BuildType
+buildType pkg
+  | specVersion pkg >= mkVersion [2,1]
+    = fromMaybe newDefault (buildTypeRaw pkg)
+  | otherwise -- cabal-version < 2.1
+    = fromMaybe Custom (buildTypeRaw pkg)
+  where
+    newDefault | isNothing (setupBuildInfo pkg) = Simple
+               | otherwise                      = Custom
+
 emptyPackageDescription :: PackageDescription
 emptyPackageDescription
     =  PackageDescription {
@@ -185,7 +215,7 @@ emptyPackageDescription
                       license      = UnspecifiedLicense,
                       licenseFiles = [],
                       specVersionRaw = Right anyVersion,
-                      buildType    = Nothing,
+                      buildTypeRaw = Nothing,
                       copyright    = "",
                       maintainer   = "",
                       author       = "",

Cabal/Distribution/Types/PackageDescription/Lens.hs

@@ -96,9 +96,9 @@ specVersionRaw :: Lens' PackageDescription (Either Version VersionRange)
 specVersionRaw f s = fmap (\x -> s { T.specVersionRaw = x }) (f (T.specVersionRaw s))
 {-# INLINE specVersionRaw #-}
 
-buildType :: Lens' PackageDescription (Maybe BuildType)
-buildType f s = fmap (\x -> s { T.buildType = x }) (f (T.buildType s))
-{-# INLINE buildType #-}
+buildTypeRaw :: Lens' PackageDescription (Maybe BuildType)
+buildTypeRaw f s = fmap (\x -> s { T.buildTypeRaw = x }) (f (T.buildTypeRaw s))
+{-# INLINE buildTypeRaw #-}
 
 setupBuildInfo :: Lens' PackageDescription (Maybe SetupBuildInfo)
 setupBuildInfo f s = fmap (\x -> s { T.setupBuildInfo = x }) (f (T.setupBuildInfo s))

Cabal/changelog

@@ -30,6 +30,9 @@
 	* Compilation with section splitting is now supported via the
 	'--enable-split-sections' flag (#4819)
 	* Support for common stanzas (#4751)
+	* Use better defaulting for `build-type`; rename `PackageDescription`'s
+	  `buildType` field to `buildTypeRaw` and introduce new `buildType`
+	  function (#4958)
 	* TODO
 
 2.0.1.1 Mikhail Glushenkov <[email protected]> December 2017

Cabal/doc/developing-packages.rst

@@ -798,12 +798,20 @@ describe the package as a whole:
 
 .. pkg-field:: build-type: identifier
 
-    :default: ``Custom``
+    :default: ``Custom`` or ``Simple``
 
     The type of build used by this package. Build types are the
     constructors of the
     `BuildType <../release/cabal-latest/doc/API/Cabal/Distribution-PackageDescription.html#t:BuildType>`__
-    type, defaulting to ``Custom``.
+    type. This field is optional and when missing, its default value
+    is inferred according to the following rules:
+
+     - When :pkg-field:`cabal-version` is set to ``2.1`` or higher,
+       the default is ``Simple`` unless a :pkg-section:`custom-setup`
+       exists, in which case the inferred default is ``Custom``.
+
+     - For lower :pkg-field:`cabal-version` values, the default is
+       ``Custom`` unconditionally.
 
     If the build type is anything other than ``Custom``, then the
     ``Setup.hs`` file *must* be exactly the standardized content

cabal-install/Distribution/Client/Dependency.hs

@@ -525,16 +525,18 @@ addDefaultSetupDependencies defaultSetupDeps params =
               PD.setupBuildInfo =
                 case PD.setupBuildInfo pkgdesc of
                   Just sbi -> Just sbi
-                  Nothing  -> case defaultSetupDeps srcpkg of
+                  Nothing -> case defaultSetupDeps srcpkg of
                     Nothing -> Nothing
-                    Just deps -> Just PD.SetupBuildInfo {
-                      PD.defaultSetupDepends = True,
-                      PD.setupDepends        = deps
-                    }
+                    Just deps | isCustom -> Just PD.SetupBuildInfo {
+                                                PD.defaultSetupDepends = True,
+                                                PD.setupDepends        = deps
+                                            }
+                              | otherwise -> Nothing
             }
           }
         }
       where
+        isCustom = PD.buildType pkgdesc == PD.Custom
         gpkgdesc = packageDescription srcpkg
         pkgdesc  = PD.packageDescription gpkgdesc
 
@@ -619,7 +621,7 @@ standardInstallPolicy installedPkgIndex sourcePkgDb pkgSpecifiers
         where
           gpkgdesc = packageDescription srcpkg
           pkgdesc  = PD.packageDescription gpkgdesc
-          bt       = fromMaybe PD.Custom (PD.buildType pkgdesc)
+          bt       = PD.buildType pkgdesc
           affected = bt == PD.Custom && hasBuildableFalse gpkgdesc
 
       -- Does this package contain any components with non-empty 'build-depends'

cabal-install/Distribution/Client/ProjectBuilding.hs

@@ -1139,7 +1139,7 @@ buildInplaceUnpackedPackage verbosity
               ifNullThen m m' = do xs <- m
                                    if null xs then m' else return xs
           monitors <- case PD.buildType (elabPkgDescription pkg) of
-            Just Simple -> listSimple
+            Simple -> listSimple
             -- If a Custom setup was used, AND the Cabal is recent
             -- enough to have sdist --list-sources, use that to
             -- determine the files that we need to track.  This can

cabal-install/Distribution/Client/ProjectPlanning.hs

@@ -1225,9 +1225,8 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
             -- Once you've implemented this, swap it for the code below.
             cuz_custom =
                 case PD.buildType (elabPkgDescription elab0) of
-                    Nothing        -> cuz "build-type is not specified"
-                    Just PD.Custom -> cuz "build-type is Custom"
-                    Just _         -> []
+                    PD.Custom -> cuz "build-type is Custom"
+                    _         -> []
             -- cabal-format versions prior to 1.8 have different build-depends semantics
             -- for now it's easier to just fallback to legacy-mode when specVersion < 1.8
             -- see, https://github.com/haskell/cabal/issues/4121
@@ -1271,7 +1270,7 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
         -- have to add dependencies on this from all other components
         setupComponent :: Maybe ElaboratedConfiguredPackage
         setupComponent
-            | fromMaybe PD.Custom (PD.buildType (elabPkgDescription elab0)) == PD.Custom
+            | PD.buildType (elabPkgDescription elab0) == PD.Custom
             = Just elab0 {
                 elabModuleShape = emptyModuleShape,
                 elabUnitId = notImpl "elabUnitId",
@@ -2795,7 +2794,7 @@ packageSetupScriptStyle pkg
   | otherwise
   = SetupNonCustomInternalLib
   where
-    buildType = fromMaybe PD.Custom (PD.buildType pkg)
+    buildType = PD.buildType pkg
 
 
 -- | Part of our Setup.hs handling policy is implemented by getting the solver

cabal-install/Distribution/Client/SetupWrapper.hs

@@ -38,7 +38,7 @@ import Distribution.Package
 import Distribution.Types.Dependency
 import Distribution.PackageDescription
          ( GenericPackageDescription(packageDescription)
-         , PackageDescription(..), specVersion
+         , PackageDescription(..), specVersion, buildType
          , BuildType(..), knownBuildTypes, defaultRenaming )
 import Distribution.PackageDescription.Parsec
          ( readGenericPackageDescription )
@@ -293,7 +293,7 @@ getSetup verbosity options mpkg = do
                                           (useCabalVersion options)
                                           (orLaterVersion (specVersion pkg))
                     }
-      buildType'  = fromMaybe Custom (buildType pkg)
+      buildType'  = buildType pkg
   checkBuildType buildType'
   (version, method, options'') <-
     getSetupMethod verbosity options' pkg buildType'

cabal-install/tests/UnitTests/Distribution/Solver/Modular/DSL.hs

@@ -350,9 +350,9 @@ exAvSrcPkg ex =
                     C.package        = pkgId
                   , C.setupBuildInfo = setup
                   , C.license = BSD3
-                  , C.buildType = if isNothing setup
-                                  then Just C.Simple
-                                  else Just C.Custom
+                  , C.buildTypeRaw = if isNothing setup
+                                     then Just C.Simple
+                                     else Just C.Custom
                   , C.category = "category"
                   , C.maintainer = "maintainer"
                   , C.description = "description"

cabal-testsuite/PackageTests/CustomPlain/plain.cabal

@@ -3,7 +3,6 @@ version:             0.1.0.0
 license:             BSD3
 author:              Edward Z. Yang
 maintainer:          [email protected]
-build-type:          Custom
 cabal-version:       >=1.10
 
 library

cabal-testsuite/PackageTests/CustomPlain/setup.cabal.out

@@ -1,6 +1,7 @@
 # Setup configure
 Resolving dependencies...
 Configuring plain-0.1.0.0...
+Warning: No 'build-type' specified. If you do not need a custom Setup.hs or ./configure script then use 'build-type: Simple'.
 # Setup build
 Preprocessing library for plain-0.1.0.0..
 Building library for plain-0.1.0.0..

cabal-testsuite/PackageTests/CustomPlain/setup.out

@@ -1,5 +1,6 @@
 # Setup configure
 Configuring plain-0.1.0.0...
+Warning: No 'build-type' specified. If you do not need a custom Setup.hs or ./configure script then use 'build-type: Simple'.
 # Setup build
 Preprocessing library for plain-0.1.0.0..
 Building library for plain-0.1.0.0..

cabal-testsuite/PackageTests/SimpleDefault/M.hs

@@ -0,0 +1 @@
+module M where

cabal-testsuite/PackageTests/SimpleDefault/Setup.hs

@@ -0,0 +1,2 @@
+main :: IO ()
+main = fail "Setup called despite `build-type:Simple`"

cabal-testsuite/PackageTests/SimpleDefault/cabal.project

@@ -0,0 +1 @@
+packages: .

cabal-testsuite/PackageTests/SimpleDefault/cabal.test.hs

@@ -0,0 +1,6 @@
+import Test.Cabal.Prelude
+main = cabalTest $ do
+    recordMode DoNotRecord $ do
+        -- TODO: Hack; see also CustomDep/cabal.test.hs
+        withEnvFilter (/= "HOME") $ do
+            cabal "new-build" ["all"]

cabal-testsuite/PackageTests/SimpleDefault/my.cabal

@@ -0,0 +1,10 @@
+cabal-version:       2.1
+name:                my
+version:             0
+-- tests whether the default is `build-type: Simple`
+-- (for cabal-version >= 2.1)
+
+library
+  exposed-modules:     M
+  build-depends:       base
+  default-language:    Haskell2010

cabal-testsuite/Test/Cabal/Prelude.hs

@@ -151,7 +151,7 @@ setup' cmd args = do
         else do
             pdfile <- liftIO $ tryFindPackageDesc (testCurrentDir env)
             pdesc <- liftIO $ readGenericPackageDescription (testVerbosity env) pdfile
-            if buildType (packageDescription pdesc) == Just Simple
+            if buildType (packageDescription pdesc) == Simple
                 then runM (testSetupPath env) full_args
                 -- Run the Custom script!
                 else do