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

JDeps outputs for kt_jvm_library targets include absolute paths #941

Open
freetheinterns opened this issue Mar 16, 2023 · 7 comments · May be fixed by #1120
Open

JDeps outputs for kt_jvm_library targets include absolute paths #941

freetheinterns opened this issue Mar 16, 2023 · 7 comments · May be fixed by #1120
Assignees
Labels
component: jvm type: bug Something isn't working

Comments

@freetheinterns
Copy link

freetheinterns commented Mar 16, 2023

Issue Summary

Outputs for .jdeps files from kt_jvm_library targets include the absolute path to kotlin-stdlib.jar. This means that cache entries which include these outputs are not shared with other users. The path in my case looks like:
.../external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar

Steps to Reproduce

Simply build any kotlin library target then cat the jdeps output file created:
bazel build //path/to/kotlin/library/target:target_lib

...
INFO: Found 1 target...
Target //path/to/kotlin/library/target:target_lib up-to-date:
  dist/bin/path/to/kotlin/library/target/target_lib.jar
  dist/bin/path/to/kotlin/library/target/target_lib.jdeps
...

cat dist/bin/path/to/kotlin/library/target/target_lib.jdeps

^
Z.cache/bazel/_bazel_ted_tenedorio/8d13339fa502dfceb2b05be00f715cc8/execroot/treehouse/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar
7
3bazel-out/k8-fastbuild/bin/bazel/jvm/impl/empty.jar
�
�bazel-out/k8-fastbuild/bin/external/maven/v1/https/maven.org/apiguardian/apiguardian-api/1.1.0/header_apiguardian-api-1.1.0.jar
�
�bazel-out/k8-fastbuild/bin/external/maven/v1/https/maven.org/junit/jupiter/junit-jupiter-api/5.7.2/header_junit-jupiter-api-5.7.2.jar
�
�bazel-out/k8-fastbuild/bin/external/maven/v1/https/maven.org/junit/platform/junit-platform-commons/1.7.2/header_junit-platform-commons-1.7.2.jar
�
�bazel-out/k8-fastbuild/bin/external/maven/v1/https/maven.org/opentest4j/opentest4j/1.2.0/header_opentest4j-1.2.0.jar
A
=external/com_github_jetbrains_kotlin/lib/annotations-13.0.jar
C
?external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk7.jar
C
?external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk8.jar
>
:external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jarC//path/to/kotlin/library/target:target_lib%
@Bencodes
Copy link
Collaborator

Bencodes commented Apr 21, 2023

I'm not super familiar with how listed jdeps are expected to be formatted but Bazel core also has absolute paths listed.

➜ cat bazel-bin/src/main/java/com/google/devtools/build/lib/server/libserver.jdeps
x
tbazel-out/darwin_arm64-fastbuild/bin/src/main/java/com/google/devtools/build/lib/server/libpid_file_watcher-hjar.jar
r
nbazel-out/darwin_arm64-fastbuild/bin/src/main/java/com/google/devtools/build/lib/server/librpc_server-hjar.jar
v
rbazel-out/darwin_arm64-fastbuild/bin/src/main/java/com/google/devtools/build/lib/server/libshutdown_hooks-hjar.jar
}
ybazel-out/darwin_arm64-fastbuild/bin/src/main/java/com/google/devtools/build/lib/libruntime/blaze_command_result-hjar.jar
{
wbazel-out/darwin_arm64-fastbuild/bin/src/main/java/com/google/devtools/build/lib/libruntime/command_dispatcher-hjar.jar
}
ybazel-out/darwin_arm64-fastbuild/bin/src/main/java/com/google/devtools/build/lib/libruntime/safe_request_logging-hjar.jar
.......

@jdai8
Copy link
Contributor

jdai8 commented Apr 21, 2023

Those look like relative paths to me?

I think the PR description was edited/redacted, which might be confusing now. But the original paths look something like:

/home/<user>/.cache/bazel/<...>/execroot/<repo>/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar

Note that this seems to only happen for the kotlin stdlib jar.

@restingbull
Copy link
Collaborator

The crux of the problem is that all of the preloaded jar (e.g. stdlib, etc.) return an absolute path. I've been able to reproduce this locally.

This is related to the runfile resolution as part of the worker. I've rearranged the runfile resolution (in part for the KSP work), but it still is unclear what the root path should be.

My current effort is trying to relativize the paths to the runfiles... without hardcoding runfiles into the heuristic. Because that will probably break later.

@restingbull restingbull added type: bug Something isn't working and removed status: open - needs triage labels Apr 21, 2023
@smocherla-brex
Copy link

smocherla-brex commented Jun 25, 2023

Looks like this might be fixed with Kotlin 1.8.20? I was looking into this and noticed that currently on master, if you build a kt_jvm_library target, there are no absolute paths to the kotlin-stdlib.jar in the .jdeps outputs.

After some bisection, I found that it seems fixed with this commit 3f578c2

Prior to this, I get different outputs if I use different --output_base values for build invocations

# commit prior to 3f578c2de02d5013fbf693b1772a8fb74b101a4b
git checkout b3bd1e65c99ae545494244dacf94ba899e470633
bazel clean
# use a custom output base to force a different .jdeps file output
bazel --output_base=$(mktemp -d)  build //src/test/kotlin/io/bazel/kotlin:assertion_test_case
smocherla@YQRW36MHXG rules_kotlin % sha256sum   bazel-bin/src/test/kotlin/io/bazel/kotlin/assertion_test_case.jdeps
80a4174ebea29b23f3fa3eea950418c82b6e9b5117eed2d971c7f9ea77b227ff  bazel-bin/src/test/kotlin/io/bazel/kotlin/assertion_test_case.jdeps

smocherla@YQRW36MHXG rules_kotlin % cat bazel-bin/src/test/kotlin/io/bazel/kotlin/assertion_test_case.jdeps

�
�/private/var/folders/lr/k_fmpjvx7qjc6lw9dmn20vym0000gp/T/tmp.FIuvQhN0/execroot/dev_io_bazel_rules_kotlin/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar
�
�bazel-out/darwin_arm64-fastbuild/bin/external/kotlin_rules_maven/v1/https/repo1.maven.org/maven2/com/google/guava/guava/31.0.1-jre/header_guava-31.0.1-jre.jar
A
=external/com_github_jetbrains_kotlin/lib/annotations-13.0.jar
C
?external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk7.jar
C
?external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk8.jar
>
:external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar
<
8external/com_github_jetbrains_kotlin/lib/kotlin-test.jar5//src/test/kotlin/io/bazel/kotlin:assertion_test_case%

# rebuild with default output base
# this will use a different absolute path and produce a different .jdeps output
bazel build //src/test/kotlin/io/bazel/kotlin:assertion_test_case
smocherla@YQRW36MHXG rules_kotlin % sha256sum   bazel-bin/src/test/kotlin/io/bazel/kotlin/assertion_test_case.jdeps
fd82da59da57df12774f49db96276174a637104a1cacddd0aa25f3882a710ad9  bazel-bin/src/test/kotlin/io/bazel/kotlin/assertion_test_case.jdeps

smocherla@YQRW36MHXG rules_kotlin % cat bazel-bin/src/test/kotlin/io/bazel/kotlin/assertion_test_case.jdeps

�
�/private/var/tmp/_bazel_smocherla/f8e4d4f6d7daa3eff93a1e3ecc243ee1/execroot/dev_io_bazel_rules_kotlin/external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar
�
�bazel-out/darwin_arm64-fastbuild/bin/external/kotlin_rules_maven/v1/https/repo1.maven.org/maven2/com/google/guava/guava/31.0.1-jre/header_guava-31.0.1-jre.jar
A
=external/com_github_jetbrains_kotlin/lib/annotations-13.0.jar
C
?external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk7.jar
C
?external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk8.jar
>
:external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar
<
8external/com_github_jetbrains_kotlin/lib/kotlin-test.jar5//src/test/kotlin/io/bazel/kotlin:assertion_test_case%

