Dependency on System.Drawing.Common (>= 7.0.0) blocks latest Microsoft.Build package

[theolivenbaum]

Not sure if this is the right place to report this, but while trying to update the Microsoft.Build package on our software to the latest version, we hit an issue due to a dependency on System.Security.Permissions, chianing to System.Windows.Extensions, which depends on System.Drawing.Common >= 7.0.0.

As System.Drawing.Common >=7.0.0 doesn't support macOS / Linux anymore (6.0.0 being the latest version that supports it through a compatibility flag), this chain of dependency blocks using to the latest package on Linux & macOS builds.

Any ideas if it is possible to force using the older package, or if the dependency version on System.Windows.Extensions could be lowered to >=6.0.0 ?

Thanks!

[ghost]

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Not sure if this is the right place to report this, but while trying to update the Microsoft.Build package on our software to the latest version, we hit an issue due to a dependency on System.Security.Permissions, chianing to System.Windows.Extensions, which depends on System.Drawing.Common >= 7.0.0.

As System.Drawing.Common >=7.0.0 doesn't support macOS / Linux anymore (6.0.0 being the latest version that supports it through a compatibility flag), this chain of dependency blocks using to the latest package on Linux & macOS builds.

Any ideas if it is possible to force using the older package, or if the dependency version on System.Windows.Extensions could be lowered to >=6.0.0 ?

Thanks!

Author:	theolivenbaum
Assignees:	-
Labels:	area-Infrastructure-libraries, untriaged
Milestone:	-

[rainersigwald]

I don't think it'll be possible to pin to a specific older version forever; we try to track the latest versions available where possible.

Do you need to update your MSBuild reference? You shouldn't generally need to, since you generally load MSBuild from a .NET SDK that contains all of its dependencies.

@baronfel and I looked into dropping the dependency, which is almost doable, except for

msbuild/src/Shared/ExceptionHandling.cs

Line 170 in 265f341

|| e is XmlSyntaxException

[danmoseley]

If it's literally just that line, could you check the exception type dynamically - GetType()? Presumably this is an exceptional path. And you already depend on reflection.

[theolivenbaum]

@rainersigwald we would love to drop the dependency on System.Drawing.Common 6.0.0, but due to how the deprecation was managed (i.e. as a breaking change including dropping support for the compatibility flag with the 7.0.0 release), we're stuck with 6.0.0 till other libraries replace their usage with other drawing libraries. While this is slowly moving forward, there are still many that have not yet updated, which forces us to use the last version that works cross platform.

I asked just because I found it very surprising that this dependency was there in the first place, and it seems it should be possible to either drop or change the constraint back to 6.0.0.

As shown in dotnet/sdk#33472, this is might also an issue when using the latest MSBuild package on other platforms, as System.Drawing.Common >= 7.0.0 is not available there.

[KalleOlaviNiemitalo]

https://source.dot.net/#System.Security.Permissions/System/Security/XmlSyntaxException.cs,64bad675b08e83cd,references doesn't list anything that creates an instance of XmlSyntaxException, and the constructor is obsolete in .NET 8 since dotnet/runtime#84383. Does MSBuild really have to recognize this type?

[danmoseley]

tagging @ericstj for thoughts as he's very familiar with unexpected dependencies on System.Drawing

[rainersigwald]

we would love to drop the dependency on System.Drawing.Common 6.0.0, but due to how the deprecation was managed (i.e. as a breaking change including dropping support for the compatibility flag with the 7.0.0 release), we're stuck with 6.0.0 till other libraries replace their usage with other drawing libraries. While this is slowly moving forward, there are still many that have not yet updated, which forces us to use the last version that works cross platform.

To be clear, I'm asking if you need to update your MSBuild reference, @theolivenbaum.

[theolivenbaum]

Need: probably not now, although we've always tracked the latest version as well and were surprised to see it break this time.

[ericstj]

In 8.0 we removed some of the dependencies on System.Drawing.Common but we did not remove the dependency originating from System.Security.Permissions since we discourage the use of System.Security.Permissions. The dependency from System.Security.Permissions to System.Windows.Extensions is required - for types in the public surface area. The dependency between System.Windows.Extensions to System.Drawing.Common is only for forwarded types, and one that @ViktorHofer and I have discussed removing. I put up a PR to do this. dotnet/runtime#88157

It think it would be a good idea to have MSBuild avoid it's dependency on System.Security.Permissions entirely. The runtime doesn't ever throw a XmlSyntaxException and I couldn't find a place where this is thrown elsewhere in the SDK. Perhaps on .NETFramework it's still thrown - but you wouldn't need a System.Security.Permissions reference there since it's part of mscorlib. Maybe you can just ifdef this code to only be present for NETFramework builds and then remove the package reference entirely.

Finally a question for the OP - @theolivenbaum - does your product actually need to deploy Microsoft.Build and it's dependency closure? Usually MSBuild as not a local dependency of an application. For example - a common usage in tasks would have Microsoft.Build referenced with PrivateAssets="All" with the expectation that the library be provided by the host when the task is loaded by MSBuild . You might be able to separate your usage of Microsoft.Build from the component that needs to use System.Drawing.Common and avoid leaking the Microsoft.Build dependencies to other projects by making it with PrivateAssets.

[KalleOlaviNiemitalo]

In .NET Framework, XmlSyntaxException can be thrown by internal classes System.Security.SecurityDocument, System.Security.Util.Parser, or System.Security.Util.Tokenizer. These are apparently used for deserializing Code Access Security types such as System.Security.PermissionSet, in AppDomain.Deserialize. Perhaps the runtime calls that when parsing security attributes from assemblies (ECMA-335 6th ed. §II.22.11 DeclSecurity).

The System.Security.SecurityElement.Escape(string) method has sometimes been used for general XML escaping unrelated to security, but that one does not throw XmlSyntaxException.

[rainersigwald]

Maybe you can just ifdef this code to only be present for NETFramework builds and then remove the package reference entirely.

Works for me, let's do that.

[KirillOsenkov]

What about the idea to check for the type dynamically, such as e != null && e.GetType.FullName == "System.Security.XmlSyntaxException"? Would love to drop that reference for .NET Framework too.

[theolivenbaum]

Finally a question for the OP - @theolivenbaum - does your product actually need to deploy Microsoft.Build and it's dependency closure? Usually MSBuild as not a local dependency of an application. For example - a common usage in tasks would have Microsoft.Build referenced with PrivateAssets="All" with the expectation that the library be provided by the host when the task is loaded by MSBuild . You might be able to separate your usage of Microsoft.Build from the component that needs to use System.Drawing.Common and avoid leaking the Microsoft.Build dependencies to other projects by making it with PrivateAssets.

Thanks for the tip, I'll investigate on our end. If I remember correctly, it is pulled on our build via a dependency on our C# to JS transpiler.

[rainersigwald]

If I remember correctly, it is pulled on our build via a dependency on our C# to JS transpiler.

That explicitly disables the hint from the MSBuildLocator package that tries to guide you to the approach @ericstj was suggesting (which I endorse). Looking at history it's been like that for a while. If y'all run into trouble removing that please file a new issue here--this is hard and we'd love to make it easier.