Skip to content

Improve build-type defaulting #4958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cabal/Distribution/PackageDescription.hs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ module Distribution.PackageDescription (
PackageDescription(..),
emptyPackageDescription,
specVersion,
buildType,
descCabalVersion,
BuildType(..),
knownBuildTypes,
Expand Down
27 changes: 14 additions & 13 deletions Cabal/Distribution/PackageDescription/Check.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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]) $
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great change, which one should mention in the changelog, It's updated in the manual (great!), but let's advertise this change in Changelog too.

Optionally, we can add a section in changelog gathering all cabal format related changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right!

Fwiw, I've got more related changes to for cabal check & build-type in the pipeline; the idea about structuring the changelog is a good one too, although I've been wanting to have an explicit cabal spec changelog in the user's guide as well... :-)

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 "
Expand Down Expand Up @@ -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 "
Expand All @@ -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 "
Expand Down Expand Up @@ -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) $
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Cabal/Distribution/PackageDescription/FieldGrammar.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Cabal/Distribution/PackageDescription/Parse.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down
34 changes: 32 additions & 2 deletions Cabal/Distribution/Types/PackageDescription.hs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ module Distribution.Types.PackageDescription (
PackageDescription(..),
specVersion,
descCabalVersion,
buildType,
emptyPackageDescription,
hasPublicLib,
hasLibs,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -185,7 +215,7 @@ emptyPackageDescription
license = UnspecifiedLicense,
licenseFiles = [],
specVersionRaw = Right anyVersion,
buildType = Nothing,
buildTypeRaw = Nothing,
copyright = "",
maintainer = "",
author = "",
Expand Down
6 changes: 3 additions & 3 deletions Cabal/Distribution/Types/PackageDescription/Lens.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
3 changes: 3 additions & 0 deletions Cabal/changelog
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions Cabal/doc/developing-packages.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 8 additions & 6 deletions cabal-install/Distribution/Client/Dependency.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion cabal-install/Distribution/Client/ProjectBuilding.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 4 additions & 5 deletions cabal-install/Distribution/Client/ProjectPlanning.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions cabal-install/Distribution/Client/SetupWrapper.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand Down Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion cabal-testsuite/PackageTests/CustomPlain/plain.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions cabal-testsuite/PackageTests/CustomPlain/setup.cabal.out
Original file line number Diff line number Diff line change
@@ -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..
1 change: 1 addition & 0 deletions cabal-testsuite/PackageTests/CustomPlain/setup.out
Original file line number Diff line number Diff line change
@@ -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..
1 change: 1 addition & 0 deletions cabal-testsuite/PackageTests/SimpleDefault/M.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module M where
2 changes: 2 additions & 0 deletions cabal-testsuite/PackageTests/SimpleDefault/Setup.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
main :: IO ()
main = fail "Setup called despite `build-type:Simple`"
1 change: 1 addition & 0 deletions cabal-testsuite/PackageTests/SimpleDefault/cabal.project
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
packages: .
6 changes: 6 additions & 0 deletions cabal-testsuite/PackageTests/SimpleDefault/cabal.test.hs
Original file line number Diff line number Diff line change
@@ -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"]
10 changes: 10 additions & 0 deletions cabal-testsuite/PackageTests/SimpleDefault/my.cabal
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion cabal-testsuite/Test/Cabal/Prelude.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down