microsoft / MSBuildLocator

An API to locate MSBuild assemblies from an installed Visual Studio location. Use this to ensure that calling the MSBuild API will use the same toolset that a build from Visual Studio or msbuild.exe would.
Other
212 stars 83 forks source link

Environment sdk version restriction negatively affects dotnet tools #153

Closed Lanayx closed 4 months ago

Lanayx commented 2 years ago

Here are the links to the main sympthoms https://github.com/neuecc/MessagePack-CSharp/issues/1387 https://github.com/fsprojects/FSharpLint/issues/536

The root of the issue seems to be these lines:

            // Components of the SDK often have dependencies on the runtime they shipped with, including that several tasks that shipped
            // in the .NET 5 SDK rely on the .NET 5.0 runtime. Assuming the runtime that shipped with a particular SDK has the same version,
            // this ensures that we don't choose an SDK that doesn't work with the runtime of the chosen application. This is not guaranteed
            // to always work but should work for now.
            if (major > Environment.Version.Major ||
                (major == Environment.Version.Major && minor > Environment.Version.Minor))
            {
                return null;
            }

So if library is built with .net5.0 and then used on the environment with just .net6.0 is installed it fails, because the check above returns null. At the same time there shouldn't be any restrictions of running tools on higher versions of sdk's.

JoeRobich commented 2 years ago

I think enabling rollForward would be the best option for these tools. They could continue to target net5.0 if they wish, but the runtime could roll up to 6.0, which would be compatible with .NET 6 SDKs. See documentation at https://docs.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#rollforward

baronfel commented 2 years ago

One thing I discovered with Ionide is that when an app targets net5.0 and is rolled forward to a newer runtime, the System.Environment.Version values don't seem to update to those of the runtime. This killed my ability to use this library entirely, since a thte time we targeted net5.0 for reach and enabled roll-forward as you describe.

jzabroski commented 2 years ago

One thing I discovered with Ionide is that when an app targets net5.0 and is rolled forward to a newer runtime, the System.Environment.Version values don't seem to update to those of the runtime.

If true, please file a bug in dotnet/runtime. That sounds very incorrect.

rainersigwald commented 2 years ago

One thing I discovered with Ionide is that when an app targets net5.0 and is rolled forward to a newer runtime, the System.Environment.Version values don't seem to update to those of the runtime.

I'm not seeing this.

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
    <RollForward>LatestMajor</RollForward>
  </PropertyGroup>

</Project>
using System;                                                                                                                                                                                                                                   namespace Net5Rollforward
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine(System.Environment.Version);
        }
    }
}
S:\play\Net5Rollforward via .NET v7.0.100-preview.5.22307.18 🎯 net5.0
❯ dotnet run
7.0.0
baronfel commented 2 years ago

Well that's very interesting. I can't find my notes from the tire fire that was this pr, but a finding like that is what led me to rip it all out. Perhaps its time for a more measured look again.

YuliiaKovalova commented 4 months ago

It has been recently addressed in scope of this ticket. https://github.com/microsoft/MSBuildLocator/issues/271

Lanayx commented 4 months ago

@YuliiaKovalova can you please link the PR or commit to see how that was fixed? I don't see any PR in the issue you mentioned

JoeRobich commented 4 months ago

@Lanayx I believe these are the PRs https://github.com/microsoft/MSBuildLocator/pull/265 & https://github.com/microsoft/MSBuildLocator/pull/281

YuliiaKovalova commented 4 months ago

Hi @Lanayx and @JoeRobich ,

You can test the changes in the scope of the recently released package: https://www.nuget.org/packages/Microsoft.Build.Locator/1.7.8

Please let us know if you have any questions/concerns.