Fix compilation avoidance bug with buildCodegenCLI (#38903)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38903

Currently Android is not rebuilding the CLI if its content changes. This is a bug.
It's resolution is a bit more complicated as we do have various `buildCodegenCLI` tasks, also in user projects.

I've removed the tasks from the user projects, as they're practically not needed (users always consume a prebuilt codegen).

And I've also updated the setup to have only one `buildCodegenCLI` in the ReactAndroid project.
This allows us to ensure the job executes only once and has correct input/outputs.

Changelog:
[Internal] [Changed] -  Fix compilation avoidance bug with buildCodegenCLI

Reviewed By: mdvacca

Differential Revision: D48199157

fbshipit-source-id: ba3be6a0ca959ac4e1240e8feb99b8274e4e2b46
This commit is contained in:
Nicola Corti
2023-08-09 14:33:00 -07:00
committed by Facebook GitHub Bot
parent fedbb7202c
commit fdab18b0a1
6 changed files with 61 additions and 109 deletions
@@ -10,7 +10,6 @@ package com.facebook.react
import com.android.build.api.variant.AndroidComponentsExtension
import com.android.build.gradle.internal.tasks.factory.dependsOn
import com.facebook.react.internal.PrivateReactExtension
import com.facebook.react.tasks.BuildCodegenCLITask
import com.facebook.react.tasks.GenerateCodegenArtifactsTask
import com.facebook.react.tasks.GenerateCodegenSchemaTask
import com.facebook.react.utils.AgpConfiguratorUtils.configureBuildConfigFields
@@ -123,24 +122,10 @@ class ReactPlugin : Plugin<Project> {
localExtension.jsRootDir.convention(localExtension.root)
}
val buildCodegenTask =
project.tasks.register("buildCodegenCLI", BuildCodegenCLITask::class.java) {
it.codegenDir.set(rootExtension.codegenDir)
val bashWindowsHome = project.findProperty("REACT_WINDOWS_BASH") as String?
it.bashWindowsHome.set(bashWindowsHome)
// Please note that appNeedsCodegen is triggering a read of the package.json at
// configuration time as we need to feed the onlyIf condition of this task.
// Therefore, the appNeedsCodegen needs to be invoked inside this lambda.
val needsCodegenFromPackageJson = project.needsCodegenFromPackageJson(rootExtension.root)
it.onlyIf { isLibrary || needsCodegenFromPackageJson }
}
// We create the task to produce schema from JS files.
val generateCodegenSchemaTask =
project.tasks.register(
"generateCodegenSchemaFromJavaScript", GenerateCodegenSchemaTask::class.java) { it ->
it.dependsOn(buildCodegenTask)
it.nodeExecutableAndArgs.set(rootExtension.nodeExecutableAndArgs)
it.codegenDir.set(rootExtension.codegenDir)
it.generatedSrcDir.set(generatedSrcDir)
@@ -5,12 +5,12 @@
* LICENSE file in the root directory of this source tree.
*/
package com.facebook.react.tasks
package com.facebook.react.tasks.internal
import com.facebook.react.utils.Os.unixifyPath
import com.facebook.react.utils.windowsAwareBashCommandLine
import org.gradle.api.file.DirectoryProperty
import org.gradle.api.file.FileCollection
import org.gradle.api.file.FileTree
import org.gradle.api.provider.Property
import org.gradle.api.tasks.*
@@ -27,21 +27,14 @@ abstract class BuildCodegenCLITask : Exec() {
@get:Internal abstract val bashWindowsHome: Property<String>
@get:InputFiles
val input: FileCollection by lazy {
codegenDir.get().files("scripts", "src", "package.json", ".babelrc", ".prettierrc")
}
val inputFiles: FileTree = project.fileTree(codegenDir) { it.include("src/**/*.js") }
@get:OutputDirectories
val output: FileCollection by lazy { codegenDir.get().files("lib", "node_modules") }
init {
// We need this condition as we want a single instance of BuildCodegenCLITask to execute
// per project. Therefore we can safely skip the task if the lib/cli/ folder is available.
onlyIf {
val cliDir = codegenDir.file("lib/cli/").get().asFile
!cliDir.exists() || cliDir.listFiles()?.size == 0
}
}
@get:OutputFiles
val outputFiles: FileTree =
project.fileTree(codegenDir) {
it.include("lib/**/*.js")
it.include("lib/**/*.js.flow")
}
override fun exec() {
commandLine(
@@ -1,75 +0,0 @@
/*
* 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.tasks
import com.facebook.react.tests.createTestTask
import java.io.File
import org.gradle.api.tasks.*
import org.junit.Assert.*
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TemporaryFolder
class BuildCodegenCLITaskTest {
@get:Rule val tempFolder = TemporaryFolder()
@Test
fun buildCodegenCli_input_isSetCorrectly() {
val task = createTestTask<BuildCodegenCLITask> { it.codegenDir.set(tempFolder.root) }
assertTrue(task.input.contains(File(tempFolder.root, "scripts")))
assertTrue(task.input.contains(File(tempFolder.root, "src")))
assertTrue(task.input.contains(File(tempFolder.root, "package.json")))
assertTrue(task.input.contains(File(tempFolder.root, ".babelrc")))
assertTrue(task.input.contains(File(tempFolder.root, ".prettierrc")))
}
@Test
fun buildCodegenCli_output_isSetCorrectly() {
val task = createTestTask<BuildCodegenCLITask> { it.codegenDir.set(tempFolder.root) }
assertTrue(task.output.contains(File(tempFolder.root, "lib")))
assertTrue(task.output.contains(File(tempFolder.root, "node_modules")))
}
@Test
fun buildCodegenCli_bashWindowsHome_isSetCorrectly() {
val bashPath = tempFolder.newFile("bash").absolutePath
val task = createTestTask<BuildCodegenCLITask> { it.bashWindowsHome.set(bashPath) }
assertEquals(bashPath, task.bashWindowsHome.get())
}
@Test
fun buildCodegenCli_onlyIf_withMissingDirectory_isSatisfied() {
File(tempFolder.root, "lib/cli/").apply { mkdirs() }
val task = createTestTask<BuildCodegenCLITask> { it.codegenDir.set(tempFolder.root) }
assertTrue(task.onlyIf.isSatisfiedBy(task))
}
@Test
fun buildCodegenCli_onlyIf_withEmptyDirectory_isSatisfied() {
File(tempFolder.root, "lib/cli/").apply { mkdirs() }
val task = createTestTask<BuildCodegenCLITask> { it.codegenDir.set(tempFolder.root) }
assertTrue(task.onlyIf.isSatisfiedBy(task))
}
@Test
fun buildCodegenCli_onlyIf_withExistingDirtyDirectory_isNotSatisfied() {
File(tempFolder.root, "lib/cli/a-file").apply {
parentFile.mkdirs()
writeText("¯\\_(ツ)_/¯")
}
val task = createTestTask<BuildCodegenCLITask> { it.codegenDir.set(tempFolder.root) }
assertFalse(task.onlyIf.isSatisfiedBy(task))
}
}
@@ -0,0 +1,27 @@
/*
* 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.tasks.internal
import com.facebook.react.tests.createTestTask
import org.junit.Assert.assertEquals
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TemporaryFolder
class BuildCodegenCLITaskTest {
@get:Rule val tempFolder = TemporaryFolder()
@Test
fun buildCodegenCli_bashWindowsHome_isSetCorrectly() {
val bashPath = tempFolder.newFile("bash").absolutePath
val task = createTestTask<BuildCodegenCLITask> { it.bashWindowsHome.set(bashPath) }
assertEquals(bashPath, task.bashWindowsHome.get())
}
}
@@ -383,6 +383,13 @@ task downloadNdkBuildDependencies {
dependsOn(downloadGtest)
}
// As ReactAndroid builds from source, the codegen needs to be built before it can be invoked.
// This is not the case for users of React Native, as we ship a compiled version of the codegen.
final def buildCodegenCLITask = tasks.register('buildCodegenCLI', BuildCodegenCLITask) {
it.codegenDir.set(file("$rootDir/node_modules/@react-native/codegen"))
it.bashWindowsHome.set(project.findProperty("REACT_WINDOWS_BASH"))
}
/**
* Finds the path of the installed npm package with the given name using Node's
* module resolution algorithm, which searches "node_modules" directories up to
@@ -572,9 +579,20 @@ android {
}
}
preBuild.dependsOn(prepareJSC, prepareBoost, prepareDoubleConversion, prepareFmt, prepareFolly, prepareGlog, prepareLibevent, prepareGtest)
preBuild.dependsOn("generateCodegenArtifactsFromSchema")
preBuild.dependsOn(preparePrefab)
preBuild.dependsOn(
buildCodegenCLITask,
generateCodegenArtifactsFromSchema,
prepareBoost,
prepareDoubleConversion,
prepareFmt,
prepareFolly,
prepareGlog,
prepareGtest,
prepareJSC,
prepareLibevent,
preparePrefab
)
generateCodegenSchemaFromJavaScript.dependsOn(buildCodegenCLITask)
sourceSets.main {
res.srcDirs = ["src/main/res/devsupport", "src/main/res/shell", "src/main/res/views/modal", "src/main/res/views/uimanager"]
@@ -193,4 +193,8 @@ afterEvaluate {
mergeJscDebugNativeLibs.mustRunAfter(externalNativeBuildHermesDebug)
mergeJscReleaseNativeLibs.mustRunAfter(externalNativeBuildHermesRelease)
}
// As RN-Tester consumes the codegen from source, we need to make sure the codegen exists before
// we can actually invoke it. It's built by the ReactAndroid:buildCodegenCLI task.
generateCodegenSchemaFromJavaScript.dependsOn(":packages:react-native:ReactAndroid:buildCodegenCLI")
}