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

friends= on tests is broken in 1.3.40+ #210

Closed
cgruber opened this issue Oct 14, 2019 · 2 comments · Fixed by #222
Closed

friends= on tests is broken in 1.3.40+ #210

cgruber opened this issue Oct 14, 2019 · 2 comments · Fixed by #222

Comments

@cgruber
Copy link
Collaborator

cgruber commented Oct 14, 2019

Kotlinc prior to 1.3.40 used a more generous heuristic (a partial string match) on the contents of -Xfriend-paths. This has caused a breakage when used with 1.3.40 or later versions of kotlinc in the present rules.

The current rules throw the whole compilation class path at this flag, BUT, also use an incorrect path separator (namely File.pathSeparator). However, the present implementation of kotlinc's JVM back-end does not use this separator, instead using a list-like property syntax, with comma-separated values (Note: The Javascript backend uses the system path separator). This inconsistency is filed with Jetbrains as KT-34277. The currently rules therefore do not split the classpath given by -Xfriend-paths at all, returning a single-item list containing the whole string of the classpath. The net effect is that prior to 1.3.40, the partial string match worked (though an over-zealous list of friends was supplied). As of 1.3.40, however, an exact match against the jarfile (implied by the given friends target) is used. This fails to match against the string given to the flag.

The existing rules need to both switch to using the separator expected, and also need to back off of sending the compilation classpath, which results in access to internals that should not be granted.

@cgruber cgruber self-assigned this Oct 14, 2019
@cgruber
Copy link
Collaborator Author

cgruber commented Oct 14, 2019

@jin and @djwhang and @timpeut for awareness. I have a plan to deal with this (and a PR), but this is for context.

cgruber added a commit to cgruber/rules_kotlin that referenced this issue Oct 26, 2019
…th elements using that separator, not the usual system path separator.

> Note: This differs between the js and jvm backends [KT34277]

Fixes bazelbuild#210

[KT34277]: https://youtrack.jetbrains.com/issue/KT-34277
cgruber added a commit that referenced this issue Oct 28, 2019
…th elements using that separator, not the usual system path separator. (#222)

> Note: This differs between the js and jvm backends [KT34277]

Fixes #210

[KT34277]: https://youtrack.jetbrains.com/issue/KT-34277
@gertvdijk
Copy link
Contributor

gertvdijk commented Feb 6, 2020

Hi! I have issues with this and bisected the problem with rules_kotlin on a954e2b. It happens when I use an extension function from another workspace as dependency.

/path/to/bazel-cache/execroot/workspace_name/src/test/kotlin/nl/myorg/mycomponent/subcomponent/MyTest.kt:252:40: error: cannot access 'toFooBar': it is internal in 'nl.myorg.mycomponent.subcomponent'

Here is toFooBar part of a kt_jvm dep from @other_workspace// and then it gets into trouble.

Could it be that paths to external workspaces are not taken into account?

(I could make a minimal working example to demonstrate the issue, but would take some time. Please let me know if you want me to take that time or that I could try something else or maybe I have missed something else?)

(Also, the commit message is very misleading; the main change is that the kotlin compiler was updated.)

jongerrish added a commit to jongerrish/rules_kotlin that referenced this issue Apr 16, 2020
…th elements using that separator, not the usual system path separator. (bazelbuild#222)

> Note: This differs between the js and jvm backends [KT34277]

Fixes bazelbuild#210

[KT34277]: https://youtrack.jetbrains.com/issue/KT-34277
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants