Skip to content

[vs17.13] Fix SdkResult Evalution when ProjectRootElement is null#11628

Closed
surayya-MS wants to merge 7 commits intodotnet:vs17.14from
surayya-MS:sdk-result-fix
Closed

[vs17.13] Fix SdkResult Evalution when ProjectRootElement is null#11628
surayya-MS wants to merge 7 commits intodotnet:vs17.14from
surayya-MS:sdk-result-fix

Conversation

@surayya-MS
Copy link
Member

Fixes #11550

Context

This is a regression caused by changes in MSBuildSdkResolver that were introduced in dotnet/sdk#45364. Specifically by adding 2 new properties.
This results in hitting the path that was not hit before - handling properties and items of SdkResult:

if ((sdkResult.PropertiesToAdd?.Any() == true) ||
(sdkResult.ItemsToAdd?.Any() == true))
{
projectList ??= new List<ProjectRootElement>();
// Inserting at the beginning will mean that the properties or items from the SdkResult will be evaluated before
// any projects from paths returned by the SDK Resolver.
projectList.Insert(0, CreateProjectForSdkResult(sdkResult));
}

When Project is created from XmlReader and not from ProjectRootElement, it results in null ProjectRootElement during Evaluation. Which results in internal exception like InternalErrorException: MSB0001: Internal MSBuild Error: .SdkResolver.1981936763.proj unexpectedly not a rooted path here:

string projectPath = $"{_projectRootElement.FullPath}.SdkResolver.{propertiesAndItemsHash}.proj";

Above created project path is just .SdkResolver.1981936763.proj with no directory. Later exception is thrown here because of it:
ErrorUtilities.VerifyThrowInternalRooted(projectFile);

or here if you use SimpleProjectRootElementCache:
ErrorUtilities.VerifyThrowInternalRooted(projectFile);

Changes Made

Changed the projet path that is created for SdkResult properties and items - if there is no ProjectRootElement then generate name like {Guid}.SdkResolver.{propertiesAndItemsHash}.proj in the current directory.

Testing

Added test. Tested manually as well

Notes

dotnet-maestro bot and others added 5 commits March 19, 2025 14:13
* Update dependencies from https://github.com/dotnet/arcade build 20250311.4

Microsoft.SourceBuild.Intermediate.arcade , Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.XliffTasks , Microsoft.DotNet.XUnitExtensions
 From Version 9.0.0-beta.25111.5 -> To Version 9.0.0-beta.25161.4

* Update Versions.props VersionPrefix

* Update dependencies from https://github.com/dotnet/arcade build 20250314.2

Microsoft.SourceBuild.Intermediate.arcade , Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.XliffTasks , Microsoft.DotNet.XUnitExtensions
 From Version 9.0.0-beta.25111.5 -> To Version 9.0.0-beta.25164.2

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Jenny Bai <v-jennybai@microsoft.com>
@surayya-MS surayya-MS requested a review from a team as a code owner March 26, 2025 10:06
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks pretty good to me but at this point I think we should fix in 17.14. If we want to service 17.13 we need to put together the evidence in support of that.

@surayya-MS surayya-MS changed the base branch from vs17.13 to vs17.14 March 27, 2025 12:15
@surayya-MS
Copy link
Member Author

surayya-MS commented Mar 27, 2025

Closing this PR because we want to target 17.14 #11636

@surayya-MS surayya-MS closed this Mar 27, 2025
@surayya-MS surayya-MS deleted the sdk-result-fix branch March 27, 2025 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants