Skip to content

Conversation

@KevinRansom
Copy link
Contributor

@KevinRansom KevinRansom commented Apr 20, 2019

Enable Pinvoke testing on coreclr.

Because pinvoke on coreclr is not supported on versions of dotnet framework lower than 3.0, we generate a flag at build time that determines whether to run the test or not.

Also adds a flag generated at build time, that lets us know whether we are running on coreclr version 3.0.
The tests run on desktop and coreclr, when applicable.

Some code was removed from the original test cases, this code was removed because it depends on a windows dll, cards.dll that is no longer shipped. And other code that relied on popdyn.dll, that I have no idea what it is or where it is. The existing test merely compiled against them. Now we also run the tests, so at last they are actually testing pinvoke.

@KevinRansom KevinRansom requested review from brettfo and dsyme April 20, 2019 22:28
@KevinRansom KevinRansom changed the title Pinvoketests Pinvoke testing Apr 20, 2019
@NinoFloris
Copy link
Contributor

This sounds too simple? I've used p/invoke since dotnet core 1.0 without problems. I'm not aware of big changes there either for 3.0?

@cartermp
Copy link
Contributor

dotnet framework lower than 3.0

I'm confused. So it's compatible with .NET Framework?

@KevinRansom
Copy link
Contributor Author

KevinRansom commented Apr 22, 2019

@cartermp, @NinoFloris this is pinvoke from fsi, it worked fine for apps compiled with fsc. The RefEmit API for emitting the necessary code to support pinvoke wasn't added until netcoreapp3.0.

The desktop fsi has always supported pinvoke from fsi. The coreclr version of fsi has not supported pinvoke until this PR: #6542

I hope this clears things up

Kevin

@KevinRansom KevinRansom merged commit 028e1df into dotnet:master Apr 22, 2019
@cartermp
Copy link
Contributor

@KevinRansom Did you intend to merge this? Build shows failure: https://dev.azure.com/dnceng/public/_build/results?buildId=165027

@KevinRansom
Copy link
Contributor Author

@cartermp bugger, it has some extra and buggy code in it.

KevinRansom added a commit that referenced this pull request Apr 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants