Retarget everything to .NET 6

[Numpsy]

An attempt at updating everything to .NET 6 all at once:

• Use the .NET 6 SDK
• Update Paket to v7
• Update FAKE to v6
• Update the .NET Test SDK to 17.7.2 (the latest version at the point the change was made)

Note: I had to fight with Paket a bit doing the updates as it kept trying to update Microsoft.Build.* to v17.7 (which needs .NET 7, and thus doesn't work) - so I hope the updated paket files are sensible

[Numpsy]

Extra note: this is a superset of the previous PRs

[backerman]

Works on my machine, but .github/workflows/publish.yml will need to be changed as well.

[knocte]

@Numpsy hey, thanks for rebasing this PR. Sorry for being a bit unresponsive the past days, I was on holidays. Seems CI is broken now, can you fix?

[Numpsy]

It looks like the nuget package restore is failing on the macOS build (which then causes the Linux build that has got further to be aborted), not sure why that is.
There has been a new release of Paket as well, so I'll see if that's useful

[Numpsy]

Saying that, just forced a retry on the build at https://github.com/Numpsy/FSharpLint/actions/runs/6942714944 and it worked, so :-(

[Numpsy]

The error looks like the one described at fsprojects/Paket#3767
(I'm not sure what the 'storage: symlink' setting in the docs section of paket.dependencies is doing offhand though)

[knocte]

Upgrading paket to v8 doesn't increase reliability?

[Numpsy]

I'll give it a go

[Numpsy]

It still seems to have a problem with Paket 8, but I did try a run without the storage:symlink setting for the docs at https://github.com/Numpsy/FSharpLint/actions/runs/6948076650 and that one worked

[knocte]

I did try a run without the storage:symlink setting for the docs at https://github.com/Numpsy/FSharpLint/actions/runs/6948076650 and that one worked

So let's propose this as a separate PR to increase paket reliability on MacOS CI?

[Numpsy]

Ok.
Such a change might relate to #578 on Windows as well

[knocte]

Let's rebase this now?

[knocte]

Let's now remove the Paket update? Since it's not really needed for this PR anymore.

[knocte]

Let's now remove the Paket update? Since it's not really needed for this PR anymore.

@Numpsy ?

[Numpsy]

Let's now remove the Paket update? Since it's not really needed for this PR anymore.

@Numpsy ?

I removed the test change to use Paket 8, but the update to 7 was there because the version it currently uses doesn't understand .NET 6/7 and it caused build warnings when updating nuget packages that target those versions

[Numpsy]

rebased to resolve conflicts with other CI build definition changes

[knocte]

rebased to resolve conflicts with other CI build definition changes

Thanks very much! BTW, do you mind if we make the commits a bit clear? For example:

1. Update the sdk version in the publish workflow
2. Build everything as .NET 6
3. update build.yml

Can you squash 1 with 3 since they both change CI?

[knocte]

If you don't know how can you merge 2 commits into 1 let me know and I'll be able to help. BTW, now that you'd be doing an interactive rebase, can you also explain (in the commit msg) the paket commands you ran that made changes into the paket.deps and paket.lock files? I'm not a paket expert so every little bit of extra info you could add would be appreciated.

[smoothdeveloper]

@knocte, on the paket side, I'd guess the changes were "adjust paket.dependencies in text editor" (for the framework restrictions, often times, you can also do with no such restriction, a bit more verbose paket.lock file, but more resilient if you need to try or target different runtimes, without redoing install), followed by paket install in this case.

Sometimes, when I want to be conservative, I use paket install --keep-patch, it updates paket.lock based on my changes in paket.dependencies but it keeps the current dependencies as is.

Other times, when I want to see what a "updating everything" does, I just erase a group in the paket.lock and run the paket install --keep-patch again (so it doesn't change the other groups).

Other time, when I want to update just one dependency, I remove the line form the paket.lock an run paket install --keep-patch.

There are more fine grained commands for handling a specific dependency.

[Numpsy]

Can you squash 1 with 3 since they both change CI?

Ok, I'll do that later (and look at rebasing on top of the 0.21.5 changes at the same time)

[Numpsy]

@knocte, on the paket side, I'd guess the changes were "adjust paket.dependencies in text editor" (for the framework restrictions, often times, you can also do with no such restriction, a bit more verbose paket.lock file, but more resilient if you need to try or target different runtimes, without redoing install), followed by paket install in this case.

Sometimes, when I want to be conservative, I use paket install --keep-patch, it updates paket.lock based on my changes in paket.dependencies but it keeps the current dependencies as is.

Other times, when I want to see what a "updating everything" does, I just erase a group in the paket.lock and run the paket install --keep-patch again (so it doesn't change the other groups).

Other time, when I want to update just one dependency, I remove the line form the paket.lock an run paket install --keep-patch.

There are more fine grained commands for handling a specific dependency.

I think that's just what I did for updating the TFMs in paket.dependencies, followed by updates to the FAKE libs and the Test SDK (I don't think there were any updates to the versions of things beyond that).

My knowledge of Paket is very slim itself - as I mentioned on the initial post the attempt at updating without any extra settings tried to update things like Microsoft.Build that I didn't want it to (or which don't work), so it needs --keep-patch or similar to restrict what gets updated

[knocte]

I think that's just what I did for updating the TFMs in paket.dependencies

Can you be more specific please, and actually put the info in the commit msg. I want to be able to replicate what you did and see the same diff that your commit has.

BTW I'm afraid this needs yet another rebase (as I'm doing many releases in the past days which include merging stuff here and there that touches these files as well, sorry).

[Numpsy]

Rebased and updated commit comments.

For the record here, the paket install does need --keep-patch, otherwise running it now tries to update dependencies to the latest available (which are now in some cases the .NET 8 versions), which wasn't wanted for this PR.

[knocte]

Thanks Richard! getting there. Now only 2 things left I think:

• I see you also updated paket but you didn't mention why or explained how you did it (manually or also with a command?).
• I generally prefer to merge PRs that have more than one commit with a merge-commit (instead of squashing or rebasing), but if this is the case I prefer to make sure that all CI statues of each of the commits is green (and in the particular case of this PR I have the feeling that some intermediate commits might not). Can you push 1 by 1 to make sure? There's two ways to do this, manually or automatically.

[Numpsy]

The paket tool was just updated with dotnet tool update paket, and doing things one change at once was what

#603
#605

were trying to do (this one contains both of those, but #603 could still be completed separately)

[knocte]

The approach of merging this work as separate PRs is different than merging a PR with separate commits. I don't prefer the former because each PR needs to have its own goal. However, a PR can have a goal but still be split in different commits with specific sub-goals which, all combined achieve the bigger goal of the PR. And in the latter approach, I prefer that each commit has a green CI status.

I've been trying to replicate what you did, with the instructions you provided, in my own branch here (not succeeding yet, so far): https://github.com/knocte/FSharpLint/commits/make_it_all_6/ I will now take your advice about how to update paket (because that's the only difference, I guess, between my branch and yours), and report further.

[Numpsy]

I had to rebase everything on top of the changes in #629, and that has also meant that all the changes to the build yaml file are now in one commit.

Also added an extra 'Update paket with dotnet tool update paket' to the commit message.

[knocte]

that all the changes to the build yaml file are now in one commit.

Ah, that makes more sense! However, to double-check that everything is good now with the commits of your PR, I pushed them one by one, cherry-picking them into a branch of mine, and I'm afraid their intermediate CI statuses are not green. See:

https://github.com/knocte/FSharpLint/commits/rw/make_it_all_6/

This means that there are some bits of the commits that depend on each other. To try to make myself understood I'm going to bring a naive example: you're correcting the README.md file in your last commit but that means the README.md file was broken in the commit that changed the name of the workflow; therefore, the changes for that commit should be squashed with the commit that changes the name of the workflow (the one titled "update the CI builds").

The above is just an example because when I say that the "README.md file is broken", what I mean is that it links to a broken link for a CI badge. It's not easy to see/know that it's broken. But CI sometimes can gives us better hints of how something is broken.

[knocte]

@Numpsy thanks for rebasing again! We're getting closer because now there's only 1 red CI status, see my branch (which is exactly the same as yours, but pushed 1 by 1):

https://github.com/knocte/FSharpLint/commits/rw/make_it_all_6/

Shouldn't you just squash the last commit with the previous one, to solve the problem?

Another thing to be improved that I find in your commit msg is:

Update paket with dotnet tool update paket, to get a version with .net 6 support

However, if I do the same on my machine, it updates to version 8.0.0, not 7.2.1 like it happened in your PR. Did you forget to append --version flag?

[knocte]

On another note, I keep trying to replicate what you did, without success:

https://github.com/knocte/FSharpLint/commits/updToNet6/

I'm not sure how you fixed the CI issues. And if I can't figure out from the info in your commit msgs, then we need better commit messages. @MrLuje ping? maybe you can help here move this forward?

[Numpsy]

Did you forget to append --version flag?

No, it's just that the initial PR was done before .NET 8 went RTM, so doing it without specifying an explicit version got 7.x at the time, and might get 8 now.

As the only code (rather than build/project) changes were already merged in #636 we could probably just squash the CI and project changes, yes.

[knocte]

As the only code (rather than build/project) changes were already merged in #636 we could probably just squash the CI and project changes, yes.

Agreed.

No, it's just that the initial PR was done before .NET 8 went RTM, so doing it without specifying an explicit version got 7.x at the time, and might get 8 now.

Got it. However, I tried to replicate again what you did, this time adding the --version flag when updating paket (to upgrade to the same version you upgraded to) and I still got CI broken. What magic trick did you do in your diff (that you're forgetting to add to the commit message) to make CI pass?

This is the branch where I followed your instructions from your commit msg bit by bit:

https://github.com/knocte/FSharpLint/commits/replicateNumpsyWork/

And this is the error that CI gets:

Could not detect any platforms from 'net8.0' in '/home/runner/.nuget/packages/system.text.encodings.web/8.0.0/lib/net8.0/System.Text.Encodings.Web.xml', please tell the package authors
Could not detect any platforms from 'net8.0' in '/home/runner/.nuget/packages/system.security.cryptography.protecteddata/8.0.0/lib/net8.0/System.Security.Cryptography.ProtectedData.xml', please tell the package authors
Could not detect any platforms from 'net8.0' in '/home/runner/.nuget/packages/microsoft.net.stringtools/17.8.3/lib/net8.0/Microsoft.NET.StringTools.pdb', please tell the package authors
Could not detect any platforms from 'net8.0' in '/home/runner/.nuget/packages/system.text.encoding.codepages/8.0.0/lib/net8.0/System.Text.Encoding.CodePages.dll', please tell the package authors
Could not detect any platforms from 'net8.0' in '/home/runner/.nuget/packages/system.security.cryptography.pkcs/8.0.0/lib/net8.0/System.Security.Cryptography.Pkcs.xml', please tell the package authors
Could not detect any platforms from 'net8.0' in '/home/runner/.nuget/packages/system.formats.asn1/8.0.0/lib/net8.0/System.Formats.Asn1.dll', please tell the package authors
Could not detect any platforms from 'net8.0' in '/home/runner/.nuget/packages/system.configuration.configurationmanager/8.0.0/lib/net8.0/System.Configuration.ConfigurationManager.xml', please tell the package authors
Could not detect any platforms from 'net8.0' in '/home/runner/.nuget/packages/system.collections.immutable/8.0.0/lib/net8.0/System.Collections.Immutable.dll', please tell the package authors
Could not detect any platforms from 'net8.0' in '/home/runner/.nuget/packages/microsoft.build.utilities.core/17.8.3/lib/net8.0/Microsoft.Build.Utilities.Core.xml', please tell the package authors
Could not detect any platforms from 'net8.0' in '/home/runner/.nuget/packages/microsoft.build.framework/17.8.3/lib/net8.0/Microsoft.Build.Framework.dll', please tell the package authors
Could not detect any platforms from 'net8.0' in '/home/runner/.nuget/packages/system.text.json/8.0.0/lib/net8.0/System.Text.Json.dll', please tell the package authors
6.0.417

Is it maybe that you're forgetting to comment about 2 modifications you did in paket.dependencies? In particular, assigning "strategy" to "min" and/or changing framework: netstandard2.0 to net6.0? You're not commenting anything about that in your commit msg and I doubt that any tool you invoked made those changes.

[Numpsy]

The changes to the .net version in paket.dependencies were what the 'editing paket.dependencies and changing frameworks to net6.0' comment in the commit is about (In the case of the Build group, I think because the fake 5 libraries were built as .net standard 2, and the fake 6 libraries are built as .net 6.0).

Did you use the --keep-patch flag when doing the paket install? That's supposed to stop it trying to update the libraries to newer versions.
(I wonder if some of this would work better if more of the entries in paket.dependencies had version contraints applied - e.g. should the Microsoft.Build.* references have a maximum version of 17.3.x so that it won't try to update to versions that require .NET 7 or 8? That's a separate change though)

[knocte]

'editing paket.dependencies and changing frameworks to net6.0' comment in the commit is about

You're still not explaining what the min: addition change is about.

Did you use the --keep-patch flag when doing the paket install?

Yes I did, since @smoothdeveloper explained it above (very useful thanks Gauthier).

I wonder if some of this would work better if more of the entries in paket.dependencies had version contraints applied - e.g.

That might be the right way to do it indeed (and maybe this is what you did, since CI in your branch is green), but if you haven't documented it I cannot really accept a PR that changes paket.lock so much without me being able to replicate it (that paket.lock file is a blackbox...).

So in the end I've succeeded in, more or less, adopting the work you did here, in this other PR: #655 , which I'm going to merge shortly. Thanks for your work! I added a co-authored git tag to it.