Add Gradle examples #1
Conversation
Signed-off-by: Andrew Druk <[email protected]>
|
Looks great to me! I say we merge as-is, and then we can iterate on it and flesh out the docs and add CI. Thoughts, @finagolfin, @Joannis? |
|
Hmm, lot of files here, anyone other than Andriy actually review it all? Unlikely to be a problem, but we should read it and make sure, at least so we can say we did. |
|
I've looked over it all and successfully run them all on my machine in Android Studio. They are nice, simple, and self-contained with no external dependencies other than the Swift SDK for Android. 
|
|
Thanks, Marc, merge when you want, I will read them all tomorrow. |
|
Can we update the screenshots with swift-andriod-examples? |
…the latest Android Studio
|
Thanks for putting together this pull request. We need to update the file headers with the license template. Also, I will most likely be able to provide more feedback later tonight or tomorrow. (I'm currently replying from my phone) |
|
Here's the updated screenshot with "swift-android-examples" (also fixed the package name to match that in the PR and cleaned up the window layouts a bit). hello-swift sample (Kotlin/Java calling into Swift)
hello-swift-callback sample (Swift calling back into Kotlin/Java)
native-activity (Java-less Swift+C+OpenGL example app)
|
|
I like the code shown from the native-activity sample the best, it is the cleanest Swift code. My vote is for putting that one in the blog post, even though the hello world one is the best emulator display capture. |
|
FTR, I prefer the hello-swift for the screenshot. Not only is the emulator screen nicer, it is showing 100% of what is going on. The native-activity sample, OTOH, is only showing a tiny fraction of what is needed to get a native activity up and running, and the sample delves into some really complex APIs (OpenGL, Choreographer, Looper). I also think that the majority of (non-game) Swift developers will be interfacing with Java, so the hello-swift is a more realistic starting point. |
There was a problem hiding this comment.
@andriydruk we can do without the gradle-wrapper.jar binary, right? It'll be downloaded automatically based on the gradle-wrapper.properties?
There was a problem hiding this comment.
No, the JAR file is required to bootstrap downloading Gradle using the information from the properties file. Gradle itself is usually downloaded to ~/.gradle
This JAR is just a bootstrap helper, and it’s generally recommended to keep it in the project
All official Android samples include it:
https://github.com/android/architecture-samples/tree/main/gradle/wrapper
There was a problem hiding this comment.
Yes, committing the gradle-wrapper jar is "the right way".
I've added license headers to all the *.swift, *.kt, *.c, and *.h files. My impression from other sample repositories (like https://github.com/swiftlang/swift-embedded-examples/tree/main) is that build scripts like |
|
If we wanted to give a bit more of a hint about what is going on, this screenshot adds a few arrow annotations. 
|
Yes, but does the new swift-java interop still require that devs write complicated Swift methods like this when bridging? If not, we are doing the swift-java people and ourselves a disservice by showing such code with older hand-written JNI bridging. |
Eventually it will be nicer (with The JNI sample is un-lovely, but it is a very stable interface and well-understood by many people who are already familiar with Android. And it's still nowhere near as complicated as the |
Nothing is being set "in stone," we are showing a screenshot of the Java interface we plan to have. That is reasonable to accompany a trunk snapshot SDK with.
I agree that we should emphasize that the new Java interop isn't settled yet.
It is downright ugly and one of the many reasons people avoid running AoT-compiled languages with Java or on Android. We would be shooting ourselves in the foot by even showing it, particularly since @ktoso and others' swift-java work is trying to abstract that away.
I guess it's in the eye of the beholder then. I think such simple Swift code that Andriy only translated from the NDK C++ sample to Swift is much nicer than the annotated JNI binding that you want us to show.
How would anybody know about the C glue used if it's not in the screenshot? Second, nobody writes that C glue themselves: that is a C wrapper provided with the Android NDK for more than a decade, that Andriy did not write and has simply copied over here for convenience. @andriydruk, can you please add the original license header back for any files like that which you simply copied over or translated? Since all google's examples are licensed under Apache 2.0 also, I don't see any issue with using them here, but we cannot simply strip out their original license info and copyright names and dates and replace them with ours. |
It is in the screenshot, in the I guess I don't care that much. I just think the hello-swift sample is much less intimidating. Maybe others can offer their input. |
|
Btw, I finally watched @madsodgaard's talk from a couple weeks ago on his work bridging Swift and Java through automatically generated JNI bindings. I highly recommend that you and everyone using Android watch it, it is only 25 mins and the approach looks great! 😄 If nothing else, watch the 2-3 min. live demo starting at the 33:10 mark, which required zero annotations. If we are on the verge of such a great automated Swift-Java interface, we would be foolish to show the older, hand-written code in these screenshots. Now, of course, caveats apply, ie all of Swift cannot be bridged yet; this is a new tool; annotations may still be needed in some places, like you say. But if he has a working sample app that works so cleanly with our Android SDK, we should show that. I have asked him to submit that sample app's source here and a screenshot, like in the video demo, both of which he has agreed to. |
|
@finagolfin I restored the license headers for two files I copied from Google samples |
|
Hi there, was just added to this repo along with @madsodgaard. Thanks for the work here already, and looking forward to integrate Mads's work! Very much agreed that there isn't much point in showing painful integration and cdecls etc. swift-java is "the" way to integrate those languages and the purpose of demos like this should also be to show off how cool/good/useful Swift is for such tasks -- in other words, showing that it's simple to pull off and that the resulting integration is nice to work with. As far as the "not complete" status of swift-java, well neither is android+swift "final" right now, so it's all in the same boat -- polishing up and making sure it all can work properly. |
| [plugins] | ||
| android-application = { id = "com.android.application", version.ref = "agp" } | ||
| kotlin-android = { id = "org.jetbrains.kotlin.android", version.ref = "kotlin" } | ||
| android-library = { id = "com.android.library", version.ref = "agp" } |
There was a problem hiding this comment.
Important note that we will have to prepare an https://github.com/swiftlang/swift-embedded-examples/blob/main/harmony/ACKNOWLEDGEMENTS.md file that covers all dependencies we pulled in
|
Anyone object to this PR getting merged? We can always add more examples in future PRs, and the screenshots discussion is orthogonal to the content of this PR. |
|
Let me review the code, so we can say more than one person did it, 😉 and I will merge this. |
|
@finagolfin do you have any additional feedback? Otherwise, we should merge this PR. Thanks @andriydruk! |
There was a problem hiding this comment.
We chatted about this earlier, but adding an explicit review then -- I don't think it's ok to be showing unsafe interop, and the example should take work from PR #2 so that we're delivering a consistent message about Swift and language interop.
| import Android | ||
|
|
||
| @_cdecl("Java_org_example_swiftlibrary_SwiftLibrary_stringFromSwift") | ||
| public func SwiftLibrary_stringFromSwift(env: UnsafeMutablePointer<JNIEnv?>, clazz: jclass) -> jstring { |
There was a problem hiding this comment.
IMHO it is not ok to be showing raw JNI like this as "the official example" when we're so heavily investing in swift-java interop. Sure, not everything is complete yet, but neither is the Android support. We should collaborate between those two initiatives, and an official example using the unsafe and boilerplate heavy ways to interop rather than our own interop effort undermine Swift's value propositio.
Instead, this should either be replaced by, or take inspiration from, the Example from the pull request over here #2
There was a problem hiding this comment.
I think it's fine, because as Marc says, some will want to see an example of the raw JNI. I don't think this undermines the automated swift-java work, rather it showcases how much better that is, by demonstrating this low-level method also. 😃
I disagree. There's a lot of value in these examples, even if they wind up not being the recommended and official way that Java interoperability. Being able to see all parts of the code in the small dependency-free In addition, other samples in this PR, like I believe we should merge this and then augment it with #2 when it is ready. The README can then guide developers towards our official recommendation, and the remaining samples will continue to be useful as references to other ways of doing things. |
|
This PR still puts the example specific stuff in the root directory, like gradle files. We discussed on Slack that we should maybe separate this into its own subdirectory? This is what PR #2 does, otherwise it might seem confusing that there are these gradle files in the root directory, that some of the examples does not use/need? |
|
@andriydruk, you have copyright on all images and files here, other than clearly marked open-source files like native-app-glue? If so, I'm fine with merging and we can reorganize as we go. |
|
I'm not entirely sold on the example given our focus on safety and based on my experience introducing folks to Swift -- they often end up pursuing the "wrong example" and then complain, but agreed that many examples are useful. Let's please follow up with reorganization so that Mad's example can be integrated nicely side-by-side such that all examples are similar to access/build. |
Agree that we should amend the doc once Mads's example is in to strongly push people to that approach. |
|
@finagolfin all images and other resources were generated using the Android Studio new project template |
|
@andriydruk, thanks for confirming, as I'm sure you know, we simply have to be careful about such issues. |
6 participants
No description provided.