Fake build project on .NET 9 #742

xperiandri · 2025-07-10T15:03:49Z

Build project is on .NET 9
launchSettings.json with multiple profiles to run the build project
Target to execute locally and issue a release: update changelog, commit, set tag
Publish successful merges to master as prerelease GitHub packages
Embed all sources into GitHub packages for easy local debugging

Copilot

Pull Request Overview

This PR migrates the FAKE build from an FSI script to a dedicated .NET 9 build project and updates solution and package version management.

Introduces build/build.fsproj targeting net9.0 and new F# build code in build/build.fs
Removes legacy build.fsx and adds the new build project to the solution
Refactors Directory.Packages.props to group package versions and adds missing dependencies

Reviewed Changes

Copilot reviewed 19 out of 21 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/FSharpLint.Core/FSharpLint.Core.fsproj	Disabled auto assembly info and swapped in `Attributes.fs`
build/build.fsproj	New FAKE build project targeting .NET 9
build/build.fs	Full replacement of the FAKE build script with fs project
Directory.Packages.props	Centralized package version groups, moved test SDK
.github/workflows/copilot-setup-steps.yml	CI runner set to an invalid label
.devcontainer/devcontainer.json	Inconsistent extension ID casing

Comments suppressed due to low confidence (2)

.github/workflows/copilot-setup-steps.yml:17

The runner label windows-2025 is not a valid GitHub-hosted runner. Use a supported label like windows-latest or windows-2022 to ensure the job runs successfully.

    runs-on: windows-2025

.devcontainer/devcontainer.json:40

[nitpick] Extension identifiers are usually all lowercase. Consider using ionide.ionide-fsharp to match the marketplace ID casing and ensure consistency.

                "Ionide.Ionide-fsharp",

knocte · 2025-07-11T14:21:14Z

Fake build project on .NET 9

What does the title of this PR really mean? If with "Fake" you mean the build system FAKE, maybe rather put it in uppercase letters? Not that I'm trying to be a nitpicker but I don't understand what "FAKE build project on .NET9" really means if master branch is already targetting .NET9. What does this PR really achieve? Can you reword the title of PR to convey that message more clearly please?

I'll continue my review now, please don't merge yet until you got a green light from me because I still think this is still reverting some improvements that we have made in the past.

.github/workflows/build+test+deploy.yml

knocte · 2025-07-11T14:23:21Z

build.ps1

@@ -0,0 +1,5 @@
+$ErrorActionPreference = 'Stop'


@xperiandri why do we need powershell script if we already have build.cmd?

To remove the conditional logic that existed for building on Windows and non-Windows.
As PowerShell exists and is available everywhere

There is no conditional logic in master, dotnet fsi works everywhere.

knocte · 2025-07-11T14:27:09Z

.github/workflows/copilot-setup-steps.yml

@@ -0,0 +1,34 @@
+name: "Copilot Setup Steps"


@xperiandri looks like this file is unrelated to this PR?

OK, I'll add it separatelly

knocte · 2025-07-11T15:28:07Z

I'll continue my review now

I'm having some connection issues atm, will continue tomorrow.

knocte · 2025-07-11T15:42:53Z

.github/workflows/build+test+deploy.yml

+      run: ./build.ps1 Publish
+
+    - name: PublishToGitHub (if master branch)
+      if: github.ref == 'refs/heads/master'


@xperiandri this logic was moved in the past from GithubActions to F#, let's keep it in F# please

knocte · 2025-07-12T14:24:26Z

@xperiandri this logic was moved in the past from GithubActions to F#, let's keep it in F# please

The reason is one that I kind of hinted in the previous PR already: if you make the Publish CI job only be run in master branch, then changing the job might lead to breaking it without noticing it in the PR status. However, doing the check in F# instead of with GitHubActions' if statement allows the job to run always, except for the last step (the nuget push). This way, the nuget pack part gets exercised everytime (and it's not rare to break nuget pack operations by mistake if you, for example, change some properties of the .fsproj files).

knocte

See review comments above.

xperiandri · 2025-07-12T19:25:22Z

I don't understand how your last comment is related to this PR.

The reason is one that I kind of hinted in the previous PR already: if you make the Publish CI job only be run in master branch, then changing the job might lead to breaking it without noticing it in the PR status. However, doing the check in F# instead of with GitHubActions' if statement allows the job to run always, except for the last step (the nuget push). This way, the nuget pack part gets exercised everytime (and it's not rare to break nuget pack operations by mistake if you, for example, change some properties of the .fsproj files).

The pipeline contains ./build.ps1 DotnetPack which is not constrained by any condition.

For me, it looks strange to use Fake but call targets separately in the pipeline. If we use Fake the pipeline should consist of.NET install and calling Fake.
The current pipeline does

      run: dotnet restore
    - name: Build
      run: dotnet fsi build.fsx -t Build
    - name: Run tests
      run: dotnet fsi build.fsx -t Test
    - name: Run FSharpLint on itself
      run: dotnet fsi build.fsx -t SelfCheck

but inside we have

"Clean"
  ==> "Build"
  ==> "Test"
  ==> "Default"

which means that we build twice, and here

"Default"
  ==> "Pack"
  ==> "Push"
  ==> "Release"

with

    - name: Build
     run: dotnet fake build
   - name: Pack
     run: dotnet fake build -t Pack

we build twice again. As @Numpsy told

And what exists now is

    - name: Upload binaries to nuget (if nugetKey present)
      env:
        nuget-key: ${{ secrets.NUGET_KEY }}
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
      run: dotnet fake build -t Push
    - name: Create Release (if tag)
      if: startsWith(github.ref, 'refs/tags/')
      id: create_release
      uses: actions/create-release@latest
      env:
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # This token is provided by Actions, you do not need to create your own token
      with:
        tag_name: ${{ github.ref }}
        release_name: ${{ github.ref }}
        body: ${{ steps.changelog_reader.outputs.log_entry }}
        draft: false
        prerelease: false
    - name: Upload binaries to release (if tag)
      if: startsWith(github.ref, 'refs/tags/')
      uses: svenstaro/upload-release-action@v1-release
      with:
        repo_token: ${{ secrets.GITHUB_TOKEN }}
        file: out/*.nupkg
        tag: ${{ github.ref }}
        overwrite: true
        file_glob: true

and this logic is extremely complicated and impossible to understand without GPT explanation

    let key = getBuildParam "nuget-key"
    match getBuildParam "GITHUB_EVENT_NAME" with
    | None ->
        match key with
        | None ->
            let key = UserInput.getUserPassword "NuGet Key: "
            push key
        | Some key ->
            push key

    | Some "push" ->
        match key with
        | None ->
            Console.WriteLine "No nuget-key env var found, skipping..."
        | Some key ->
            if isTag then
                push key
            else
                match getBuildParam "GITHUB_SHA" with
                | None ->
                    failwith "GITHUB_SHA should have been populated"
                | Some commitHash ->
                    let gitArgs = sprintf "describe --exact-match --tags %s" commitHash
                    let proc =
                        CreateProcess.fromRawCommandLine "git" gitArgs
                        |> Proc.run
                    if proc.ExitCode <> 0 then
                        // commit is not a tag, so go ahead pushing a prerelease
                        push key
                    else
                        Console.WriteLine "Commit mapped to a tag, skipping pushing prerelease..."
    | _ ->
        Console.WriteLine "Github event name not 'push', skipping..."

I think it is a crazy mess.

I have clean working code and pipelines from https://github.com/TheAngryByrd/MiniScaffold implemented in 3 my repos and multiple others. Let it have a few adjustments that you want.

CI pipeline for everything does Pack
Publish to GitHub packages from master
Publish to NuGet from v* tag

All pipelines look like

    - uses: actions/checkout@v4
    - name: Setup .NET
      uses: actions/setup-dotnet@v4
      with:
        global-json-file: global.json
    - name: Restore tools
      run: dotnet tool restore
    - name: Build
      shell: pwsh
      run: ./build.ps1 <call target>

Clean, easy to read and maintain. One pipeline has only a pipeline-level trigger condition and calls Fake.

knocte · 2025-07-15T11:39:27Z

we build twice again

First let me ask you this basic question: if you clone the project, and you try to run a target that runs tests, shouldn't the build system know that the project hasn't been built yet and then launch the build before trying to run the tests? I hope yes, then I would agree.

Now, if you agree with the above, it's only normal to think that other targets should behave similarly, e.g. if you instruct the build system to "Pack", then it should build first if there was no build before, right?

Alright, so now that we agree on basic functionality of a build system, let's talk performance! If you instruct the build system to "Build" first, and after that, you instruct it to "Pack", should it build twice? Well, if you're using a good build system, it shouldn't. And this is not a conversation about CI, this is just talking build systems. So let's see what happens if you use Make: what you normally do in this case is create a target with the same name as the binary file that gets generated by the build; after that you make that target depend on all the source files that are needed to generate that binary; and then you create a target called "Build" and make it depend on the source files and on the binary file. This way, if you instruct Make to build twice (e.g. running "make" twice in your terminal) it will actually only run the build once! Because it detects that the source files didn't change after the last binary was generated. After this, you could create a target for packing, and so on.

Now, does FAKE work this way to avoid building twice? I'm not sure! I told you I'm not a fan of FAKE (and not a fan of MSBuild either, truth to be told) so I didn't learn it enough to know if this is possible. But if this is possible, then we should be able to make use of the FAKE feature that allows this, without having to change our CI steps (remember, we have 2 steps: one for building, another for packing, so that when it fails, we know which part failed! if we didn't care about this information, then, for sure, we would only need a CI step that just packs).

I hope you understood where I'm coming from here.

I think it is a crazy mess.

You're welcome to refactor that code if you think you can make it more readable. But to me, you're just overhauling a bunch of things here which are regressing in terms of what the build system can do right now in the master branch, for example: if we merged this PR as it is, and after that we created a new tag (a new version), would your new build scripts be able to figure out that the commit is a tag and, therefore, not push a pre-release version to nuget.org for that commit?

xperiandri · 2025-07-15T15:19:41Z

I have no device without .NET 9 SDK installed to verify, but I suppose it fixes that

knocte · 2025-07-15T15:28:05Z

I have no device without .NET 9 SDK installed to verify, but I suppose it fixes that

Did you mean to write this comment in PR 748? Because I assume that with "without .NET 9" you mean "with only .net8", something which doesn't seem related to this PR.

xperiandri · 2025-07-15T15:29:52Z

Sorry, wrong PR

…m YAML to Fake

Copilot AI review requested due to automatic review settings July 10, 2025 15:03

xperiandri force-pushed the fake-project branch from 06c2e5e to 2d95d00 Compare July 10, 2025 15:04

Copilot AI reviewed Jul 10, 2025

View reviewed changes

xperiandri force-pushed the fake-project branch 2 times, most recently from 7d3381f to 8afbd19 Compare July 11, 2025 00:18

xperiandri mentioned this pull request Jul 11, 2025

Fake build project and GitHub actions per scenario #739

Closed

knocte reviewed Jul 11, 2025

View reviewed changes

.github/workflows/build+test+deploy.yml Show resolved Hide resolved

knocte reviewed Jul 11, 2025

View reviewed changes

knocte requested changes Jul 12, 2025

View reviewed changes

xperiandri force-pushed the fake-project branch from 8afbd19 to 63d87e9 Compare July 18, 2025 10:10

xperiandri added 6 commits July 18, 2025 14:28

feat(ci): Fake build project

1af4cad

feat(ci): run Fornax to build docs

b0c6594

feat(ci): self check the solution

4ad7693

refactor(ci): move NuGet.config package source mapping creation fro…

c9ef3ba

…m YAML to Fake

feat(docs): resolve full paths to find FSharpLint.Core.dll

5789c96

feat(ci): improve change log reading

9b314f0

xperiandri force-pushed the fake-project branch from 63d87e9 to 9b314f0 Compare July 18, 2025 10:33

fixup! feat(ci): self check the solution

44c5744

Fake build project on .NET 9 #742

Are you sure you want to change the base?

Fake build project on .NET 9 #742

Uh oh!

Conversation

xperiandri commented Jul 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

knocte commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

knocte Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

xperiandri Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

knocte Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

knocte Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

xperiandri Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

knocte commented Jul 11, 2025

Uh oh!

knocte Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

knocte commented Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knocte left a comment

Choose a reason for hiding this comment

Uh oh!

xperiandri commented Jul 12, 2025

Uh oh!

knocte commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xperiandri commented Jul 15, 2025

Uh oh!

knocte commented Jul 15, 2025

Uh oh!

xperiandri commented Jul 15, 2025

Uh oh!

Uh oh!

knocte commented Jul 11, 2025 •

edited

Loading

knocte commented Jul 12, 2025 •

edited

Loading

knocte commented Jul 15, 2025 •

edited

Loading