diff --git a/ChangeLog.md b/ChangeLog.md index 6edfb923da..0a4171ae1b 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 21f2339370..d61eb44078 100644 --- a/src/Stack/Build/ConstructPlan.hs +++ b/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 +-- +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 = "" diff --git a/test/integration/lib/StackTest.hs b/test/integration/lib/StackTest.hs index c29943620a..cab1669fff 100644 --- a/test/integration/lib/StackTest.hs +++ b/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 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..caeef88259 --- /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 "that 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..66e4ade778 --- /dev/null +++ b/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 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