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

[jasonmalinowski]

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

MSBuildLocator/src/MSBuildLocator/MSBuildLocator.cs

Line 93 in ecd04bf

var instance = GetInstances(VisualStudioInstanceQueryOptions.Default).FirstOrDefault();

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:

dotnet/roslyn#40886
dotnet/roslyn@43e13de

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]

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]

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]

@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]

@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.