Minor improvements and fixes

src/app/Fake.Core.Process/Process.fs

@@ -394,14 +394,21 @@ module Process =
     let private getStartedProcesses, _, private setStartedProcesses =
         FakeVar.defineAllowNoContext<ProcessList> startedProcessesVar
 
+    let private processListLock = obj ()
+
     let private doWithProcessList f =
         if Context.isFakeContext () then
-            match getStartedProcesses () with
-            | Some h -> Some(f h)
-            | None ->
-                let h = new ProcessList()
-                setStartedProcesses h
-                Some(f h)
+            // The get-or-create must be atomic: under the parallel target runner two workers can both
+            // observe None on the first process start, each allocate a ProcessList and register their
+            // own PID, and the second setStartedProcesses then overwrites the first - so the losing
+            // list's PID is never tracked and killAllCreatedProcesses (Ctrl+C cleanup) leaks it.
+            lock processListLock (fun () ->
+                match getStartedProcesses () with
+                | Some h -> Some(f h)
+                | None ->
+                    let h = new ProcessList()
+                    setStartedProcesses h
+                    Some(f h))
         else
             None
 

src/app/Fake.Core.Target/Target.fs

@@ -922,7 +922,11 @@ module Target =
         // Centralized handling of target context and next target logic...
         [<NoComparison>]
         [<NoEquality>]
-        type RunnerHelper = GetNextTarget of TargetContext * AsyncReplyChannel<Async<TargetContext * Target option>>
+        type RunnerHelper =
+            | GetNextTarget of TargetContext * AsyncReplyChannel<Async<TargetContext * Target option>>
+            /// Sent when the build is cancelled so the scheduler can release workers parked in the
+            /// wait list even if no further GetNextTarget message will ever arrive to trigger a drain.
+            | Cancel
 
         type IRunnerHelper =
             abstract GetNextTarget: TargetContext -> Async<TargetContext * Target option>
@@ -945,6 +949,15 @@ module Target =
                             let! msg = inbox.Receive()
 
                             match msg with
+                            | Cancel ->
+                                // Release every parked worker so runOptimal's Task.WhenAll can
+                                // complete during a cancelled build. A worker parked in waitList
+                                // awaits a TaskCompletionSource that nothing else ever completes once
+                                // no further GetNextTarget arrives, which would otherwise hang forever.
+                                for w: TaskCompletionSource<TargetContext * Target option> in waitList do
+                                    w.SetResult(ctx, None)
+
+                                waitList <- []
                             | GetNextTarget (newCtx, reply) ->
                                 let failwithf pf =
                                     // handle reply before throwing.
@@ -966,7 +979,10 @@ module Target =
                                     runningTasks
                                     |> List.filter (fun t -> not (known.ContainsKey(String.toLower t.Name)))
 
-                                if known.Count = targetCount then
+                                // Drain when everything is done, or when the build is cancelled: in
+                                // either case no more work should be handed out, and every waiting
+                                // worker must be released so the run can finish instead of deadlocking.
+                                if known.Count = targetCount || ctx.CancellationToken.IsCancellationRequested then
                                     for w: TaskCompletionSource<TargetContext * Target option> in waitList do
                                         w.SetResult(ctx, None)
 
@@ -1070,11 +1086,16 @@ module Target =
                             let! msg = inbox.Receive()
 
                             match msg with
+                            | Cancel -> ()
                             | GetNextTarget (_, reply) ->
                                 reply.Reply(async { return raise <| exn ("mailbox failed", e) })
                 }
 
             let mbox = MailboxProcessor.Start(body)
