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

Remove Drawing.Common .NETCoreApp PNSEs #80435

Merged
merged 1 commit into from
Jan 11, 2023

Conversation

ViktorHofer
Copy link
Member

System.Drawing.Common was throwing PNSEs on non Windows platforms when accessing public API which makes sense as the library isn't supposed to be used on other platforms.

That said, we already have a runtime hook that throws when DllImports are invoked. We intentionally want to allow accessing Drawing exchange types to not break customers when they don't rely on GDI+ or other native functions.

This is also part of a wider effort that allows this library to be ported to another repository (which doesn't support targeting RIDs).

System.Drawing.Common was throwing PNSEs on non Windows platforms when
accessing public API which makes sense as the library isn't supposed to
be used on other platforms.

That said, we already have a runtime hook that throws when DllImports
are invoked. We intentionally want to allow accessing Drawing exchange
types to not break customers when they don't rely on GDI+ or other
native functions.

This is also part of a wider effort that allows this library to be
ported to another repository (which doesn't support targeting RIDs).
@ghost
Copy link

ghost commented Jan 10, 2023

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

Issue Details

System.Drawing.Common was throwing PNSEs on non Windows platforms when accessing public API which makes sense as the library isn't supposed to be used on other platforms.

That said, we already have a runtime hook that throws when DllImports are invoked. We intentionally want to allow accessing Drawing exchange types to not break customers when they don't rely on GDI+ or other native functions.

This is also part of a wider effort that allows this library to be ported to another repository (which doesn't support targeting RIDs).

Author: ViktorHofer
Assignees: ViktorHofer
Labels:

area-System.Drawing

Milestone: -

@ericstj
Copy link
Member

ericstj commented Jan 10, 2023

Interesting that we had this Windows check already in the windows specific assembly:

if (!OperatingSystem.IsWindows())
{
NativeLibrary.SetDllImportResolver(Assembly.GetExecutingAssembly(), static (_, _, _) =>
throw new PlatformNotSupportedException(SR.PlatformNotSupported_Unix));
}

That's what makes this change not need to touch the library source at all. This will result in a TypeInitializationException being thrown with inner exception of PlatformNotSupportedException (rather than directly a PNSE). That's what the behavior was in 6.0 anyway so perhaps that's OK, but it is a difference from what shipped in 7.0.

@ViktorHofer ViktorHofer merged commit 7c265c3 into dotnet:main Jan 11, 2023
@ViktorHofer ViktorHofer deleted the SimplifyDrawingCommon branch January 11, 2023 08:16
@ghost ghost locked as resolved and limited conversation to collaborators Feb 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants