fix: correct requirement computation

.vscode/launch.json

@@ -12,12 +12,12 @@
             // "args": ["run", "build", "test", "--workspace", "tests/simple", "--parallel", "1", "--debug", "--log" ],
             // "args": ["run", "build", "test", "plan", "apply", "--workspace", "tests/indirect-target", "--parallel", "1", "--debug", "--log" ],
 
-            "args": ["run", "build", "--workspace", "../../matis/matis-next", "--retry", "--debug", "--local-only", "--log" ],
+            // "args": ["run", "build", "--workspace", "../../matis/matis-next", "--configuration", "local", "--retry", "--debug", "--local-only", "--log", "--what-if" ],
+            "args": ["run", "build", "test", "dist", "--configuration", "Debug", "--log", "--debug", "--parallel", "1", "--retry" ],
 
             // "args": ["run", "build", "--workspace", "tests/cluster-layers", "--force", "--debug", "-p", "1", "--whatif" ],
             // "args": ["run", "build", "--workspace", "tests/cluster-layers", "--force", "--debug", "-p", "1", "--whatif" ],
             // "args": ["run", "plan", "dist", "build", "test", "--workspace", "../insights", "--log", "-c", "dev", "--debug", "-p", "1", "--whatif" ],
-            // "args": ["run", "build", "test", "dist", "--log", "--debug", "--parallel", "1", "--retry", "--local-only", "-p", "terrabuild" ],
             // "args": [ "run", "build", "--workspace", "tests/basic", "--parallel", "1", "--force", "--debug", "--log" ],
             // "args": ["run", "build", "--workspace", "tests/simple", "--debug", "--parallel", "1", "--force" ],
             // "cwd": "${workspaceFolder}/tests/indirect-target",

src/Terrabuild.Tests/Core/GraphPipeline/Batch.fs

@@ -21,7 +21,8 @@ let ``check partition computation``() =
           Node.Artifacts = ArtifactMode.Workspace
           Node.Action = action
           Node.Build = BuildMode.Auto
-          Node.Batch = BatchMode.Partition }
+          Node.Batch = BatchMode.Partition
+          Node.Required = true }
 
     let addNode (node: Node) nodes = nodes |> Map.add node.Id node
 
@@ -88,7 +89,8 @@ let ``check partition/all computation``() =
           Node.Artifacts = ArtifactMode.Workspace
           Node.Action = action
           Node.Build = BuildMode.Auto
-          Node.Batch = group }
+          Node.Batch = group
+          Node.Required = true }
 
     let addNode (node: Node) nodes = nodes |> Map.add node.Id node
 

src/Terrabuild/Core/GraphDef.fs

@@ -61,6 +61,7 @@ type Node = {
     Build: BuildMode
     Batch: BatchMode
     Action: RunAction
+    Required: bool
 }
 
 

src/Terrabuild/Core/GraphPipeline/Action.fs

@@ -1,7 +1,6 @@
 
 module GraphPipeline.Action
 
-
 open System
 open Collections
 open Serilog
@@ -17,7 +16,7 @@ let build (options: ConfigOptions.Options) (cache: Cache.ICache) (graph: Graph)
 
     let getNodeAction (node: Node) hasChildBuilding =
         // task is forced to build
-        if node.Action = RunAction.Exec then
+        if node.Build = BuildMode.Always then
             Log.Debug("{NodeId} is marked for build", node.Id)
             (RunAction.Exec, DateTime.MaxValue)
 
@@ -90,8 +89,16 @@ let build (options: ConfigOptions.Options) (cache: Cache.ICache) (graph: Graph)
     | Status.SubscriptionError edi ->
         forwardInvalidArg("Failed to compute actions", edi.SourceException)
 