+            // Poke the mailbox on cancellation so it drains the wait list even when every worker is
+            // parked and no further GetNextTarget would otherwise be sent. Posting is thread-safe and
+            // keeps all waitList mutation on the mailbox thread.
+            ctx.CancellationToken.Register(fun () -> mbox.Post Cancel) |> ignore
 
             { new IRunnerHelper with
                 member _.GetNextTarget ctx =

src/app/Fake.DotNet.Testing.VSTest/VSTest.fs

@@ -171,7 +171,15 @@ module VSTest =
             Trace.trace (sprintf "Saved args to '%s' with value: %s" path generatedArgs))
         |> CreateProcess.addOnFinally (fun () -> File.Delete path)
         |> CreateProcess.addOnExited (fun _ exitCode ->
-            if exitCode > 0 && parameters.ErrorLevel <> ErrorLevel.DontFailBuild then
+            // A negative exit code means the test host itself crashed (e.g. -532462766 for an
+            // unhandled CLR exception, -1073741819 for an access violation) rather than reporting
+            // failing tests. Such a crash must fail the build even under DontFailBuild, otherwise a
+            // runner that dies mid-run reports zero failures and the build goes green. Positive
+            // non-zero codes are ordinary test failures and honour the configured ErrorLevel.
+            let crashed = exitCode < 0
+            let testsFailed = exitCode > 0 && parameters.ErrorLevel <> ErrorLevel.DontFailBuild
+
+            if crashed || testsFailed then
                 let message =
                     sprintf "%sVSTest test run failed with exit code %i" Environment.NewLine exitCode
 

src/app/Fake.Runtime/FakeRuntime.fs

@@ -46,6 +46,35 @@ let internal filterValidAssembly (logLevel: VerboseLevel) (isSdk, isReferenceAss
 
         None
 
+// Fold the resolved dependency set into the cache key. The script hash computed upstream
+// covers only the script text and fsi args, not the paket-resolved references. Without this,
+// a paket.lock change that pulls in new package versions leaves the script hash - and therefore
+// CachedAssemblyFilePath - unchanged, so the DLL compiled against the OLD versions is reused: a
+// MissingMethodException/TypeLoadException at runtime, or silently stale behaviour from inlined
+// values baked into the old assembly. Re-hashing the combination keeps the cache file name a
+// fixed length. Prefer the lock file content when present; otherwise fall back to the resolved
+// references and runtime assemblies so the key still moves when the dependency set changes.
+let internal computeDependencyAwareHash
+    (existingHash: string)
+    (lockFilePath: string)
+    (references: string list)
+    (runtimeAssemblies: Runners.AssemblyInfo list)
+    =
+    let dependencyHash =
+        let depInput =
+            if File.Exists lockFilePath then
+                File.ReadAllText lockFilePath
+            else
+                references
+                @ (runtimeAssemblies
+                   |> List.map (fun a -> sprintf "%s;%s;%s" a.FullName a.Version a.Location))
+                |> List.sort
+                |> String.concat "\n"
+
+        HashGeneration.getStringHash depInput
+
+    HashGeneration.getStringHash (existingHash + "|" + dependencyHash)
+
 let paketCachingProvider
     (config: FakeConfig)
     cacheDir
@@ -494,19 +523,21 @@ let paketCachingProvider
                     References = references @ context.Config.CompileOptions.FsiOptions.References
                     Debug = Some Yaaf.FSharp.Scripting.DebugMode.Portable }
 
-            { context with
-                Config =
-                    { context.Config with
-                        CompileOptions = { context.Config.CompileOptions with FsiOptions = newAdditionalArgs }
-                        RuntimeOptions =
-                            { context.Config.RuntimeOptions with
-                                _RuntimeDependencies =
-                                    runtimeAssemblies @ context.Config.RuntimeOptions.RuntimeDependencies
-                                _NativeLibraries = nativeLibraries @ context.Config.RuntimeOptions.NativeLibraries }
-
-                     } },
+            let newContext =
+                { context with
+                    Hash = computeDependencyAwareHash context.Hash lockFilePath.FullName references runtimeAssemblies
+                    Config =
+                        { context.Config with
+                            CompileOptions = { context.Config.CompileOptions with FsiOptions = newAdditionalArgs }
+                            RuntimeOptions =
+                                { context.Config.RuntimeOptions with
+                                    _RuntimeDependencies =
+                                        runtimeAssemblies @ context.Config.RuntimeOptions.RuntimeDependencies
+                                    _NativeLibraries = nativeLibraries @ context.Config.RuntimeOptions.NativeLibraries } } }
+
+            newContext,
             let assemblyPath, warningsFile =
-                context.CachedAssemblyFilePath + ".dll", context.CachedAssemblyFilePath + ".warnings"
+                newContext.CachedAssemblyFilePath + ".dll", newContext.CachedAssemblyFilePath + ".warnings"
 
             if File.Exists(assemblyPath) && File.Exists(warningsFile) then
                 Some

src/app/Fake.Tools.Git/Commit.fs

@@ -15,7 +15,7 @@ module Commit =
     /// <param name="repositoryDir">The git repository.</param>
     /// <param name="message">The commit message text.</param>
     let exec repositoryDir message =
-        sprintf "commit -m \"%s\"" message
+        Args.toWindowsCommandLine [ "commit"; "-m"; message ]
         |> CommandHelper.runSimpleGitCommand repositoryDir
         |> Trace.trace
 
@@ -27,6 +27,6 @@ module Commit =
     /// <param name="shortMessage">The commit short (title) message text.</param>
     /// <param name="extendedMessage">The commit extended (description) message text.</param>
     let execExtended repositoryDir shortMessage extendedMessage =
-        sprintf "commit -m \"%s\" -m \"%s\"" shortMessage extendedMessage
+        Args.toWindowsCommandLine [ "commit"; "-m"; shortMessage; "-m"; extendedMessage ]
         |> CommandHelper.runSimpleGitCommand repositoryDir
         |> Trace.trace

src/app/Fake.netcore/Program.fs

@@ -432,9 +432,11 @@ let main (args: string[]) =
         // See https://github.com/fsharp/FAKE/issues/2406
         printfn "(Warning) Error while Console.ResetColor:"
         reportExn VerboseLevel.Normal e
-#if !NETSTANDARD1_6
-    //if !TargetHelper.ExitCode.exitCode <> 0 then exit !TargetHelper.ExitCode.exitCode
+    // Honour an exit code a script signalled via Environment.ExitCode (e.g. FAKE 4 style scripts
+    // that set it instead of throwing). This must not be gated behind NETSTANDARD1_6: the
+    // Fake.netcore project defines that constant while the fake-cli tool does not, so gating it
+    // made the two shipped runners disagree and let Fake.netcore exit 0 on a failed build.
     if Environment.ExitCode <> 0 then
         exitCode <- Environment.ExitCode
-#endif
+
     exitCode

src/test/Fake.Core.UnitTests/Fake.Core.Target.fs

@@ -153,6 +153,64 @@ let tests =
                Expect.isNone target "expected no next target"
 
 
+           Fake.ContextHelper.fakeContextTestCase "cancellation releases workers parked in the parallel runner"
+           <| fun _ ->
+               // Diamond so that after "a" is handed out (but never reported finished) every other
+               // worker has nothing runnable and parks in the wait list awaiting a TaskCompletionSource.
+               Target.create "a" ignore
+               Target.create "b" ignore
+               Target.create "c" ignore
+               Target.create "d" ignore
+
+               "a" ==> "b" |> ignore
+               "a" ==> "c" |> ignore
+               "b" ==> "d" |> ignore
+               "c" ==> "d" |> ignore
+
+               let order = Target.determineBuildOrder "d"
+               let targets = order |> Seq.concat |> Seq.toList
+
+               use cts = new System.Threading.CancellationTokenSource()
+
+               let ctx = TargetContext.Create "d" targets [] cts.Token
+
+               let mgr = Target.ParallelRunner.createCtxMgr order ctx
+
+               let waitTask (t: System.Threading.Tasks.Task<_>) message =
+                   if not (t.Wait 10000) then
+                       failwithf "%s" message
+
+                   t.Result
+
+               // Take the only initially-runnable target ("a") without ever reporting it finished.
+               let firstTarget =
+                   let t = mgr.GetNextTarget ctx |> Async.StartAsTask
+                   let _, target = waitTask t "GetNextTarget for the first target hung"
+                   target
+
+               Expect.isSome firstTarget "expected the first runnable target"
+               Expect.equal firstTarget.Value.Name "a" "Expected target a"
+
+               // Two more workers ask for work; both must park because b/c depend on the unfinished a.
+               let parked = [ for _ in 1..2 -> mgr.GetNextTarget ctx |> Async.StartAsTask ]
+
+               // Let the workers actually park before cancelling, so this exercises the Cancel-driven
+               // drain (no further GetNextTarget will arrive) rather than the GetNextTarget check.
+               Async.Sleep 500 |> Async.RunSynchronously
+
+               Expect.isFalse
+                   (parked |> List.exists (fun t -> t.IsCompleted))
+                   "workers should be parked, not completed, before cancellation"
+
+               cts.Cancel()
+
+               for t in parked do
+                   let _, target =
+                       waitTask t "a parked worker did not complete after cancellation (deadlock)"
+
+                   Expect.isNone target "a cancelled worker must be released with no target"
+
+
            Fake.ContextHelper.fakeContextTestCase "check simple parallelism"
            <| fun _ ->
                Target.create "a" ignore

src/test/Fake.Core.UnitTests/Fake.DotNet.Testing.VSTest.fs

@@ -70,4 +70,51 @@ let tests =
                    [ "assembly1.dll"; "assembly2.dll"; "/Parallel"; "/InIsolation" ]
                    "Expected arg file to be correct")
 
