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
220 stars 83 forks source link

Should MSBuildLocator.RegisterDefaults register the highest VS version instead of first? #81

Open jasonmalinowski opened 4 years ago

jasonmalinowski commented 4 years ago

Right now MSBuildLocator.RegisterDefaults registers the first instance it finds on the machine, whatever that is:

https://github.com/microsoft/MSBuildLocator/blob/ecd04bf208ed0cc3e67d46e7eb90359df7315504/src/MSBuildLocator/MSBuildLocator.cs#L93

This provides to be wrong in a lot of cases: for example, any tool using RegisterDefaults won't be able to analyze the Roslyn solution on a machine with both VS2017 and VS2019. RegisterDefaults will pick the 2017 version, which will break because we're using the new "GetPathsOfAllDirectoriesAbove" feature which doesn't exist in 2017. Just yesterday both I and @sharwell were independently replacing this in two separate tools with the exact same PR:

https://github.com/dotnet/roslyn/pull/40886 https://github.com/dotnet/roslyn/pull/40574/commits/43e13de123dce589518b51dd48bbe82aadba5361

Should this just be the automatic behavior? Because .editorconfig support uses this new MSBuild feature, anybody using any tool using the default function that has both VS2017 and VS2019 on their machine is going to get potentially very unexpected results.

tbolon commented 3 years ago

Excellent remark, impacting also docfx which still uses VS 2017 in our build server instead of VS 2019, resulting in multiple errors when trying to build projects not compatible with VS 2017.

moh-hassan commented 3 years ago

using "Microsoft.Build.Locator" Version="1.4.1", get the recent Version (NET5 on my machine, I have net2.0, 2.1, 3.1 and 5.0.4) I can control the version of MsBuild using the next code:

  var visualStudioInstances = MSBuildLocator .QueryVisualStudioInstances();
  //select NET5, or whatever by modifying Version.Major 
    var instance = visualStudioInstances .FirstOrDefault(x => x.Version.Major.ToString() == "5");
//register the instance
   MSBuildLocator.RegisterInstance(instance);   

Note: MSBuildLocator.QueryVisualStudioInstances result is dependent on the TargetFrameWork of the project which is controled by DiscoveryType In my case i use NetCore SDK project

In FullFramework (e.g net472) project, the result is different and based on the actual path of installed Visual Studio.

asos-tomlonghurst commented 2 years ago

@moh-hassan This is simpler and will work with newer versions as they get installed:

        var query = MSBuildLocator.QueryVisualStudioInstances();

        var instance = query.MaxBy(x => x.Version);

        if (instance == null)
        {
            throw new ArgumentException("Please install the latest .NET SDK");
        }

        MSBuildLocator.RegisterInstance(instance);

or

        var query = MSBuildLocator.QueryVisualStudioInstances();

        var instance = query.OrderBy(x => x.Version).LastOrDefault();

        if (instance == null)
        {
            throw new ArgumentException("Please install the latest .NET SDK");
        }

        MSBuildLocator.RegisterInstance(instance);
jzabroski commented 2 years ago

@rainersigwald @Forgind I realize nobody is pushing you to triage this stuff since nobody in management seems to care, but this would be a pretty huge improvement.

@yaakov-h Ideally, RegisterLatest would be RegisterDefault. Most toolchains should operate under pseudo-"compile at head globally" assumption.

I can see why RegisterDefault is the lowest version, though. But the problem is, it's NOT the lowest version, it's the first version. Lowest version would make sense as that would then match nuget behavior, which is to prevent some degree of non-determinism by having scripts that automatically migrate up versions when some dependency changes.

Even better would be if Microsoft just copied what Google has been doing in Bazel/Blaze.