Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Remove the System.Runtime.InteropServices.PInvoke contract #8626

Merged
merged 4 commits into from
May 21, 2016

Conversation

botaberg
Copy link

Per issue #7754, removing the System.Runtime.InteropServices.PInvoke contract.

This contract was introduced several months ago, but is no longer aligned with our direction for .NET Core APIs.

I bumped the version numbers of System.Runtime.InteropServices and System.Runtime back down from 4.1.0.0 to 4.0.21.0. If we need to, we can keep the 4.1.0.0 version around for the new UTF8 changes (#8607). I am wondering what extra we need to do for System.Runtime though, if we want to bump the version numbers down again.

@botaberg
Copy link
Author

@yizhang82 @tijoytom @weshaggard @ericstj Please take a look.

@mellinoe
Copy link
Contributor

I don't think we can lower version numbers at this point since the RC2 packages are public and supported, can we?

@ericstj
Copy link
Member

ericstj commented May 18, 2016

System.Runtime already has other additions that establish 4.1.0.0, just have a look at https://github.com/dotnet/corefx/commits/master/src/System.Runtime/ref/System.Runtime.cs.

InteropServices needs to stay at 4.1.0.0 as well for the UTF8 change you mention.

The only thing this change should do is delete IS.Pinvoke, delete dependencies on it, and remove the explicit type forwards from IS. You may also need to fix up https://github.com/dotnet/corefx/blob/ca5d1174dbaa12b8b6e55dc494fcd4609ed553cc/src/System.Runtime.CompilerServices.Unsafe/tests/UnsafeTests.cs.

// See the LICENSE file in the project root for more information.

[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Runtime.InteropServices.GCHandle))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Runtime.InteropServices.GCHandleType))]
Copy link
Author

Choose a reason for hiding this comment

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

@ericstj This change also removes explicit type forwards of GCHandle from System.Runtime. Should I leave this change in here?

John Bottenberg added 2 commits May 18, 2016 10:21
…InteropServices as part of the removal of S.R.IS.PInvoke. Also, remove a reference to PInvokeMarshal in the Unsafe tests.
@KrzysztofCwalina
Copy link
Member

KrzysztofCwalina commented May 18, 2016

cc: @raja-krishnan, @terrajobst
Hmm, do we really need to remove it? It's a great modern surface area API. Does it hurt the "new direction"?

@ericstj
Copy link
Member

ericstj commented May 19, 2016

It's irrelevant since InteropServices already is part of netstandard and thus must be supported everywhere. Removing it from netstandard means breaking changes (marshal) which are against the direction we are heading for API convergence. What benefit do you see in keeping it?

@jkotas
Copy link
Member

jkotas commented May 20, 2016

LGTM. There are merge conflicts that need to be fixed before this is merged.

@jkotas jkotas merged commit 1b61b69 into dotnet:master May 21, 2016
@karelz karelz modified the milestone: 1.0.0-rtm Dec 3, 2016
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…refx#8626)

Revert the changes that introduced the System.Runtime.InteropServices.PInvoke contract

Don't bump down version numbers of System.Runtime and System.Runtime.InteropServices as part of the removal of S.R.IS.PInvoke. Also, remove a reference to PInvokeMarshal in the Unsafe tests.

Commit migrated from dotnet/corefx@1b61b69
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
8 participants