Make java library earlier on classpath #1797
Conversation
Co-Authored-By: Farid Zakaria <farid.m.zakaria@gmail.com>
vinnybod
requested review from
WojciechMazur,
mbland and
simuons
as code owners
| #$runner bazel build --extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error --all_incompatible_changes -- test/... | ||
| $runner bazel test "${test_output_flag}" --extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error -- test/... | ||
| $runner bazel test "${test_output_flag}" --extra_toolchains=//scala:minimal_direct_source_deps -- test/... | ||
| $runner bazel test "${test_output_flag}" --extra_toolchains=//scala:minimal_direct_source_deps --build_tag_filters=-no-strict-deps --test_tag_filters=-no-strict-deps -- test/... |
There was a problem hiding this comment.
I don't love that I had to do this, but afaik there isn't a way to disable "strict deps" on a target level like you can with "unused deps"
There was a problem hiding this comment.
I think this patch should do the job without changes to the testing script. We should be able to revert these changes and the tag from the test (however it would be worth it to add comment linking to the change in the toolchain)
diff --git a/scala/BUILD b/scala/BUILD
index 2f0b40b..ee98b80 100644
--- a/scala/BUILD
+++ b/scala/BUILD
@@ -29,6 +29,11 @@ scala_toolchain(
dependency_tracking_method = "ast",
strict_deps_mode = "error",
unused_dependency_checker_mode = "error",
+ # Exclude the shadowing test package from strict-deps to allow classpath-order coverage without disabling strict-deps globally in CI runs.
+ dependency_tracking_strict_deps_patterns = [
+ "", # keep default include
+ "-//test/src/main/scala/scalarules/test/java_classpath_order",
+ ],
)
[
WojciechMazur
left a comment
WojciechMazur
left a comment
There was a problem hiding this comment.
Thank you for these changes, looks good. I've left some 2 comments regarding only minor improvements
|
|
||
| full_jars = [ctx.outputs.jar] | ||
| if java_jar: | ||
| full_jars.append(java_jar.jar) |
There was a problem hiding this comment.
Potentially we might also align this part as well: currently it would use the old order: Scala -> Java
I'm not sure if can come up with example which would require that, but at least both list would have the same order
| #$runner bazel build --extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error --all_incompatible_changes -- test/... | ||
| $runner bazel test "${test_output_flag}" --extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error -- test/... | ||
| $runner bazel test "${test_output_flag}" --extra_toolchains=//scala:minimal_direct_source_deps -- test/... | ||
| $runner bazel test "${test_output_flag}" --extra_toolchains=//scala:minimal_direct_source_deps --build_tag_filters=-no-strict-deps --test_tag_filters=-no-strict-deps -- test/... |
There was a problem hiding this comment.
I think this patch should do the job without changes to the testing script. We should be able to revert these changes and the tag from the test (however it would be worth it to add comment linking to the change in the toolchain)
diff --git a/scala/BUILD b/scala/BUILD
index 2f0b40b..ee98b80 100644
--- a/scala/BUILD
+++ b/scala/BUILD
@@ -29,6 +29,11 @@ scala_toolchain(
dependency_tracking_method = "ast",
strict_deps_mode = "error",
unused_dependency_checker_mode = "error",
+ # Exclude the shadowing test package from strict-deps to allow classpath-order coverage without disabling strict-deps globally in CI runs.
+ dependency_tracking_strict_deps_patterns = [
+ "", # keep default include
+ "-//test/src/main/scala/scalarules/test/java_classpath_order",
+ ],
)
[2 participants
Description
I have a scala_library with some java files where I would like to override the class for a particular dependency (i.e. something from @maven//).
In order for this to work, the Java file needs to be first on the classpath.
@rules_scala seems to be placing the generated java_library at the end of the CLASSPATH.
This change makes it so that it's first.
This is a copy of #1752 but with a test added