-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Handle autolinking for pure C++ turbo modules without includesGeneratedCode
#56938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
3dc7df3
1899f34
0cfd8d9
bacd360
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,8 @@ import java.io.File | |
| import org.gradle.api.DefaultTask | ||
| import org.gradle.api.file.DirectoryProperty | ||
| import org.gradle.api.file.RegularFileProperty | ||
| import org.gradle.api.provider.MapProperty | ||
| import org.gradle.api.tasks.Input | ||
| import org.gradle.api.tasks.InputFile | ||
| import org.gradle.api.tasks.OutputDirectory | ||
| import org.gradle.api.tasks.TaskAction | ||
|
|
@@ -22,12 +24,15 @@ abstract class GenerateAutolinkingNewArchitecturesFileTask : DefaultTask() { | |
|
|
||
| init { | ||
| group = "react" | ||
| generatedPureCxxCmakeListsPaths.convention(emptyMap<String, String>()) | ||
| } | ||
|
|
||
| @get:InputFile abstract val autolinkInputFile: RegularFileProperty | ||
|
|
||
| @get:OutputDirectory abstract val generatedOutputDirectory: DirectoryProperty | ||
|
|
||
| @get:Input abstract val generatedPureCxxCmakeListsPaths: MapProperty<String, String> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need a Can we replace it with a single optional The autolinking task can derive each dep's CMake path itself: when cmakeListsPath is null and isPureCxxDependency is true, just look at {pureCxxDir}/{libraryName}/jni/CMakeLists.txt.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cortinico updated to a |
||
|
|
||
| @TaskAction | ||
| fun taskAction() { | ||
| val model = JsonUtils.fromAutolinkingConfigJson(autolinkInputFile.get().asFile) | ||
|
|
@@ -55,7 +60,7 @@ abstract class GenerateAutolinkingNewArchitecturesFileTask : DefaultTask() { | |
| packages.joinToString("\n") { dep -> | ||
| var addDirectoryString = "" | ||
| val libraryName = dep.libraryName | ||
| val cmakeListsPath = dep.cmakeListsPath | ||
| val cmakeListsPath = cmakeListsPathForDependency(dep) | ||
| val cxxModuleCMakeListsPath = dep.cxxModuleCMakeListsPath | ||
| if (libraryName != null && cmakeListsPath != null) { | ||
| // If user provided a custom cmakeListsPath, let's honor it. | ||
|
|
@@ -96,6 +101,12 @@ abstract class GenerateAutolinkingNewArchitecturesFileTask : DefaultTask() { | |
| return CMAKE_TEMPLATE.replace("{{ libraryIncludes }}", libraryIncludes) | ||
| } | ||
|
|
||
| private fun cmakeListsPathForDependency( | ||
| dep: ModelAutolinkingDependenciesPlatformAndroidJson | ||
| ): String? { | ||
| return dep.cmakeListsPath ?: dep.libraryName?.let { generatedPureCxxCmakeListsPaths.get()[it] } | ||
| } | ||
|
|
||
| internal fun generateCppFileContent( | ||
| packages: List<ModelAutolinkingDependenciesPlatformAndroidJson> | ||
| ): String { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| /* | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| package com.facebook.react | ||
|
|
||
| import java.io.File | ||
| import org.assertj.core.api.Assertions.assertThat | ||
| import org.intellij.lang.annotations.Language | ||
| import org.junit.Rule | ||
| import org.junit.Test | ||
| import org.junit.rules.TemporaryFolder | ||
|
|
||
| class ReactPluginTest { | ||
|
|
||
| @get:Rule val tempFolder = TemporaryFolder() | ||
|
|
||
| @Test | ||
| fun getPureCxxCodegenDependencies_filtersDependenciesCorrectly() { | ||
| val includesGeneratedCode = createPackage("includes-generated-code", true) | ||
| val withoutGeneratedCode = createPackage("without-generated-code", false) | ||
| val withoutIncludesGeneratedCode = createPackage("without-includes-generated-code", null) | ||
| val withoutCodegenConfig = createPackageWithoutCodegenConfig("without-codegen-config") | ||
| val missingNonPureCxxPackage = File(tempFolder.root, "missing-non-pure-cxx-package") | ||
|
|
||
| val autolinkingFile = | ||
| createAutolinkingFile( | ||
| """ | ||
| { | ||
| "reactNativeVersion": "1000.0.0", | ||
| "dependencies": { | ||
| "includes-generated-code": { | ||
| "root": "${includesGeneratedCode.invariantSeparatorsPath}", | ||
| "name": "includes-generated-code", | ||
| "platforms": { | ||
| "android": { | ||
| "sourceDir": "${includesGeneratedCode.invariantSeparatorsPath}/android", | ||
| "packageImportPath": "import com.facebook.react.IncludesGeneratedCodePackage;", | ||
| "packageInstance": "new IncludesGeneratedCodePackage()", | ||
| "buildTypes": [], | ||
| "libraryName": "IncludesGeneratedCode", | ||
| "isPureCxxDependency": true | ||
| } | ||
| } | ||
| }, | ||
| "without-generated-code": { | ||
| "root": "${withoutGeneratedCode.invariantSeparatorsPath}", | ||
| "name": "without-generated-code", | ||
| "platforms": { | ||
| "android": { | ||
| "sourceDir": "${withoutGeneratedCode.invariantSeparatorsPath}/android", | ||
| "packageImportPath": "import com.facebook.react.WithoutGeneratedCodePackage;", | ||
| "packageInstance": "new WithoutGeneratedCodePackage()", | ||
| "buildTypes": [], | ||
| "libraryName": "WithoutGeneratedCode", | ||
| "isPureCxxDependency": true | ||
| } | ||
| } | ||
| }, | ||
| "without-includes-generated-code": { | ||
| "root": "${withoutIncludesGeneratedCode.invariantSeparatorsPath}", | ||
| "name": "without-includes-generated-code", | ||
| "platforms": { | ||
| "android": { | ||
| "sourceDir": "${withoutIncludesGeneratedCode.invariantSeparatorsPath}/android", | ||
| "packageImportPath": "import com.facebook.react.WithoutIncludesGeneratedCodePackage;", | ||
| "packageInstance": "new WithoutIncludesGeneratedCodePackage()", | ||
| "buildTypes": [], | ||
| "libraryName": "WithoutIncludesGeneratedCode", | ||
| "isPureCxxDependency": true | ||
| } | ||
| } | ||
| }, | ||
| "without-codegen-config": { | ||
| "root": "${withoutCodegenConfig.invariantSeparatorsPath}", | ||
| "name": "without-codegen-config", | ||
| "platforms": { | ||
| "android": { | ||
| "sourceDir": "${withoutCodegenConfig.invariantSeparatorsPath}/android", | ||
| "packageImportPath": "import com.facebook.react.WithoutCodegenConfigPackage;", | ||
| "packageInstance": "new WithoutCodegenConfigPackage()", | ||
| "buildTypes": [], | ||
| "libraryName": "WithoutCodegenConfig", | ||
| "isPureCxxDependency": true | ||
| } | ||
| } | ||
| }, | ||
| "missing-non-pure-cxx-package": { | ||
| "root": "${missingNonPureCxxPackage.invariantSeparatorsPath}", | ||
| "name": "missing-non-pure-cxx-package", | ||
| "platforms": { | ||
| "android": { | ||
| "sourceDir": "${missingNonPureCxxPackage.invariantSeparatorsPath}/android", | ||
| "packageImportPath": "import com.facebook.react.MissingNonPureCxxPackage;", | ||
| "packageInstance": "new MissingNonPureCxxPackage()", | ||
| "buildTypes": [], | ||
| "libraryName": "MissingNonPureCxxPackage", | ||
| "isPureCxxDependency": false | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| """ | ||
| .trimIndent() | ||
| ) | ||
|
|
||
| val result = ReactPlugin().getPureCxxCodegenDependencies(autolinkingFile) | ||
|
|
||
| assertThat(result.map { it.name }) | ||
| .containsExactly("without-generated-code", "without-includes-generated-code") | ||
| } | ||
|
|
||
| private fun createPackage(name: String, includesGeneratedCode: Boolean? = null): File { | ||
| val folder = tempFolder.newFolder(name) | ||
| File(folder, "package.json").writeText(packageJson(includesGeneratedCode)) | ||
| return folder | ||
| } | ||
|
|
||
| private fun createPackageWithoutCodegenConfig(name: String): File { | ||
| val folder = tempFolder.newFolder(name) | ||
| File(folder, "package.json").writeText(packageJson()) | ||
| return folder | ||
| } | ||
|
|
||
| private fun createAutolinkingFile(@Language("JSON") input: String) = | ||
| tempFolder.newFile("autolinking.json").apply { writeText(input) } | ||
|
|
||
| private fun packageJson(includesGeneratedCode: Boolean?): String { | ||
| val includesGeneratedCodeLine = | ||
| includesGeneratedCode?.let { ""","includesGeneratedCode": $it""" } ?: "" | ||
| // language=JSON | ||
| return """ | ||
| { | ||
| "version": "1.0.0", | ||
| "codegenConfig": { | ||
| "name": "TestSpec", | ||
| "type": "modules", | ||
| "jsSrcsDir": "src"$includesGeneratedCodeLine | ||
| } | ||
| } | ||
| """ | ||
| .trimIndent() | ||
| } | ||
|
|
||
| private fun packageJson(): String { | ||
| // language=JSON | ||
| return """ | ||
| { | ||
| "version": "1.0.0" | ||
| } | ||
| """ | ||
| .trimIndent() | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here there is a bunch of duplicated code.
Can you extract the common logic from
configureCodegeninto a reusable helper, then call it here for pure C++ deps instead?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cortinico extracted duplicated task registration code to
registerCodegenTasks