From b2b705913e5cf65604120e1c13bc94179e39bf6c Mon Sep 17 00:00:00 2001 From: Jiaxiang Chen Date: Tue, 27 Sep 2022 02:25:39 -0700 Subject: [PATCH 1/3] use buildDir for relative path --- .../kotlin/com/google/devtools/ksp/KspOptions.kt | 11 +++++++++++ .../ksp/processing/impl/CodeGeneratorImpl.kt | 10 +++++----- .../com/google/devtools/ksp/Incremental.kt | 16 ++++++++++++++-- .../ksp/KotlinSymbolProcessingExtension.kt | 2 +- .../ksp/test/AbstractKSPCompilerPluginTest.kt | 1 + .../google/devtools/ksp/gradle/KspSubplugin.kt | 4 ++++ .../devtools/ksp/test/KSPCmdLineOptionsIT.kt | 1 + .../com/google/devtools/ksp/test/PlaygroundIT.kt | 2 +- 8 files changed, 38 insertions(+), 9 deletions(-) diff --git a/common-util/src/main/kotlin/com/google/devtools/ksp/KspOptions.kt b/common-util/src/main/kotlin/com/google/devtools/ksp/KspOptions.kt index d0d34a4bcb..ae113c3989 100644 --- a/common-util/src/main/kotlin/com/google/devtools/ksp/KspOptions.kt +++ b/common-util/src/main/kotlin/com/google/devtools/ksp/KspOptions.kt @@ -27,6 +27,7 @@ class KspOptions( val projectBaseDir: File, val compileClasspath: List, val javaSourceRoots: List, + val buildDir: File, val classOutputDir: File, val javaOutputDir: File, @@ -58,6 +59,7 @@ class KspOptions( var projectBaseDir: File? = null val compileClasspath: MutableList = mutableListOf() val javaSourceRoots: MutableList = mutableListOf() + var buildDir: File? = null var classOutputDir: File? = null var javaOutputDir: File? = null @@ -91,6 +93,7 @@ class KspOptions( requireNotNull(projectBaseDir) { "A non-null projectBaseDir must be provided" }, compileClasspath, javaSourceRoots, + requireNotNull(buildDir) { "A non-null buildDir must be provided" }, requireNotNull(classOutputDir) { "A non-null classOutputDir must be provided" }, requireNotNull(javaOutputDir) { "A non-null javaOutputDir must be provided" }, requireNotNull(kotlinOutputDir) { "A non-null kotlinOutputDir must be provided" }, @@ -181,6 +184,13 @@ enum class KspCliOption( false ), + BUILD_DIR_OPTION( + "buildDir", + "", + "path to build dirs", + false + ), + KSP_OUTPUT_DIR_OPTION( "kspOutputDir", "", @@ -288,6 +298,7 @@ fun KspOptions.Builder.processOption(option: KspCliOption, value: String) = when KspCliOption.CACHES_DIR_OPTION -> cachesDir = File(value) KspCliOption.KSP_OUTPUT_DIR_OPTION -> kspOutputDir = File(value) KspCliOption.PROJECT_BASE_DIR_OPTION -> projectBaseDir = File(value) + KspCliOption.BUILD_DIR_OPTION -> buildDir = File(value) KspCliOption.PROCESSING_OPTIONS_OPTION -> { val (k, v) = value.split('=', ignoreCase = false, limit = 2) processingOptions.put(k, v) diff --git a/common-util/src/main/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImpl.kt b/common-util/src/main/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImpl.kt index f9293bc6c0..a67e67f309 100644 --- a/common-util/src/main/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImpl.kt +++ b/common-util/src/main/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImpl.kt @@ -32,7 +32,7 @@ class CodeGeneratorImpl( private val javaDir: File, private val kotlinDir: File, private val resourcesDir: File, - private val projectBase: File, + private val buildDir: File, private val anyChangesWildcard: KSFile, private val allSources: List, private val isIncremental: Boolean @@ -93,7 +93,7 @@ class CodeGeneratorImpl( ) { val path = pathOf(packageName, fileName, extensionName) val files = classes.map { - it.containingFile ?: NoSourceFile(projectBase, it.qualifiedName?.asString().toString()) + it.containingFile ?: NoSourceFile(buildDir, it.qualifiedName?.asString().toString()) } associate(files, path, extensionToDirectory(extensionName)) } @@ -158,14 +158,14 @@ class CodeGeneratorImpl( if (!isIncremental) return - val output = outputPath.relativeTo(projectBase) + val output = outputPath.relativeTo(buildDir) sources.forEach { source -> - sourceToOutputs.getOrPut(File(source.filePath).relativeTo(projectBase)) { mutableSetOf() }.add(output) + sourceToOutputs.getOrPut(File(source.filePath).relativeTo(buildDir)) { mutableSetOf() }.add(output) } } val outputs: Set - get() = fileMap.values.mapTo(mutableSetOf()) { it.relativeTo(projectBase) } + get() = fileMap.values.mapTo(mutableSetOf()) { it.relativeTo(buildDir) } override val generatedFile: Collection get() = fileOutputStreamMap.keys.map { fileMap[it]!! } diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/Incremental.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/Incremental.kt index af41837dbb..05307fbbdb 100644 --- a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/Incremental.kt +++ b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/Incremental.kt @@ -194,6 +194,7 @@ class IncrementalContext( private val rebuild = !cachesUpToDateFile.exists() private val baseDir = options.projectBaseDir + private val buildDir = options.buildDir private val logsDir = File(options.cachesDir, "logs").apply { mkdirs() } private val buildTime = Date().time @@ -219,7 +220,16 @@ class IncrementalContext( private val sourceToOutputsMap = FileToFilesMap(File(options.cachesDir, "sourceToOutputs")) - private fun String.toRelativeFile() = File(this).relativeTo(baseDir) + // Ugly, but better than copying the private logics out of stdlib. + // TODO: get rid of `relativeTo` if possible. + private fun String.toRelativeFile(): File { + return try { + File(this).relativeTo(baseDir) + } catch (e: IllegalArgumentException) { + File(this).relativeTo(buildDir) + } + } + private val KSFile.relativeFile get() = filePath.toRelativeFile() @@ -440,7 +450,9 @@ class IncrementalContext( val outRoot = options.kspOutputDir val bakRoot = File(options.cachesDir, "backups") - fun File.abs() = File(baseDir, path) + // Can I assume output is always in buildDir? + fun File.abs() = File(buildDir, path) + // Can I assume cachesDir always has a common parent as buildDir? fun File.bak() = File(bakRoot, abs().toRelativeString(outRoot)) // Copy recursively, including last-modified-time of file and its parent dirs. diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingExtension.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingExtension.kt index f912513c7d..92ca1d419b 100644 --- a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingExtension.kt +++ b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingExtension.kt @@ -205,7 +205,7 @@ abstract class AbstractKotlinSymbolProcessingExtension( options.javaOutputDir, options.kotlinOutputDir, options.resourceOutputDir, - options.projectBaseDir, + options.buildDir, anyChangesWildcard, ksFiles, options.incremental diff --git a/compiler-plugin/src/test/kotlin/com/google/devtools/ksp/test/AbstractKSPCompilerPluginTest.kt b/compiler-plugin/src/test/kotlin/com/google/devtools/ksp/test/AbstractKSPCompilerPluginTest.kt index d57acae2f7..86ab7af5a1 100644 --- a/compiler-plugin/src/test/kotlin/com/google/devtools/ksp/test/AbstractKSPCompilerPluginTest.kt +++ b/compiler-plugin/src/test/kotlin/com/google/devtools/ksp/test/AbstractKSPCompilerPluginTest.kt @@ -63,6 +63,7 @@ abstract class AbstractKSPCompilerPluginTest : AbstractKSPTest(FrontendKinds.Cla kotlinOutputDir = File(testRoot, "kspTest/src/main/kotlin") resourceOutputDir = File(testRoot, "kspTest/src/main/resources") projectBaseDir = testRoot + buildDir = testRoot cachesDir = File(testRoot, "kspTest/kspCaches") kspOutputDir = File(testRoot, "kspTest") }.build(), diff --git a/gradle-plugin/src/main/kotlin/com/google/devtools/ksp/gradle/KspSubplugin.kt b/gradle-plugin/src/main/kotlin/com/google/devtools/ksp/gradle/KspSubplugin.kt index 144baa435b..cbb8ed0d9c 100644 --- a/gradle-plugin/src/main/kotlin/com/google/devtools/ksp/gradle/KspSubplugin.kt +++ b/gradle-plugin/src/main/kotlin/com/google/devtools/ksp/gradle/KspSubplugin.kt @@ -163,6 +163,10 @@ class KspGradleSubplugin @Inject internal constructor(private val registry: Tool "returnOkOnError", project.findProperty("ksp.return.ok.on.error")?.toString() ?: "true" ) + options += SubpluginOption( + "buildDir", + project.project.buildDir.canonicalPath + ) kspExtension.apOptions.forEach { options += SubpluginOption("apoption", "${it.key}=${it.value}") diff --git a/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/KSPCmdLineOptionsIT.kt b/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/KSPCmdLineOptionsIT.kt index 80f8cce60d..5e8d186650 100644 --- a/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/KSPCmdLineOptionsIT.kt +++ b/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/KSPCmdLineOptionsIT.kt @@ -38,6 +38,7 @@ class KSPCmdLineOptionsIT { "-Xplugin=${kspApiJar.absolutePath}", "-P", "plugin:$kspPluginId:apclasspath=${processorJar.absolutePath}", "-P", "plugin:$kspPluginId:projectBaseDir=${project.root}/build", + "-P", "plugin:$kspPluginId:buildDir=${project.root}/build", "-P", "plugin:$kspPluginId:classOutputDir=${project.root}/build", "-P", "plugin:$kspPluginId:javaOutputDir=${project.root}/build/out", "-P", "plugin:$kspPluginId:kotlinOutputDir=${project.root}/build/out", diff --git a/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/PlaygroundIT.kt b/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/PlaygroundIT.kt index 0e3d0fe35e..2e16375603 100644 --- a/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/PlaygroundIT.kt +++ b/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/PlaygroundIT.kt @@ -43,8 +43,8 @@ class PlaygroundIT { @Test fun testPlayground() { - // FIXME: `clean` fails to delete files on windows. Assume.assumeFalse(System.getProperty("os.name").startsWith("Windows", ignoreCase = true)) + // FIXME: `clean` fails to delete files on windows. val gradleRunner = GradleRunner.create().withProjectDir(project.root) gradleRunner.buildAndCheck("clean", "build") gradleRunner.buildAndCheck("clean", "build") From 542ea7dad57292b5c0463a386865b8784bcef97a Mon Sep 17 00:00:00 2001 From: Ting-Yuan Huang Date: Mon, 10 Oct 2022 11:32:05 -0700 Subject: [PATCH 2/3] Fixup to the buildDir change The source-to-output map and path converter for lookup tracker needs to be multiplexed as well. --- .../ksp/processing/impl/CodeGeneratorImpl.kt | 10 ++++++- .../processing/impl/CodeGeneratorImplTest.kt | 1 + .../com/google/devtools/ksp/Incremental.kt | 30 +++++++++++-------- .../ksp/KotlinSymbolProcessingExtension.kt | 1 + .../ksp/impl/KotlinSymbolProcessing.kt | 1 + .../ksp/impl/test/AbstractKSPAATest.kt | 1 + 6 files changed, 30 insertions(+), 14 deletions(-) diff --git a/common-util/src/main/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImpl.kt b/common-util/src/main/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImpl.kt index a67e67f309..ed017edb10 100644 --- a/common-util/src/main/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImpl.kt +++ b/common-util/src/main/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImpl.kt @@ -32,6 +32,7 @@ class CodeGeneratorImpl( private val javaDir: File, private val kotlinDir: File, private val resourcesDir: File, + private val projectBase: File, private val buildDir: File, private val anyChangesWildcard: KSFile, private val allSources: List, @@ -154,13 +155,20 @@ class CodeGeneratorImpl( associate(sources, file) } + private val File.relativeFile: File + get() = + if (this.startsWith(buildDir)) + relativeTo(buildDir) + else + relativeTo(projectBase) + private fun associate(sources: List, outputPath: File) { if (!isIncremental) return val output = outputPath.relativeTo(buildDir) sources.forEach { source -> - sourceToOutputs.getOrPut(File(source.filePath).relativeTo(buildDir)) { mutableSetOf() }.add(output) + sourceToOutputs.getOrPut(File(source.filePath).relativeFile) { mutableSetOf() }.add(output) } } diff --git a/common-util/src/test/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImplTest.kt b/common-util/src/test/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImplTest.kt index cd958f979a..2f8d0e7080 100644 --- a/common-util/src/test/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImplTest.kt +++ b/common-util/src/test/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImplTest.kt @@ -30,6 +30,7 @@ class CodeGeneratorImplTest { kotlinDir, resourcesDir, baseDir, + baseDir, AnyChanges(baseDir), emptyList(), true diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/Incremental.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/Incremental.kt index 05307fbbdb..2de08ff2e4 100644 --- a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/Incremental.kt +++ b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/Incremental.kt @@ -166,9 +166,17 @@ object symbolCollector : KSDefaultVisitor<(LookupSymbol) -> Unit, Unit>() { } } -internal class RelativeFileToPathConverter(val baseDir: File) : FileToPathConverter { +internal class RelativeFileToPathConverter( + val baseDir: File, + val buildDir: File, +) : FileToPathConverter { override fun toPath(file: File): String = file.path - override fun toFile(path: String): File = File(path).relativeTo(baseDir) + override fun toFile(path: String): File = + if (path.startsWith(buildDir.path)) { + File(path).relativeTo(buildDir) + } else { + File(path).relativeTo(baseDir) + } } class IncrementalContext( @@ -207,7 +215,7 @@ class IncrementalContext( // Disable incremental processing if somehow DualLookupTracker failed to be registered. // This may happen when a platform hasn't support incremental compilation yet. E.g, Common / Metadata. private val isIncremental = options.incremental && lookupTracker is DualLookupTracker - private val PATH_CONVERTER = RelativeFileToPathConverter(baseDir) + private val PATH_CONVERTER = RelativeFileToPathConverter(baseDir, buildDir) private val symbolLookupTracker = (lookupTracker as? DualLookupTracker)?.symbolTracker ?: LookupTracker.DO_NOTHING private val symbolLookupCacheDir = File(options.cachesDir, "symbolLookups") @@ -222,13 +230,11 @@ class IncrementalContext( // Ugly, but better than copying the private logics out of stdlib. // TODO: get rid of `relativeTo` if possible. - private fun String.toRelativeFile(): File { - return try { - File(this).relativeTo(baseDir) - } catch (e: IllegalArgumentException) { + private fun String.toRelativeFile(): File = + if (this.startsWith(buildDir.path)) File(this).relativeTo(buildDir) - } - } + else + File(this).relativeTo(baseDir) private val KSFile.relativeFile get() = filePath.toRelativeFile() @@ -447,13 +453,11 @@ class IncrementalContext( } private fun updateOutputs(outputs: Set, cleanOutputs: Collection) { - val outRoot = options.kspOutputDir val bakRoot = File(options.cachesDir, "backups") - // Can I assume output is always in buildDir? + // Assuming that output is always in buildDir. fun File.abs() = File(buildDir, path) - // Can I assume cachesDir always has a common parent as buildDir? - fun File.bak() = File(bakRoot, abs().toRelativeString(outRoot)) + fun File.bak() = File(bakRoot, path) // Copy recursively, including last-modified-time of file and its parent dirs. // diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingExtension.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingExtension.kt index 92ca1d419b..ad20a1994f 100644 --- a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingExtension.kt +++ b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingExtension.kt @@ -205,6 +205,7 @@ abstract class AbstractKotlinSymbolProcessingExtension( options.javaOutputDir, options.kotlinOutputDir, options.resourceOutputDir, + options.projectBaseDir, options.buildDir, anyChangesWildcard, ksFiles, diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/KotlinSymbolProcessing.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/KotlinSymbolProcessing.kt index 91efb6d957..3b117f8ead 100644 --- a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/KotlinSymbolProcessing.kt +++ b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/KotlinSymbolProcessing.kt @@ -71,6 +71,7 @@ class KotlinSymbolProcessing( options.kotlinOutputDir, options.resourceOutputDir, options.projectBaseDir, + options.buildDir, anyChangesWildcard, ksFiles, options.incremental diff --git a/kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/impl/test/AbstractKSPAATest.kt b/kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/impl/test/AbstractKSPAATest.kt index e2d622ce79..fc001c7365 100644 --- a/kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/impl/test/AbstractKSPAATest.kt +++ b/kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/impl/test/AbstractKSPAATest.kt @@ -88,6 +88,7 @@ abstract class AbstractKSPAATest : AbstractKSPTest(FrontendKinds.FIR) { kotlinOutputDir = File(testRoot, "kspTest/src/main/kotlin") resourceOutputDir = File(testRoot, "kspTest/src/main/resources") projectBaseDir = testRoot + buildDir = File(testRoot, "kspTest/build") cachesDir = File(testRoot, "kspTest/kspCaches") kspOutputDir = File(testRoot, "kspTest") }.build() From 4b8a4e2bdade7fc09247a251abc17fbb1470d270 Mon Sep 17 00:00:00 2001 From: Jiaxiang Chen Date: Tue, 11 Oct 2022 00:09:35 -0700 Subject: [PATCH 3/3] use try catch clause for more accurate relative path check. --- .../ksp/processing/impl/CodeGeneratorImpl.kt | 6 +++-- .../com/google/devtools/ksp/Incremental.kt | 9 +++++--- .../google/devtools/ksp/test/PlaygroundIT.kt | 22 +++++++++++++++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/common-util/src/main/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImpl.kt b/common-util/src/main/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImpl.kt index ed017edb10..f840b8c208 100644 --- a/common-util/src/main/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImpl.kt +++ b/common-util/src/main/kotlin/com/google/devtools/ksp/processing/impl/CodeGeneratorImpl.kt @@ -156,11 +156,13 @@ class CodeGeneratorImpl( } private val File.relativeFile: File - get() = - if (this.startsWith(buildDir)) + get() { + val buildDirPrefix = if (buildDir.path.startsWith("/")) buildDir.path else buildDir.path.replace("\\", "/") + return if (this.startsWith(buildDirPrefix)) relativeTo(buildDir) else relativeTo(projectBase) + } private fun associate(sources: List, outputPath: File) { if (!isIncremental) diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/Incremental.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/Incremental.kt index 2de08ff2e4..d3990914a1 100644 --- a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/Incremental.kt +++ b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/Incremental.kt @@ -230,11 +230,14 @@ class IncrementalContext( // Ugly, but better than copying the private logics out of stdlib. // TODO: get rid of `relativeTo` if possible. - private fun String.toRelativeFile(): File = - if (this.startsWith(buildDir.path)) + private fun String.toRelativeFile(): File { + val buildDirPrefix = if (buildDir.path.startsWith("/")) buildDir.path else buildDir.path.replace("\\", "/") + return if (this.startsWith(buildDirPrefix)) { File(this).relativeTo(buildDir) - else + } else { File(this).relativeTo(baseDir) + } + } private val KSFile.relativeFile get() = filePath.toRelativeFile() diff --git a/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/PlaygroundIT.kt b/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/PlaygroundIT.kt index 2e16375603..73640fa1e5 100644 --- a/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/PlaygroundIT.kt +++ b/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/PlaygroundIT.kt @@ -65,6 +65,28 @@ class PlaygroundIT { project.restore("workload/build.gradle.kts") } + @Test + fun testArbitraryBuildDir() { + Assume.assumeTrue(System.getProperty("os.name").startsWith("Windows", ignoreCase = true)) + val gradleRunner = GradleRunner.create().withProjectDir(project.root) + + File(project.root, "workload/build.gradle.kts") + .appendText("project.buildDir = File(\"D:/build/\")") + val result = gradleRunner.withArguments("build").build() + + Assert.assertEquals(TaskOutcome.SUCCESS, result.task(":workload:build")?.outcome) + + val artifact = File("D:/build/libs/workload-1.0-SNAPSHOT.jar") + Assert.assertTrue(artifact.exists()) + + JarFile(artifact).use { jarFile -> + Assert.assertTrue(jarFile.getEntry("TestProcessor.log").size > 0) + Assert.assertTrue(jarFile.getEntry("hello/HELLO.class").size > 0) + Assert.assertTrue(jarFile.getEntry("g/G.class").size > 0) + Assert.assertTrue(jarFile.getEntry("com/example/AClassBuilder.class").size > 0) + } + } + @Test fun testAllowSourcesFromOtherPlugins() { fun checkGBuilder() {