FAKE icon indicating copy to clipboard operation
FAKE copied to clipboard

Build with .NET 8.0

Open Thorium opened this issue 1 year ago • 20 comments

Changes with this PR:

  • Change global.json .NET 6 to 8
  • Search and replace targetframeworks from fsproj files to add net8.0
  • Add net8.0 to paket.dependencies
  • dotnet paket install to find .NET8 compatible dependencies
  • Expecto had to be hardcoded for now, because some tests are running on netstandard2.0 library (hopefully we can update this separately later)
  • MSBuild.StructuredLogger problem: DisableInternalBinLog = true had to be added to build.fsx NuGet commands (hopefully we can update this separately later)
  • A few places of code had new overrides so had to explicitly type to strings
  • SdkAssemblyResolver to default .NET 8 as well
  • Readme update
  • GitHub pipeline configs: Add .NET 8 install.

Possible issue: If tests restore still previous fake-cli 6.1.1 and not this version, then they might fail.

Thorium avatar Sep 03 '24 20:09 Thorium

The real issue with the build.fsx is that it's recursive: In the beginning it says "use paket source release/dotnetcore" which is a local folder, that is having the latest build result. If there is nothing yet, then it uses latest release from the NuGet.

But how can you build .NET 8 local package, when there is no .NET 8 compatible packages in NuGet?

Thorium avatar Sep 03 '24 23:09 Thorium

But how can you build .NET 8 local package, when there is no .NET 8 compatible packages in NuGet?

How was it done when .NET 6.0 support was first added?

Do the Microsoft.Build.* package references in build.fsx need unpinning from 17.3.2 to support .NET 8 buiids?

Numpsy avatar Sep 03 '24 23:09 Numpsy

If I build this locally with dotnet build or dotnet pack it works on both my Mac and Windows. But if I try to run it with dotnet fake run it fails on "Fake is not defined" which doesn't make sense to me. Maybe I'm missing something trivial. At least it does read the dll so that is already further than the current Fake 6.1.1 which just fails for https://github.com/fsprojects/FAKE/issues/2314#issuecomment-2327156343

I have a bad hunch that version update could have been done by releasing some -alpha package without Fake, and then moved to build.fsx when it's uploaded to NuGet. But hopefully there is a way to avoid that, as it sounds slow and error-prone.

Thorium avatar Sep 04 '24 07:09 Thorium

But if I try to run it with dotnet fake run it fails on "Fake is not defined" which doesn't make sense to me

Not sure if this is an issue or not, but in case it is -

When testing a local build I noticed that build.fsx.lock has a RESTRICTION: || (== net6.0) (== netstandard2.0) at the top, and wondered if that could be an issue if dealing with the latest versions of libraries like Microsoft.Build which don't have either of those TFMs (just .NET 8.0 and FW 4.7.2)?

I tried adding .NET 8.0 in the fsx file at https://github.com/fsprojects/FAKE/commit/2852870eb7f942cb923442cecbee7926189727a4 as a test and it failed to load some v8.0.0 libraries - https://github.com/Numpsy/FAKE/actions/runs/10702782852/job/29671766410#step:10:950 - so not sure if that counts as an improvement.

Anyway - is it possible that the default frameworks at https://github.com/fsprojects/FAKE/blob/fac72951389da2798f396e32239fb03a032f36ea/src/app/Fake.Runtime/FakeHeader.fs#L182 should be different in a .NET 8.0 build?

Numpsy avatar Sep 04 '24 14:09 Numpsy

if you run locally dotnet fake --version then you can observe that it has picked FakePath: .../net6.0/... for whatever reason even there should be both net6.0 and net8.0 in the tool package, and the global.json tells to use net8.0

I think this is some kind of issue of "dotnet tool restore" picking the framework from latest version instead of instructed 1.0.0-local version. There is --framework parameter on dotnet tool install but that doesn't work on local tools like this.

Thorium avatar Sep 05 '24 19:09 Thorium

lf I open .fake\ dll with IlSpy, it seems to be compiled to .NET core 3.1, which could explain why it can't handle the libraries referenced: image

The reason is CompileRunner.fs calling FSharpChecker.Create() and I expect this is related: https://github.com/dotnet/fsharp/issues/14243

This seems to be the case in both current master and this branch.

Thorium avatar Sep 06 '24 06:09 Thorium

When testing a local build I noticed that build.fsx.lock has a RESTRICTION: || (== net6.0) (== netstandard2.0) at the top,

Oh, but netstandard 2.0 is not compatible with .net8. Shouldn't it be 2.1?

Thorium avatar Sep 06 '24 08:09 Thorium

I'm not entirely sure if those restrictions are a 'can be consumed by' or a 'same family of TFMs as' ? The last build does seem to have picked the .NET Standard 2.1 version of FSharp.Core though - https://github.com/fsprojects/FAKE/actions/runs/10729495009/job/29756204428#step:10:404

Numpsy avatar Sep 06 '24 08:09 Numpsy

I made an attempt at debugging the cli tool locally to see if I could see what's going on with the failure to load System.Globalization.dll seen in the CI builds, and made an observation:

When debugging it, the call to LoadFromAssemblyName at https://github.com/fsprojects/FAKE/blob/3a3e00c73041162b83f878960189e5736e836dad/src/app/Fake.Runtime/CoreCache.fs#L331 manages to load the assembly - image

However, the isFramework check a few lines down then discards the loaded dll. This is different from the current .NET 6.0 build, which keeps it.

Looking at the difference, it appears that the .NET 6.0 version of System.Globalization has the .NETFrameworkAssembly attribute, and the .NET 8 version doesn't.

Searching about a bit for that attribute value I spotted https://github.com/dotnet/runtime/pull/89490, where said property was removed from all in-box assembles in .NET 8.0.

So - I wonder if the logic there is just wrong in .NET 8 and newer?

Numpsy avatar Sep 17 '24 18:09 Numpsy

I guess the latest failure is becuse https://github.com/fsprojects/FAKE/blob/3a3e00c73041162b83f878960189e5736e836dad/build.fsx#L807 needs to specify whether to publish as .NET 6.0 or 8.0? (assuing it still needs to build as 6.0 rather than just using 8.0)

Numpsy avatar Sep 19 '24 13:09 Numpsy

But how can you build .NET 8 local package, when there is no .NET 8 compatible packages in NuGet?

How was it done when .NET 6.0 support was first added?

We might need to go with a route where there is "fake-cli 8.0.0-alpha -package" in NuGet to not break any existing fake implementations but still be able to call the global tool.

Thorium avatar Sep 22 '24 17:09 Thorium

it needs rebase again

xperiandri avatar Sep 23 '24 11:09 xperiandri

When trying to reason the build-log: There are 3 parallel threads all writing to same output, and many of the commands are just starting a process and then monitoring exit-code.

Thorium avatar Sep 24 '24 18:09 Thorium

When trying to reason the build-log: There are 3 parallel threads all writing to same output, and many of the commands are just starting a process and then monitoring exit-code.

Would it be worthwhlie trying to get something like https://github.com/EnricoMi/publish-unit-test-result-action plugged into the build and publish the test result xml files? Might be easier to see the results vs. trawling through all the text

Numpsy avatar Sep 25 '24 23:09 Numpsy

I like the idea, but maybe separately in PR to master-branch and I'll merge it here then?

Thorium avatar Sep 26 '24 09:09 Thorium

It turns out the latest FAKE 6.1.3 builds .NET 8 projects just fine (after this PR https://github.com/fsprojects/FAKE/pull/2835), so I think the priority of this struggle just dropped.

Thorium avatar Sep 26 '24 17:09 Thorium

I like the idea, but maybe separately in PR to master-branch and I'll merge it here then?

I'll have a look at what it would involve

Numpsy avatar Sep 29 '24 08:09 Numpsy

It turns out the latest FAKE 6.1.3 builds .NET 8 projects just fine (after this PR #2835), so I think the priority of this struggle just dropped.

Ok, some test results are getting published now - e.g. https://github.com/fsprojects/FAKE/actions/runs/11105226220/job/30852070808

Numpsy avatar Sep 30 '24 12:09 Numpsy

Test Results

  4 files   -   5    4 suites   - 5   52m 6s :stopwatch: + 4m 59s 437 tests  -   2  435 :white_check_mark:  -   4  0 :zzz: ±0  2 :x: +2  479 runs   - 772  476 :white_check_mark:  - 775  0 :zzz: ±0  3 :x: +3 

For more details on these failures, see this check.

Results for commit 6b18ea85. ± Comparison against base commit 6f2fc43b.

This pull request removes 2 tests.
[Fake.DotNet.FxCop.Tests; Test failure on non-Windows platforms]
[Fake.DotNet.ILMerge.Tests; Test failure on non-Windows platforms]

github-actions[bot] avatar Sep 30 '24 16:09 github-actions[bot]

Related to this:

I had a try at updating to Paket 9 rather than the alpha version it's currently using, and

  1. the Paket cli tool is now .NET 8, so it doesn't work when the .NET SDK is locked to version 6.
  2. I tried updating Paket.Core to v9 and it build in Visual Studio (17.12) but the CI failed with https://github.com/Numpsy/FAKE/actions/runs/11815951713/job/32918222457#step:6:745

I'm currently wondering if (2) might be because the dll was built with .NET 9 and has compressed metadata enabled so the .NET 6 compiler can't consume it, but I haven't looked into it any more than that so far

Numpsy avatar Nov 13 '24 11:11 Numpsy