Check for deps on exe-only packages #2195

ChangeLog.md

@@ -103,6 +103,10 @@ Other enhancements:
 * `--cwd DIR` can now be passed to `stack exec` in order to execute the
   program in a different directory. See:
   [#3264](https://github.com/commercialhaskell/stack/issues/3264)
+* Plan construction will detect if you add an executable-only package
+  as a library dependency, resulting in much clearer error
+  messages. See:
+  [#2195](https://github.com/commercialhaskell/stack/issues/2195).
 
 Bug fixes:
 

src/Stack/Build/ConstructPlan.hs

@@ -180,10 +180,10 @@ constructPlan ls0 baseConfigOpts0 locals extraToBuild0 localDumpPkgs loadPackage
     u <- askUnliftIO
 
     econfig <- view envConfigL
-    let onWanted = void . addDep False . packageName . lpPackage
+    let onWanted = void . addDep False Nothing . packageName . lpPackage
     let inner = do
             mapM_ onWanted $ filter lpWanted locals
-            mapM_ (addDep False) $ Set.toList extraToBuild0
+            mapM_ (addDep False Nothing) $ Set.toList extraToBuild0
     lp <- getLocalPackages
     let ctx = mkCtx econfig (unliftIO u . getPackageVersions) lp
     ((), m, W efinals installExes dirtyReason deps warnings parents) <-
@@ -357,6 +357,13 @@ addFinal lp package isAllInOne = do
                 }
     tell mempty { wFinals = Map.singleton (packageName package) res }
 
+-- | Is this package being used as a library, or just as a build tool?
+-- If the former, we need to ensure that a library actually
+-- exists. See
+-- <https://github.com/commercialhaskell/stack/issues/2195>
+data DepType = AsLibrary | AsBuildTool
+  deriving Show
+
 -- | Given a 'PackageName', adds all of the build tasks to build the
 -- package, if needed.
 --
@@ -368,14 +375,15 @@ addFinal lp package isAllInOne = do
 -- directly wanted. This makes sense - if we left out packages that are
 -- deps, it would break the --only-dependencies build plan.
 addDep :: Bool -- ^ is this being used by a dependency?
+       -> Maybe DepType
        -> PackageName
        -> M (Either ConstructPlanException AddDepRes)
-addDep treatAsDep' name = do
+addDep treatAsDep' mdepType name = do
     ctx <- ask
     let treatAsDep = treatAsDep' || name `Set.notMember` wanted ctx
     when treatAsDep $ markAsDep name
     m <- get
-    case Map.lookup name m of
+    eres <- case Map.lookup name m of
         Just res -> do
             planDebug $ "addDep: Using cached result for " ++ show name ++ ": " ++ show res
             return res
@@ -408,6 +416,27 @@ addDep treatAsDep' name = do
                             installPackage treatAsDep name ps (Just installed)
             updateLibMap name res
             return res
+    case (mdepType, eres) of
+      (Just AsLibrary, Right res)
+        | not (adrHasLibrary res) -> return $ Left $ BuildDependencyHasNoLibrary name
+      _ -> return eres
+  where
+    adrHasLibrary :: AddDepRes -> Bool
+    adrHasLibrary (ADRToInstall task) = taskHasLibrary task
+    adrHasLibrary (ADRFound _ (Library _ _)) = True
+    adrHasLibrary (ADRFound _ (Executable _)) = False
+
+    taskHasLibrary :: Task -> Bool
+    taskHasLibrary task =
+      case taskType task of
+        TTFiles lp _ -> packageHasLibrary $ lpPackage lp
+        TTIndex package _ _ -> packageHasLibrary package
+
+    packageHasLibrary :: Package -> Bool
+    packageHasLibrary package =
+      case packageLibraries package of
+        HasLibraries _ -> True
+        NoLibraries -> False
 
 -- FIXME what's the purpose of this? Add a Haddock!
 tellExecutables :: PackageSource -> M ()
@@ -592,8 +621,8 @@ addPackageDeps :: Bool -- ^ is this being used by a dependency?
 addPackageDeps treatAsDep package = do
     ctx <- ask
     deps' <- packageDepsWithTools package
-    deps <- forM (Map.toList deps') $ \(depname, range) -> do
-        eres <- addDep treatAsDep depname
+    deps <- forM (Map.toList deps') $ \(depname, (range, depType)) -> do
+        eres <- addDep treatAsDep (Just depType) depname
         let getLatestApplicable = do
                 vs <- liftIO $ getVersions ctx depname
                 return (latestApplicableVersion range vs)
@@ -793,7 +822,7 @@ psLocal PSIndex{} = False
 
 -- | Get all of the dependencies for a given package, including guessed build
 -- tool dependencies.
-packageDepsWithTools :: Package -> M (Map PackageName VersionRange)
+packageDepsWithTools :: Package -> M (Map PackageName (VersionRange, DepType))
 packageDepsWithTools p = do
     ctx <- ask
     -- TODO: it would be cool to defer these warnings until there's an
@@ -816,9 +845,16 @@ packageDepsWithTools p = do
             Nothing -> return (Just warning)
             Just _ -> return Nothing
     tell mempty { wWarnings = (map toolWarningText warnings ++) }
-    return $ Map.unionsWith intersectVersionRanges
-           $ packageDeps p
-           : toolDeps
+    return $ Map.unionsWith
+               (\(vr1, dt1) (vr2, dt2) ->
+                    ( intersectVersionRanges vr1 vr2
+                    , case dt1 of
+                        AsLibrary -> AsLibrary
+                        AsBuildTool -> dt2
+                    )
+               )
+           $ ((, AsLibrary) <$> packageDeps p)
+           : (Map.map (, AsBuildTool) <$> toolDeps)
 
 -- | Warn about tools in the snapshot definition. States the tool name
 -- expected, the package name using it, and found packages. If the
@@ -881,6 +917,8 @@ data ConstructPlanException
     | DependencyPlanFailures Package (Map PackageName (VersionRange, LatestApplicableVersion, BadDependency))
     | UnknownPackage PackageName -- TODO perhaps this constructor will be removed, and BadDependency will handle it all
     -- ^ Recommend adding to extra-deps, give a helpful version number?
+    | BuildDependencyHasNoLibrary !PackageName
+    -- ^ See description of 'DepType'
     deriving (Typeable, Eq, Ord, Show)
 
 deriving instance Ord VersionRange
@@ -942,6 +980,7 @@ pprintExceptions exceptions stackYaml parentMap wanted =
        go (name, (_range, Just version, DependencyMismatch{})) =
            Map.singleton name version
        go _ = Map.empty
+    getExtras (BuildDependencyHasNoLibrary _) = Map.empty
     pprintExtra (name, version) =
       fromString (concat ["- ", packageNameString name, "-", versionString version])
 
@@ -978,6 +1017,8 @@ pprintExceptions exceptions stackYaml parentMap wanted =
         | name `HashSet.member` wiredInPackages =
             Just $ flow "Can't build a package with same name as a wired-in-package:" <+> (styleCurrent . display $ name)
         | otherwise = Just $ flow "Unknown package:" <+> (styleCurrent . display $ name)
+    pprintException (BuildDependencyHasNoLibrary name) =
+      Just $ flow "You have a build dependency on a package that provides no library:" <+> (styleCurrent . display $ name)
 
     pprintFlags flags
         | Map.null flags = ""

test/integration/lib/StackTest.hs

@@ -108,6 +108,18 @@ stackCheckStderr args check = do
         then error $ "Exited with exit code: " ++ show ec
         else check err
 
+-- | Same as 'stackCheckStderr', but ensures that the Stack process
+-- fails.
+stackErrStderr :: [String] -> (String -> IO ()) -> IO ()
+stackErrStderr args check = do
+    stackExe <- getEnv "STACK_EXE"
+    logInfo $ "Running: " ++ stackExe ++ " " ++ unwords (map showProcessArgDebug args)
+    (ec, _, err) <- readProcessWithExitCode stackExe args ""
+    hPutStr stderr err
+    if ec == ExitSuccess
+        then error "Stack process succeeded, but it shouldn't"
+        else check err
+
 doesNotExist :: FilePath -> IO ()
 doesNotExist fp = do
     logInfo $ "doesNotExist " ++ fp

test/integration/tests/2195-depend-on-exe/Main.hs

@@ -0,0 +1,11 @@
+import Control.Monad (unless)
+import Data.List (isInfixOf)
+import StackTest
+
+main :: IO ()
+main = stackErrStderr ["build"] (expectMessage "that provides no library")
+
+expectMessage :: String -> String -> IO ()
+expectMessage msg stderr =
+    unless (msg `isInfixOf` stderr)
+        (error $ "Expected a warning: \n" ++ show msg)

test/integration/tests/2195-depend-on-exe/files/files.cabal

@@ -0,0 +1,10 @@
+name:                files
+version:             0.1.0.0
+build-type:          Simple
+cabal-version:       >=1.10
+
+library
+  hs-source-dirs:      src
+  exposed-modules:     Lib
+  build-depends:       happy
+  default-language:    Haskell2010

test/integration/tests/2195-depend-on-exe/files/stack.yaml

@@ -0,0 +1 @@
+resolver: lts-9.9