-    let nodes = graph.Nodes |> Map.addMap (nodes |> Seq.map (|KeyValue|) |> Map.ofSeq)
-    let rootNodes = graph.RootNodes |> Set.filter (fun nodeId -> nodes[nodeId].Action <> RunAction.Ignore)
+    let mutable nodes = graph.Nodes |> Map.addMap (nodes |> Seq.map (|KeyValue|) |> Map.ofSeq)
+    let rootNodes =
+        graph.RootNodes
+        |> Set.filter (fun nodeId -> nodes[nodeId].Action = RunAction.Exec)
+
+    // root node to execute are required
+    rootNodes
+    |> Set.iter (fun nodeId ->
+        let node = { nodes[nodeId] with Required = true }
+        nodes <- nodes |> Map.add node.Id node)
 
     let graph =
         { graph with

src/Terrabuild/Core/GraphPipeline/Batch.fs

@@ -72,7 +72,7 @@ let computeBatches (graph: Graph) =
     graph.Nodes
     |> Seq.choose (fun (KeyValue(_, node)) ->
         match node with
-        | { Action = RunAction.Exec; ClusterHash = Some clusterHash } -> Some (clusterHash, node)
+        | { Action = RunAction.Exec; ClusterHash = Some clusterHash; Required = true } -> Some (clusterHash, node)
         | _ -> None)
     |> Seq.groupBy fst
     |> Seq.collect (fun (clusterHash, items) ->
@@ -192,7 +192,8 @@ let private createBatchNodes (options: ConfigOptions.Options) (configuration: Co
                   GraphDef.Node.TargetHash = headNode.TargetHash
                   GraphDef.Node.Action = RunAction.Exec
                   GraphDef.Node.Build = headNode.Build
-                  GraphDef.Node.Batch = headNode.Batch }
+                  GraphDef.Node.Batch = headNode.Batch
+                  GraphDef.Node.Required = false }
 
             Some (batch.BatchId, batchNode)
     )

src/Terrabuild/Core/GraphPipeline/Cascade.fs

@@ -1,39 +1,42 @@
 module GraphPipeline.Cascade
 
-open System.Collections.Generic
 open Collections
 open GraphDef
 open Serilog
 
 let build (graph: Graph) =
-    // For each node: have we already processed it as "required"?
-    // false means "seen but only processed as not-required"
-    let processedRequired = Dictionary<string, bool>()
-    let mutable requiredNodes = Set.empty
-
-    let rec propagate parentIsRequired nodeId =
-        let node = graph.Nodes[nodeId]
-
-        if node.Action <> RunAction.Ignore then
-            let nodeRequired = parentIsRequired || node.Action = RunAction.Exec
-
-            match processedRequired.TryGetValue(nodeId) with
-            | true, alreadyRequired when alreadyRequired || not nodeRequired ->
-                // Already processed with >= this requiredness
-                ()
-            | _ ->
-                // Either first time, or upgrade from not-required -> required
-                processedRequired[nodeId] <- nodeRequired
-                if nodeRequired then
-                    Log.Debug("Node {NodeId} is marked as required", nodeId)
-                    requiredNodes <- requiredNodes |> Set.add node.Id
-                node.Dependencies |> Seq.iter (propagate nodeRequired)
-
-    graph.RootNodes |> Seq.iter (propagate false)
-
-    // keep only runnable root nodes
-    let rootNodes =
-        graph.RootNodes
-        |> Set.filter (fun rootNodeId -> requiredNodes |> Set.contains rootNodeId)
-
-    { graph with Graph.RootNodes = rootNodes }
+
+    let node2dependents = 
+        graph.Nodes
+        |> Seq.collect (fun (KeyValue(nodeId, node)) -> node.Dependencies |> Seq.map (fun depId -> depId, nodeId))
+        |> Seq.groupBy fst
+        |> Map.ofSeq
+        |> Map.map (fun _ depIds -> depIds |> Seq.map snd |> Set.ofSeq)
+
+    let mutable nodes = graph.Nodes
+
+    let mutable nodeRequirements = Map.empty
+    let rec getNodeRequirements nodeId =
+        match nodeRequirements |> Map.tryFind nodeId with
+        | Some requirement -> requirement
+        | _ ->
+            let node = nodes[nodeId]
+            let isRequired =
+                if node.Required then
+                    node.Required
+                else
+                    node2dependents
+                    |> Map.tryFind nodeId
+                    |> Option.defaultValue Set.empty
+                    |> Seq.exists getNodeRequirements
+
+            Log.Debug("Node {NodeId} has requirement {Requirement}", node.Id, isRequired)
+            nodeRequirements <- nodeRequirements |> Map.add nodeId isRequired
+            let node = { node with Required = isRequired }
+            nodes <- nodes |> Map.add node.Id node
+            isRequired
+
+    for nodeId in graph.Nodes.Keys do
+        getNodeRequirements nodeId |> ignore
+
+    { graph with Graph.Nodes = nodes }

