Skip to content

Minor improvements and fixes#2900

Merged
xperiandri merged 3 commits into
fsprojects:masterfrom
Thorium:fix/critical-silent-build-failures
Jul 13, 2026
Merged

Minor improvements and fixes#2900
xperiandri merged 3 commits into
fsprojects:masterfrom
Thorium:fix/critical-silent-build-failures

Conversation

@Thorium

@Thorium Thorium commented Jul 6, 2026

Copy link
Copy Markdown
Member

Fix silent build failures: cancellation deadlock, stale caches... Parallel builds no longer hang on Ctrl+C, script cache invalidates when paket.lock changes, and crashes/failures can't slip through as green.

  • Target: release workers parked in the wait list on cancellation (Cancel message + drain on IsCancellationRequested) so a cancelled build exits instead of deadlocking
  • FakeRuntime: fold the resolved dependency set into the script cache key so a paket.lock change stops reusing a DLL compiled against old packages
  • Process: make ProcessList get-or-create atomic so concurrently started processes aren't lost from Ctrl+C cleanup
  • VSTest: negative exit codes (crashed test host) fail the build even under DontFailBuild
  • Git.Commit: escape commit messages via Args.toWindowsCommandLine instead of naive quoting
  • Fake.netcore: honour Environment.ExitCode set by scripts in both shipped runners (was gated behind a define only one of them had)

…codes

Parallel builds no longer hang on Ctrl+C, script cache invalidates
when paket.lock changes, and crashes/failures can't slip through as green.

- Target: release workers parked in the wait list on cancellation (Cancel
  message + drain on IsCancellationRequested) so a cancelled build exits
  instead of deadlocking
- FakeRuntime: fold the resolved dependency set into the script cache key
  so a paket.lock change stops reusing a DLL compiled against old packages
- Process: make ProcessList get-or-create atomic so concurrently started
  processes aren't lost from Ctrl+C cleanup
- VSTest: negative exit codes (crashed test host) fail the build even
  under DontFailBuild
- Git.Commit: escape commit messages via Args.toWindowsCommandLine instead
  of naive quoting
- Fake.netcore: honour Environment.ExitCode set by scripts in both shipped
  runners (was gated behind a define only one of them had)
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Test Results

   12 files  + 2     12 suites  +2   27m 38s ⏱️ + 11m 23s
  457 tests + 5    456 ✅ +  6  1 💤 ±0  0 ❌  - 1 
1 305 runs  +99  1 302 ✅ +100  3 💤 ±0  0 ❌  - 1 

Results for commit 5bb2d3b. ± Comparison against base commit e37773e.

♻️ This comment has been updated with latest results.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses several sources of “green-but-broken” builds by improving cancellation behavior, cache invalidation correctness, and process/exit-code handling in FAKE’s runners and tooling.

Changes:

  • Prevent parallel target execution from deadlocking on cancellation by draining waiting workers when CancellationToken is triggered.
  • Make script compilation caching sensitive to resolved Paket dependencies (not just script text/FSI args) to avoid reusing stale compiled assemblies.
  • Tighten failure signaling across runners/tools: treat crashed VSTest hosts as failures, preserve script-set Environment.ExitCode, and harden git commit argument escaping.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/app/Fake.Tools.Git/Commit.fs Builds git commit args via Args.toWindowsCommandLine to properly escape commit messages.
src/app/Fake.Runtime/FakeRuntime.fs Incorporates dependency/lock-state into the script cache key to invalidate stale cached DLLs.
src/app/Fake.netcore/Program.fs Ensures Environment.ExitCode set by scripts is honored consistently across runners.
src/app/Fake.DotNet.Testing.VSTest/VSTest.fs Treats negative (crash) exit codes as build failures even under DontFailBuild.
src/app/Fake.Core.Target/Target.fs Adds cancellation signaling to the parallel scheduler to drain parked workers and avoid hangs.
src/app/Fake.Core.Process/Process.fs Makes started-process tracking get-or-create atomic to avoid losing PIDs under parallel starts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/app/Fake.Core.Target/Target.fs
Comment thread src/app/Fake.DotNet.Testing.VSTest/VSTest.fs
Comment thread src/app/Fake.Runtime/FakeRuntime.fs Outdated
Thorium and others added 2 commits July 11, 2026 19:52
The Copilot-test commit referenced FakeRuntime.computeDependencyAwareHash,
which was never committed, so the unit tests could not compile. Restore the
pure helper by extracting the dependency-aware cache-key logic out of the
paketCachingProvider closure (behaviour-identical), and apply fantomas
formatting to the new Target cancellation test so CheckFormatting passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@xperiandri
xperiandri merged commit e8e1cae into fsprojects:master Jul 13, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants