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

Bazel issue 13553 desugar workaround #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Bencodes
Copy link
Owner

@Bencodes Bencodes commented Jun 28, 2021

PR demonstrating how to work around the desugaring bug that was introduced in the latest version of Kotlin.

@Bencodes Bencodes force-pushed the bazel-issue-13553-desugar-workaround branch from 8b2c757 to fa9cf4c Compare June 28, 2021 02:36
@Bencodes Bencodes force-pushed the bazel-issue-13553-desugar-workaround branch from fa9cf4c to 6915e45 Compare June 28, 2021 02:37
@@ -8,3 +8,20 @@ android_binary(
"@maven//:org_jetbrains_kotlinx_kotlinx_coroutines_core_jvm",
],
)

java_import(
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually needs to be a kt_jvm_import to preserve the Kotlin metadata


java_import(
name = "kotlinx_coroutines_core_jvm_fixed",
jars = ["@kotlinx_coroutines_core_fixed//:v1/https/repo1.maven.org/maven2/org/jetbrains/kotlinx/kotlinx-coroutines-core-jvm/1.5.0/kotlinx-coroutines-core-jvm-1.5.0.jar"],
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also need to supply the srcjar here if you want to use the Intellij aspects

@@ -18,7 +18,13 @@ http_archive(

load("@rules_jvm_external//:defs.bzl", "maven_install")

overridden_targets = {
# Override the maven dep with the fixed target
"org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm": "@//:kotlinx_coroutines_core_jvm_fixed",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the actual fix? just curious

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 15 in the BUILD file

@Kernald
Copy link

Kernald commented Sep 23, 2022

Looks like this isn't really possible anymore with rules_jvm_external 4.3 and above - the jar isn't visible anymore. (Probably?) caused by bazel-contrib/rules_jvm_external#649.

@Bencodes
Copy link
Owner Author

@Kernald it's still possible and I made some changes inside rules jvm external to make it easier. I'll try to update this PR today with the new way.

@Bencodes
Copy link
Owner Author

@Kernald This actually seems to have been fixed using the latest rolling release (6.0.0-pre.20220909.2).

@Kernald
Copy link

Kernald commented Sep 23, 2022

That's great to know! I guess I might just wait a bit to update rules_jvm_external here.

@Bencodes
Copy link
Owner Author

The missing classpath issues are still logged to the console though

❯ bazel build //...
Starting local Bazel server and connecting to it...
INFO: Analyzed target //:bazel_desugar_issue_13553 (64 packages loaded, 1211 targets configured).
INFO: Found 1 target...
INFO: From Desugaring v1/https/repo1.maven.org/maven2/org/jetbrains/kotlinx/kotlinx-coroutines-core-jvm/1.6.4/kotlinx-coroutines-core-jvm-1.6.4.jar for Android:
Warning in external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlinx/kotlinx-coroutines-core-jvm/1.6.4/kotlinx-coroutines-core-jvm-1.6.4.jar:kotlinx/coroutines/internal/ClassValueCtorCache$cache$1.class:
Type `java.lang.ClassValue` was not found, it is required for default or static interface methods desugaring of `kotlinx.coroutines.internal.ClassValueCtorCache$cache$1`
Warning in external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlinx/kotlinx-coroutines-core-jvm/1.6.4/kotlinx-coroutines-core-jvm-1.6.4.jar:kotlinx/coroutines/debug/AgentPremain.class:
Type `sun.misc.SignalHandler` was not found, it is required for default or static interface methods desugaring of `kotlinx.coroutines.debug.AgentPremain$$InternalSyntheticLambda$2$7895cd395e43c061a299e224a1d3672f97bd4610fe97f0e188c9c199a1620b54$0`
Warning in external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlinx/kotlinx-coroutines-core-jvm/1.6.4/kotlinx-coroutines-core-jvm-1.6.4.jar:kotlinx/coroutines/debug/AgentPremain$DebugProbesTransformer.class:
Type `java.lang.instrument.ClassFileTransformer` was not found, it is required for default or static interface methods desugaring of `kotlinx.coroutines.debug.AgentPremain$DebugProbesTransformer`Target //:bazel_desugar_issue_13553 up-to-date:
  bazel-bin/bazel_desugar_issue_13553_deploy.jar
  bazel-bin/bazel_desugar_issue_13553_unsigned.apk
  bazel-bin/bazel_desugar_issue_13553.apk
INFO: Elapsed time: 19.251s, Critical Path: 6.05s
INFO: 41 processes: 14 internal, 13 darwin-sandbox, 14 worker.
INFO: Build completed successfully, 41 total actions

@Bencodes
Copy link
Owner Author

If you want to work around this using the latest rules_jvm_external you can reference the downloaded artifacts by pinning the maven install, and referencing the files that get downloaded:

aar_import(
    name = "androidx_core_core",
    aar = "@androidx_core_core_1_8_0//file:file",
    srcjar = "@androidx_core_core_aar_sources_1_8_0//file:file",
    visibility = ["//visibility:public"],
    deps = [
        # Add missing dependencies here
    ],
)

@aptenodytes-forsteri
Copy link

aptenodytes-forsteri commented Oct 6, 2022

We needed to do this to get it to work: https://gist.github.com/aptenodytes-forsteri/772d75cf41644166d8190e125e1ba367

  • --patch-module to avoid error: package exists in another module: jdk.unsupported
  • jvm_import instead of java_import
  • Custom combined jar file to try to keep as identical to the downloaded jar file from maven as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants