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

Sample for IDynamicInterfaceCastable #3744

Merged
merged 11 commits into from
Jul 28, 2020

Conversation

elinor-fung
Copy link
Member

Summary

Add a sample using the new IDynamicInterfaceCastable API.

cc @AaronRobinsonMSFT @jkoritzinsky

@elinor-fung elinor-fung requested a review from a team as a code owner July 20, 2020 17:34
core/interop/IDynamicInterfaceCastable/README.md Outdated Show resolved Hide resolved
core/interop/IDynamicInterfaceCastable/README.md Outdated Show resolved Hide resolved
core/interop/IDynamicInterfaceCastable/README.md Outdated Show resolved Hide resolved
core/interop/IDynamicInterfaceCastable/README.md Outdated Show resolved Hide resolved
core/interop/IDynamicInterfaceCastable/README.md Outdated Show resolved Hide resolved

This sample provides an implementation of `IDynamicInterfaceCastable` that projects a native object as implementing different known managed interfaces. It uses COM conventions (e.g. `QueryInterface`) for interacting with the native object, but does not require the actual COM system.

Note: The sample uses `Marshal` APIs as part of interacting with the native library. The introduction of [C# function pointers](https://github.com/dotnet/csharplang/blob/994c41586e07e38fb6b30902b1715b4025d80c52/proposals/function-pointers.md) will allow that interaction to occur in a more performant manner.
Copy link
Member

Choose a reason for hiding this comment

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

The function pointers are shipping in previews already. Can the sample be modified to use them?

Copy link
Member Author

Choose a reason for hiding this comment

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

The new syntax for specifying the calling convention (e.g. cdecl -> unmanaged[Cdecl]) isn't in the shipped previews yet. I was planning on updating the sample once they can use the new syntax.

Co-authored-by: Youssef Victor <31348972+Youssef1313@users.noreply.github.com>
@elinor-fung
Copy link
Member Author

@dotnet/docs This sample involves building a native library. To make it a bit simpler for users to run, it hooks the native build in such that dotnet run and dotnet build will build the native library. This is the same system as that used for the marshaling and hosting samples. The prerequisite to make this work is the have the C++ compiler for the platform be installed and on the path when building. The CI build is failing because the C++ compiler is not found - is there a recommendation for what we should do here? Maybe have the CI set some property such that the projects could conditionally skip doing the native build?

Copy link
Member

@IEvangelist IEvangelist left a comment

Choose a reason for hiding this comment

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

I have some nits for the C# bits, otherwise this is looking awesome!

elinor-fung and others added 3 commits July 20, 2020 13:50
Co-authored-by: David Pine <david.pine@microsoft.com>
Co-authored-by: David Pine <david.pine@microsoft.com>
Copy link
Member

@IEvangelist IEvangelist left a comment

Choose a reason for hiding this comment

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

This actually needs a README.md with metadata, so that it is indexed by the samples browser.

Here is an example:
https://github.com/dotnet/samples/blob/master/async/async-and-await/cs/readme.md

Copy link
Member

@IEvangelist IEvangelist left a comment

Choose a reason for hiding this comment

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

Needs a README.md

@adegeo
Copy link
Contributor

adegeo commented Jul 21, 2020

@dotnet/docs This sample involves building a native library. To make it a bit simpler for users to run, it hooks the native build in such that dotnet run and dotnet build will build the native library. This is the same system as that used for the marshaling and hosting samples. The prerequisite to make this work is the have the C++ compiler for the platform be installed and on the path when building. The CI build is failing because the C++ compiler is not found - is there a recommendation for what we should do here? Maybe have the CI set some property such that the projects could conditionally skip doing the native build?

I would love to get CI working for this. I checked on my computer that has the latest VS2019 preview and I don't have the following according to the readme:

  • Using Developer Command Prompt for VS 2019 Int Preview, cl.exe doesn't resolve to anything.
  • There is no x64 Native Tools Command Prompt for VS 2019 shortcut on my machine.

@elinor-fung
Copy link
Member Author

This actually needs a README.md with metadata, so that it is indexed by the samples browser.

Nice - I had no idea that was a thing. Will update.

There is no x64 Native Tools Command Prompt for VS 2019 shortcut on my machine.

Do you have a C++ workload installed for VS? I'll make the readme point out
https://docs.microsoft.com/cpp/build/building-on-the-command-line#download-and-install-the-tools.

The CI is only on Windows currently, right?

@adegeo
Copy link
Contributor

adegeo commented Jul 22, 2020

Do you have a C++ workload installed for VS? I'll make the readme point out
https://docs.microsoft.com/cpp/build/building-on-the-command-line#download-and-install-the-tools.

The CI is only on Windows currently, right?

Ahh that is probably the reason. I believe the CI machine has all workloads installed, so the command would work there. CI is only on Windows yes. Tomorrow I'll work on getting Preview 7 installed and the CI to use dotnet via the developer command prompt. It currently only runs it directly.

@elinor-fung
Copy link
Member Author

@adegeo I updated the sample so that it should automatically find the MSVC compiler based on the bitness of the dotnet executable doing the build. It works locally for me. With that, hopefully moving Preview 7 is all that is needed for the CI.

@adegeo
Copy link
Contributor

adegeo commented Jul 23, 2020

Awesome! CI passed!

Co-authored-by: Youssef Victor <31348972+Youssef1313@users.noreply.github.com>
@elinor-fung
Copy link
Member Author

@IEvangelist I have updated the README to include metadata for indexing by the samples browser. Do you have any additional concerns with this?

Co-authored-by: Youssef Victor <31348972+Youssef1313@users.noreply.github.com>
@elinor-fung
Copy link
Member Author

@adegeo Seems like the CI is now expecting the added global.json to correspond a project in its current directory or a parent directory, but not finding one?

Running "LocateProjects "d:\a\samples\samples" --pullrequest 3744 --owner dotnet --repo samples"
1|core/interop/IDynamicInterfaceCastable/global.json|

When it was passing before, global.json wasn't listed by LocateProjects.

Copy link
Member

@IEvangelist IEvangelist left a comment

Choose a reason for hiding this comment

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

Looks good to me, we'll :shipit: - thank you 🙏

@IEvangelist IEvangelist merged commit 227b5de into dotnet:master Jul 28, 2020
@elinor-fung elinor-fung deleted the castableSample branch July 28, 2020 20:16
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.

5 participants