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.

Sign up for GitHub

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

Support not bundling SemanticDB files in the output JAR #50

Merged
merged 3 commits into from
Aug 27, 2024
Merged

Support not bundling SemanticDB files in the output JAR #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a698dd2
Support not bundling SemanticDB files in the output JAR
Aug 19, 2024
edf4124
fixup! Don't run the SemanticDB phase in the bootstrap compiler
Aug 27, 2024
49479f7
fixup! Rename scalacopts to common_scalacopts in phase_zinc_compile
Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/newdocs/phases.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,4 @@ _NOTE: This is a work in progress_
Bazel scala annex allows users to customize `scala_binary`, `scala_library`, and `scala_test` by adding/replacing [phases](#phases).
Each of these rules has a corresponding `make_<rule>(*extras)` method which takes as arguments a list of of extra config items.

For an example of this in action, see [/rules/scala_with_scalafmt.bzl](/rules/scala_with_scalafmt.bzl) and [/rules/scalafmt/ext.blz](/rules/scalafmt/ext.bzl)
For an example of this in action, see [/rules/scala_with_scalafmt.bzl](/rules/scala_with_scalafmt.bzl) and [/rules/scalafmt/ext.blz](/rules/scalafmt/ext.bzl)
25 changes: 14 additions & 11 deletions rules/private/phases.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ load(":phases/phase_library_defaultinfo.bzl", _phase_library_defaultinfo = "phas
load(":phases/phase_noop.bzl", _phase_noop = "phase_noop")
load(":phases/phase_resources.bzl", _phase_resources = "phase_resources")
load(":phases/phase_scalafmt_nondefault_outputs.bzl", _phase_scalafmt_nondefault_outputs = "phase_scalafmt_nondefault_outputs")
load(":phases/phase_semanticdb.bzl", _phase_semanticdb = "phase_semanticdb")
load(":phases/phase_singlejar.bzl", _phase_singlejar = "phase_singlejar")
load(":phases/phase_test_launcher.bzl", _phase_test_launcher = "phase_test_launcher")
load(":phases/phase_zinc_compile.bzl", _phase_zinc_compile = "phase_zinc_compile")
Expand All @@ -20,6 +21,10 @@ adjust_phases = _adjust_phases

run_phases = _run_phases

phase_binary_deployjar = _phase_binary_deployjar

phase_binary_launcher = _phase_binary_launcher

phase_bootstrap_compile = _phase_bootstrap_compile

phase_classpaths = _phase_classpaths
Expand All @@ -30,24 +35,22 @@ phase_coverage_jacoco = _phase_coverage_jacoco

phase_ijinfo = _phase_ijinfo

phase_noop = _phase_noop
phase_javainfo = _phase_javainfo
Copy link

Choose a reason for hiding this comment

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

Looks like these were sorted alphabetically. Is that what is going on here?

Copy link
Author

Choose a reason for hiding this comment

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

That's right.


phase_resources = _phase_resources
phase_library_defaultinfo = _phase_library_defaultinfo

phase_zinc_compile = _phase_zinc_compile
phase_noop = _phase_noop

phase_zinc_depscheck = _phase_zinc_depscheck
phase_resources = _phase_resources

phase_javainfo = _phase_javainfo
phase_scalafmt_nondefault_outputs = _phase_scalafmt_nondefault_outputs

phase_library_defaultinfo = _phase_library_defaultinfo
phase_semanticdb = _phase_semanticdb

phase_singlejar = _phase_singlejar

phase_binary_deployjar = _phase_binary_deployjar

phase_binary_launcher = _phase_binary_launcher
phase_test_launcher = _phase_test_launcher

phase_scalafmt_nondefault_outputs = _phase_scalafmt_nondefault_outputs
phase_zinc_compile = _phase_zinc_compile

phase_test_launcher = _phase_test_launcher
phase_zinc_depscheck = _phase_zinc_depscheck
42 changes: 42 additions & 0 deletions rules/private/phases/phase_semanticdb.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
load("@bazel_skylib//lib:paths.bzl", "paths")
load("@rules_scala_annex//rules:providers.bzl", _ScalaConfiguration = "ScalaConfiguration")

#
# PHASE: semanticdb
#
# Configures the compiler to output SemanticDB metadata. Note that this phase won't work without the
# SemanticDB compiler plugin being enabled.
#
def phase_semanticdb(ctx, g):
scala_configuration = ctx.attr.scala[_ScalaConfiguration]

if scala_configuration.semanticdb_bundle:
return struct(outputs = [], scalacopts = [])

outputs = []
semanticdb_directory = paths.join("_semanticdb/", ctx.label.name)
Copy link

Choose a reason for hiding this comment

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

Is it possible for the name to contain characters that are illegal to use as file names?

Copy link
Author

@jadenPete jadenPete Aug 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

From Bazel's documentation on labels:

Allowed characters in package names are the lowercase letters a through z, the uppercase letters A through Z, the digits 0 through 9, the characters ! \"#$%&'()*+,-.;<=>?@[]^_`{|} (yes, there's a space character in there!), and of course forward slash / (since it's the directory separator).

I assume legal target names can't include slashes because that'd make the package and name indistinguishable.

I think the only illegal characters in Unix filenames are / and the null character, both of which Bazel doesn't allow, so no, it shouldn't be possible. Windows is a different story, but I think it's highly unlikely that special characters other than - and _ will be used anyways, and even if they are, an error should be reported.

semanticdb_target_root = paths.join(paths.dirname(ctx.outputs.jar.path), semanticdb_directory)

for source in ctx.files.srcs:
if source.extension == "scala":
output_filename = paths.join(
semanticdb_directory,
"META-INF",
"semanticdb",
"{}.semanticdb".format(source.path),
)

outputs.append(ctx.actions.declare_file(output_filename))

if scala_configuration.version.startswith("2"):
Copy link

Choose a reason for hiding this comment

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

I'm not sure how much we care about this, but how does this work for things like Scalajs or Scala Native?

Copy link
Author

Choose a reason for hiding this comment

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

Hmm, it depends on what version is set to. The only other place in which we use it is phase_bootstrap_compile, where we also check if we're dealing with Scala 2 or 3.

Now that I think about it, is SemanticDB even a concept in Scala.js and Scala Native? I think both Scala.js and Scala Native use the same compiler, so it should be. For now, I'll leave this as is and dig into it further if folks want Scala.js and/or Scala Native support (assuming that's ok with you).

scalacopts = [
"-P:semanticdb:failures:error",
"-P:semanticdb:targetroot:{}".format(semanticdb_target_root),
]
else:
scalacopts = [
"-semanticdb-target:{}".format(semanticdb_target_root),
"-Ysemanticdb",
]

return struct(outputs = outputs, scalacopts = scalacopts)
12 changes: 10 additions & 2 deletions rules/private/phases/phase_zinc_compile.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def phase_zinc_compile(ctx, g):
]

zincs = [dep[_ZincInfo] for dep in ctx.attr.deps if _ZincInfo in dep]
common_scalacopts = ctx.attr.scalacopts + g.semanticdb.scalacopts

args = ctx.actions.args()

Expand All @@ -45,7 +46,7 @@ def phase_zinc_compile(ctx, g):
args.add_all("--compiler_classpath", g.classpaths.compiler)
args.add_all("--classpath", g.classpaths.compile)
args.add_all(scala_configuration.global_scalacopts, format_each = "--compiler_option=%s")
args.add_all(ctx.attr.scalacopts, format_each = "--compiler_option=%s")
args.add_all(common_scalacopts, format_each = "--compiler_option=%s")
args.add_all(javacopts, format_each = "--java_compiler_option=%s")
args.add(ctx.label, format = "--label=%s")
args.add("--main_manifest", mains_file)
Expand Down Expand Up @@ -73,7 +74,14 @@ def phase_zinc_compile(ctx, g):
] + [zinc.deps_files for zinc in zincs],
)

outputs = [g.classpaths.jar, mains_file, analysis_store, analysis_store_text, used, tmp]
outputs = [
g.classpaths.jar,
mains_file,
analysis_store,
analysis_store_text,
used,
tmp,
] + g.semanticdb.outputs

