← Back to task

Commit cebb8894

commit cebb8894f5da27945e78a7d01a86f664bcd39502
Author: Ben Sima <ben@bensima.com>
Date:   Wed Nov 26 14:08:46 2025

    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

diff --git a/Omni/Bild.hs b/Omni/Bild.hs
index 8d009368..1bb1c83a 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 8c48f837..f61eb2ea 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