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

Fix associates compile classpath #1271

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rbeazleyspot
Copy link
Contributor

@rbeazleyspot rbeazleyspot commented Feb 3, 2025

This is a (surprisingly) small PR to resolve #1250.

Any friends mentioned were not added to the classpath so KotlinBuilder now adds them and it keeps the compilation units friends close by adding them to the head of the classpath

Any friends jars that were aded were effectively just the ABI jars so associates.bzl now uses the class jars (which contain any internal declarations)


I had a hard time (ie failed) writing a test for this, i can only be sure the existing mechanism works ok.

Locally i redefined the tool chain to enable the relevant experimental options

define_kt_toolchain(
        name = "default_toolchain",
        experimental_remove_private_classes_in_abi_jars = True,
        experimental_treat_internal_as_private_in_abi_jars = True,
        experimental_use_abi_jars = True,
        language_version = "2.1",
        api_version = "2.1",
    )

with these in place the basic associates test fails, applying these commits makes it pass

an integration test could be added once a release is cut maybe?

@rbeazleyspot
Copy link
Contributor Author

rbeazleyspot commented Feb 3, 2025

well it looks like the same code fails on CI :)

so i guess ill look into this

-- edit
Still worth getting your feedback on this whilst i scratch my head

@rbeazleyspot
Copy link
Contributor Author

rookie mistake, not of course adding the associates java_outputs as an input to the compilation action, thanks for the help debugging @gabrielborglund

@rbeazleyspot
Copy link
Contributor Author

I've tried to work out why the integration tests are failing, no idea ! sorry :/

@rbeazleyspot
Copy link
Contributor Author

FYI, i believe with this change in place and considering a build graph that looks like this:

A - deps=[B], associates=[C]
B - deps=[C, D]

the compilation class path will look like this

C-full-runtime.jar (Collected from friends)
B-abi.jar (plus stdlib etc)
C-abi.jar (Collected from the transitive deps)
D.abi.jar

which is a change from

B-abi.jar (plus stdlib etc)
C-abi.jar (Collected from the transitive deps)
D.abi.jar

.... its possible a different angle could be taken and when the java_info for the associate_deps is collected here in compile.bzl

then it could be massaged to drop any ABI jars ie set compile_jar to output_jar

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.

Using plugin:org.jetbrains.kotlin.jvm.abi:treatInternalAsPrivate=true breaks Associates
1 participant