Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use stderr for Cabal's diagnostic messages instead of stdout #4789

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Cabal/Distribution/Simple/Program/GHC.hs
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,8 @@ ghcInvocation :: ConfiguredProgram -> Compiler -> Platform -> GhcOptions
-> ProgramInvocation
ghcInvocation prog comp platform opts =
(programInvocation prog (renderGhcOptions comp platform opts)) {
progInvokePathEnv = fromNubListR (ghcOptExtraPath opts)
progInvokePathEnv = fromNubListR (ghcOptExtraPath opts),
progInvokeOutAsErr = True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only do this for GHC? Perhaps it should be the default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure. Do these other programs write non-essential messages to stdout? I think the use of stdout for progress messages as GHC does is not normal, but I suppose Cabal could be proactive and force everything to stderr.

}

renderGhcOptions :: Compiler -> Platform -> GhcOptions -> [String]
Expand Down
15 changes: 11 additions & 4 deletions Cabal/Distribution/Simple/Program/Run.hs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import Distribution.Verbosity

import System.Exit (ExitCode (..), exitWith)
import System.FilePath
import System.IO (stderr)

import qualified Data.ByteString.Lazy as LBS
import qualified Data.Map as Map
Expand All @@ -60,7 +61,8 @@ data ProgramInvocation = ProgramInvocation {
progInvokeCwd :: Maybe FilePath,
progInvokeInput :: Maybe IOData,
progInvokeInputEncoding :: IOEncoding, -- ^ TODO: remove this, make user decide when constructing 'progInvokeInput'.
progInvokeOutputEncoding :: IOEncoding
progInvokeOutputEncoding :: IOEncoding,
progInvokeOutAsErr :: Bool
}

data IOEncoding = IOEncodingText -- locale mode text
Expand All @@ -81,7 +83,8 @@ emptyProgramInvocation =
progInvokeCwd = Nothing,
progInvokeInput = Nothing,
progInvokeInputEncoding = IOEncodingText,
progInvokeOutputEncoding = IOEncodingText
progInvokeOutputEncoding = IOEncodingText,
progInvokeOutAsErr = False
}

simpleProgramInvocation :: FilePath -> [String] -> ProgramInvocation
Expand Down Expand Up @@ -121,16 +124,20 @@ runProgramInvocation verbosity
progInvokeEnv = envOverrides,
progInvokePathEnv = extraPath,
progInvokeCwd = mcwd,
progInvokeInput = Nothing
progInvokeInput = Nothing,
progInvokeOutAsErr = outAsErr
} = do
pathOverride <- getExtraPathEnv envOverrides extraPath
menv <- getEffectiveEnvironment (envOverrides ++ pathOverride)
exitCode <- rawSystemIOWithEnv verbosity
path args
mcwd menv
Nothing Nothing Nothing
Nothing childStdout Nothing
when (exitCode /= ExitSuccess) $
exitWith exitCode
where
childStdout | outAsErr = Just stderr
| otherwise = Nothing

runProgramInvocation verbosity
ProgramInvocation {
Expand Down
36 changes: 22 additions & 14 deletions Cabal/Distribution/Simple/Utils.hs
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ import System.FilePath as FilePath
, getSearchPath, joinPath, takeDirectory, splitExtension
, splitDirectories, searchPathSeparator )
import System.IO
( Handle, hSetBinaryMode, hGetContents, stderr, stdout, hPutStr, hFlush
, hClose, hSetBuffering, BufferMode(..) )
( Handle, hSetBinaryMode, hGetContents, stderr, stdout, hPutStr, hPutStrLn
, hFlush, hClose, hSetBuffering, BufferMode(..) )
import System.IO.Error
import System.IO.Unsafe
( unsafeInterleaveIO )
Expand Down Expand Up @@ -453,7 +453,8 @@ notice :: Verbosity -> String -> IO ()
notice verbosity msg = withFrozenCallStack $ do
when (verbosity >= normal) $ do
ts <- getPOSIXTime
hPutStr stdout . withMetadata ts NormalMark FlagTrace verbosity
hFlush stdout
hPutStr stderr . withMetadata ts NormalMark FlagTrace verbosity
. wrapTextVerbosity verbosity
$ msg

