From a00fef2d437acfcf24c36354a295002771509891 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Thu, 30 Jan 2020 13:06:20 -0700 Subject: [PATCH 1/4] Get files helper function --- scala/private/phases/phase_compile.bzl | 31 +++++--------------------- scala/private/rule_impls.bzl | 7 ++++++ 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/scala/private/phases/phase_compile.bzl b/scala/private/phases/phase_compile.bzl index e98946899..e7381843a 100644 --- a/scala/private/phases/phase_compile.bzl +++ b/scala/private/phases/phase_compile.bzl @@ -13,6 +13,7 @@ load( "@io_bazel_rules_scala//scala/private:rule_impls.bzl", _compile_scala = "compile_scala", _expand_location = "expand_location", + _get_files_with_extension = "get_files_with_extension", ) load(":resources.bzl", _resource_paths = "paths") @@ -197,29 +198,17 @@ def _compile_or_empty( merged_provider = scala_compilation_provider, ) else: - in_srcjars = [ - f - for f in ctx.files.srcs - if f.basename.endswith(_srcjar_extension) - ] + in_srcjars = _get_files_with_extension(ctx, _srcjar_extension) all_srcjars = depset(in_srcjars, transitive = [srcjars]) - java_srcs = [ - f - for f in ctx.files.srcs - if f.basename.endswith(_java_extension) - ] + java_srcs = _get_files_with_extension(ctx, _java_extension) # We are not able to verify whether dependencies are used when compiling java sources # Thus we disable unused dependency checking when java sources are found if len(java_srcs) != 0: unused_dependency_checker_mode = "off" - sources = [ - f - for f in ctx.files.srcs - if f.basename.endswith(_scala_extension) - ] + java_srcs + sources = _get_files_with_extension(ctx, _scala_extension) + java_srcs _compile_scala( ctx, ctx.label, @@ -341,18 +330,10 @@ def _create_scala_compilation_provider(ctx, ijar, source_jar, deps_providers): def _pack_source_jar(ctx): # collect .scala sources and pack a source jar for Scala - scala_sources = [ - f - for f in ctx.files.srcs - if f.basename.endswith(_scala_extension) - ] + scala_sources = _get_files_with_extension(ctx, _scala_extension) # collect .srcjar files and pack them with the scala sources - bundled_source_jars = [ - f - for f in ctx.files.srcs - if f.basename.endswith(_srcjar_extension) - ] + bundled_source_jars = _get_files_with_extension(ctx, _srcjar_extension) scala_source_jar = java_common.pack_sources( ctx.actions, output_jar = ctx.outputs.jar, diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index c5747d1ca..1ac5c0674 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -23,6 +23,13 @@ load( ) load(":resources.bzl", _resource_paths = "paths") +def get_files_with_extension(ctx, extension): + return [ + f + for f in ctx.files.srcs + if f.basename.endswith(extension) + ] + def expand_location(ctx, flags): if hasattr(ctx.attr, "data"): data = ctx.attr.data From 72fe84cdf4f711021123a25d08b516f39dede297 Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Thu, 30 Jan 2020 13:22:21 -0700 Subject: [PATCH 2/4] Remove extra computation --- scala/private/phases/phase_compile.bzl | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/scala/private/phases/phase_compile.bzl b/scala/private/phases/phase_compile.bzl index e7381843a..9fd05e72d 100644 --- a/scala/private/phases/phase_compile.bzl +++ b/scala/private/phases/phase_compile.bzl @@ -198,17 +198,17 @@ def _compile_or_empty( merged_provider = scala_compilation_provider, ) else: + java_srcs = _get_files_with_extension(ctx, _java_extension) + scala_srcs = _get_files_with_extension(ctx, _scala_extension) in_srcjars = _get_files_with_extension(ctx, _srcjar_extension) all_srcjars = depset(in_srcjars, transitive = [srcjars]) - java_srcs = _get_files_with_extension(ctx, _java_extension) - # We are not able to verify whether dependencies are used when compiling java sources # Thus we disable unused dependency checking when java sources are found if len(java_srcs) != 0: unused_dependency_checker_mode = "off" - sources = _get_files_with_extension(ctx, _scala_extension) + java_srcs + sources = scala_srcs + java_srcs _compile_scala( ctx, ctx.label, @@ -247,7 +247,7 @@ def _compile_or_empty( # so set ijar == jar ijar = ctx.outputs.jar - source_jar = _pack_source_jar(ctx) + source_jar = _pack_source_jar(ctx, scala_srcs, in_srcjars) scala_compilation_provider = _create_scala_compilation_provider(ctx, ijar, source_jar, deps_providers) # compile the java now @@ -328,23 +328,16 @@ def _create_scala_compilation_provider(ctx, ijar, source_jar, deps_providers): runtime_deps = runtime_deps, ) -def _pack_source_jar(ctx): - # collect .scala sources and pack a source jar for Scala - scala_sources = _get_files_with_extension(ctx, _scala_extension) - - # collect .srcjar files and pack them with the scala sources - bundled_source_jars = _get_files_with_extension(ctx, _srcjar_extension) - scala_source_jar = java_common.pack_sources( +def _pack_source_jar(ctx, scala_srcs, in_srcjars): + return java_common.pack_sources( ctx.actions, output_jar = ctx.outputs.jar, - sources = scala_sources, - source_jars = bundled_source_jars, + sources = scala_srcs, + source_jars = in_srcjars, java_toolchain = find_java_toolchain(ctx, ctx.attr._java_toolchain), host_javabase = find_java_runtime_toolchain(ctx, ctx.attr._host_javabase), ) - return scala_source_jar - def _jacoco_offline_instrument(ctx, input_jar): if not ctx.configuration.coverage_enabled or not hasattr(ctx.attr, "_code_coverage_instrumentation_worker"): return _empty_coverage_struct From 08e2dbc0a8a33df5e36d49f1bd8587ebc7e1d58f Mon Sep 17 00:00:00 2001 From: Bor Kae Hwang Date: Thu, 30 Jan 2020 14:47:17 -0700 Subject: [PATCH 3/4] Move extension to shared file --- scala/private/phases/phase_compile.bzl | 9 +++------ scala/private/phases/phase_scalafmt.bzl | 7 ++++++- scala/private/rule_impls.bzl | 6 ++++++ 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/scala/private/phases/phase_compile.bzl b/scala/private/phases/phase_compile.bzl index 9fd05e72d..2a9bbb559 100644 --- a/scala/private/phases/phase_compile.bzl +++ b/scala/private/phases/phase_compile.bzl @@ -14,15 +14,12 @@ load( _compile_scala = "compile_scala", _expand_location = "expand_location", _get_files_with_extension = "get_files_with_extension", + _java_extension = "java_extension", + _scala_extension = "scala_extension", + _srcjar_extension = "srcjar_extension", ) load(":resources.bzl", _resource_paths = "paths") -_java_extension = ".java" - -_scala_extension = ".scala" - -_srcjar_extension = ".srcjar" - _empty_coverage_struct = struct( external = struct( replacements = {}, diff --git a/scala/private/phases/phase_scalafmt.bzl b/scala/private/phases/phase_scalafmt.bzl index 5e8284c35..284ab7db7 100644 --- a/scala/private/phases/phase_scalafmt.bzl +++ b/scala/private/phases/phase_scalafmt.bzl @@ -3,6 +3,11 @@ # # Outputs to format the scala files when it is explicitly specified # +load( + "@io_bazel_rules_scala//scala/private:rule_impls.bzl", + _scala_extension = "scala_extension", +) + def phase_scalafmt(ctx, p): if ctx.attr.format: manifest, files = _build_format(ctx) @@ -17,7 +22,7 @@ def _build_format(ctx): manifest_content = [] for src in ctx.files.srcs: # only format scala source files, not generated files - if src.path.endswith(".scala") and src.is_source: + if src.path.endswith(_scala_extension) and src.is_source: file = ctx.actions.declare_file("{}.fmt.output".format(src.short_path)) files.append(file) ctx.actions.run( diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 1ac5c0674..32902fe76 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -23,6 +23,12 @@ load( ) load(":resources.bzl", _resource_paths = "paths") +java_extension = ".java" + +scala_extension = ".scala" + +srcjar_extension = ".srcjar" + def get_files_with_extension(ctx, extension): return [ f From d6b5a36de321f3a6b530c9e567b6837bb8e4f49a Mon Sep 17 00:00:00 2001 From: borkaehw Date: Thu, 30 Jan 2020 22:25:08 -0700 Subject: [PATCH 4/4] Move functions to paths.bzl --- scala/private/paths.bzl | 12 ++++++++++++ scala/private/phases/phase_compile.bzl | 9 ++++++--- scala/private/phases/phase_scalafmt.bzl | 2 +- scala/private/rule_impls.bzl | 13 ------------- 4 files changed, 19 insertions(+), 17 deletions(-) create mode 100644 scala/private/paths.bzl diff --git a/scala/private/paths.bzl b/scala/private/paths.bzl new file mode 100644 index 000000000..09b916d4f --- /dev/null +++ b/scala/private/paths.bzl @@ -0,0 +1,12 @@ +java_extension = ".java" + +scala_extension = ".scala" + +srcjar_extension = ".srcjar" + +def get_files_with_extension(ctx, extension): + return [ + f + for f in ctx.files.srcs + if f.basename.endswith(extension) + ] diff --git a/scala/private/phases/phase_compile.bzl b/scala/private/phases/phase_compile.bzl index 2a9bbb559..1e730a574 100644 --- a/scala/private/phases/phase_compile.bzl +++ b/scala/private/phases/phase_compile.bzl @@ -10,14 +10,17 @@ load( _coverage_replacements_provider = "coverage_replacements_provider", ) load( - "@io_bazel_rules_scala//scala/private:rule_impls.bzl", - _compile_scala = "compile_scala", - _expand_location = "expand_location", + "@io_bazel_rules_scala//scala/private:paths.bzl", _get_files_with_extension = "get_files_with_extension", _java_extension = "java_extension", _scala_extension = "scala_extension", _srcjar_extension = "srcjar_extension", ) +load( + "@io_bazel_rules_scala//scala/private:rule_impls.bzl", + _compile_scala = "compile_scala", + _expand_location = "expand_location", +) load(":resources.bzl", _resource_paths = "paths") _empty_coverage_struct = struct( diff --git a/scala/private/phases/phase_scalafmt.bzl b/scala/private/phases/phase_scalafmt.bzl index 284ab7db7..a42410980 100644 --- a/scala/private/phases/phase_scalafmt.bzl +++ b/scala/private/phases/phase_scalafmt.bzl @@ -4,7 +4,7 @@ # Outputs to format the scala files when it is explicitly specified # load( - "@io_bazel_rules_scala//scala/private:rule_impls.bzl", + "@io_bazel_rules_scala//scala/private:paths.bzl", _scala_extension = "scala_extension", ) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 32902fe76..c5747d1ca 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -23,19 +23,6 @@ load( ) load(":resources.bzl", _resource_paths = "paths") -java_extension = ".java" - -scala_extension = ".scala" - -srcjar_extension = ".srcjar" - -def get_files_with_extension(ctx, extension): - return [ - f - for f in ctx.files.srcs - if f.basename.endswith(extension) - ] - def expand_location(ctx, flags): if hasattr(ctx.attr, "data"): data = ctx.attr.data