Kotlin Native support

[sebleclerc]

Hi there!
I'm really looking forward to use pbandk for my Kotlin Multiplatform project, but it's coming short of Kotlin Native support. I'm willing to dig into this but first I wanted to know if any of you (contributors) have any tip or advance regarding what will be required to implement? Or anything I that could be helpful to me?

Thanks!

[garyp]

Hi @sebleclerc. We don't have any immediate need for Kotlin Native support internally at Streem, so if you wanted to contribute it that would be very appreciated. I'd suggest you take a look at the existing PR for Kotlin Native support that was done against the cretz/pbandk repo (which this project forked from): cretz/pb-and-k#15. I haven't yet looked at it in depth, so I'm not sure if it's usable as-is or needs more work. But probably should be a good starting point for you either way.

A few other tips:

• @cretz had wanted to make a set of "pure" projects in that previous PR (pure kotlin dependency-free Marshaller/Unmarshaller and native cretz/pb-and-k#15 (comment)). I'm not against that approach but I also don't think it's a requirement to get something merged. So it's up to you if you want to do the "pure" projects or not.
• It'd be great if you can include running the conformance-native tests under CI as part of this PR. See https://github.com/streem/pbandk/blob/master/.circleci/config.yml and https://github.com/streem/pbandk/blob/master/conformance/test-conformance.sh for more details.
• Something to keep in mind is that this project is still using the old kotlin-native-gradle-plugin instead of the more recent kotlin-multiplatform plugin. Hopefully that's not an impediment to adding Kotlin Native support. But if you find you do need functionality only available in the newer kotlin-multiplatform plugin, we can probably get pbandk updated to use kotlin-multiplatform. It just hasn't been a pressing need so far.

[sebleclerc]

@garyp Native support is essential for our usage of KMP, so that's why I want to contribute.

Thanks for pointing that PR! For sure I'll have a look and probably base a few things on this. I like the idea of the Pure Kotlin approach. I will evaluate it. Keeping the way it is will limit the number of changed files.

For sure I will want to add the conformance-native to the CI. Shouldn't be that hard.

As per the Gradle plugin, do you know how much work it involves and the time required? If it's not that much of a deal, it can be really interesting to move forward with newer ways of KMP. For sure the sooner it's done, the less work it involves 😄

[garyp]

As per the Gradle plugin, do you know how much work it involves and the time required? If it's not that much of a deal, it can be really interesting to move forward with newer ways of KMP. For sure the sooner it's done, the less work it involves 😄

@sebleclerc We haven't looked yet at the effort involved to switch to the new gradle plugin. But I have played with it on other KMP projects and it doesn't seem like it'd be a huge effort. I'm pretty busy with other projects until around late April. But if you're ready to work on Kotlin Native support and this is a blocker for you, I'll find some time to update to the new kotlin-multiplatform gradle plugin. I want to encourage more contributors to pbandk 😄

[sebleclerc]

@garyp Alright! Thanks, I’ll keep you posted.

[sebleclerc]

@garyp I did got the PR into my fork. I ran current conformance to make sure everything was working and I had 3 tests that weren't:

Can you confirm that these are also not working on your side ? Thanks

[sebleclerc]

@garyp Also, I might be blocked by the multiplatform plugin. Not sure ... but when I try to build the :runtime:runtime-native project, it fails because it can't find any kotlinx serialization references. I did add a reference to kotlinx serialization inside the runtime-native project, but nothing. Anything that can be of help? Thanks!

This is my build.gradle for now

buildscript {
    ext.kotlin_version = '1.3.61'
    ext.kotlin_serialization_version = '0.14.0'
    ext.kotlin_native_gradle_plugin_version = '1.3.41'
    ext.spring_boot_gradle_plugin_version = '2.1.7.RELEASE'

    repositories {
        google()
        jcenter()
        maven {
            url 'https://dl.bintray.com/jetbrains/kotlin-native-dependencies'
        }
        maven {
            url 'https://plugins.gradle.org/m2/'
        }
    }
    dependencies {
        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
        classpath "org.jetbrains.kotlin:kotlin-serialization:$kotlin_version"
        classpath "org.jetbrains.kotlin:kotlin-native-gradle-plugin:$kotlin_native_gradle_plugin_version"
        classpath "org.springframework.boot:spring-boot-gradle-plugin:$spring_boot_gradle_plugin_version"
    }
}

allprojects {
    group 'com.github.streem.pbandk'
    version '0.8.0'

    repositories {
        google()
        jcenter()
    }
}

project(':runtime:runtime-common') {
    apply plugin: 'kotlin-platform-common'
    apply plugin: 'kotlinx-serialization'

    dependencies {
        compile "org.jetbrains.kotlin:kotlin-stdlib-common:$kotlin_version"
        compile "org.jetbrains.kotlinx:kotlinx-serialization-runtime-common:$kotlin_serialization_version"
        testCompile "org.jetbrains.kotlin:kotlin-test-annotations-common:$kotlin_version"
        testCompile "org.jetbrains.kotlin:kotlin-test-common:$kotlin_version"
    }

    task generateWellKnownTypes {
        dependsOn ':protoc-gen-kotlin:protoc-gen-kotlin-jvm:installDist'
        doFirst {
            def protocPath = System.getProperty('protoc.path')
            if (protocPath == null) throw new InvalidUserDataException('System property protoc.path must be set')
            runProtoGen(Paths.get(protocPath, 'include').toString(), 'src/main/kotlin', 'pbandk.wkt', 'debug', 'google/protobuf')
        }
    }

    archivesBaseName = 'pbandk-runtime-common'
    publishSettings(project, archivesBaseName, 'Common library for pbandk protobuf code')
}

project(':runtime:runtime-js') {
    apply plugin: 'kotlin-platform-js'
    apply plugin: 'kotlinx-serialization'

    dependencies {
        expectedBy project(':runtime:runtime-common')
        compile "org.jetbrains.kotlin:kotlin-stdlib-js:$kotlin_version"
        compile "org.jetbrains.kotlinx:kotlinx-serialization-runtime-js:$kotlin_serialization_version"
        testCompile "org.jetbrains.kotlin:kotlin-test-js:$kotlin_version"
    }
    compileKotlin2Js {
        kotlinOptions.moduleKind = 'commonjs'
    }
    archivesBaseName = 'pbandk-runtime-js'
    publishSettings(project, archivesBaseName, 'JS library for pbandk protobuf code')
}

project(':runtime:runtime-jvm') {
    apply plugin: 'kotlin-platform-jvm'
    apply plugin: 'kotlinx-serialization'
    sourceCompatibility = '1.6'
    compileKotlin {
        kotlinOptions.jvmTarget = '1.6'
    }
    compileTestKotlin {
        kotlinOptions.jvmTarget = '1.6'
    }
    dependencies {
        expectedBy project(':runtime:runtime-common')
        compile "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version"
        compile "org.jetbrains.kotlinx:kotlinx-serialization-runtime:$kotlin_serialization_version"
        compile 'com.google.protobuf:protobuf-java:3.11.1'
        testCompile 'junit:junit:4.12'
        testCompile "org.jetbrains.kotlin:kotlin-test-junit:$kotlin_version"
        testCompile "org.jetbrains.kotlin:kotlin-test:$kotlin_version"
    }
    test {
        testLogging {
            outputs.upToDateWhen {false}
            showStandardStreams = true
            exceptionFormat = 'full'
            events 'passed', 'skipped', 'failed'
        }
    }

    task generateTestTypes {
        dependsOn ':protoc-gen-kotlin:protoc-gen-kotlin-jvm:installDist'
        doFirst {
            runProtoGen('src/test/proto', 'src/test/kotlin', 'pbandk.testpb', 'debug', 'pbandk/testpb')
        }
        doFirst {
            exec {
                commandLine('protoc', '-Isrc/test/proto', '--java_out=src/test/java', 'src/test/proto/pbandk/testpb/test.proto')
            }
        }
    }

    archivesBaseName = 'pbandk-runtime-jvm'
    publishSettings(project, archivesBaseName, 'JVM library for pbandk protobuf code')
}

project(':runtime:runtime-native') {
    apply plugin: 'konan'
    apply plugin: 'kotlinx-serialization'

    konanArtifacts {
        library('pbandk') {
            enableMultiplatform true
        }
    }

    dependencies {
        expectedBy project(':runtime:runtime-common')
        compile "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version"
        compile "org.jetbrains.kotlinx:kotlinx-serialization-runtime:$kotlin_serialization_version"
    }

    archivesBaseName = 'pbandk-runtime-native'
    publishSettings(project, archivesBaseName, 'Native library for pbandk protobuf code')
}

project(':protoc-gen-kotlin:protoc-gen-kotlin-common') {
    apply plugin: 'kotlin-platform-common'
    apply plugin: 'kotlinx-serialization'

    dependencies {
        compile project(':runtime:runtime-common')
        compile "org.jetbrains.kotlin:kotlin-stdlib-common:$kotlin_version"
        compile "org.jetbrains.kotlinx:kotlinx-serialization-runtime-common:$kotlin_serialization_version"
        testCompile "org.jetbrains.kotlin:kotlin-test-annotations-common:$kotlin_version"
        testCompile "org.jetbrains.kotlin:kotlin-test-common:$kotlin_version"
    }

    task generateProto {
        dependsOn ':protoc-gen-kotlin:protoc-gen-kotlin-jvm:installDist'
        doFirst {
            runProtoGen('src/main/proto', 'src/main/kotlin', 'pbandk.gen.pb', 'debug')
        }
    }

    archivesBaseName = 'protoc-gen-kotlin-common'
    publishSettings(project, archivesBaseName, 'Common library for pbandk protobuf code generator')
}

project(':protoc-gen-kotlin:protoc-gen-kotlin-jvm') {
    apply plugin: 'kotlin-platform-jvm'
    apply plugin: 'kotlinx-serialization'
    apply plugin: 'application'
    apply plugin: 'org.springframework.boot'

    sourceCompatibility = '1.8'
    compileKotlin {
        kotlinOptions.jvmTarget = '1.8'
    }
    compileTestKotlin {
        kotlinOptions.jvmTarget = '1.8'
    }
    mainClassName = 'pbandk.gen.MainKt'
    applicationName = 'protoc-gen-kotlin'
    dependencies {
        expectedBy project(':protoc-gen-kotlin:protoc-gen-kotlin-common')
        compile project(':runtime:runtime-jvm')
        compile "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlin_version"
        compile "org.jetbrains.kotlinx:kotlinx-serialization-runtime:$kotlin_serialization_version"
        testCompile 'junit:junit:4.12'
        testCompile "org.jetbrains.kotlin:kotlin-test-junit:$kotlin_version"
        testCompile "org.jetbrains.kotlin:kotlin-test:$kotlin_version"
    }
    test {
        testLogging {
            outputs.upToDateWhen {false}
            showStandardStreams = true
            exceptionFormat = 'full'
            events 'passed', 'skipped', 'failed'
        }
    }
    jar {
        enabled = true
    }
    bootJar {
        classifier = 'jvm8'
        launchScript()
    }
    configurations.archives.artifacts.removeIf { it.name ==~ /.*-boot/ && it.type ==~ /zip|tar/ }
    artifacts {
        archives bootJar
    }

    archivesBaseName = 'protoc-gen-kotlin-jvm'
    publishSettings(project, archivesBaseName, 'JVM library for pbandk protobuf code generator')
}

/*
project(':protoc-gen-kotlin:protoc-gen-kotlin-native') {
    apply plugin: 'konan'
    konanArtifacts {
        program('protoc-gen-kotlin-native') {
            enableMultiplatform true
            libraries {
                allLibrariesFrom project(':runtime:runtime-native')
            }
        }
    }
    dependencies {
        expectedBy project(':protoc-gen-kotlin:protoc-gen-kotlin-common')
    }
}
*/

project(':conformance:conformance-common') {
    apply plugin: 'kotlin-platform-common'
    apply plugin: 'kotlinx-serialization'
    dependencies {
        compile project(':runtime:runtime-common')
        compile "org.jetbrains.kotlin:kotlin-stdlib-common:$kotlin_version"
        compile "org.jetbrains.kotlinx:kotlinx-serialization-runtime-common:$kotlin_serialization_version"
        testCompile "org.jetbrains.kotlin:kotlin-test-annotations-common:$kotlin_version"
        testCompile "org.jetbrains.kotlin:kotlin-test-common:$kotlin_version"
    }

    task generateProto {
        dependsOn ':protoc-gen-kotlin:protoc-gen-kotlin-jvm:installDist'
        doFirst {
            runProtoGen('src/main/proto', 'src/main/kotlin', 'pbandk.conformance.pb', 'debug')
        }
    }
}

project(':conformance:conformance-js') {
    apply plugin: 'kotlin-platform-js'
    apply plugin: 'kotlinx-serialization'
    dependencies {
        expectedBy project(':conformance:conformance-common')
        compile project(':runtime:runtime-js')
        compile "org.jetbrains.kotlin:kotlin-stdlib-js:$kotlin_version"
        compile "org.jetbrains.kotlinx:kotlinx-serialization-runtime-js:$kotlin_serialization_version"
        testCompile "org.jetbrains.kotlin:kotlin-test-js:$kotlin_version"
    }
    compileKotlin2Js {
        kotlinOptions.moduleKind = 'commonjs'
    }
}

project(':conformance:conformance-jvm') {
    apply plugin: 'kotlin-platform-jvm'
    apply plugin: 'kotlinx-serialization'
    apply plugin: 'application'
    sourceCompatibility = '1.8'
    compileKotlin {
        kotlinOptions.jvmTarget = '1.8'
    }
    compileTestKotlin {
        kotlinOptions.jvmTarget = '1.8'
    }
    mainClassName = 'pbandk.conformance.MainKt'
    dependencies {
        expectedBy project(':conformance:conformance-common')
        compile project(':runtime:runtime-jvm')
        compile "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlin_version"
        compile "org.jetbrains.kotlinx:kotlinx-serialization-runtime:$kotlin_serialization_version"
        testCompile 'junit:junit:4.12'
        testCompile "org.jetbrains.kotlin:kotlin-test-junit:$kotlin_version"
        testCompile "org.jetbrains.kotlin:kotlin-test:$kotlin_version"
    }
    test {
        testLogging {
            outputs.upToDateWhen {false}
            showStandardStreams = true
            exceptionFormat = 'full'
            events 'passed', 'skipped', 'failed'
        }
    }
}

project(':conformance:conformance-native') {
    apply plugin: 'konan'
    apply plugin: 'kotlinx-serialization'

    konanArtifacts {
        program('pbandk-conformance') {
            enableMultiplatform true
            libraries {
                allLibrariesFrom project(':runtime:runtime-native')
            }
        }
    }
    dependencies {
        expectedBy project(':conformance:conformance-common')
    }
}

import java.nio.file.Paths
allprojects {
    ext.runProtoGen = { inPath, outPath, kotlinPackage = null, logLevel = null, inSubPath = null ->
        // Build CLI args
        def args = ['protoc']
        args << '--kotlin_out='
        if (kotlinPackage != null) args[-1] += "kotlin_package=$kotlinPackage,"
        if (logLevel != null) args[-1] += "log=$logLevel,"
        args[-1] += 'json_use_proto_names=true,'
        args[-1] += 'empty_arg:' + Paths.get(outPath)
        args << '--plugin=protoc-gen-kotlin=' +
            Paths.get(project.rootDir.toString(), 'protoc-gen-kotlin/protoc-gen-kotlin-jvm/build/install/protoc-gen-kotlin/bin/protoc-gen-kotlin')
        if (System.properties['os.name'].toLowerCase().contains('windows')) args[-1] += '.bat'
        def includePath = Paths.get(inPath)
        if (!includePath.absolute) includePath = Paths.get(project.projectDir.toString(), inPath)
        args << '-I' << includePath
        def filePath = includePath
        if (inSubPath != null) filePath = includePath.resolve(inSubPath)
        args += filePath.toFile().listFiles().findAll {
            it.isFile() && it.toString().endsWith('.proto')
        }
        // Run it
        exec { commandLine(*args) }
    }
}

def publishSettings(project, projectName, projectDescription) {
    project.with {
        if (!project.hasProperty('ossrhUsername')) return
        apply plugin: 'maven'
        apply plugin: 'signing'

        task packageSources(type: Jar) {
            classifier = 'sources'
            from sourceSets.main.allSource
            if (project.name.endsWith('-jvm') || project.name.endsWith('-js')) {
                duplicatesStrategy = 'exclude'
                def commonProject = project.parent.subprojects.find { it.name.endsWith('-common') }
                from(sourceSets.main.allSource + commonProject.sourceSets.main.allSource)
            }
        }

        task packageJavadoc(type: Jar) {
            // Empty to satisfy Sonatype's javadoc.jar requirement
            classifier 'javadoc'
        }

        artifacts {
            archives packageSources, packageJavadoc
        }

        signing {
            sign configurations.archives
        }

        uploadArchives {
            repositories {
                mavenDeployer {
                    beforeDeployment { MavenDeployment deployment -> signing.signPom(deployment) }
                    repository(url: 'https://oss.sonatype.org/service/local/staging/deploy/maven2/') {
                        authentication(userName: ossrhUsername, password: ossrhPassword)
                    }
                    snapshotRepository(url: 'https://oss.sonatype.org/content/repositories/snapshots/') {
                        authentication(userName: ossrhUsername, password: ossrhPassword)
                    }
                    pom.project {
                        name projectName
                        packaging 'jar'
                        description projectDescription
                        url 'https://github.com/streem/pbandk'
                        scm {
                            connection 'scm:git:[email protected]:streem/pbandk.git'
                            developerConnection 'scm:git:[email protected]:streem/pbandk.git'
                            url '[email protected]:streem/pbandk.git'
                        }
                        licenses {
                            license {
                                name 'MIT License'
                                url 'https://opensource.org/licenses/MIT'
                            }
                        }
                        developers {
                            developer {
                                id 'streem'
                                name 'Streem, Inc.'
                                url 'https://github.com/streem'
                            }
                        }
                    }
                }
            }
        }
    }
}

[garyp]

@garyp I did got the PR into my fork. I ran current conformance to make sure everything was working and I had 3 tests that weren't:

Can you confirm that these are also not working on your side ? Thanks

If I run the conformance tests using conformance/test-conformance.sh from the master branch of pbandk, all of the tests pass. Were you running this in a clean checkout or was this the result after the Kotlin/Native changes were added?

[garyp]

@sebleclerc Regarding the kotlinx-serialization error you're getting, your dependency should be on org.jetbrains.kotlinx:kotlinx-serialization-runtime-native rather than org.jetbrains.kotlinx:kotlinx-serialization-runtime in the :runtime:runtime-native subproject.

Also, you might need to update pbandk to a more recent gradle version. The kotlinx-serialization README says:

For Native artifact, Gradle metadata is required (put the line enableFeaturePreview('GRADLE_METADATA') in your gradle.properties) and minimal supported version of Gradle is 5.3.

[sebleclerc]

If I run the conformance tests using conformance/test-conformance.sh from the master branch of pbandk, all of the tests pass. Were you running this in a clean checkout or was this the result after the Kotlin/Native changes were added?

I tried with both. I'll try and clone from the streem repo. But one thing I just saw if that a compile from runtime-jvm is using version 3.11.1 of Protobuf and I did my tests from the latest 3.11.4. Maybe it has something to do with it?

[sebleclerc]

Also, I did:

• Update gradle version to 5.3 ✅
• Add GRADLE_METADATA ✅
• Update the compile to use runtime... it didn't worked at first because of this error:

Build file '/Users/seb/Develop/studyo-pbandk/build.gradle' line: 132

A problem occurred evaluating root project 'pbandk'.
> Could not find method compile() for arguments [org.jetbrains.kotlinx:kotlinx-serialization-runtime-native:0.14.0] on object of type org.gradle.api.internal.artifacts.dsl.dependencies.DefaultDependencyHandler.

Those dependencies just work fine inside runtime-js and runtime-jvm but can't get them to work in the native one... 🤔Still think it can have something to do with the multiplatform version/plugin/settings

[garyp]

But one thing I just saw if that a compile from runtime-jvm is using version 3.11.1 of Protobuf and I did my tests from the latest 3.11.4. Maybe it has something to do with it?

Yes, a newer version of protobuf can definitely report new failures since protobuf is constantly expanding the conformance tests with every release. That's why the pbandk instructions for running the conformance tests (https://github.com/streem/pbandk/blob/master/README.md#conformance-tests) reference a specific version of protobuf: 3.10.1.

I'll also try running the tests with 3.11.4 and see if I get these new failures. If so, then these are probably conformance bugs we'll want to fix.

[sebleclerc]

I did bump the project to gradle 5.3 but since then I can't build ( and installDist for conformance test) the project.

I did a test on a fresh clone from the main streem repo and I have the same error when syncing with gradle:
Could not resolve: project :runtime:runtime-common

[sebleclerc]

From what I read here ( link ) this is the new way of setting a project for Kotlin multiplatform. We define project where they each have their build.gradle with a root one too. And we define source sets for each with common code with platform specific and dependencies for each.

Is that what you meant @garyp when you talked about updating the multiplatform plugin? I don't mind digging into this if this will help me with the native support of pbandk. Also, if this is the case, do you mind moving on to using build.gradle.kts files ?

[garyp]

@sebleclerc I didn't quite understand what that link was talking about. When I was referring to updating the multiplatform plugin, I was talking about setting up the project to follow the structure described at https://kotlinlang.org/docs/reference/building-mpp-with-gradle.html. If you want to dig into updating pbandk to use that new structure (and plugin), we'll gladly accept a PR.

Also, if this is the case, do you mind moving on to using build.gradle.kts files ?

Sounds great 👍

[garyp]

@sebleclerc I was able to get everything building (haven't tried actually running this code yet...) with a newer gradle version. First I ran ./gradlew wrapper --gradle-version=5.6.4 to update the version (5.6.4 is the latest release in the 5.x series). Then I made this slight patch to build.gradle:

diff --git a/build.gradle b/build.gradle
index 56e43cc..78828b5 100644
--- a/build.gradle
+++ b/build.gradle
@@ -72,12 +72,12 @@ project(':runtime:runtime-js') {
 project(':runtime:runtime-jvm') {
     apply plugin: 'kotlin-platform-jvm'
     apply plugin: 'kotlinx-serialization'
-    sourceCompatibility = '1.6'
+    sourceCompatibility = '1.8'
     compileKotlin {
-        kotlinOptions.jvmTarget = '1.6'
+        kotlinOptions.jvmTarget = '1.8'
     }
     compileTestKotlin {
-        kotlinOptions.jvmTarget = '1.6'
+        kotlinOptions.jvmTarget = '1.8'
     }
     dependencies {
         expectedBy project(':runtime:runtime-common')

I think we should be ok updating the :runtime:runtime-jvm project to Java 8. From what I understand it was using Java 6 to maintain Android support. But as long as an Android project is using a recent-enough version of the Android gradle plugin (https://developer.android.com/studio/write/java8-support), it should be able to use pbandk even if pbandk was compiled against Java 8. I don't see a need to support ancient Android gradle plugin versions.

[garyp]

I was also able to get everything to build (again, haven't tried running the code) with gradle 6.2.2 (the latest release in the 6.x series). If you're going to be changing the entire gradle project structure, you might as well update to gradle 6.2.2.

[sebleclerc]

That is exactly what I’m doing right now. I’m fairly advanced right now. I hope to have a PR in the next day or 2

[sebleclerc]

Oh and after updating Gradle I forgot to update wrapper 🤦‍♂️ that might be why it wasn’t working

[sebleclerc]

You can see PR #29