Skip to content

build: write build.fsx against FAKE 6 and merge with dotnet-ilrepack - #4423

Open
fpellet wants to merge 28 commits into
masterfrom
fake6
Open

build: write build.fsx against FAKE 6 and merge with dotnet-ilrepack#4423
fpellet wants to merge 28 commits into
masterfrom
fake6

Conversation

@fpellet

@fpellet fpellet commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Part of #4348 — the "upgrade to fake 6" step of the plan, after #4422 removed the FAKE runner from the entry points.

What changes

FAKE 6 as a library. build.fsx was written against FakeLib.dll, i.e. FAKE 4.64.17 — a .NETFramework 4.5 assembly from 2017 that only happens to load under .NET 10. It is now written against the FAKE 6 modules. Those target net6.0/netstandard2.0, so they cannot live in the Build group, which is pinned to framework >= net461 for FSharp.Formatting and the reference assemblies; they get a new BuildScript group restricted to net10.0, the framework dotnet fsi runs on. build.sh / build.cmd generate its load scripts after the restore, so running dotnet fsi build.fsx <Target> directly assumes one of the entry points has run at least once.

dotnet-ilrepack instead of ILRepack.exe. ILRepack.exe is a .NET Framework binary, so on Linux FAKE ran it through Mono. The dotnet-ilrepack tool ships the same repacker as a .NET tool. Mono also supplied the .NET Framework reference assemblies implicitly; under dotnet they have to be passed explicitly, so the repacker is pointed at the same directory MSBuild already resolves net461 against (TargetFrameworkRootPath from Directory.Build.props).

MergePaketTool therefore no longer needs Mono. It is still needed for the net461 test passes and for PublishNuGet, which pushes with the merged net461 paket.exe.

Behaviour differences worth reviewing

  • isMono guards became Environment.isWindows. FAKE 4 derived isMono from Environment.OSVersion.Platform, so it was true on Linux and not isMono really meant "on Windows". FAKE 6 tests for the Mono runtime, which is false under .NET 10 everywhere, so a literal translation would have silently enabled those targets on Linux.

  • DotNet.test defaults to Debug, unlike build/publish/pack. With --no-build that would look for bin/Debug, so the configuration is now explicit on every test invocation.

  • BuildServer.isLocalBuild changes meaning on CI. FAKE 4 reported isLocalBuild = true even on GitHub Actions; FAKE 6 detects the build server and reports false. The doc targets are guarded by it, so GenerateDocs / GenerateReferenceDocs / GenerateHelp now no longer run on the Windows runner. That looks desirable — docs/tools/generate.fsx is still bound to FSharp.Formatting 3 and its Razor templates, and the docs targets still shell out to the FAKE 4 runner for it — but it is a side effect of the library swap rather than a deliberate decision, so say the word if it should be made explicit instead.

  • github fsharp/FAKE modules/Octokit/Octokit.fsx was re-resolved in paket.lock (13eee5a7…e8e1cae7…) as a side effect of the restore; it has no SHA in paket.dependencies. build.fsx #loads it unconditionally, so it affects every target, not just ReleaseGitHub. The new revision still exposes the createClient / createDraft / uploadFile API the script uses, so nothing is broken — but it can be reverted or pinned the way enricosada/add_icon_to_exe already is, if preferred. Replace vendored source files and legacy build dependencies #4350 proposes replacing it outright.

  • The SDK install directory is pinned to LocalApplicationData/dotnetcore through CustomInstallDir. FAKE 6's DotNet.install defaults to ~/.dotnet and can also settle on a dotnet found through PATH; the CI installs the SDK and the .NET 8 runtime the integration tests need into the FAKE 4 location.

Fixes found along the way

Targets that were silently doing the wrong thing, each in its own commit:

  • QuickTest built the test assembly without the version property, so Loading assembly metadata works compared 1.0.0 against RELEASE_NOTES.md and failed. Only visible locally — CI runs BuildPackage.
  • The netcore unit-test pass ran on the wrong framework, and the bootstrapper unit tests were not run at all.
  • Empty Skip* variables now count as unset, as they did under FAKE 4.
  • PublishNuGet resolves the merged paket.exe absolutely, since ToolType.CreateFullFramework() only prefixes Mono for .exe paths.
  • Without a runner to parse it, the command line is translated by the script itself; extra target arguments and empty parameter names are now rejected instead of silently changing which target runs.

Verification

dotnet fsi build.fsx <Target> type-checks and runs; the generated dotnet build/test/pack/publish/restore/exec command lines were compared against the FAKE 4 originals, including --filter, --logger, --no-build, --output and the PackageReleaseNotesFile quoting.

Not in scope

The remaining steps of #4348 — replacing the doc generator, dropping the last Mono uses, and moving further onto standard commands — are left for follow-ups.

fpellet added 19 commits August 31, 2026 22:08
build.fsx is about to be written against FAKE 6 instead of the FakeLib.dll of
the Build group. Those modules target net6.0/netstandard2.0, so they cannot
live in the Build group, which is pinned to `framework >= net461` for
FSharp.Formatting and the .NET Framework reference assemblies.

Add a BuildScript group restricted to net10.0, the framework `dotnet fsi` runs
on, and have the two entry points generate its load scripts after the restore.
ILRepack.exe is a .NET Framework binary, so on Linux FAKE used to run it
through Mono. The dotnet-ilrepack tool ships the same repacker as a .NET tool
and needs no Mono.

Mono also supplied the .NET Framework reference assemblies implicitly; under
dotnet they have to be passed explicitly, so point the repacker at the same
directory MSBuild already resolves net461 against.
FakeLib.dll is FAKE 4.64.17, a .NETFramework 4.5 assembly from 2017 that only
happens to load under .NET 10. Rewrite the script against the FAKE 6 modules of
the BuildScript group instead.

Two behaviour-preserving details worth calling out:

- FAKE 4 derived `isMono` from Environment.OSVersion.Platform, so it was true
  on Linux and the `not isMono` guards really meant "on Windows". FAKE 6 tests
  for the Mono runtime, which is false under .NET 10 everywhere, so those
  guards become Environment.isWindows rather than a literal translation.

- DotNet.test defaults to the Debug configuration, unlike build, publish and
  pack. With --no-build that would look for bin/Debug, so the configuration is
  now explicit on every test invocation.

The docs targets keep shelling out to the FAKE 4 runner: docs/tools/generate.fsx
is still bound to FSharp.Formatting 3 and its Razor templates.
The load scripts build.fsx reads are generated, so running `dotnet fsi
build.fsx` on its own needs build.sh or build.cmd to have run first. Mono is
now only needed for the net461 test passes.
…Test

Paket.Tests.fsproj sets Authors and Description for the assembly metadata
tests, but not Version: `Loading assembly metadata works` compares the version
baked into Paket.Tests.dll with the one in RELEASE_NOTES.md, and that version
is injected by the build script.

The Build target passes it, so RunTests is green against the assembly it
produced. QuickTest builds through dotnet test instead, without the property,
and since it depends on Clean the rebuild always happens: the assembly ended up
at the default 1.0.0 and the test failed. CI only runs BuildPackage, so this
stayed invisible.
FAKE 6's DotNet.install defaults to ~/.dotnet and can also settle on a dotnet
found through PATH, while FAKE 4 installed into LocalApplicationData/dotnetcore.
The CI installs the SDK and the .NET 8 runtime the integration tests need into
that directory, so pin it through CustomInstallDir.
Array.tryFind kept the first bare argument and silently dropped the rest, while
the FAKE runner used to let the last one win. Fail with an explicit message
instead of quietly running a different target.
MergePaketTool no longer runs ILRepack.exe under Mono, the target name is no
longer passed as an environment variable, and build.sh does not run on Mono.
The FAKE 4 call passed TimeSpan.FromMinutes 5. to ExecProcess, so a hung
ILRepack was killed and reported as a build failure. DotNet.exec defaults
to Options.Timeout = None, which let a stuck merge run until the CI job
timeout instead.
…ath can't be derived

Seq.exactlyOne threw a bare ArgumentException as soon as Directory.Build.props
gained a second, conditioned TargetFrameworkRootPath, and a value written with
another MSBuild property left the literal $(...) in the ILRepack /lib argument,
surfacing as an unresolvable mscorlib. Name the props file in both cases.
The path was built with backslashes, so on Linux it named a file that does
not exist. Nuspec.Load turns a missing file into Nuspec.All without a word,
so the test compared its expectations against an empty nuspec: no
frameworkAssemblies (System.Data, System.Xml) and no <references> filter,
hence all three libraries in the generated Xml.

This stayed invisible because the test never ran anywhere: [<Flaky>] is only
applied under TESTSUITE_RUNS_ON_DOTNETCORE, and the netcore unit-test pass
was asking for a framework the project does not target.
@fpellet fpellet changed the title build: write build.fsx against FAKE 6 and merge with dotnet-ilrepack Draft: build: write build.fsx against FAKE 6 and merge with dotnet-ilrepack Sep 2, 2026
@fpellet
fpellet marked this pull request as draft September 2, 2026 07:00
@fpellet

fpellet commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Previous build on fake 4 crash on GenerateHelp, and not execute:

  • GenerateReferenceDocs
  • GenerateDocs
  • All
  • MergePaketTool
  • AddIconToExe
  • RunIntegrationTestsNet
  • RunIntegrationTestsNetCore
  • SignAssemblies
  • CalculateDownloadHash
  • NuGet
  • BuildPackage

