From a3b354c346f2f08333b3efa76bdb505dedc8fd0f Mon Sep 17 00:00:00 2001 From: Michael Snoyman Date: Sun, 22 Oct 2017 09:43:38 +0300 Subject: [PATCH] Check for deps on exe-only packages #2195 --- ChangeLog.md | 4 ++ src/Stack/Build/ConstructPlan.hs | 48 +++++++++++++++++-- test/integration/lib/StackTest.hs | 31 +++++++++--- .../tests/2195-depend-on-exe/Main.hs | 11 +++++ .../2195-depend-on-exe/files/files.cabal | 7 +++ .../tests/2195-depend-on-exe/files/stack.yaml | 1 + 6 files changed, 90 insertions(+), 12 deletions(-) create mode 100644 test/integration/tests/2195-depend-on-exe/Main.hs create mode 100644 test/integration/tests/2195-depend-on-exe/files/files.cabal create mode 100644 test/integration/tests/2195-depend-on-exe/files/stack.yaml diff --git a/ChangeLog.md b/ChangeLog.md index 271ec55d04..675c677e84 100644 --- a/ChangeLog.md +++ b/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: diff --git a/src/Stack/Build/ConstructPlan.hs b/src/Stack/Build/ConstructPlan.hs index 37dbd5aa52..ba458e5c7f 100644 --- a/src/Stack/Build/ConstructPlan.hs +++ b/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 +-- +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 = diff --git a/test/integration/lib/StackTest.hs b/test/integration/lib/StackTest.hs index c29943620a..05f8196d62 100644 --- a/test/integration/lib/StackTest.hs +++ b/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 diff --git a/test/integration/tests/2195-depend-on-exe/Main.hs b/test/integration/tests/2195-depend-on-exe/Main.hs new file mode 100644 index 0000000000..11f81255a9 --- /dev/null +++ b/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) diff --git a/test/integration/tests/2195-depend-on-exe/files/files.cabal b/test/integration/tests/2195-depend-on-exe/files/files.cabal new file mode 100644 index 0000000000..5edf065804 --- /dev/null +++ b/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 diff --git a/test/integration/tests/2195-depend-on-exe/files/stack.yaml b/test/integration/tests/2195-depend-on-exe/files/stack.yaml new file mode 100644 index 0000000000..6b15e3d046 --- /dev/null +++ b/test/integration/tests/2195-depend-on-exe/files/stack.yaml @@ -0,0 +1 @@ +resolver: lts-9.9