Expand All @@ -464,7 +465,8 @@ noticeNoWrap :: Verbosity -> String -> IO ()
noticeNoWrap verbosity msg = withFrozenCallStack $ do
when (verbosity >= normal) $ do
ts <- getPOSIXTime
hPutStr stdout . withMetadata ts NormalMark FlagTrace verbosity $ msg
hFlush stdout
hPutStr stderr . withMetadata ts NormalMark FlagTrace verbosity $ msg

-- | Pretty-print a 'Disp.Doc' status message at 'normal' verbosity
-- level. Use this if you need fancy formatting.
Expand All @@ -473,7 +475,8 @@ noticeDoc :: Verbosity -> Disp.Doc -> IO ()
noticeDoc verbosity msg = withFrozenCallStack $ do
when (verbosity >= normal) $ do
ts <- getPOSIXTime
hPutStr stdout . withMetadata ts NormalMark FlagTrace verbosity
hFlush stdout
hPutStr stderr . withMetadata ts NormalMark FlagTrace verbosity
. Disp.renderStyle defaultStyle $ msg

-- | Display a "setup status message". Prefer using setupMessage'
Expand All @@ -491,15 +494,17 @@ info :: Verbosity -> String -> IO ()
info verbosity msg = withFrozenCallStack $
when (verbosity >= verbose) $ do
ts <- getPOSIXTime
hPutStr stdout . withMetadata ts NeverMark FlagTrace verbosity
hFlush stdout
hPutStr stderr . withMetadata ts NeverMark FlagTrace verbosity
. wrapTextVerbosity verbosity
$ msg

infoNoWrap :: Verbosity -> String -> IO ()
infoNoWrap verbosity msg = withFrozenCallStack $
when (verbosity >= verbose) $ do
ts <- getPOSIXTime
hPutStr stdout . withMetadata ts NeverMark FlagTrace verbosity
hFlush stdout
hPutStr stderr . withMetadata ts NeverMark FlagTrace verbosity
$ msg

-- | Detailed internal debugging information
Expand All @@ -510,31 +515,34 @@ debug :: Verbosity -> String -> IO ()
debug verbosity msg = withFrozenCallStack $
when (verbosity >= deafening) $ do
ts <- getPOSIXTime
hPutStr stdout . withMetadata ts NeverMark FlagTrace verbosity
hFlush stdout
hPutStr stderr . withMetadata ts NeverMark FlagTrace verbosity
. wrapTextVerbosity verbosity
$ msg
-- ensure that we don't lose output if we segfault/infinite loop
hFlush stdout
hFlush stderr

-- | A variant of 'debug' that doesn't perform the automatic line
-- wrapping. Produces better output in some cases.
debugNoWrap :: Verbosity -> String -> IO ()
debugNoWrap verbosity msg = withFrozenCallStack $
when (verbosity >= deafening) $ do
ts <- getPOSIXTime
hPutStr stdout . withMetadata ts NeverMark FlagTrace verbosity
hFlush stdout
hPutStr stderr . withMetadata ts NeverMark FlagTrace verbosity
$ msg
-- ensure that we don't lose output if we segfault/infinite loop
hFlush stdout
hFlush stderr

-- | Perform an IO action, catching any IO exceptions and printing an error
-- if one occurs.
chattyTry :: String -- ^ a description of the action we were attempting
-> IO () -- ^ the action itself
-> IO ()
chattyTry desc action =
catchIO action $ \exception ->
putStrLn $ "Error while " ++ desc ++ ": " ++ show exception
catchIO action $ \exception -> do
hFlush stdout
hPutStrLn stderr $ "Error while " ++ desc ++ ": " ++ show exception

-- | Run an IO computation, returning @e@ if it raises a "file
-- does not exist" error.
Expand Down Expand Up @@ -741,7 +749,7 @@ rawSystemExitWithEnv verbosity path args env = withFrozenCallStack $ do
debug verbosity $ path ++ " returned " ++ show exitcode
exitWith exitcode

