Merge pull request #3505 from commercialhaskell/2195-dep-on-exe

Check for deps on exe-only packages #2195

ChangeLog.md

@@ -100,6 +100,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

@@ -358,6 +358,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, Eq)
+
 -- | Given a 'PackageName', adds all of the build tasks to build the
 -- package, if needed.
 --
@@ -594,7 +601,7 @@ 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
+    deps <- forM (Map.toList deps') $ \(depname, (range, depType)) -> do
         eres <- addDep treatAsDep depname
         let getLatestApplicable = do
                 vs <- liftIO $ getVersions ctx depname
@@ -608,6 +615,8 @@ addPackageDeps treatAsDep package = do
                             _ -> Couldn'tResolveItsDependencies (packageVersion package)
                 mlatestApplicable <- getLatestApplicable
                 return $ Left (depname, (range, mlatestApplicable, bd))
+            Right adr | depType == AsLibrary && not (adrHasLibrary adr) ->
+                return $ Left (depname, (range, Nothing, HasNoLibrary))
             Right adr -> do
                 addParent depname range Nothing
                 inRange <- if adrVersion adr `withinRange` range
@@ -669,6 +678,23 @@ addPackageDeps treatAsDep package = do
       where
         val = (First mversion, [(packageIdentifier package, range)])
 
+    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 p _ _ -> packageHasLibrary p
+
+    packageHasLibrary :: Package -> Bool
+    packageHasLibrary p =
+      case packageLibraries p of
+        HasLibraries _ -> True
+        NoLibraries -> False
+
 checkDirtiness :: PackageSource
                -> Installed
                -> Package
@@ -795,7 +821,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
@@ -818,9 +844,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
@@ -895,6 +928,8 @@ data BadDependency
     = NotInBuildPlan
     | Couldn'tResolveItsDependencies Version
     | DependencyMismatch Version
+    | HasNoLibrary
+    -- ^ See description of 'DepType'
     deriving (Typeable, Eq, Ord, Show)
 
 -- TODO: Consider intersecting version ranges for multiple deps on a
@@ -1002,6 +1037,9 @@ pprintExceptions exceptions stackYaml parentMap wanted =
         -- packages are needed. Instead lets give the user the shortest
         -- path from a target to the package.
         Couldn'tResolveItsDependencies _version -> Nothing
+        HasNoLibrary -> Just $
+            styleError (display name) <+>
+            align (flow "is a library dependency, but the package provides no library")
       where
         goodRange = styleGood (fromString (Cabal.display range))
         latestApplicable mversion =

test/integration/lib/StackTest.hs

@@ -24,9 +24,12 @@ run cmd args = do
     ec <- run' cmd args
     unless (ec == ExitSuccess) $ error $ "Exited with exit code: " ++ show ec
 
+stackExe :: IO String
+stackExe = getEnv "STACK_EXE"
+
 stack' :: [String] -> IO ExitCode
 stack' args = do
-    stackEnv <- getEnv "STACK_EXE"
+    stackEnv <- stackExe
     run' stackEnv args
 
 stack :: [String] -> IO ()
@@ -89,25 +92,39 @@ runRepl cmd args actions = do
 
 repl :: [String] -> Repl () -> IO ()
 repl args action = do
-    stackExe <- getEnv "STACK_EXE"
-    ec <- runRepl stackExe ("repl":args) action
+    stackExe' <- stackExe
+    ec <- runRepl stackExe' ("repl":args) action
     unless (ec == ExitSuccess) $ return ()
         -- TODO: Understand why the exit code is 1 despite running GHCi tests
         -- successfully.
         -- else error $ "Exited with exit code: " ++ show ec
 
+stackStderr :: [String] -> IO (ExitCode, String)
+stackStderr args = do
+    stackExe' <- stackExe
+    logInfo $ "Running: " ++ stackExe' ++ " " ++ unwords (map showProcessArgDebug args)
+    (ec, _, err) <- readProcessWithExitCode stackExe' args ""
+    hPutStr stderr err
+    return (ec, err)
+
 -- | Run stack with arguments and apply a check to the resulting
 -- stderr output if the process succeeded.
 stackCheckStderr :: [String] -> (String -> IO ()) -> IO ()
 stackCheckStderr args check = do
-    stackExe <- getEnv "STACK_EXE"
-    logInfo $ "Running: " ++ stackExe ++ " " ++ unwords (map showProcessArgDebug args)
-    (ec, _, err) <- readProcessWithExitCode stackExe args ""
-    hPutStr stderr err
+    (ec, err) <- stackStderr args
     if ec /= ExitSuccess
         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
+    (ec, err) <- stackStderr args
+    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 "package 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,7 @@
+name:                files
+version:             0.1.0.0
+build-type:          Simple
+cabal-version:       >=1.10
+
+library
+  build-depends:       happy

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

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