From bed8d403949376f0907679c04d643c8a238b1074 Mon Sep 17 00:00:00 2001 From: Arunkumar Date: Wed, 28 Feb 2024 13:43:25 +0800 Subject: [PATCH] Relativize jar paths in jdeps and rework BazelRunFiles to return relative paths --- .../kotlin/builder/utils/BazelRunFiles.kt | 2 +- .../bazel/kotlin/builder/utils/PathUtils.kt | 28 ++++++++++++++ .../io/bazel/kotlin/plugin/jdeps/BUILD.bazel | 1 + .../kotlin/plugin/jdeps/JdepsGenExtension.kt | 23 +++++++++-- .../kotlin/builder/KotlinJvmTestBuilder.java | 31 +++++++++------ .../tasks/jvm/KotlinBuilderJvmJdepsTest.kt | 38 +++++++++++++++++++ 6 files changed, 107 insertions(+), 16 deletions(-) create mode 100644 src/main/kotlin/io/bazel/kotlin/builder/utils/PathUtils.kt diff --git a/src/main/kotlin/io/bazel/kotlin/builder/utils/BazelRunFiles.kt b/src/main/kotlin/io/bazel/kotlin/builder/utils/BazelRunFiles.kt index 3003763de..1f09a994b 100644 --- a/src/main/kotlin/io/bazel/kotlin/builder/utils/BazelRunFiles.kt +++ b/src/main/kotlin/io/bazel/kotlin/builder/utils/BazelRunFiles.kt @@ -32,7 +32,7 @@ object BazelRunFiles { @JvmStatic fun resolveVerifiedFromProperty(key: String) = System.getProperty(key) - ?.let { path -> runfiles.rlocation(path) } + ?.let { path -> runfiles.rlocation(path).relativizeToPwd() } ?.let { File(it) } ?.also { if (!it.exists()) { diff --git a/src/main/kotlin/io/bazel/kotlin/builder/utils/PathUtils.kt b/src/main/kotlin/io/bazel/kotlin/builder/utils/PathUtils.kt new file mode 100644 index 000000000..f5478988f --- /dev/null +++ b/src/main/kotlin/io/bazel/kotlin/builder/utils/PathUtils.kt @@ -0,0 +1,28 @@ +package io.bazel.kotlin.builder.utils + +import java.io.File +import java.nio.file.Paths +import kotlin.io.path.absolute +import kotlin.io.path.exists + +/** + * Try to relativize this path to be relative to current working directory. Original path is + * returned as-is if relativization can't be done. + * Ensures returned path exists on the file system + */ +fun String.relativizeToPwd(): String { + val pwd = System.getenv("PWD") ?: "" + if (startsWith(pwd)) { + val rpath = replace(pwd, "") + if (File(rpath).exists()) { + return rpath + } + } + // Handle cases where path is in runfiles. + // In this case relativize from current directory to runfiles dir + val runfilesRPath = Paths.get(".").absolute().relativize(Paths.get(this).absolute()) + if (runfilesRPath.exists()) { + return runfilesRPath.toString() + } + return this +} diff --git a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/BUILD.bazel b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/BUILD.bazel index 131fa544d..15822ec8c 100644 --- a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/BUILD.bazel +++ b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/BUILD.bazel @@ -23,6 +23,7 @@ kt_bootstrap_library( visibility = ["//src:__subpackages__"], deps = [ "//kotlin/compiler:kotlin-compiler", + "//src/main/kotlin/io/bazel/kotlin/builder/utils", "//src/main/kotlin/io/bazel/kotlin/builder/utils/jars", "//src/main/protobuf:deps_java_proto", "@kotlin_rules_maven//:com_google_protobuf_protobuf_java", diff --git a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt index b6343d3ee..23c7d5f9a 100644 --- a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt +++ b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt @@ -4,6 +4,7 @@ import com.google.devtools.build.lib.view.proto.Deps import com.intellij.openapi.project.Project import com.intellij.psi.PsiElement import io.bazel.kotlin.builder.utils.jars.JarOwner +import io.bazel.kotlin.builder.utils.relativizeToPwd import org.jetbrains.kotlin.analyzer.AnalysisResult import org.jetbrains.kotlin.config.CompilerConfiguration import org.jetbrains.kotlin.container.StorageComponentContainer @@ -92,8 +93,10 @@ class JdepsGenExtension( // Ignore Java source local to this module. null } + is KotlinJvmBinarySourceElement -> (sourceElement.binaryClass as VirtualFileKotlinClass).file.canonicalPath + else -> null } } @@ -134,14 +137,17 @@ class JdepsGenExtension( is FunctionImportedFromObject -> { collectTypeReferences(resultingDescriptor.containingObject.defaultType) } + is PropertyImportedFromObject -> { collectTypeReferences(resultingDescriptor.containingObject.defaultType) } + is JavaMethodDescriptor -> { getClassCanonicalPath( (resultingDescriptor.containingDeclaration as ClassDescriptor).typeConstructor, )?.let { explicitClassesCanonicalPaths.add(it) } } + is FunctionDescriptor -> { resultingDescriptor.returnType?.let { collectTypeReferences(it, isExplicit = false, collectTypeArguments = false) @@ -154,20 +160,25 @@ class JdepsGenExtension( ?: return explicitClassesCanonicalPaths.add(virtualFileClass.file.path) } + is ParameterDescriptor -> { getClassCanonicalPath(resultingDescriptor)?.let { explicitClassesCanonicalPaths.add(it) } } + is FakeCallableDescriptorForObject -> { collectTypeReferences(resultingDescriptor.type) } + is JavaPropertyDescriptor -> { getClassCanonicalPath(resultingDescriptor)?.let { explicitClassesCanonicalPaths.add(it) } } + is PropertyDescriptor -> { when (resultingDescriptor.containingDeclaration) { is ClassDescriptor -> collectTypeReferences( (resultingDescriptor.containingDeclaration as ClassDescriptor).defaultType, ) + else -> { val virtualFileClass = (resultingDescriptor).getContainingKotlinJvmBinaryClass() as? VirtualFileKotlinClass @@ -177,6 +188,7 @@ class JdepsGenExtension( } collectTypeReferences(resultingDescriptor.type, isExplicit = false) } + else -> return } } @@ -192,6 +204,7 @@ class JdepsGenExtension( collectTypeReferences(it) } } + is FunctionDescriptor -> { descriptor.returnType?.let { collectTypeReferences(it) } descriptor.valueParameters.forEach { valueParameter -> @@ -204,6 +217,7 @@ class JdepsGenExtension( collectTypeReferences(it) } } + is PropertyDescriptor -> { collectTypeReferences(descriptor.type) descriptor.annotations.forEach { annotation -> @@ -213,6 +227,7 @@ class JdepsGenExtension( collectTypeReferences(annotation.type) } } + is LocalVariableDescriptor -> { collectTypeReferences(descriptor.type) } @@ -318,21 +333,21 @@ class JdepsGenExtension( unusedDeps.forEach { jarPath -> val dependency = Deps.Dependency.newBuilder() dependency.kind = Deps.Dependency.Kind.UNUSED - dependency.path = jarPath + dependency.path = jarPath.relativizeToPwd() rootBuilder.addDependency(dependency) } explicitDeps.forEach { (jarPath, _) -> val dependency = Deps.Dependency.newBuilder() dependency.kind = Deps.Dependency.Kind.EXPLICIT - dependency.path = jarPath + dependency.path = jarPath.relativizeToPwd() rootBuilder.addDependency(dependency) } - implicitDeps.keys.subtract(explicitDeps.keys).forEach { + implicitDeps.keys.subtract(explicitDeps.keys).forEach { jarPath -> val dependency = Deps.Dependency.newBuilder() dependency.kind = Deps.Dependency.Kind.IMPLICIT - dependency.path = it + dependency.path = jarPath.relativizeToPwd() rootBuilder.addDependency(dependency) } diff --git a/src/test/kotlin/io/bazel/kotlin/builder/KotlinJvmTestBuilder.java b/src/test/kotlin/io/bazel/kotlin/builder/KotlinJvmTestBuilder.java index 39c07aeb8..880f04a8a 100644 --- a/src/test/kotlin/io/bazel/kotlin/builder/KotlinJvmTestBuilder.java +++ b/src/test/kotlin/io/bazel/kotlin/builder/KotlinJvmTestBuilder.java @@ -16,13 +16,11 @@ */ package io.bazel.kotlin.builder; +import static java.util.stream.Collectors.collectingAndThen; +import static io.bazel.kotlin.builder.utils.PathUtilsKt.relativizeToPwd; + import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; -import io.bazel.kotlin.builder.Deps.AnnotationProcessor; -import io.bazel.kotlin.builder.Deps.Dep; -import io.bazel.kotlin.builder.toolchain.CompilationTaskContext; -import io.bazel.kotlin.model.CompilationTaskInfo; -import io.bazel.kotlin.model.JvmCompilationTask; import java.util.EnumSet; import java.util.HashSet; @@ -31,6 +29,13 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import io.bazel.kotlin.builder.Deps.AnnotationProcessor; +import io.bazel.kotlin.builder.Deps.Dep; +import io.bazel.kotlin.builder.toolchain.CompilationTaskContext; +import io.bazel.kotlin.builder.utils.PathUtilsKt; +import io.bazel.kotlin.model.CompilationTaskInfo; +import io.bazel.kotlin.model.JvmCompilationTask; + public final class KotlinJvmTestBuilder extends KotlinAbstractTestBuilder { @SuppressWarnings({"unused", "WeakerAccess"}) @@ -114,15 +119,19 @@ private Dep executeTask( .filter(p -> !p.isEmpty()) .toArray(String[]::new) ); + final String compileJar = relativizeToPwd(outputs.getAbijar().isEmpty() + ? outputs.getJar() : outputs.getAbijar()); return Dep.builder() .label(taskBuilder.getInfo().getLabel()) - .compileJars(ImmutableList.of( - outputs.getAbijar().isEmpty() ? outputs.getJar() : outputs.getAbijar() - )) - .jdeps(outputs.getJdeps()) - .runtimeDeps(ImmutableList.copyOf(taskBuilder.getInputs().getClasspathList())) - .sourceJar(taskBuilder.getOutputs().getSrcjar()) + .compileJars(ImmutableList.of(compileJar)) + .jdeps(relativizeToPwd(outputs.getJdeps())) + .runtimeDeps(taskBuilder.getInputs() + .getClasspathList() + .stream() + .map(PathUtilsKt::relativizeToPwd) + .collect(collectingAndThen(Collectors.toList(), ImmutableList::copyOf))) + .sourceJar(relativizeToPwd(taskBuilder.getOutputs().getSrcjar())) .build(); }); } diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinBuilderJvmJdepsTest.kt b/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinBuilderJvmJdepsTest.kt index c47ef7f80..beadb4aa7 100644 --- a/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinBuilderJvmJdepsTest.kt +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinBuilderJvmJdepsTest.kt @@ -25,6 +25,7 @@ import org.junit.runner.RunWith import org.junit.runners.Parameterized import java.io.BufferedInputStream import java.nio.file.Files +import java.nio.file.Path import java.nio.file.Paths import java.util.function.Consumer @@ -1662,6 +1663,43 @@ class KotlinBuilderJvmJdepsTest(private val enableK2Compiler: Boolean) { assertImplicit(jdeps).doesNotContain(depWithReturnTypesSuperType) } + @Test + fun `assert jdeps path are relative`() { + val foo = runCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "Foo.kt", + """ + package something + + open class Foo { } + """, + ) + } + val dependingTarget = runJdepsCompileTask { c: KotlinJvmTestBuilder.TaskBuilder -> + c.addSource( + "FunctionUsingStdlib.kt", + """ + package something + + class Bar: Foo() + + fun main(param: List) { + val foo = Foo() + } + """, + ) + c.addDirectDependencies(foo) + } + assertThat( + depsProto(dependingTarget) + .dependencyList + .asSequence() + .filter { it.kind == Deps.Dependency.Kind.EXPLICIT } + .map { Paths.get(it.path) } + .none(Path::isAbsolute), + ).isTrue() + } + private fun depsProto(jdeps: Dep) = Deps.Dependencies.parseFrom(BufferedInputStream(Files.newInputStream(Paths.get(jdeps.jdeps()!!))))