execution_requirements_tags = {
"supports-multiplex-workers": "1",
Expand Down
22 changes: 14 additions & 8 deletions rules/providers.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@ load(
ScalaConfiguration = provider(
doc = "Scala compile-time and runtime configuration",
fields = {
"version": "The Scala full version.",
"compiler_classpath": "The compiler classpath.",
"runtime_classpath": "The runtime classpath.",
"global_plugins": "Globally enabled compiler plugins",
"global_scalacopts": "Globally enabled compiler options",
"runtime_classpath": "The runtime classpath.",
"semanticdb_bundle": "Whether to bundle SemanticDB files in the resulting JAR. Note that in Scala 2, this requires the SemanticDB compiler plugin.",
"use_ijar": "Whether to use ijars for this Scala compiler",
"version": "The Scala full version.",
},
)

Expand All @@ -24,18 +25,14 @@ def _declare_scala_configuration_implementation(ctx):
global_plugins = ctx.attr.global_plugins,
global_scalacopts = ctx.attr.global_scalacopts,
runtime_classpath = ctx.attr.runtime_classpath,
version = ctx.attr.version,
semanticdb_bundle = ctx.attr.semanticdb_bundle,
use_ijar = ctx.attr.use_ijar,
version = ctx.attr.version,
),
]

declare_scala_configuration = rule(
attrs = {
"version": attr.string(mandatory = True),
"runtime_classpath": attr.label_list(
mandatory = True,
providers = [JavaInfo],
),
"compiler_classpath": attr.label_list(
mandatory = True,
providers = [JavaInfo],
Expand All @@ -47,6 +44,15 @@ declare_scala_configuration = rule(
"global_scalacopts": attr.string_list(
doc = "Scalac options that will always be enabled.",
),
"runtime_classpath": attr.label_list(
mandatory = True,
providers = [JavaInfo],
),
"semanticdb_bundle": attr.bool(
default = True,
doc = "Whether to bundle SemanticDB files in the resulting JAR. Note that in Scala 2, this requires the SemanticDB compiler plugin.",
),
"version": attr.string(mandatory = True),
},
doc = "Creates a `ScalaConfiguration`.",
implementation = _declare_scala_configuration_implementation,
Expand Down
54 changes: 31 additions & 23 deletions rules/scala.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,65 +484,79 @@ Generates Scaladocs.

configure_bootstrap_scala = rule(
attrs = {
"version": attr.string(mandatory = True),
"compiler_classpath": attr.label_list(
mandatory = True,
providers = [JavaInfo],
),
"runtime_classpath": attr.label_list(
mandatory = True,
providers = [JavaInfo],
),
"global_plugins": attr.label_list(
doc = "Scalac plugins that will always be enabled.",
providers = [JavaInfo],
),
"global_scalacopts": attr.string_list(
doc = "Scalac options that will always be enabled.",
),
"runtime_classpath": attr.label_list(
mandatory = True,
providers = [JavaInfo],
),
"semanticdb_bundle": attr.bool(
default = True,
doc = "Whether to bundle SemanticDB files in the resulting JAR. Note that in Scala 2, this requires the SemanticDB compiler plugin.",
Copy link

Choose a reason for hiding this comment

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

Should we provide some default for Scala 2? Is that even possible?

Copy link

Choose a reason for hiding this comment

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

Would we even want to do that if we could?

Copy link
Author

Choose a reason for hiding this comment

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

What do you mean by this? The default is set to true, which matches the default for the compiler plugin (bundling SemanticDB files in the JAR).

),
"use_ijar": attr.bool(
doc = "Whether to use ijars for this compiler.",
default = True,
),
"version": attr.string(mandatory = True),
},
implementation = _configure_bootstrap_scala_implementation,
)

_configure_zinc_scala = rule(
attrs = {
"version": attr.string(mandatory = True),
"runtime_classpath": attr.label_list(
"compiler_bridge": attr.label(
allow_single_file = True,
mandatory = True,
providers = [JavaInfo],
),
"compiler_classpath": attr.label_list(
mandatory = True,
providers = [JavaInfo],
),
"compiler_bridge": attr.label(
allow_single_file = True,
mandatory = True,
),
"deps_direct": attr.string(default = "error"),
"deps_used": attr.string(default = "error"),
"global_plugins": attr.label_list(
doc = "Scalac plugins that will always be enabled.",
providers = [JavaInfo],
),
"global_scalacopts": attr.string_list(
doc = "Scalac options that will always be enabled.",
),
"incremental": attr.bool(
doc = "Whether Zinc's incremental compilation will be available for this Zinc compiler. If True, this requires additional configuration to use incremental compilation.",
default = False,
),
"log_level": attr.string(
doc = "Compiler log level",
default = "warn",
),
"runtime_classpath": attr.label_list(
mandatory = True,
providers = [JavaInfo],
),
"semanticdb_bundle": attr.bool(
default = True,
doc = "Whether to bundle SemanticDB files in the resulting JAR. Note that in Scala 2, this requires the SemanticDB compiler plugin.",
),
"use_ijar": attr.bool(
doc = "Whether to use ijars for this compiler.",
default = True,
),
"deps_direct": attr.string(default = "error"),
"deps_used": attr.string(default = "error"),
"incremental": attr.bool(
doc = "Whether Zinc's incremental compilation will be available for this Zinc compiler. If True, this requires additional configuration to use incremental compilation.",
default = False,
"version": attr.string(mandatory = True),
"_code_coverage_instrumentation_worker": attr.label(
default = "@rules_scala_annex//src/main/scala/higherkindness/rules_scala/workers/jacoco/instrumenter",
allow_files = True,
executable = True,
cfg = "host",
),
"_compile_worker": attr.label(
default = "@rules_scala_annex//src/main/scala/higherkindness/rules_scala/workers/zinc/compile",
Expand All @@ -556,12 +570,6 @@ _configure_zinc_scala = rule(
executable = True,
cfg = "host",
),
"_code_coverage_instrumentation_worker": attr.label(
default = "@rules_scala_annex//src/main/scala/higherkindness/rules_scala/workers/jacoco/instrumenter",
allow_files = True,
executable = True,
cfg = "host",
),
},
implementation = _configure_zinc_scala_implementation,
)
Expand Down
8 changes: 6 additions & 2 deletions rules/scala/private/provider.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ load(
load(
"//rules/private:phases.bzl",
_phase_bootstrap_compile = "phase_bootstrap_compile",
_phase_semanticdb = "phase_semanticdb",
_phase_zinc_compile = "phase_zinc_compile",
_phase_zinc_depscheck = "phase_zinc_depscheck",
)
Expand All @@ -20,8 +21,9 @@ def configure_bootstrap_scala_implementation(ctx):
global_plugins = ctx.attr.global_plugins,
global_scalacopts = ctx.attr.global_scalacopts,
runtime_classpath = ctx.attr.runtime_classpath,
version = ctx.attr.version,
semanticdb_bundle = ctx.attr.semanticdb_bundle,
use_ijar = ctx.attr.use_ijar,
version = ctx.attr.version,
),
_ScalaRulePhase(
phases = [
Expand All @@ -37,8 +39,9 @@ def configure_zinc_scala_implementation(ctx):
global_plugins = ctx.attr.global_plugins,
global_scalacopts = ctx.attr.global_scalacopts,
runtime_classpath = ctx.attr.runtime_classpath,
version = ctx.attr.version,
semanticdb_bundle = ctx.attr.semanticdb_bundle,
use_ijar = ctx.attr.use_ijar,
version = ctx.attr.version,
),
_CodeCoverageConfiguration(
instrumentation_worker = ctx.attr._code_coverage_instrumentation_worker,
Expand All @@ -56,6 +59,7 @@ def configure_zinc_scala_implementation(ctx):
),
_ScalaRulePhase(
phases = [
("+", "classpaths", "semanticdb", _phase_semanticdb),
("=", "compile", "compile", _phase_zinc_compile),
("+", "compile", "depscheck", _phase_zinc_depscheck),
],
Expand Down
38 changes: 36 additions & 2 deletions tests/annex_test_install.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"dependency_tree": {
"__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL",
"__INPUT_ARTIFACTS_HASH": -1203700734,
"__RESOLVED_ARTIFACTS_HASH": 1444768052,
"__INPUT_ARTIFACTS_HASH": 13201521,
"__RESOLVED_ARTIFACTS_HASH": 787994482,
"conflict_resolution": {
"com.google.protobuf:protobuf-java:3.11.4": "com.google.protobuf:protobuf-java:3.15.8",
"com.thesamet.scalapb:lenses_2.13:0.9.0": "com.thesamet.scalapb:lenses_2.13:0.11.4",
Expand Down Expand Up @@ -605,6 +605,40 @@
"sha256": "1e144d19da246a2401b4c7d1254be4e9b599f2f03a55bb44e2c23e9a3ddcbb50",
"url": "https://repo.maven.apache.org/maven2/org/scalactic/scalactic_2.13/3.2.19/scalactic_2.13-3.2.19-sources.jar"
},
{
"coord": "org.scalameta:semanticdb-scalac_2.13.14:4.9.9",
"dependencies": [
"org.scala-lang:scala-library:2.13.14"
],
"directDependencies": [
"org.scala-lang:scala-library:2.13.14"
],
"file": "v1/https/repo.maven.apache.org/maven2/org/scalameta/semanticdb-scalac_2.13.14/4.9.9/semanticdb-scalac_2.13.14-4.9.9.jar",
"mirror_urls": [
"https://repo.maven.apache.org/maven2/org/scalameta/semanticdb-scalac_2.13.14/4.9.9/semanticdb-scalac_2.13.14-4.9.9.jar",
"https://maven-central.storage-download.googleapis.com/maven2/org/scalameta/semanticdb-scalac_2.13.14/4.9.9/semanticdb-scalac_2.13.14-4.9.9.jar",
"https://mirror.bazel.build/repo1.maven.org/maven2/org/scalameta/semanticdb-scalac_2.13.14/4.9.9/semanticdb-scalac_2.13.14-4.9.9.jar"
],
"sha256": "c5269b5b7264fc5082357acdb4d9cf6790f49b4195f127878bd4f46659e3c52b",
"url": "https://repo.maven.apache.org/maven2/org/scalameta/semanticdb-scalac_2.13.14/4.9.9/semanticdb-scalac_2.13.14-4.9.9.jar"
},
{
"coord": "org.scalameta:semanticdb-scalac_2.13.14:jar:sources:4.9.9",
"dependencies": [
"org.scala-lang:scala-library:jar:sources:2.13.14"
],
"directDependencies": [
"org.scala-lang:scala-library:jar:sources:2.13.14"
],
"file": "v1/https/repo.maven.apache.org/maven2/org/scalameta/semanticdb-scalac_2.13.14/4.9.9/semanticdb-scalac_2.13.14-4.9.9-sources.jar",
"mirror_urls": [
"https://repo.maven.apache.org/maven2/org/scalameta/semanticdb-scalac_2.13.14/4.9.9/semanticdb-scalac_2.13.14-4.9.9-sources.jar",
"https://maven-central.storage-download.googleapis.com/maven2/org/scalameta/semanticdb-scalac_2.13.14/4.9.9/semanticdb-scalac_2.13.14-4.9.9-sources.jar",
"https://mirror.bazel.build/repo1.maven.org/maven2/org/scalameta/semanticdb-scalac_2.13.14/4.9.9/semanticdb-scalac_2.13.14-4.9.9-sources.jar"
],
"sha256": "feb2dafb9d70a854d38886d32164614e8e251c071610a1964a18bfa6cf4039db",
"url": "https://repo.maven.apache.org/maven2/org/scalameta/semanticdb-scalac_2.13.14/4.9.9/semanticdb-scalac_2.13.14-4.9.9-sources.jar"
},
{
"coord": "org.scalatest:scalatest-compatible:3.2.19",
"dependencies": [],
Expand Down
1 change: 1 addition & 0 deletions tests/plugins/semanticdb/A.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class A
1 change: 1 addition & 0 deletions tests/plugins/semanticdb/B.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class B
Loading
Loading