Skip to content
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

Relativize jar paths in jdeps and rework BazelRunFiles to return relative paths #1120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
28 changes: 28 additions & 0 deletions src/main/kotlin/io/bazel/kotlin/builder/utils/PathUtils.kt
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions src/main/kotlin/io/bazel/kotlin/plugin/jdeps/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -92,8 +93,10 @@ class JdepsGenExtension(
// Ignore Java source local to this module.
null
}

is KotlinJvmBinarySourceElement ->
(sourceElement.binaryClass as VirtualFileKotlinClass).file.canonicalPath

else -> null
}
}
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -177,6 +188,7 @@ class JdepsGenExtension(
}
collectTypeReferences(resultingDescriptor.type, isExplicit = false)
}

else -> return
}
}
Expand All @@ -192,6 +204,7 @@ class JdepsGenExtension(
collectTypeReferences(it)
}
}

is FunctionDescriptor -> {
descriptor.returnType?.let { collectTypeReferences(it) }
descriptor.valueParameters.forEach { valueParameter ->
Expand All @@ -204,6 +217,7 @@ class JdepsGenExtension(
collectTypeReferences(it)
}
}

is PropertyDescriptor -> {
collectTypeReferences(descriptor.type)
descriptor.annotations.forEach { annotation ->
Expand All @@ -213,6 +227,7 @@ class JdepsGenExtension(
collectTypeReferences(annotation.type)
}
}

is LocalVariableDescriptor -> {
collectTypeReferences(descriptor.type)
}
Expand Down Expand Up @@ -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)
}

Expand Down
31 changes: 20 additions & 11 deletions src/test/kotlin/io/bazel/kotlin/builder/KotlinJvmTestBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<JvmCompilationTask> {

@SuppressWarnings({"unused", "WeakerAccess"})
Expand Down Expand Up @@ -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();
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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<Unit>) {
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()!!))))

Expand Down