-- Closes the passed in handles before returning.
-- Closes the passed in handles before returning (excluding standard handles).
rawSystemIOWithEnv :: Verbosity
-> FilePath
-> [String]
Expand Down
3 changes: 2 additions & 1 deletion Cabal/Distribution/Utils/LogProgress.hs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import Distribution.Utils.Progress
import Distribution.Verbosity
import Distribution.Simple.Utils
import Text.PrettyPrint
import System.IO (hPutStrLn, stderr)

type CtxMsg = Doc
type LogMsg = Doc
Expand Down Expand Up @@ -54,7 +55,7 @@ runLogProgress verbosity (LogProgress m) =
}
step_fn :: LogMsg -> NoCallStackIO a -> NoCallStackIO a
step_fn doc go = do
putStrLn (render doc)
hPutStrLn stderr (render doc)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be replaced with notice as well, I think...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted this particular change. Changing runLogProgress.step_fn to use notice instead of hPutStrLn breaks too many tests as notice will wrap the messages with -----BEGIN CABAL OUTPUT----- markers, causing them to get caught in the test output.

go
fail_fn :: Doc -> NoCallStackIO a
fail_fn doc = do
Expand Down
8 changes: 4 additions & 4 deletions cabal-install/Distribution/Client/GenBounds.hs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import Distribution.Simple.Compiler
import Distribution.Simple.Program
( ProgramDb )
import Distribution.Simple.Utils
( tryFindPackageDesc )
( notice, tryFindPackageDesc, warn )
import Distribution.System
( Platform )
import Distribution.Deprecated.Text
Expand Down Expand Up @@ -120,13 +120,13 @@ genBounds verbosity packageDBs repoCtxt comp platform progdb mSandboxPkgInfo
let epd = finalizePD mempty defaultComponentRequestedSpec
(const True) platform cinfo [] gpd
case epd of
Left _ -> putStrLn "finalizePD failed"
Rufflewind marked this conversation as resolved.
Show resolved Hide resolved
Left _ -> warn verbosity "finalizePD failed"
Right (pd,_) -> do
let needBounds = filter (not . hasUpperBound . depVersion) $
enabledBuildDepends pd defaultComponentRequestedSpec

if (null needBounds)
then putStrLn
then notice verbosity
"Congratulations, all your dependencies have upper bounds!"
else go needBounds
where
Expand All @@ -135,7 +135,7 @@ genBounds verbosity packageDBs repoCtxt comp platform progdb mSandboxPkgInfo
verbosity packageDBs repoCtxt comp platform progdb
mSandboxPkgInfo globalFlags freezeFlags

putStrLn boundsNeededMsg
notice verbosity boundsNeededMsg

let isNeeded pkg = unPackageName (packageName pkg)
`elem` map depName needBounds
Expand Down
6 changes: 3 additions & 3 deletions cabal-install/Distribution/Client/Install.hs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import System.Directory
import System.FilePath
( (</>), (<.>), equalFilePath, takeDirectory )
import System.IO
( openFile, IOMode(AppendMode), hClose )
( openFile, IOMode(AppendMode), hPutStr, hClose, stderr )
import System.IO.Error
( isDoesNotExistError, ioeGetFileName )

Expand Down Expand Up @@ -1230,11 +1230,11 @@ executeInstallPlan verbosity jobCtl keepGoing useLogFile plan0 installPkg =
Nothing -> return ()
Just (mkLogFileName, _) -> do
let logName = mkLogFileName pkgid uid
putStr $ "Build log ( " ++ logName ++ " ):\n"
hPutStr stderr $ "Build log ( " ++ logName ++ " ):\n"
printFile logName

printFile :: FilePath -> IO ()
printFile path = readFile path >>= putStr
printFile path = readFile path >>= hPutStr stderr

-- | Call an installer for an 'SourcePackage' but override the configure
-- flags with the ones given by the 'ReadyPackage'. In particular the
Expand Down