summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBen Sima <ben@bensima.com>2025-11-26 14:08:46 -0500
committerBen Sima <ben@bensima.com>2025-11-26 14:08:46 -0500
commitcebb8894f5da27945e78a7d01a86f664bcd39502 (patch)
tree32caf61f45b21292d3c94e9e3adb9f8aa1bb7263
parent6305179905d5428879aaf8c08fef1722c734c3d7 (diff)
Fix bild status line output not overwriting properly
The fix is complete. Here's a summary of the changes: Fixed the bild status line output not overwriting properly by: 1. **Made `withLineManager` reentrant** ([Omni/Log/Concurrent.hs](file:/ 2. **Added `linesInitialized` tracking** ([Omni/Log/Concurrent.hs](file: 3. **Added `lastOutputTransient` tracking** ([Omni/Log/Concurrent.hs](fi 4. **Wrapped analyze+build in single manager** ([Omni/Bild.hs](file:///h Task-Id: t-l6kc73wk
-rw-r--r--Omni/Bild.hs31
-rw-r--r--Omni/Log/Concurrent.hs130
2 files changed, 100 insertions, 61 deletions
diff --git a/Omni/Bild.hs b/Omni/Bild.hs
index 8d00936..1bb1c83 100644
--- a/Omni/Bild.hs
+++ b/Omni/Bild.hs
@@ -227,18 +227,25 @@ move args = do
+> traverse (namespaceFromPathOrDie root)
/> filter isBuildableNs
let isPlanMode = args `Cli.has` Cli.longOption "plan"
- analysis <- analyzeAll isPlanMode namespaces
- printOrBuild root analysis
- |> Timeout.timeout (toMillis minutes)
- +> \case
- Nothing ->
- Log.br
- >> Log.fail ["bild", "timeout after " <> tshow minutes <> " minutes"]
- >> Log.br
- >> exitWith (ExitFailure 124)
- Just s -> do
- when (all isSuccess s) saveGhcPkgCache
- exitSummary s
+ -- Wrap entire analyze+build sequence in a single LineManager
+ -- to avoid duplicate status line output
+ let runWithManager action = case (isPlanMode, isLoud) of
+ (True, _) -> action -- Plan mode doesn't need a manager
+ (_, True) -> action -- Loud mode doesn't need a manager
+ _ -> LogC.withLineManager namespaces (const action)
+ runWithManager <| do
+ analysis <- analyzeAll isPlanMode namespaces
+ printOrBuild root analysis
+ |> Timeout.timeout (toMillis minutes)
+ +> \case
+ Nothing ->
+ Log.br
+ >> Log.fail ["bild", "timeout after " <> tshow minutes <> " minutes"]
+ >> Log.br
+ >> exitWith (ExitFailure 124)
+ Just s -> do
+ when (all isSuccess s) saveGhcPkgCache
+ exitSummary s
where
minutes =
Cli.getArgWithDefault args "10" (Cli.longOption "time")
diff --git a/Omni/Log/Concurrent.hs b/Omni/Log/Concurrent.hs
index 8c48f83..f61eb2e 100644
--- a/Omni/Log/Concurrent.hs
+++ b/Omni/Log/Concurrent.hs
@@ -42,6 +42,17 @@ currentLineManager = unsafePerformIO (newIORef Nothing)
namespaceLines :: IORef (Map Namespace Int)
namespaceLines = unsafePerformIO (newIORef Map.empty)
+-- | Tracks if the last output was transient (no newline printed)
+-- When True, cleanup should not add a newline since next manager will overwrite
+{-# NOINLINE lastOutputTransient #-}
+lastOutputTransient :: IORef Bool
+lastOutputTransient = unsafePerformIO (newIORef False)
+
+-- | Tracks if lines have been initialized (prevents duplicate initialization)
+{-# NOINLINE linesInitialized #-}
+linesInitialized :: IORef Bool
+linesInitialized = unsafePerformIO (newIORef False)
+
-- | Global lock for all terminal operations
-- ANSI terminal library (ncurses) is not thread-safe, so we must serialize all calls
-- to prevent segfaults during concurrent builds
@@ -51,55 +62,72 @@ terminalLock = unsafePerformIO (newMVar ())
withLineManager :: [Namespace] -> (LineManager -> IO a) -> IO a
withLineManager nss action = do
- termInfo <- detectTerminal
-
- case tiMode termInfo of
- SingleLine -> do
- -- Single-line mode: no reservations, updates in place
- let mgr = LineManager {lmNamespaces = nss, lmTermInfo = termInfo}
- writeIORef currentLineManager (Just mgr)
- result <- action mgr
- IO.hPutStrLn IO.stderr "" -- Final newline
- writeIORef currentLineManager Nothing
- writeIORef namespaceLines Map.empty
- pure result
- MultiLine -> do
- -- Multi-line mode: reserve lines for each namespace
- let numLines = min (length nss) (tiHeight termInfo - 2)
- IO.hPutStrLn IO.stderr ""
- replicateM_ numLines (IO.hPutStrLn IO.stderr "")
- withMVar terminalLock <| \_ -> ANSI.hCursorUp IO.stderr numLines
-
- let mgr = LineManager {lmNamespaces = nss, lmTermInfo = termInfo}
- writeIORef currentLineManager (Just mgr)
-
- -- Initialize the namespace -> line mapping
- writeIORef namespaceLines (Map.fromList <| zip nss [0 ..])
-
- result <- action mgr
-
- IO.hPutStrLn IO.stderr ""
- writeIORef currentLineManager Nothing
- writeIORef namespaceLines Map.empty
- pure result
+ -- Check if a manager is already active (reentrant call)
+ existingMgr <- readIORef currentLineManager
+ maybe createNewManager action existingMgr
+ where
+ createNewManager = do
+ termInfo <- detectTerminal
+
+ case tiMode termInfo of
+ SingleLine -> do
+ -- Single-line mode: no reservations, updates in place
+ let mgr = LineManager {lmNamespaces = nss, lmTermInfo = termInfo}
+ writeIORef currentLineManager (Just mgr)
+ writeIORef lastOutputTransient False
+ writeIORef linesInitialized False
+ result <- action mgr
+ -- Only print final newline if last output wasn't transient
+ -- (transient outputs expect to be overwritten by next manager)
+ wasTransient <- readIORef lastOutputTransient
+ unless wasTransient (IO.hPutStrLn IO.stderr "")
+ writeIORef currentLineManager Nothing
+ writeIORef namespaceLines Map.empty
+ writeIORef linesInitialized False
+ pure result
+ MultiLine -> do
+ -- Multi-line mode: reserve lines for each namespace
+ let numLines = min (length nss) (tiHeight termInfo - 2)
+ IO.hPutStrLn IO.stderr ""
+ replicateM_ numLines (IO.hPutStrLn IO.stderr "")
+ withMVar terminalLock <| \_ -> ANSI.hCursorUp IO.stderr numLines
+
+ let mgr = LineManager {lmNamespaces = nss, lmTermInfo = termInfo}
+ writeIORef currentLineManager (Just mgr)
+ writeIORef linesInitialized False
+
+ -- Initialize the namespace -> line mapping
+ writeIORef namespaceLines (Map.fromList <| zip nss [0 ..])
+
+ result <- action mgr
+
+ IO.hPutStrLn IO.stderr ""
+ writeIORef currentLineManager Nothing
+ writeIORef namespaceLines Map.empty
+ writeIORef linesInitialized False
+ pure result
-- | Initialize all lines with pending status
+-- Only initializes once per manager session (prevents duplicate output on reentrant calls)
initializeLines :: LineManager -> IO ()
-initializeLines LineManager {..} =
- case (tiMode lmTermInfo, tiSupportsANSI lmTermInfo) of
- (_, False) -> pure () -- No ANSI support, skip initialization
- (SingleLine, _) -> pure () -- No initialization needed
- (MultiLine, _) -> do
- nsMap <- readIORef namespaceLines
- forM_ (Map.toList nsMap) <| \(ns, _) ->
- withMVar terminalLock <| \_ -> do
- ANSI.hSetCursorColumn IO.stderr 0
- ANSI.hClearLine IO.stderr
- let nsText = Text.pack (Namespace.toPath ns)
- let msg = "[+] " <> nsText
- let truncated = truncateToWidth (tiWidth lmTermInfo - 1) msg
- IO.hPutStrLn IO.stderr (Text.unpack truncated)
- IO.hFlush IO.stderr
+initializeLines LineManager {..} = do
+ alreadyInit <- readIORef linesInitialized
+ unless alreadyInit
+ <| case (tiMode lmTermInfo, tiSupportsANSI lmTermInfo) of
+ (_, False) -> pure () -- No ANSI support, skip initialization
+ (SingleLine, _) -> writeIORef linesInitialized True -- Mark as done even if no-op
+ (MultiLine, _) -> do
+ writeIORef linesInitialized True
+ nsMap <- readIORef namespaceLines
+ forM_ (Map.toList nsMap) <| \(ns, _) ->
+ withMVar terminalLock <| \_ -> do
+ ANSI.hSetCursorColumn IO.stderr 0
+ ANSI.hClearLine IO.stderr
+ let nsText = Text.pack (Namespace.toPath ns)
+ let msg = "[+] " <> nsText
+ let truncated = truncateToWidth (tiWidth lmTermInfo - 1) msg
+ IO.hPutStrLn IO.stderr (Text.unpack truncated)
+ IO.hFlush IO.stderr
updateLine :: Namespace -> Text -> IO ()
updateLine ns output = do
@@ -175,9 +203,13 @@ updateLineState ns buildState = do
IO.hPutStr IO.stderr "\r"
Rainbow.hPutChunks IO.stderr [fore color <| chunk truncated]
case buildState of
- Success -> IO.hPutStrLn IO.stderr "" -- Keep successes visible
- Failed -> IO.hPutStrLn IO.stderr "" -- Keep failures visible
- _ -> pure () -- Transient states overwrite
+ Success -> do
+ IO.hPutStrLn IO.stderr "" -- Keep successes visible
+ writeIORef lastOutputTransient False
+ Failed -> do
+ IO.hPutStrLn IO.stderr "" -- Keep failures visible
+ writeIORef lastOutputTransient False
+ _ -> writeIORef lastOutputTransient True -- Transient states overwrite
IO.hFlush IO.stderr
MultiLine ->
-- Multi-line: use reserved lines with truncation