src/Terrabuild/Core/GraphPipeline/Node.fs

@@ -147,24 +147,15 @@ let build (options: ConfigOptions.Options) (configuration: Configuration.Workspa
             let build =
                 let defaultForce = if options.Force then BuildMode.Always else BuildMode.Auto
                 targetConfig.Build |> Option.defaultValue defaultForce
-            let buildAction =
-                if build = BuildMode.Always then RunAction.Exec
-                else RunAction.Ignore
+            let required = build = BuildMode.Always
 
             let targetOutput =
                 if cache = ArtifactMode.None then Set.empty
                 else targetConfig.Outputs
 
             let targetClusterHash =
-                if batchable then
-                    let batchContent = [
-                        targetConfig.Hash
-                        $"{buildAction}"
-                    ]
-                    let batchHash = batchContent |> Hash.sha256strings
-                    Some batchHash
-                else
-                    None
+                if batchable then Some targetConfig.Hash
+                else None
 
             let node =
                 { Node.Id = nodeId
@@ -184,8 +175,9 @@ let build (options: ConfigOptions.Options) (configuration: Configuration.Workspa
                   Node.ClusterHash = targetClusterHash
                   Node.ProjectHash = projectConfig.Hash
                   Node.TargetHash = targetHash
+                  Node.Action = RunAction.Ignore
+                  Node.Required = required }
 
-                  Node.Action = buildAction }
             if allNodes.TryAdd(nodeId, node) |> not then raiseBugError "Unexpected graph building race"
 
         if processedNodes.TryAdd(nodeId, true) then processNode()

src/Terrabuild/Program.fs

@@ -143,12 +143,12 @@ let processCommandLine (parser: ArgumentParser<TerrabuildArgs>) (result: ParseRe
         let graph = GraphPipeline.Action.build options cache graph
         if options.Debug then graph |> Json.Serialize |> IO.writeTextFile (logFile $"action.json")
 
-        let graph = GraphPipeline.Batch.build options config graph
-        if options.Debug then graph |> Json.Serialize |> IO.writeTextFile (logFile $"batch.json")
-
         let graph = GraphPipeline.Cascade.build graph
         if options.Debug then graph |> Json.Serialize |> IO.writeTextFile (logFile $"cascade.json")
 
+        let graph = GraphPipeline.Batch.build options config graph
+        if options.Debug then graph |> Json.Serialize |> IO.writeTextFile (logFile $"batch.json")
+
         if options.Debug then
             let markdown =
                 [

tests/basic/results/terrabuild-debug.action.json

@@ -25,7 +25,8 @@
       "artifacts": "none",
       "build": "always",
       "batch": "all",
-      "action": "exec"
+      "action": "exec",
+      "required": true
     }
   },
   "rootNodes": [

tests/basic/results/terrabuild-debug.batch.json

@@ -25,7 +25,8 @@
       "artifacts": "none",
       "build": "always",
       "batch": "all",
-      "action": "exec"
+      "action": "exec",
+      "required": true
     }
   },
   "rootNodes": [

tests/basic/results/terrabuild-debug.cascade.json

@@ -25,7 +25,8 @@
       "artifacts": "none",
       "build": "always",
       "batch": "all",
-      "action": "exec"
+      "action": "exec",
+      "required": true
     }
   },
   "rootNodes": [

tests/basic/results/terrabuild-debug.node.json

@@ -25,7 +25,8 @@
       "artifacts": "none",
       "build": "always",
       "batch": "all",
-      "action": "exec"
+      "action": "ignore",
+      "required": true
     }
   },
   "rootNodes": [

tests/cluster-layers/results/terrabuild-debug.action.json

@@ -45,7 +45,8 @@
       "artifacts": "managed",
       "build": "always",
       "batch": "all",
-      "action": "exec"
+      "action": "exec",
+      "required": true
     },
     "workspace/path#b:build": {
       "id": "workspace/path#b:build",
@@ -92,7 +93,8 @@
       "artifacts": "managed",
       "build": "always",
       "batch": "all",
-      "action": "exec"
+      "action": "exec",
+      "required": true
     },
     "workspace/path#c:build": {
       "id": "workspace/path#c:build",
@@ -140,7 +142,8 @@
       "artifacts": "managed",
       "build": "always",
       "batch": "all",
-      "action": "exec"
+      "action": "exec",
+      "required": true
     },
     "workspace/path#d:build": {
       "id": "workspace/path#d:build",
@@ -158,7 +161,7 @@
       ],
       "projectHash": "E27EE8CFF9BD151E97E09A8A953929B6DC5E4E242F0EB1BD94FD92562DFC1EB6",
       "targetHash": "F59792CEAF216E5D8146633727BF9C85E5B5D86F6806D1424676E0100DA55430",