System.Net.Http.WinHttpHandler references System.Buffers 4.0.3.0, but net461 resolves
to the netstandard2.0 asset of System.Buffers 4.6.1, stamped 4.0.2.0. MSBuild only
unifies references upwards, so AutoGenerateBindingRedirects warns with MSB3277 and
emits no redirect at all, and every HTTP response dies in WinHttpResponseParser with
a FileLoadException.

Only bin/net461/paket.exe is affected -- the one the .NET Framework integration tests
drive. The shipped bin/merged/paket.exe repacks System.Buffers and WinHttpHandler into
a single assembly, so it never resolves the reference.
Mirrors the < /dev/null already used by the Linux job. The Windows job hung for 59
minutes after 24 integration tests without emitting a line, and a process waiting on
stdin is the cheapest hypothesis to rule out. Requires shell: cmd, since < NUL is not
PowerShell syntax.
…t assemblies

The spawned bin/net461/paket.exe was only half the problem: tests that drive
Paket.Core in-process run under the test host, which binds against
Paket.IntegrationTests.dll.config, not paket.exe.config. #1298 still died in
WinHttpResponseParser through Paket.Dependencies.FindPackageVersions.

Auditing every assembly reference in both net461 test outputs against the versions
actually shipped there turns up System.Buffers as the only reference not covered by
a redirect, in both projects.
Only failures were printed, so the run that hangs past the 60-minute timeout leaves
no record of which test never returned -- and the timeout kills it before the trx is
written. RunTests already reports at this verbosity.
The suite runs in a few minutes when healthy, so 30 minutes is still a wide margin,
and a run that hangs reports back twice as fast.
…est suite

ExecProcessWithLambdas (FakeLib 4.64.17) ends with an unbounded WaitForExit() that the
7-minute timeout above it does not cover:

  IL_020b:  WaitForExit(int32)   // the timeout
  IL_0217:  Kill()               // "Process %s %s timed out."
  IL_02b0:  WaitForExit()        // unbounded, after
  IL_02b6:  get_ExitCode()

That call returns only once the redirected streams reach EOF. With node reuse on,
dotnet build/run/pack leave MSBuild worker nodes behind holding the inherited stdout
and stderr handles, so EOF never comes and the run hangs with no output -- which is
why PROCESS TIMED OUT never appeared in the log.

ConditionSpecs is the only fixture that nests dotnet run and dotnet pack, and it is
where the Windows run stops.
The scenario is named i001195-broken-appconfig and the test asserts paket reports a
parse error naming Project1 and app.config. The brokenness was a '<<<<<<< HEAD'
conflict marker left in the file on purpose; dd0b881 stripped it as if it were a
stray merge residue, which turned the fixture into valid XML. paket then had nothing
to report and the test failed with 'paket should fail'.
The suite reached its end for the first time: 226 tests, 212 passed, 12 failed. None
of the failures comes from the FAKE 6 port -- they are the debt of a suite that had
never run in CI on any platform. Each is marked with what it actually does, so the
212 that do pass are protected from here on.

Notably 'apply framework restriction' is a real resolution bug, not a stale bound: it
resolves Microsoft.AspNetCore.WebUtilities 10.0.11 under 'framework: =net8.0' even
though that version only ships lib/net10.0. Raising the assertion would have disarmed
the test.

Tracked by #4346.
Both passed in the previous Windows run and failed in the next, with the endpoints
they depend on healthy throughout -- they fail on third-party availability, not on
anything in this repository.

#3014 calls api.github.com, which allows unauthenticated callers 60 requests an hour
per IP, shared across hosted runners; Paket takes its GitHub credentials from
paket.dependencies, so there is no token to hand it from CI. #1635 asserts on a
specific 401 from myget.org, so any hiccup produces a different message.

Flaky is the category this repository already keeps for exactly this, and it moves
them off the main job.
@fpellet

fpellet commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

Update — the Windows job now runs the integration suite, for the first time

Green on c6bb869e9. The extra commits since the description are all consequences of one finding, so here is the chain.

The Windows job used to pass by accident

On master, run 33528762012, job 99926228529: GitHub reports success, while the log reports Status: FailureGenerateHelp fails with System.Exception: generating documentation failed (#4002). build.cmd did not propagate dotnet fsi's exit code, so everything after GenerateHelpMergePaketTool, AddIconToExe, the integration tests, NuGet, BuildPackage — had never actually run on Windows.

That settles the BuildServer.isLocalBuild point raised in the description: on this branch the doc targets are skipped, the build gets past that point, and RunIntegrationTestsNet executes. It is worth knowing that the full suite runs nowhere else either — the Linux build job passes SkipIntegrationTests=true, and the flaky job filters to TestCategory=Flaky, i.e. a single integration test. This branch is the first time the integration suite has run in CI at all, which is why it turned up so much.

Two real bugs it uncovered

Missing System.Buffers binding redirect (99a265b7c, 89b84776d). System.Net.Http.WinHttpHandler references System.Buffers 4.0.3.0, but net461 resolves the netstandard2.0 asset of System.Buffers 4.6.1, which is stamped 4.0.2.0. MSBuild only unifies references upwards, so AutoGenerateBindingRedirects reports MSB3277 and emits nothing at all — and every HTTP response died in WinHttpResponseParser with a FileLoadException. The redirect is added to src/Paket/App.config and to both net461 test assemblies: tests that drive Paket.Core in-process bind against <testassembly>.dll.config, not paket.exe.config. Only bin/net461/paket.exe was affected; the shipped bin/merged/paket.exe repacks both assemblies and never resolves the reference, so there is no release note.

The suite could hang forever (9b50f5137). ExecProcessWithLambdas (FakeLib 4.64.17) ends with an unbounded WaitForExit() after its timeout logic; it returns only once the redirected streams reach EOF. With node reuse on, dotnet build/run/pack leaves MSBuild worker nodes behind holding the inherited stdout/stderr handles, so EOF never comes — which is why PROCESS TIMED OUT never appeared in the log despite the 7-minute setting. Reproducible on Linux too. Setting MSBUILDDISABLENODEREUSE=1 in TestHelper took ConditionSpecs from an infinite hang to 2m28s.

Separately, 7bf9a000e: i001195-broken-appconfig had stopped being broken. Its app.config deliberately contained a <<<<<<< HEAD marker to make it invalid XML, and dd0b881e6 stripped it as if it were merge residue — so paket had nothing to report and the test failed with "paket should fail". Restored. It will probably be stripped again by the same tooling; happy to make the fixture malformed in a less conflict-marker-shaped way if you prefer.

What I parked, and why (3f35b7b29, c6bb869e9)

The first complete run finished 226 tests with 12 failures. None of them comes from this PR — they are the debt of a suite that had never run. Rather than grow this PR into an open-ended investigation, each is marked with what it actually does and tracked by #4346:

test(s) marked reason
apply framework restriction (×4) Ignore framework restriction not applied to the resolution
#2694, #3558, #3983 Ignore the expected .nupkg is never produced
#3317 Ignore dotnet pack exits 1
#4183 Ignore Many groups detected map [], floating deps dropped
#2684 Ignore second restore still reports mismatching hashes
run fsi …deterministic output Ignore 2 of 8 scripts no longer produce their expected output
#3014, #1635 Flaky network-dependent, see below

One deserves a closer look before merge: apply framework restriction is not a stale bound. It resolves Microsoft.AspNetCore.WebUtilities 10.0.11 under framework: =net8.0, although that version ships only lib/net10.0 and 8.0.21 is the newest one that fits. Raising the assertion to < 11 would have disarmed a test that is catching a genuine resolution bug, so it is ignored with that diagnosis written above it instead.

#3014 and #1635 are network flakes, not failures: they hit api.github.com (60 unauthenticated requests an hour per IP, shared across hosted runners) and myget.org, they passed in one Windows run and failed in the next with both endpoints healthy, and they pass locally. [<Flaky>] is the category this repo already keeps for that — with the consequence that the Linux flaky job now inherits them.

Result

Windows on c6bb869e9: Status: Ok. net461 integration 224 discovered / 211 passed / 13 skipped in 16 minutes; net10.0 227 / 207 / 20. Unit tests unchanged.

And the green now means something: with the pwsh default the job reported success over Status: Failure, whereas with shell: cmd (0a26cdf3d, added to close stdin) the failing run correctly reported Process completed with exit code 1. Supporting changes: normal verbosity on the integration run so that a hang names the test that never returned (a721b1346), and the timeout cut from 60 to 30 minutes now that the suite takes ~16 (be223af75).

Still open for you to call

  • The Octokit.fsx re-resolution noted in the description is unchanged — leave it, revert it, or pin it the way enricosada/add_icon_to_exe is.
  • Skipping the doc targets on CI is currently a side effect of isLocalBuild rather than a deliberate guard. Given that GenerateHelp fails on the Windows runner anyway (Build broken since myget roslyn-tools is gone #4002), skipping looks right, but say the word if you would rather it were explicit.

@fpellet
fpellet marked this pull request as ready for review September 4, 2026 22:34
@fpellet fpellet changed the title Draft: build: write build.fsx against FAKE 6 and merge with dotnet-ilrepack build: write build.fsx against FAKE 6 and merge with dotnet-ilrepack Sep 4, 2026
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.

1 participant