diff options
| author | Ben Sima <ben@bensima.com> | 2025-11-26 14:08:46 -0500 |
|---|---|---|
| committer | Ben Sima <ben@bensima.com> | 2025-11-26 14:08:46 -0500 |
| commit | cebb8894f5da27945e78a7d01a86f664bcd39502 (patch) | |
| tree | 32caf61f45b21292d3c94e9e3adb9f8aa1bb7263 /Omni | |
| parent | 6305179905d5428879aaf8c08fef1722c734c3d7 (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
Diffstat (limited to 'Omni')
| -rw-r--r-- | Omni/Bild.hs | 31 | ||||
| -rw-r--r-- | Omni/Log/Concurrent.hs | 130 |
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 |