-      "clusterHash": "6EE8E1E7205ABAE10C58BC4B6DA4CA7129380A596049BBC2264C973C2C5331F1",
+      "clusterHash": "89F54C1D6CCEDA2279E58F176EEC7D265891E610EFCBC76C9725473565B6A245",
       "operations": [
         {
           "image": "mcr.microsoft.com/dotnet/sdk:9.0.202",
@@ -182,7 +185,8 @@
       "artifacts": "managed",
       "build": "always",
       "batch": "all",
-      "action": "exec"
+      "action": "exec",
+      "required": true
     },
     "workspace/path#e:build": {
       "id": "workspace/path#e:build",
@@ -200,7 +204,7 @@
       ],
       "projectHash": "0913D77CAAD0C42E3E5521DA44216CEEB02C56ADB452C6F1999EA983DC9C9BFB",
       "targetHash": "DF61287AD8999932BA40DAB1C891C748A8210C4D6648EE24113F45428CE8A63C",
-      "clusterHash": "6EE8E1E7205ABAE10C58BC4B6DA4CA7129380A596049BBC2264C973C2C5331F1",
+      "clusterHash": "89F54C1D6CCEDA2279E58F176EEC7D265891E610EFCBC76C9725473565B6A245",
       "operations": [
         {
           "image": "mcr.microsoft.com/dotnet/sdk:9.0.202",
@@ -224,7 +228,8 @@
       "artifacts": "managed",
       "build": "always",
       "batch": "all",
-      "action": "exec"
+      "action": "exec",
+      "required": true
     },
     "workspace/path#f:build": {
       "id": "workspace/path#f:build",
@@ -264,7 +269,8 @@
       "artifacts": "managed",
       "build": "always",
       "batch": "all",
-      "action": "exec"
+      "action": "exec",
+      "required": true
     },
     "workspace/path#g:build": {
       "id": "workspace/path#g:build",
@@ -303,7 +309,8 @@
       "artifacts": "managed",
       "build": "always",
       "batch": "all",
-      "action": "exec"
+      "action": "exec",
+      "required": true
     }
   },
   "rootNodes": [