-              Expect.isFalse (File.Exists argFile) "File should be deleted" ]
+              Expect.isFalse (File.Exists argFile) "File should be deleted"
+
+          // A negative exit code means the test host itself crashed (e.g. an unhandled CLR exception)
+          // rather than reporting failing tests, so it must fail the build even under DontFailBuild -
+          // otherwise a runner that dies mid-run reports zero failures and the build goes green.
+          testCase "Test that a crashed test host (negative exit code) fails the build even under DontFailBuild"
+          <| fun _ ->
+              let cp =
+                  VSTest.createProcess
+                      Path.GetTempFileName
+                      (fun param ->
+                          { param with
+                              ToolPath = "vstest.exe"
+                              ErrorLevel = Fake.Testing.Common.DontFailBuild })
+                      [| "assembly.dll" |]
+
+              use state = cp.Hook.PrepareState()
+
+              let runWithExitCode (exitCode: int) =
+                  cp.Hook.RetrieveResult(state, System.Threading.Tasks.Task.FromResult { RawExitCode = exitCode })
+                  |> Async.RunSynchronously
+                  |> ignore
+
+              // Negative exit code = crash -> must throw regardless of DontFailBuild.
+              Expect.throws
+                  (fun () -> runWithExitCode -532462766)
+                  "a crashed test host must fail the build even under DontFailBuild"
+
+              // Positive non-zero exit code = ordinary test failure -> honours DontFailBuild (no throw).
+              runWithExitCode 1
+
+          // Under the default ErrorLevel, an ordinary test failure (positive exit code) must fail the build.
+          testCase "Test that a positive exit code fails the build under the default ErrorLevel"
+          <| fun _ ->
+              let cp =
+                  VSTest.createProcess
+                      Path.GetTempFileName
+                      (fun param -> { param with ToolPath = "vstest.exe" })
+                      [| "assembly.dll" |]
+
+              use state = cp.Hook.PrepareState()
+
+              Expect.throws
+                  (fun () ->
+                      cp.Hook.RetrieveResult(state, System.Threading.Tasks.Task.FromResult { RawExitCode = 1 })
+                      |> Async.RunSynchronously
+                      |> ignore)
+                  "a non-zero exit code must fail the build under the default ErrorLevel" ]