From commit 3f578c2 onwards, I see relative paths in the jdeps file and consistent outputs regardless of the output base.

smocherla@YQRW36MHXG rules_kotlin % git checkout 3f578c2de02d5013fbf693b1772a8fb74b101a4b
smocherla@YQRW36MHXG rules_kotlin % bazel --output_base=$(mktemp -d) build //src/test/kotlin/io/bazel/kotlin:assertion_test_case
smocherla@YQRW36MHXG rules_kotlin % sha256sum bazel-bin/src/test/kotlin/io/bazel/kotlin/assertion_test_case.jdeps
14d4296997c1a25381978406c6816de059dd76e4020dee9086d4792870ec9fd6  bazel-bin/src/test/kotlin/io/bazel/kotlin/assertion_test_case.jdeps
smocherla@YQRW36MHXG rules_kotlin % cat bazel-bin/src/test/kotlin/io/bazel/kotlin/assertion_test_case.jdeps
�
�bazel-out/darwin_arm64-fastbuild/bin/external/kotlin_rules_maven/v1/https/repo1.maven.org/maven2/com/google/guava/guava/31.0.1-jre/header_guava-31.0.1-jre.jar
A
=external/com_github_jetbrains_kotlin/lib/annotations-13.0.jar
C
?external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk7.jar
C
?external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk8.jar
>
:external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar
<
8external/com_github_jetbrains_kotlin/lib/kotlin-test.jar5//src/test/kotlin/io/bazel/kotlin:assertion_test_case%

# rebuild with default output base
# and we see that the contents of the jdeps file are the same
bazel build //src/test/kotlin/io/bazel/kotlin:assertion_test_case
smocherla@YQRW36MHXG rules_kotlin % sha256sum bazel-bin/src/test/kotlin/io/bazel/kotlin/assertion_test_case.jdeps
14d4296997c1a25381978406c6816de059dd76e4020dee9086d4792870ec9fd6  bazel-bin/src/test/kotlin/io/bazel/kotlin/assertion_test_case.jdeps
smocherla@YQRW36MHXG rules_kotlin % cat bazel-bin/src/test/kotlin/io/bazel/kotlin/assertion_test_case.jdeps

�
�bazel-out/darwin_arm64-fastbuild/bin/external/kotlin_rules_maven/v1/https/repo1.maven.org/maven2/com/google/guava/guava/31.0.1-jre/header_guava-31.0.1-jre.jar
A
=external/com_github_jetbrains_kotlin/lib/annotations-13.0.jar
C
?external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk7.jar
C
?external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk8.jar
>
:external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar
<
8external/com_github_jetbrains_kotlin/lib/kotlin-test.jar5//src/test/kotlin/io/bazel/kotlin:assertion_test_case%

I'm not sure which commit between 1.8.10 and 1.8.20 fixed this though (the changelog is pretty huge), but seems like this would solve the remote caching problem for these outputs.

@arunkumar9t2
Copy link
Contributor

Able to confirm that 1.8.20+ fixes the issue. For a different reason we are still on 1.8.10 and managed to workaround this issue by relativizing the path.

My current effort is trying to relativize the paths to the runfiles... without hardcoding runfiles into the heuristic. Because that will probably break later.

For this, the absolute path can be inferred from Bazel injected env variable PWD. So something like this on all dependency.path in JdepsExtension helps. Only tested workers so far, might also work with sandboxing.

private fun relativize(jarPath: String): String {
    val pwd = System.getenv("PWD")
    return if (jarPath.startsWith(pwd)) {
      jarPath.replace(pwd, "")
    } else jarPath
  }

@Bencodes
Copy link
Collaborator

@arunkumar9t2 this seems worth upstreaming to future proof jdeps from regressions like these in the future.

@arunkumar9t2
Copy link
Contributor

Sure will work on it 👍🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: jvm type: bug Something isn't working
Projects
None yet
6 participants