Version 1.5.3 misses a trailing directory separator path

[CamiloTerevinto]

In a project, where the MS Build instance is registered with MSBuildLocator.RegisterDefaults(); before calling a method that ends up doing:

var globalProperties = new Dictionary<string, string>
{
    { "SolutionDir", $"{Path.GetDirectoryName("path")}{Path.DirectorySeparatorChar}" }
};
var projectCollection = new ProjectCollection(globalProperties);
var project = projectCollection.LoadProject("path");

Both the above and doing just new Project("path"); fails with an InvalidProjectFileException.

Looking at the ProjectCollection, the difference between Microsoft.Build.Locator 1.4.1 and 1.5.3 is in ProjectCollection.Toolsets[0]._properties[0].

1.4.1: "MSBuildSDKsPath"="C:\\Program Files\\dotnet\\sdk\\6.0.303\\Sdks"
1.5.3: "MSBuildSDKsPath"="C:\\Program Files\\dotnet\\sdk\\6.0.303Sdks"

This might need to be fixed in the Microsoft.Build.Evaluation package instead of here, though.

[caaavik-msft]

Just in case it helps anyone else, I got stuck on this for a good chunk of the day as this was the error I was presented which was misleading:

Unhandled exception: Microsoft.Build.Exceptions.InvalidProjectFileException: The SDK 'Microsoft.NET.Sdk' specified could not be found.  C:\path\to\project.csproj
   at Microsoft.Build.Shared.ProjectErrorUtilities.ThrowInvalidProject(String errorSubCategoryResourceName, IElementLocation elementLocation, String resourceName, Object[] args)
   at Microsoft.Build.Shared.ProjectErrorUtilities.ThrowInvalidProject[T1](IElementLocation elementLocation, String resourceName, T1 arg0)
   at Microsoft.Build.Evaluation.Evaluator`4.ExpandAndLoadImportsFromUnescapedImportExpressionConditioned(String directoryOfImportingFile, ProjectImportElement importElement, List`1& projects, SdkResult& sdkResult, Boolean throwOnFileNotExistsError)
   at Microsoft.Build.Evaluation.Evaluator`4.ExpandAndLoadImports(String directoryOfImportingFile, ProjectImportElement importElement, SdkResult& sdkResult)
   at Microsoft.Build.Evaluation.Evaluator`4.EvaluateImportElement(String directoryOfImportingFile, ProjectImportElement importElement)
   at Microsoft.Build.Evaluation.Evaluator`4.PerformDepthFirstPass(ProjectRootElement currentProjectOrImport)
   at Microsoft.Build.Evaluation.Evaluator`4.Evaluate()
   at Microsoft.Build.Evaluation.Evaluator`4.Evaluate(IEvaluatorData`4 data, Project project, ProjectRootElement root, ProjectLoadSettings loadSettings, Int32 maxNodeCount, PropertyDictionary`1 environmentProperties, ILoggingService loggingService, IItemFactory`2 itemFactory, IToolsetProvider toolsetProvider, ProjectRootElementCacheBase projectRootElementCache, BuildEventContext buildEventContext, ISdkResolverService sdkResolverService, Int32 submissionId, EvaluationContext evaluationContext, Boolean interactive)
   at Microsoft.Build.Execution.ProjectInstance.Initialize(ProjectRootElement xml, IDictionary`2 globalProperties, String explicitToolsVersion, String explicitSubToolsetVersion, Int32 visualStudioVersionFromSolution, BuildParameters buildParameters, ILoggingService loggingService, BuildEventContext buildEventContext, ISdkResolverService sdkResolverService, Int32 submissionId, Nullable`1 projectLoadSettings, EvaluationContext evaluationContext)
   at Microsoft.Build.Execution.ProjectInstance..ctor(String projectFile, IDictionary`2 globalProperties, String toolsVersion, String subToolsetVersion, ProjectCollection projectCollection, Nullable`1 projectLoadSettings, EvaluationContext evaluationContext)
   at Microsoft.Build.Execution.ProjectInstance..ctor(String projectFile, IDictionary`2 globalProperties, String toolsVersion)

I would recommend that in addition to fixing the missing directory separator, that a bugfix also includes checks to ensure that the directories exist so that a more meaningful error message can be shown as this error message was not very helpful.

[Danielku15]

I can also confirm this issue. Problem is that at this place a simple concatination is done instead of using Path.Combine

MSBuildLocator/src/MSBuildLocator/MSBuildLocator.cs

Lines 311 to 322 in 850d27b

	private static void ApplyDotNetSdkEnvironmentVariables(string dotNetSdkPath)
	{
	const string MSBUILD_EXE_PATH = nameof(MSBUILD_EXE_PATH);
	const string MSBuildExtensionsPath = nameof(MSBuildExtensionsPath);
	const string MSBuildSDKsPath = nameof(MSBuildSDKsPath);
	var variables = new Dictionary<string, string>
	{
	[MSBUILD_EXE_PATH] = dotNetSdkPath + "MSBuild.dll",
	[MSBuildExtensionsPath] = dotNetSdkPath,
	[MSBuildSDKsPath] = dotNetSdkPath + "Sdks"
	};

[jharjung]

Lost a full day of work to figuring this out after Microsoft.Build and Microsoft.CodeAnalysis started spewing "SDK not found" errors :(

I also considered a reflection hack as an interim workaround like MarkPflug above, but it turned out to be simpler to just wipe the offending environment variables after calling RegisterDefaults. Works for my build, but no idea if it has other implications.

MSBuildLocator.RegisterDefaults();

// Workaround for https://github.com/microsoft/MSBuildLocator/issues/176
Environment.SetEnvironmentVariable("MSBuildSDKsPath", null);
Environment.SetEnvironmentVariable("MSBUILD_EXE_PATH", null);

Downgrading Microsoft.Build.Locator nuget back to 1.4.1 also appears to work (on .net 6.0.4, Microsoft.Build 17.3.1, Microsoft.CodeAnalysis 4.2).