dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.02k stars 4.03k forks source link

MSBuild throws 'Cannot modify an evaluated object' #35760

Closed GeertvanHorrik closed 1 year ago

GeertvanHorrik commented 5 years ago

For the original ticket, see See https://github.com/microsoft/msbuild/issues/4376

Version Used:

16

Steps to Reproduce:

This happens when running dotnet-format combined with msbuild-sdk-extras.

For example, run it on https://github.com/wildgums/orc.controls

Expected behavior

The project should be loaded successfully (it can build, load, etc, but when running dotnet format, it fails.

Actual behavior

The project should be loaded successfully (it can build, load, etc, but when running dotnet format, it fails.

I've been investigating the source code, and found that it's correctly splitting multi-targeted builds, but then needs to re-check for the target version which it doesn't.

image

So it's trying to remove a property that's been added while re-evaluating the project. Then the code should check for the value again (it might have been set by the re-evaluation).

I think the fix is to re-check for the property existence just before removing the property.

GeertvanHorrik commented 5 years ago

Happy to PR if that gets it fixed faster.

GeertvanHorrik commented 5 years ago

Writing a PR with a fix now, but it makes sense to remove the explicit target framework if it was not available in the first place.

Should we just keep it on the project and not remove it once it has been set? Then this exception is done, the "only" thing is that we have updated the project, but does that matter much?

GeertvanHorrik commented 5 years ago

Another option is to do a try/catch around the removal of the property, but not sure whether that's a good solution...

GeertvanHorrik commented 5 years ago

Just did a debug session with dotnet-format, if I skip the resetting, it does work (but I have no idea whether this has implications for other parts of Roslyn).

CyrusNajmabadi commented 1 year ago

I dont' really understand what this has to do with roslyn-ide. Could you clarify what it is you'd like us to change/fix here in our code @GeertvanHorrik ? Thanks!

ghost commented 1 year ago

Closing this issue as we've seen no reply to the request for more information. If you are able to get the requested information, please add it to the issue and we will retriage it.