src/test/Fake.Core.UnitTests/Fake.Runtime.fs

@@ -282,4 +282,49 @@ printfn "other.fsx"
                        && e.Message.Contains "' doesn't exist")
                       |> Flip.Expect.isTrue (sprintf "Expected a good error message, but got: %s" e.Message)
               finally
-                  Directory.Delete(tmpDir, true) ]
+                  Directory.Delete(tmpDir, true)
+
+          testCase "paket.lock content change invalidates the compiled-script cache hash"
+          <| fun _ ->
+              let lockPath = Path.GetTempFileName()
+
+              try
+                  let baseHash = "scripthash"
+
+                  File.WriteAllText(lockPath, "NUGET\n    FSharp.Core (6.0.0)")
+                  let hash1 = FakeRuntime.computeDependencyAwareHash baseHash lockPath [] []
+
+                  // Same lock content must yield a stable hash, otherwise the cache would never be reused.
+                  FakeRuntime.computeDependencyAwareHash baseHash lockPath [] []
+                  |> Flip.Expect.equal "identical lock content must produce a stable hash" hash1
+
+                  // A paket.lock change must change the hash so the stale DLL is not reused.
+                  File.WriteAllText(lockPath, "NUGET\n    FSharp.Core (7.0.0)")
+                  let hash2 = FakeRuntime.computeDependencyAwareHash baseHash lockPath [] []
+
+                  Expect.notEqual hash2 hash1 "a paket.lock change must change the script cache hash"
+              finally
+                  File.Delete lockPath
+
+          testCase "resolved dependency set drives cache invalidation when no lock file exists"
+          <| fun _ ->
+              let missingLock = Path.GetTempFileName()
+              File.Delete missingLock // ensure it does not exist so the reference fallback is used
+
+              let assembly version : AssemblyInfo =
+                  { FullName = "Lib, Version=" + version
+                    Version = version
+                    Location = "/packages/lib.dll" }
+
+              let hashV1 =
+                  FakeRuntime.computeDependencyAwareHash "h" missingLock [ "/packages/ref.dll" ] [ assembly "1.0.0.0" ]
+
+              // Re-ordering the same reference set must not change the hash (input is sorted).
+              FakeRuntime.computeDependencyAwareHash "h" missingLock [ "/packages/ref.dll" ] [ assembly "1.0.0.0" ]
+              |> Flip.Expect.equal "an unchanged resolved dependency set must produce a stable hash" hashV1
+
+              // A changed resolved version must change the hash even without a lock file present.
+              let hashV2 =
+                  FakeRuntime.computeDependencyAwareHash "h" missingLock [ "/packages/ref.dll" ] [ assembly "2.0.0.0" ]
+
+              Expect.notEqual hashV2 hashV1 "a changed resolved dependency set must change the hash" ]