Skip to content

Xamarin.iOS and Xamarin.Android bindings - #623

Merged
bemartin merged 17 commits into
masterfrom
bemartin/xamarin
Sep 22, 2020
Merged

bemartin merged 17 commits into
masterfrom
bemartin/xamarin

Conversation

@bemartin

Copy link
Copy Markdown
Contributor

This PR includes:

  • Android and iOS Xamarin bindings for methods available in both Java and Obj-C wrappers
  • Test projects to validate the bindings
  • build-xamarin.sh script to build C++ SDK, Obj-C/Java wrappers and Xamarin bindings
  • Initial Xamarin.md documentation on building and consuming the Xamarin bindings

@bemartin bemartin changed the title Bemartin/xamarin [WIP] Xamarin.iOS and Xamarin.Android bindings Sep 14, 2020
Comment thread wrappers/xamarin/sdk/OneDsCppSdk.Standard/Enums.cs Outdated
@maxgolov

Copy link
Copy Markdown
Contributor

I like all of it. Could you please double-check that the test failures are not related to your commit?

@maxgolov

Max Golovanov (maxgolov) commented Sep 16, 2020 •

Copy link
Copy Markdown
Contributor

When you integrate it - I'd like to compile it, get the assembly, then diff versus what we generate using C++/CX on Windows. If there are some significant differences, we can address these in a separate PR. BTW, old nuget packaging scripts are located here:
https://github.com/microsoft/cpp_client_telemetry/blob/master/tools/build-nugets.cmd
https://github.com/microsoft/cpp_client_telemetry/tree/master/tools/NuGet/uap

We are not using these scripts for a while now. But it might be something that we can restart doing, including both Xamarin for other OS + Windows 10 C++/CX projection in one nuget. I think if we use GitHub Actions cache or some external storage account, we might be able to aggregate different platform SDK bits into one C# nuget.

@bemartin

Copy link
Copy Markdown
Contributor Author

I like all of it. Could you please double-check that the test failures are not related to your commit?

I re-executed the tests and they are all green now. It seems to have been some transient issue. I did not see any issues when running the tests locally either

@maxgolov

Copy link
Copy Markdown
Contributor

bemartin - could you convert this to "Ready for Review"? I think it should be safe to add your change to the master.

@bemartin

Copy link
Copy Markdown
Contributor Author

When you integrate it - I'd like to compile it, get the assembly, then diff versus what we generate using C++/CX on Windows. If there are some significant differences, we can address these in a separate PR. BTW, old nuget packaging scripts are located here:
https://github.com/microsoft/cpp_client_telemetry/blob/master/tools/build-nugets.cmd
https://github.com/microsoft/cpp_client_telemetry/tree/master/tools/NuGet/uap

We are not using these scripts for a while now. But it might be something that we can restart doing, including both Xamarin for other OS + Windows 10 C++/CX projection in one nuget. I think if we use GitHub Actions cache or some external storage account, we might be able to aggregate different platform SDK bits into one C# nuget.

I will look into the nugets scripts a little bit later and see if I can revive this as a follow up PR. Is that ok?
I have a couple of additional changes to make but I am hoping to publish the PR today or tomorrow

@bemartin

Copy link
Copy Markdown
Contributor Author

bemartin - could you convert this to "Ready for Review"? I think it should be safe to add your change to the master.

I have to fix a couple of small things that I found while testing locally but I will publish it ASAP

@bemartin
bemartin marked this pull request as ready for review September 17, 2020 23:00
Comment thread build-xamarin.sh
@@ -0,0 +1,120 @@
#!/bin/sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a way we can call this script from build.sh, so that we have one single entry point? That may not be a hard requirement, but would be nice. I am thinking of this like, once we integrate the vcpkg install mstelemetry - we can also create a triplet that compiles with Xamarin / C# projeciton from there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we could but this script builds the C++ code, Obj-c and Java wrappers and then the Xamarin bindings. This will be adding about 20 minutes of build time and will require anybody using this script to have VS with Xamarin installed.
This script also requires running build on Mac since it's building Obj-c code.

Are we ok with those requirements/constraints?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not suggesting to do it every time, but maybe an option in the build.sh to trigger build-xamarin.sh as well. Maybe not in this PR, but after the merge of mine: #644 - then we can control in vcpkg install mstelemetry:x64-macos-csharp , for example, to build both - core and Xamarin.

@maxgolov

Copy link
Copy Markdown
Contributor

I would suggest we merge it-as is. All checks passed.

@bemartin bemartin changed the title [WIP] Xamarin.iOS and Xamarin.Android bindings Xamarin.iOS and Xamarin.Android bindings Sep 22, 2020
@bemartin
bemartin merged commit d945778 into master Sep 22, 2020
@maxgolov
Max Golovanov (maxgolov) deleted the bemartin/xamarin branch October 6, 2020 16:52
Max Golovanov (maxgolov) pushed a commit that referenced this pull request Oct 19, 2020
Xamarin.iOS and Xamarin.Android bindings
theta3 pushed a commit that referenced this pull request Jan 19, 2021
Xamarin.iOS and Xamarin.Android bindings
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.

3 participants