Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vs17.13] Don't mark synthesized projects dirty when SDKs define properties #11478

Open
wants to merge 8 commits into
base: vs17.13
Choose a base branch
from

Conversation

surayya-MS
Copy link
Member

Context

Fixes #11394
This is a regression. The bug appeared after Expand MSBuildSdkResolver.

Customer impact

Without this fix the customers that opt-into MsBuildUseSimpleProjectRootElementCacheConcurrency will get System.NotImplementedException. This includes slngen and quickbuild.

Details

The NotImplementedException is thrown here:

internal override void OnProjectRootElementDirtied(ProjectRootElement sender, ProjectXmlChangedEventArgs e)
{
throw new NotImplementedException();
}

Previously the SdkResult of MSBuildSdkResolver was empty and ProjectRootElement was never created for it. Now, it contains 2 properties, and when ProjectRootElement is created, every change marks it as dirty. The fix is not to mark it dirty when it is from SdkResult

Changes made

Implemented internal CreateNotDirty that creates ProjectRootElement that cannot be dirtied.

Testing

Added unit test for CreateNotDirty. Also manually tested that the exception is not thrown anymore.

Risks
Low - existing tests ensure that other scenarios are not broken, added new test, also tested manually this exact case.

surayya-MS and others added 6 commits February 19, 2025 15:28
Try disabling AV in the official build script to avoid build breaks that manifest as

```
D:\a\_work\1\s\.packages\microsoft.dotnet.arcade.sdk\9.0.0-beta.24516.2\tools\Sign.proj(72,5): error MSB4018: The "Microsoft.DotNet.SignTool.SignToolTask" task failed unexpectedly.
System.Runtime.Serialization.SerializationException: Type 'System.AssemblyLoadEventArgs' in assembly 'mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089' is not marked as serializable.
   at Microsoft.Build.BackEnd.Components.RequestBuilder.AssemblyLoadsTracker.CurrentDomainOnAssemblyLoad(Object sender, AssemblyLoadEventArgs args)
   at System.AppDomain.OnAssemblyLoadEvent(RuntimeAssembly LoadedAssembly)
   at Microsoft.Build.Framework.ITask.Execute()
   at Microsoft.Build.BackEnd.TaskExecutionHost.Execute()
   at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() [D:\a\_work\1\s\.packages\microsoft.dotnet.arcade.sdk\9.0.0-beta.24516.2\tools\Sign.proj]
```

---------

Co-authored-by: Jan Provaznik <[email protected]>
@surayya-MS
Copy link
Member Author

This is approved by tactics in #11454
but we need to target 17.13

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.

2 participants