Merge pull request #1507 from BlythMeister/ParallelFix

URGENT: Bug Fix For Parallel From #1396
fsprojects · Mar 28, 2017 · 8857221 · 8857221
2 parents 566327f + ed2df1a
commit 8857221
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 4 deletions.
diff --git a/src/app/FakeLib/TargetHelper.fs b/src/app/FakeLib/TargetHelper.fs
@@ -501,10 +501,20 @@ let determineBuildOrder (target : string) =
     let rec SetTargetLevel newLevel target  = 
         match targetLevels.TryGetValue target with
         | true, exDependencyLevel -> 
-            if exDependencyLevel.level < newLevel then
-                exDependencyLevel.dependants |> List.iter (fun x -> SetTargetLevel (newLevel - 1) x)
+            let minLevel = targetLevels
+                           |> Seq.filter(fun x -> x.Value.dependants.Contains target)
+                           |> Seq.map(fun x -> x.Value.level)
+                           |> fun x -> match x.Any() with
+                                       | true -> x |> Seq.min
+                                       | _ -> -1
+
             if exDependencyLevel.dependants.Length > 0 then
-                targetLevels.[target] <- {level = newLevel; dependants = exDependencyLevel.dependants}
+                if (exDependencyLevel.level < newLevel && newLevel < minLevel) || (exDependencyLevel.level > newLevel) then
+                    targetLevels.[target] <- {level = newLevel; dependants = exDependencyLevel.dependants}
+
+                if exDependencyLevel.level < newLevel then
+                    exDependencyLevel.dependants |> List.iter (fun x -> SetTargetLevel (newLevel - 1) x)
+
         | _ -> ()     
 
     let addTargetLevel ((dependantTarget:option<TargetTemplate<unit>>), (target: TargetTemplate<unit>), _, level, _ ) =
@@ -520,18 +530,24 @@ let determineBuildOrder (target : string) =
         | (true, exDependencyLevel), _ when exDependencyLevel.level < level -> Some (exDependencyLevel)
         | _ -> None
 
+        let (|LevelRemain|_|) = function
+        | (true, exDependencyLevel), _ -> Some (exDependencyLevel)
+        | _ -> None
+
         let (|NewTarget|_|) = function
         | (false, _), _ -> Some ()
         | _ -> None
 
         match targetLevels.TryGetValue target.Name, dependantTarget with
         | LevelIncreaseWithDependantTarget (exDependencyLevel, dt) ->
-            SetTargetLevel (exDependencyLevel.level - 1) dt.Name
             targetLevels.[target.Name] <- {level = exDependencyLevel.level; dependants = (appendDepentantOption exDependencyLevel.dependants dependantTarget)}
+            SetTargetLevel (exDependencyLevel.level - 1) dt.Name
         |  LevelIncreaseWithNoDependantTarget (exDependencyLevel) -> 
             targetLevels.[target.Name] <- {level = exDependencyLevel.level; dependants = (appendDepentantOption exDependencyLevel.dependants dependantTarget)}
         |  LevelDecrease (exDependencyLevel) -> 
+            targetLevels.[target.Name] <- {level = level; dependants = (appendDepentantOption exDependencyLevel.dependants dependantTarget)}
             exDependencyLevel.dependants |> List.iter (fun x -> SetTargetLevel (level - 1) x)
+        |  LevelRemain (exDependencyLevel) -> 
             targetLevels.[target.Name] <- {level = level; dependants = (appendDepentantOption exDependencyLevel.dependants dependantTarget)}
         | NewTarget -> 
             targetLevels.[target.Name] <- {level = level; dependants=(appendDepentantOption [] dependantTarget)}

diff --git a/src/test/FsCheck.Fake/TestParallelBuildOrder.fs b/src/test/FsCheck.Fake/TestParallelBuildOrder.fs
@@ -154,6 +154,59 @@ let ``Initial Targets Can Run Concurrently``() =
         | _ ->
             failwithf "unexpected order: %A" order
 
+[<Fact>]
+let ``BlythMeisters Scenario Of Complex Build Order Is Correct``() =
+    TargetDict.Clear()
+    Target "PrepareBuild" DoNothing
+    Target "CreateWholeCaboodle" DoNothing
+    Target "UpdateVersions" DoNothing
+    Target "PreBuildVerifications" DoNothing
+    Target "BuildWholeCaboodle" DoNothing
+    Target "RunUnitTests" DoNothing
+    Target "RunIntTests" DoNothing
+    Target "CreateDBNugets" DoNothing
+    Target "DropIntDatabases" DoNothing
+    Target "DeployIntDatabases" DoNothing
+    Target "CreateNugets" DoNothing
+    Target "PublishNugets" DoNothing
+
+    "PrepareBuild" ==> "CreateWholeCaboodle" |> ignore
+    "PrepareBuild" ==> "UpdateVersions" |> ignore
+    "CreateWholeCaboodle" ==> "PreBuildVerifications" |> ignore
+    "UpdateVersions" ==> "PreBuildVerifications" |> ignore
+    "PreBuildVerifications" ==> "BuildWholeCaboodle" |> ignore
+    "PreBuildVerifications" ==> "CreateDBNugets" |> ignore
+    "PreBuildVerifications" ==> "DropIntDatabases" |> ignore
+    "BuildWholeCaboodle" ==> "CreateNugets" |> ignore
+    "BuildWholeCaboodle" ==> "RunUnitTests" |> ignore
+    "BuildWholeCaboodle" ==> "RunIntTests" |> ignore
+    "CreateDBNugets" ==> "DeployIntDatabases" |> ignore
+    "DropIntDatabases" ==> "DeployIntDatabases" |> ignore
+    "DeployIntDatabases" ==> "RunIntTests" |> ignore
+    "CreateNugets" ==> "PublishNugets" |> ignore
+    "CreateDBNugets" ==> "PublishNugets" |> ignore
+    "RunUnitTests" ==> "PublishNugets" |> ignore
+    "RunIntTests" ==> "PublishNugets" |> ignore
+
+    let order = determineBuildOrder "PublishNugets"
+    validateBuildOrder order "PublishNugets"
+
+    match order with
+        | [
+           TargetSet ["PrepareBuild"];
+           TargetSet ["CreateWholeCaboodle"; "UpdateVersions"];
+           TargetSet ["PreBuildVerifications"];
+           TargetSet ["BuildWholeCaboodle"; "CreateDBNugets"; "DropIntDatabases"];
+           TargetSet ["CreateNugets"; "DeployIntDatabases"; "RunUnitTests"];
+           TargetSet ["RunIntTests"];
+           TargetSet ["PublishNugets"];
+           ] ->
+            // as expected
+            ()
+
+        | _ ->
+            failwithf "unexpected order: %A" order
+
 [<Fact>]
 let ``Spurs run as early as possible``() =
     TargetDict.Clear()