SetupHooks: add tests

This adds tests for error messages related to SetupHooks:

  - error when returning an invalid component diff in a
    per-component pre-configure hook
  - error when declaring pre-build rules whose dependency
    graph contains cycles
  - error when we cannot find a dependency of a pre-build rule
  - warning when there are pre-build rules that are declared but
    never demanded

It also adds some correctness tests for SetupHooks, e.g. that
pre-build rules are run in dependency order (see the
`SetupHooksRuleOrdering` test).

Author: sheaf

Cabal-hooks/src/Distribution/Simple/SetupHooks.hs

@@ -90,7 +90,7 @@ module Distribution.Simple.SetupHooks
     -- $rulesAPI
   , RulesM
   , registerRule, registerAction
-  , declareRuleDependencies
+  , addRuleMonitors
 
     -- **** Local name generation for t'ActionId'
   , FreshT
@@ -354,10 +354,9 @@ Rules can declare various kinds of dependencies:
 Rules are considered __out-of-date__ precisely when any of the following
 conditions apply:
 
-  [O1] a file dependency of a rule has changed in some way,
-  [O2] the environment passed to the computation of rules has changed,
-  [O3] there has been a relevant change in the set of files and
+  [O1] there has been a relevant change in the set of files and
        directories monitored by the rules.
+  [O2] the environment passed to the computation of rules has changed,
 
 If the rules are out-of-date, the build system is expected to re-run the
 computation that computes all rules.
@@ -394,7 +393,7 @@ Defining pre-build rules can be done in the following style:
 >   return $ do
 >     -- IO actions allowed here
 >     myData <- liftIO someIOAction
->     declareRuleDependencies [ MonitorDir "someSearchDir" DirContents ]
+>     addRuleMonitors [ MonitorDir "someSearchDir" DirContents ]
 >     registerRule $ simpleRule action1 deps1 outs1
 >     registerRule $ simpleRule action1 deps2 outs2
 >     registerRule $ simpleRule action1 deps3 outs3
@@ -405,10 +404,10 @@ rather than directly using the v'Rules', v'Rule' and v'Action' constructors,
 which insulates us from internal changes to the t'Rules', t'Rule' and t'Action'
 datatypes, respectively.
 
-We use 'declareRuleDependencies' to declare that the collection of rules as a
-whole depends on. In this case, we declare that they depend on the contents of
-the "searchDir" directory. This means that the rules will be computed anew
-whenever the contents of this directory change.
+We use 'addRuleMonitorss' to declare a monitored directory that the collection
+of rules as a whole depends on. In this case, we declare that they depend on the
+contents of the "searchDir" directory. This means that the rules will be
+computed anew whenever the contents of this directory change.
 
 Additional convenience functions are also provided, such as the 'generateModules'
 function which can be used to generate a collection of modules ex nihilo without
@@ -566,19 +565,17 @@ register zeroId succId rule = FreshT $ do
 registerRule :: Monad m => Rule -> RulesT m ()
 registerRule r = Writer.tell ( [r], [] )
 
--- | Declare a dependency for the collection of all rules.
+-- | Declare additional monitored objects for the collection of all rules.
 --
--- When this dependency changes, the rules are re-computed.
-declareRuleDependencies :: Monad m => [ MonitorFileOrDir ] -> RulesT m ()
-declareRuleDependencies mons = Writer.tell ( [], mons )
+-- When these monitored objects change, the rules are re-computed.
+addRuleMonitors :: Monad m => [ MonitorFileOrDir ] -> RulesT m ()
+addRuleMonitors mons = Writer.tell ( [], mons )
 
 -- | Register an action. Returns a unique identifier for that action.
 registerAction :: Action -> FreshT Action ActionId Identity ActionId
 registerAction = register ( ActionId 1 ) ( \ ( ActionId i ) -> ActionId ( i + 1 ) )
 
 -- | Find a file in the given search directories.
---
---
 findFileInDirs :: FilePath -> [FilePath] -> IO (Maybe Location)
 findFileInDirs file dirs =
   findFirstFile

Cabal/src/Distribution/Simple/SetupHooks/Internal.hs

@@ -201,7 +201,7 @@ data PreConfPackageInputs = PreConfPackageInputs
 -- of this datatype.
 data PreConfPackageOutputs = PreConfPackageOutputs
   { buildOptions :: BuildOptions
-  , extraConfiguredPrograms :: ConfiguredProgs
+  , extraConfiguredProgs :: ConfiguredProgs
   }
   deriving (Generic, Show)
 
@@ -215,7 +215,7 @@ noPreConfPackageOutputs :: PreConfPackageInputs -> PreConfPackageOutputs
 noPreConfPackageOutputs (PreConfPackageInputs{localBuildConfig = lbc}) =
   PreConfPackageOutputs
     { buildOptions = LBC.withBuildOptions lbc
-    , extraConfiguredPrograms = Map.empty
+    , extraConfiguredProgs = Map.empty
     }
 
 -- | Package-wide post-configure step.
@@ -347,12 +347,12 @@ instance Semigroup PreConfPkgSemigroup where
           do
             PreConfPackageOutputs
               { buildOptions = opts1
-              , extraConfiguredPrograms = progs1
+              , extraConfiguredProgs = progs1
               } <-
               f1 inputs
             PreConfPackageOutputs
               { buildOptions = opts2
-              , extraConfiguredPrograms = progs2
+              , extraConfiguredProgs = progs2
               } <-
               f2 $
                 PreConfPackageInputs
