chore: bootstrap CRT build for native platforms #91

aajtodd · 2024-02-09T17:46:03Z

Description of changes:

This PR bootstraps building CRT for native platforms via CMake (which is what CRT uses to build with). It also sets up generating the cinterop for Kotlin using the CRT header files and proves everything links.

Each K/N target gets it's own CMake build and install dir under the root build dir:

build/
    cmake-build/
        <target-name>/            -- e.g. linuxX64
    crt-libs/
        <target-name>/            -- e.g. linuxX64

This is a good stopping point as we have a proof of concept build to iterate on. I expect there to be bugs and issues with where we are at right now. I've setup kn-main as the integration branch to merge this work and all future work into until such a time we are ready to release K/N support.

Summary

Adds aws-c-* CRT submodules under crt/
Adds a CMake build CMakeLists.txt for building the CRT components.
- NOTE: This is mostly boilerplate taken from aws-crt-ffi and the original one from the bootstrap-kn branch.
Adds docker images based on dockcross for building Linux target(s).
- NOTE: The dockcross-<target> scripts are auto generated!
- These images are really thin wrappers around dockcross. This is done for two reasons (1) I expect we'll want to publish our own images to ECR at some point and (2) if we need to customize anything about the images we are setup to do so.
Adds custom build-support plugin to abstract some of the build logic around invoking CMake for the various targets.
Adds a C interop file for CRT.
I verified we can build CRT for linuxX64 on macos via docker as well as spot testing iosArm64, iosSimulatorArm64, and iosX64 (simulator on X64 arch, unfortunately/confusingly it wasn't natmed iosSimulatorX64 😞). I also ran tests for iosX64 simulator and can see they fail due to unimplemented methods/tests.

Open Questions and Follow on Work

Management of docker images in CI/release vs local build? Currently you have to build and tag the docker images locally as a prerequisite to building (e.g. ./docker-images/build-all.sh)
Moving target enablement to our aws-kotlin-repo-tools build plugin. This may require making that project a composite build for the time being and/or publishing versions that only aws-crt-kotlin uses (since we can't cut and consume a release that actually enables targets that aws-sdk-kotlin and smithy-kotlin don't yet support).
Min ios/tvos/watchos targets. I asked the Swift team and they had some good feedback but also said they may bump the versions they use before an eventual GA.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

ianbotsf · 2024-02-09T18:07:54Z

.gitmodules

+[submodule "crt/aws-c-common"]
+	path = crt/aws-c-common
+	url = https://github.com/awslabs/aws-c-common
+[submodule "crt/aws-c-auth"]
+	path = crt/aws-c-auth
+	url = https://github.com/awslabs/aws-c-auth
+[submodule "crt/aws-c-cal"]
+	path = crt/aws-c-cal
+	url = https://github.com/awslabs/aws-c-cal
+[submodule "crt/aws-c-compression"]
+	path = crt/aws-c-compression
+	url = https://github.com/awslabs/aws-c-compression
+[submodule "crt/aws-c-http"]
+	path = crt/aws-c-http
+	url = https://github.com/awslabs/aws-c-http
+[submodule "crt/aws-c-io"]
+	path = crt/aws-c-io
+	url = https://github.com/awslabs/aws-c-io
+[submodule "crt/aws-checksums"]
+	path = crt/aws-checksums
+	url = https://github.com/awslabs/aws-checksums
+[submodule "crt/aws-c-mqtt"]
+	path = crt/aws-c-mqtt
+	url = https://github.com/awslabs/aws-c-mqtt
+[submodule "crt/s2n"]
+	path = crt/s2n
+	url = https://github.com/aws/s2n-tls
+[submodule "crt/aws-lc"]
+	path = crt/aws-lc
+	url = https://github.com/aws/aws-lc
+[submodule "crt/aws-c-sdkutils"]
+	path = crt/aws-c-sdkutils
+	url = https://github.com/awslabs/aws-c-sdkutils


Question: Does the addition of submodules here mean they need to be kept in sync with the versions referenced in aws-crt-java?

I don't think they have to be kept in sync necessarily since their usage is independent. We probably don't want them to drift too much but I don't think I'm overly concerned at the moment if they differ.

CMakeLists.txt

README.md

ianbotsf · 2024-02-09T18:21:11Z

aws-crt-kotlin/build.gradle.kts

+    // create a single "umbrella" cinterop will all the aws-c-* API's we want to consume
+    // see: https://github.com/JetBrains/kotlin-native/issues/2423#issuecomment-466300153
+    targets.withType<KotlinNativeTarget> {


Question: I don't quite understand the referenced comment or how it applies to this interop config. Why is a single "umbrella" cinterop better than individual cinterops per library?

I don't recall exactly since it's been years since this comment was written but I do remember something around if you tried to generate separate cinterops there were issues with the types being able to be passed back and forth (which matters to us since these libraries all actually re-use parts from one another). At any rate we don't lose anything going this route and we don't gain anything from what I can see trying to create a separate interop definition per/CRT module.

ianbotsf · 2024-02-09T18:32:25Z

README.md

 ## Building

-Kotlin Multiplatform projects are in [Alpha](https://kotlinlang.org/docs/reference/evolution/components-stability.html). The CRT interfaces are subject to change.
+CRT interfaces are subject to change.



Suggestion: We should add a note to the readme about needing to fetch/update submodules.

Yeah this is all a WIP right now as I figure out the build instructions for local vs CI.

ianbotsf · 2024-02-09T18:35:48Z

README.md

-Kotlin Multiplatform projects are in [Alpha](https://kotlinlang.org/docs/reference/evolution/components-stability.html). The CRT interfaces are subject to change.
+CRT interfaces are subject to change.

 ### Linux/Unix


Question: What's the updated procedure for building on Linux now? When building on my dev machine running AL2, I see the following error:

> Task :aws-crt-kotlin:cmakeBuildIosArm64 FAILED cmake --build build/cmake-build/iosArm64 --config RelWithDebInfo -- -sdk iphoneos GNU Make 3.82 Built for x86_64-koji-linux-gnu Copyright (C) 2010 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Reading makefiles... Reading makefile `Makefile'... Updating makefiles.... Considering target file `Makefile'. Looking for an implicit rule for `Makefile'. No implicit rule found for `Makefile'. Finished prerequisites of target file `Makefile'. gmake: *** No rule to make target `iphoneos'. No need to remake target `Makefile'. Updating goal targets.... Considering target file `iphoneos'. File `iphoneos' does not exist. Looking for an implicit rule for `iphoneos'. No implicit rule found for `iphoneos'. Finished prerequisites of target file `iphoneos'. Must remake target `iphoneos'. Failed to remake target file `iphoneos'. FAILURE: Build failed with an exception.

Similar question for Windows.

Yeah this is failing in CI for same reason. Kotlin plugin disables targets that can't build on a given host but it seems they don't disable the cinterop tasks even though that target won't be built. I imagine we'll just have to do the same and only enable the cmake tasks if a given target is enabled.

ianbotsf · 2024-02-09T18:43:06Z

build-support/src/main/kotlin/aws/sdk/kotlin/gradle/crt/CMakeTasks.kt

+internal fun Project.registerCmakeConfigureTask(
+    knTarget: KotlinNativeTarget,
+    buildType: CMakeBuildType,
+): TaskProvider<Task> {


Question: Why are registerCmakeConfigureTask, registerCmakeBuildTask, registerCmakeInstallTask, and runCmake marked as internal? They seem to be used only in this file and thus feel like they could be private.

sure we can make them private, I was probably just toying around with how to structure this 🤷

ianbotsf · 2024-02-09T18:46:52Z

build-support/src/main/kotlin/aws/sdk/kotlin/gradle/crt/CMakeTasks.kt

+        project.logger.info("$exeName ${exeArgs.joinToString(separator = " ")}")
+        // FIXME - remove
+        println("$exeName ${exeArgs.joinToString(separator = " ")}")


Question: Can this FIXME be addressed now? Or is the redundant println still needed for some reason?

it can be removed now I was debugging something and didn't want the verbosity of --info.

…n host

lauzadis · 2024-02-12T14:35:02Z

CMakeLists.txt

+    add_subdirectory(crt/aws-c-sdkutils)
+    add_subdirectory(crt/aws-c-io)
+    add_subdirectory(crt/aws-c-cal)
+    add_subdirectory(crt/aws-c-compression)
+    add_subdirectory(crt/aws-c-http)
+    add_subdirectory(crt/aws-c-auth)
+    add_subdirectory(crt/aws-checksums)


question: aws-c-mqtt is not added here, is that intentional?

We don't use it yet so yeah, if/when we bind it we can add it.

sonarqubecloud · 2024-02-12T19:13:27Z

Quality Gate passed

Issues
4 New issues

Measures
2 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

aajtodd · 2024-02-13T14:03:55Z

I'm going to merge this as is and address CI in a follow on PR

aajtodd added 8 commits February 7, 2024 10:58

add crt submodules

723da7f

add cmake build for CRT

9acdfd9

proof of concept build

40e98b8

fix build and add install

4a94449

add cinterop tasks and fix simulator sdk install task

6aee202

fix test on ios and cmake up-to-date checks

3f32d35

dont lint or format crt submodules

379b5a9

cleanup readme

297c845

aajtodd requested a review from a team as a code owner February 9, 2024 17:46

ianbotsf reviewed Feb 9, 2024

View reviewed changes

aajtodd added 2 commits February 12, 2024 11:31

feedback

034b04c

only enable cmake tasks if the target is actually enabled on the give…

d80121b

…n host

lauzadis approved these changes Feb 12, 2024

View reviewed changes

aajtodd added 2 commits February 12, 2024 12:40

first stab at simplifying CI and using new docker images

4af6e55

debug docker ci

d1b1f8f

ianbotsf approved these changes Feb 12, 2024

View reviewed changes

aajtodd added 3 commits February 12, 2024 13:33

try same job

f5c464f

try no pull

0548a1b

cleanup and implement caching improvements

4d305b8

aajtodd merged commit bbc7cf1 into kn-main Feb 13, 2024

aajtodd deleted the kn-bootstrap-build branch February 13, 2024 14:04

chore: bootstrap CRT build for native platforms #91

chore: bootstrap CRT build for native platforms #91

Uh oh!

Conversation

aajtodd commented Feb 9, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 12, 2024

Quality Gate passed

Uh oh!

aajtodd commented Feb 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants