cake-build / cake

:cake: Cake (C# Make) is a cross platform build automation system.
https://cakebuild.net
MIT License
3.89k stars 726 forks source link

Upgrade the Cake executable to target .NET 4.7.2 #2253

Closed cnickel closed 2 years ago

cnickel commented 6 years ago

As the title suggests, can you please update the the Cake executable to target the 4.7.2 version of the .NET Framework?

When building addins, iIt would be beneficial to use features introduced in versions of the framework >4.6.1. For example. In an addin that I'm creating I require the long path syntax support of DirectoryInfo constructor introduced in 4.6.2

Thank you.

knocte commented 6 years ago

For example. In an addin that I'm creating I require the long path syntax support of DirectoryInfo constructor introduced in 4.6.2

Then you need 4.6.2, not 4.7.2.

jnm2 commented 6 years ago

Is there something wrong with an addin targeting net472 even though Cake itself needs only net461?

knocte commented 6 years ago

Not all platforms support 4.7.2 already. I don't think Cake should upgrade for the sake of upgrading.

jnm2 commented 6 years ago

Yes, my point is that Cake should target the lowest common denominator and leave it up to addin authors to target their addin to something newer if they want to make that call.

devlead commented 6 years ago

@knocte so this isn't an upgrade just for the sake of it, we have given it some thought, even not stated fully in this issue.

@jnm2 this came up as guidance just recently shifted from Microsoft when they recently stated

Sorry but we messed up. We tried to make .NET Framework 4.6.1 retroactively implement .NET > Standard 2.0. This was a mistake as we don't have a time machine and there is a tail of bugs.

If you want to consume .NET Standard 1.5+ from .NET Framework, I recommend to be on 4.7.2.

So if we want to support .NET standard 2 without issues we need to target 4.7.2 or we need all addins to multitarget both .NET Framework and .NET standard.

Issue we're having is that addin authors get weekly support issues because 4.6.1 causing netstandard issues. And as previous guidance from Microsoft was target .NET standard 2 and you'll support 4.6.1 and up, this has resulted in many packages on nuget has dropped framework monikers in favor of only having .NET standard monikers, which resulted in some addins dependencies no longer even available for 4.6.1.

By targetting 4.7.2 the guidance would be clear, implementation would be clean and cause less issues.

Of course we could change our guidance, have all addin authors multitarget, choose lower versions of dependencies for framework moniker, if def and polyfill the api differences that might cause. And then publish new versions. In some cases that could mean exposing users to risk as I.e. security fixes only available in latest versions, libgit2sharp is a really good recent example.

So Microsoft has put us and many others in a hard spot here.

knocte commented 6 years ago

The correct way to solve this problem then, is to have a stable release (which stays at 4.6.1), and a development release (which targets 4.7.2 so that devs having problems can target it). Then most people will pin to Cake stable version and people having problems will use latest dev version. Until 4.7.2 becomes widespread enough that you can call the latest dev version a stable version.

This would also go inline with the recommendation that other cake devs seem to give* about pinning in general. By having two clearly defined version release series, you can more clearly send the message that pinning to a version (in this case stable) is a recommended thing to do to avoid surprises.

* see https://github.com/cake-build/cake/issues/2264#issuecomment-418079995

devlead commented 6 years ago

So what would 4.7.2 add in that scenario? I mean if you run 4.6.1 assembly on 4.7.2 .NET standard will work.

So if we were to go the guidance route, we could just stay on 4.6.1.

Currently we ship for

Each moniker adds a bit of a support burden.

knocte commented 6 years ago

Each moniker adds a bit of a support burden.

In my experience, having stable vs dev versions relaxes the support burden. Because you can just make bugfix releases (no new features) in the stable series, while leaving new features in the dev series (then decreasing the chances of regressions, which are the main culprit of issues/supportRequests).

devlead commented 6 years ago

If people want zero regressions and no new features, then they should as we recommend pin their version of Cake and the versions of the addins they're using. I.e. 0.30.0 will always run on .NET 4.6.1. Pinning all versions is the recommendation for reproducible builds.

If there's an serious enough bug/regression with a version, then we'll do a hotfix release for that, an example of that is 0.28.1 which is a hotfix release for 0.28.0, so we do have a channel for that already. i.e. if 0.30.0 would need patching after 0.31.0 is release, then we would release 0.30.x from the latest 0.30.0 release.

Today if someone wants the latest and greatest features on a machine with older .NET version than 4.6.1, then they've got the option to use Cake.CoreCLR and sideload .NET Core runtime as part of their Cake bootstrapping process.

Also we do already have a dev channel on MyGet, which get a new version for each merge to develop. https://www.myget.org/gallery/cake So if developers want to try the unstable and unreleased bits, they can do so already.

jnm2 commented 6 years ago

@devlead That's a separate reason to move Cake to net472, one which I'd support.

I don't think that Cake should target a different framework version to enable an addin to access a newer API, since the addin can always target a newer framework version without Cake changing its target framework. Is that correct?

cnickel commented 6 years ago

@jnm2, It doesn't seem to be the case that I can just set the target framework in the addin to >= 4.6.2. From what I have been witnessing as long as Cake.exe is targeting 4.6.1, I get an exception on the DirectoryInfo constructor when using the long path syntax. If I locally retarget Cake to >4.6.1 then all is right in the world.

Alternately, I can set <AppContextSwitchOverrides value="Switch.System.IO.UseLegacyPathHandling=false" /> in the Cake.exe.config file and utilize the long paths as I expect locally, however the issue I have here is that on the build servers when the bootstrapper is executed and it brings down all the Cake bits, the config file gets overwritten.

To add a little clarity on the submission on this specific Github issue, I asked the question in Gitter if there were plans to upgrade the target framework, the answer was yes but no issue had been created that I would watch for progress. Through the conversation I was suggested that I create one. When I did, I supplied my need for the description. I don't think it should be construed that the Cake team is going to upgrade the framework for the sake of the request for a single addin.

jnm2 commented 6 years ago

Thanks for explaining! For people watching the proposals that come through, it looked a bit surprising. But it's okay. :D

Makes sense. You need Cake targeting net462 or newer because you can't control .NET Framework quirking from inside an addin. I would be interested to have a discussion issue with this title because if .NET Framework 4.8 (released next year) adds any quirking rules, this may come up again.

devlead commented 6 years ago

So we've decided to for now stay on 4.6.1, but add documentation/guidance and probably blog around .NET Standard pitfalls.

@cnickel to solve your issue, would us including the mention config sort your issue? Does it have implications wen running on 4.6.1?

cnickel commented 6 years ago

@devlead, For the time being I have been able to work around the issue by using a combination of the cake.exe.config entry as mentioned above, along with the bootstrapper argument -SkipToolPackageRestore and committing the cake tool directory to the repository. While I am not thrilled by this solution it will have to suffice until Cake targets something >= 4.6.2.

I appreciate the dialog and the responsiveness regarding this issue, and would like to thank everyone for their insight. I will leave the issue for the Cake team to close, in the case that they would like to leave any additional thoughts/comments.

mvestergaard commented 4 years ago

Is there any chance this can be taken up for consideration again? I've recently needed to do some tooling that use Microsoft.Build https://www.nuget.org/packages/Microsoft.Build/, which in the latest version (tied to MSBuild 16, or VS2019) only targets net472, and netcoreapp2.1. Meanwhile other dependencies of the tooling, don't support netstandard, so it's kind of a shitty limbo state where I can't move in either direction.

devlead commented 4 years ago

@mvestergaard So are these dependencies tools or assemblies?

Personally I more and more think the Cake global tool is the way forward it's compiled for .NET Core 2.1 and 3.0. What would be the scenarios it wouldn't solve?

mvestergaard commented 4 years ago

@devlead As mentioned, other parts of our build pipeline rely on dependencies that are not netstandard compatible.

mvestergaard commented 4 years ago

Oh, and to your first question, these are custom cake addins that rely on external dependencies for things. In particular one of those that is not netstandard compatible, at least as of yet, is this thing https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/ used for validating database migrations.

devlead commented 4 years ago

Ok, reason I ask tool or assemblies, is because only assemblies will be an issue with Cake.Tool it'll run any tool fine, it'll even fallback to mono if available on posix systems.

See that that package refers to Microsoft.SqlServer.DacFx is now available with netstandard2 moniker at https://www.nuget.org/packages/Microsoft.SqlServer.DacFx

But do acknowledge the problem, issue is that we don't know how which runtimes people are using, so while switching to i.e. 4.8 would solve problem for some, it might break even more builds for people running on mono or previous .NET versions.

But perhaps we could do something "smart" so people could opt-in to i.e. supplying a 4.8 exe too or something.

At same time main future investment should probably go into Cake.Tool as tables are turning and dependencies are starting to be netstandard/netcore only. We've recently seen a huge adoption in Cake.Tool which now has more daily downloads than Cake.exe. doing any special solution might add to our maintenance burden.

mvestergaard commented 4 years ago

I would definitely like to move to Cake.Tool too when possible. While I have you, is there any way to bootstrap Cake.Tool, same as the build.ps1/build.sh of old? I would like to avoid requiring the global installation, as it would be quite a task both to set it up on each CI server, but also changing the workflow developers are used to.

Regarding your other suggestion, I was also thinking that an "easy" solution would be to be able to opt in to a Cake.exe targeting a newer framework version. How much effort would it be to go that route?

devlead commented 4 years ago

With some thinking we might come up with something clever, resources limited though, and there's new and shiny things we want to solve too 😎

You could look at Cake's projects own bootstrappers, we use Cake.Tool to build Cake itself. https://github.com/cake-build/cake/blob/develop/build.ps1 https://github.com/cake-build/cake/blob/develop/build.sh

mvestergaard commented 4 years ago

Great, thanks for the feedback @devlead. Right now our workaround to the issues we have is that people must have VS2017 installed, which keeps us safe from these dependency issues. But that's not really a viable long term solution.

I'll be looking into how far off we are from being able to port to Cake.Tool.

pascalberger commented 2 years ago

While with 2.0 we will update target Frameworks, we will also simplify runners and stop shipping Cake Runner for .NET Framework and .NET Core. Cake .NET tool will still be supported on .NET Core 3.1 and higher.

See https://github.com/cake-build/cake/discussions/3575 for details.