@@ -370,7 +370,7 @@ instance Semigroup PreConfPkgSemigroup where
             return $
               PreConfPackageOutputs
                 { buildOptions = opts2
-                , extraConfiguredPrograms = progs1 <> progs2
+                , extraConfiguredProgs = progs1 <> progs2
                 }
 
 -- | A newtype to hang off the @Semigroup PreConfComponentHook@ instance.

Cabal/src/Distribution/Simple/SetupHooks/Rule.hs

@@ -185,6 +185,9 @@ data Rule
     -- it will pass these locations as the first argument to the action.
     , results :: !(NE.NonEmpty Location)
     -- ^ Results of this rule; see t'Result'.
+    --
+    -- When the build system executes the action associated to this rule,
+    -- it will pass these locations as the second argument to the action.
     , actionId :: !ActionId
     -- ^ To run this rule, which t'Action' should we execute?
     --
@@ -200,7 +203,7 @@ data Rule
     -- environment passed to the rule.
     --
     -- A value of 'Nothing' means: always re-run the rule when the
-    -- environment passes to it changes.
+    -- environment passed to it changes.
     }
   deriving (Generic, Show)
 
@@ -220,7 +223,8 @@ instance Binary Rule
 instance Structured Rule
 
 -- | A (fully resolved) location of a dependency or result of a rule,
--- consisting of an absolute path and of a file path relative to that base path.
+-- consisting of a base directory and of a file path relative to that base
+-- directory path.
 --
 -- In practice, this will be something like @( dir, toFilePath modName )@,
 -- where:
@@ -269,7 +273,7 @@ newtype Action
         -- \^ Locations of the __dependencies__ of this action,
         -- as declared by the rule that this action is executing.
         -> NE.NonEmpty Location
-        -- \^ Locations in which the __results_ of this action
+        -- \^ Locations in which the __results__ of this action
         -- should be put.
         -> IO ()
     }
@@ -286,8 +290,9 @@ simpleAction f = Action{action = f}
 -- constructor.
 --
 -- Actions are registered using 'registerAction', and rules are registered
--- using 'registerRule'. Additional rule dependencies (whose changes should
--- trigger rule recompilation) are declared using 'declareRuleDependencies'.
+-- using 'registerRule'. One can declare additional monitored files or
+-- directories using 'addRuleMonitors'; a change in these will trigger the
+-- recomputation of all rules.
 --
 -- The @env@ type parameter represents an extra argument, which usually
 -- consists of information known to Cabal such as 'LocalBuildInfo' and

cabal-testsuite/PackageTests/SetupHooksBadDiff1/Setup.hs

@@ -0,0 +1,7 @@
+module Main where
+
+import Distribution.Simple ( defaultMainWithSetupHooks )
+import SetupHooks ( setupHooks )
+
+main :: IO ()
+main = defaultMainWithSetupHooks setupHooks

cabal-testsuite/PackageTests/SetupHooksBadDiff1/SetupHooks.hs

@@ -0,0 +1,18 @@
+module SetupHooks where
+
+import Distribution.Simple.SetupHooks
+
+import Control.Monad ( void )
+
+setupHooks :: SetupHooks
+setupHooks =
+  noSetupHooks
+    { configureHooks =
+        noConfigureHooks
+          { preConfComponentHook = Just pccHook }
+    }
+
+pccHook :: PreConfComponentHook
+pccHook _ = return $
+  PreConfComponentOutputs $ ComponentDiff $ CExe emptyExecutable
+    -- Bad: component is a library, but we returned an executable!

cabal-testsuite/PackageTests/SetupHooksBadDiff1/cabal.project

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

cabal-testsuite/PackageTests/SetupHooksBadDiff1/setup-hooks-bad-diff1-test.cabal

@@ -0,0 +1,16 @@
+cabal-version:       2.2
+name:                setup-hooks-bad-diff1-test
+version:             0.1.0.0
+synopsis:            Test 1 for a bad component diff
+license:             BSD-3-Clause
+author:              NA
+maintainer:          NA
+category:            Testing
+build-type:          Hooks
+
+custom-setup
+  setup-depends: Cabal-hooks, base
+
+library
+  build-depends:       base
+  default-language:    Haskell2010

cabal-testsuite/PackageTests/SetupHooksBadDiff1/setup.out

@@ -0,0 +1,5 @@
+# Setup configure
+Configuring setup-hooks-bad-diff1-test-0.1.0.0...
+Error: [Cabal-9491]
+Hooks: mismatched component types in per-component configure hook.
+Trying to apply an executable diff to a library.

cabal-testsuite/PackageTests/SetupHooksBadDiff1/setup.test.hs

@@ -0,0 +1,3 @@
+import Test.Cabal.Prelude
+main = setupTest $ do
+  fails $ setup "configure" []

cabal-testsuite/PackageTests/SetupHooksBadDiff2/Setup.hs

@@ -0,0 +1,7 @@
+module Main where
+
+import Distribution.Simple ( defaultMainWithSetupHooks )
+import SetupHooks ( setupHooks )
+
+main :: IO ()
+main = defaultMainWithSetupHooks setupHooks