C interop build integration support#29
Conversation
sources/frontend-api/src/org/jetbrains/amper/frontend/schema/settings.kt
Outdated
Show resolved
Hide resolved
sources/amper-cli/src/org/jetbrains/amper/tasks/native/taskBuilderNative.kt
Outdated
Show resolved
Hide resolved
sources/amper-cli/src/org/jetbrains/amper/tasks/native/taskBuilderNative.kt
Outdated
Show resolved
Hide resolved
sources/frontend/schema/src/org/jetbrains/amper/frontend/tree/CinteropDiscovery.kt
Outdated
Show resolved
Hide resolved
|
Hey! I'd like to thank you again for the effort and the reactivity. I'm not using a lot of sugar in my PR comments, so if something sounds harsh or direct at first glance, please read the text while imagining a gentle and polite tone 😄 I haven't finished reviewing by any means, but I figured it was worth doing a first pass quickly. |
joffrey-bion
left a comment
There was a problem hiding this comment.
Thanks for your updates! Here is another pass. Sorry for the delay, but also let's wait for @Jeffset to be back from vacation (end of January)
|
|
||
| @SchemaDoc("C/Objective-C interop settings for native targets") | ||
| val cinterop: CinteropSettings by nested() | ||
| val cinterop: Map<String, CinteropModule> by value(default = emptyMap()) |
There was a problem hiding this comment.
What are the keys in this map used for? If they don't serve any purpose from the user's perspective, I think we should just make this a list.
There was a problem hiding this comment.
Hey @joffrey-bion, the original idea was to follow the pattern proposed in the Kotlin/Native documentation, where each cinterops declaration has its own alias, for example:
cinterops {
val libcurl by creating {
definitionFile.set(project.file("src/nativeInterop/cinterop/libcurl.def"))
packageName("com.jetbrains.handson.http")
compilerOpts("-I/path")
includeDirs.allHeaders("path")
}
}cinterop:
hello:
defFile: 'src/nativeInterop/cinterop/libcurl.def'In that model, using a map makes sense to allow identification by name.
That said, since this alias is not currently used anywhere, I agree there’s no practical benefit from the user’s perspective. We can simplify this and switch to a list.
sources/amper-cli/src/org/jetbrains/amper/compilation/KotlinNativeCompiler.kt
Outdated
Show resolved
Hide resolved
sources/amper-cli/src/org/jetbrains/amper/compilation/KotlinNativeCompiler.kt
Outdated
Show resolved
Hide resolved
sources/amper-cli/src/org/jetbrains/amper/compilation/KotlinNativeCompiler.kt
Outdated
Show resolved
Hide resolved
sources/amper-cli/src/org/jetbrains/amper/compilation/KotlinNativeCompiler.kt
Outdated
Show resolved
Hide resolved
sources/amper-cli/src/org/jetbrains/amper/compilation/KotlinNativeCompiler.kt
Outdated
Show resolved
Hide resolved
sources/amper-cli/src/org/jetbrains/amper/tasks/native/NativeLinkTask.kt
Outdated
Show resolved
Hide resolved
sources/frontend/schema/src/org/jetbrains/amper/frontend/tree/CinteropDiscovery.kt
Outdated
Show resolved
Hide resolved
sources/frontend-api/src/org/jetbrains/amper/frontend/schema/settings.kt
Outdated
Show resolved
Hide resolved
sources/amper-cli/src/org/jetbrains/amper/tasks/native/taskBuilderNative.kt
Outdated
Show resolved
Hide resolved
Sure, @joffrey-bion, np |
…tiveCompiler.kt Co-authored-by: Joffrey Bion <joffrey.bion@gmail.com>
…inkTask.kt Co-authored-by: Joffrey Bion <joffrey.bion@gmail.com>
No description provided.