diff --git a/scala/private/phases/phase_compile.bzl b/scala/private/phases/phase_compile.bzl index 2362823a1..d5b72ea3c 100644 --- a/scala/private/phases/phase_compile.bzl +++ b/scala/private/phases/phase_compile.bzl @@ -3,7 +3,6 @@ # # DOCUMENT THIS # -load("@bazel_skylib//lib:paths.bzl", _paths = "paths") load("@bazel_skylib//lib:dicts.bzl", "dicts") load("@bazel_tools//tools/jdk:toolchain_utils.bzl", "find_java_runtime_toolchain", "find_java_toolchain") load( @@ -12,10 +11,10 @@ load( ) load( "@io_bazel_rules_scala//scala/private:rule_impls.bzl", - _adjust_resources_path_by_default_prefixes = "adjust_resources_path_by_default_prefixes", _compile_scala = "compile_scala", _expand_location = "expand_location", ) +load(":resources.bzl", _resource_paths = "paths") _java_extension = ".java" @@ -491,38 +490,10 @@ def _try_to_compile_java_jar( java_compilation_provider = provider, ) -def _adjust_resources_path(resource, resource_strip_prefix): - if resource_strip_prefix: - return _adjust_resources_path_by_strip_prefix(resource, resource_strip_prefix) - else: - return _adjust_resources_path_by_default_prefixes(resource.path) - def _add_resources_cmd(ctx): - res_cmd = [] - for f in ctx.files.resources: - c_dir, res_path = _adjust_resources_path( - f, - ctx.attr.resource_strip_prefix, - ) - target_path = res_path - if target_path[0] == "/": - target_path = target_path[1:] - line = "{target_path}={c_dir}{res_path}\n".format( - res_path = res_path, - target_path = target_path, - c_dir = c_dir, - ) - res_cmd.extend([line]) - return "".join(res_cmd) - -def _adjust_resources_path_by_strip_prefix(resource, resource_strip_prefix): - path = resource.path - prefix = _paths.join(resource.owner.workspace_root, resource_strip_prefix) - if not path.startswith(prefix): - fail("Resource file %s is not under the specified prefix %s to strip" % (path, prefix)) - - clean_path = path[len(prefix):] - return prefix, clean_path + paths = _resource_paths(ctx.files.resources, ctx.attr.resource_strip_prefix) + lines = ["{target}={source}\n".format(target = p[0], source = p[1]) for p in paths] + return "".join(lines) def _collect_java_providers_of(deps): providers = [] diff --git a/scala/private/resources.bzl b/scala/private/resources.bzl new file mode 100644 index 000000000..ff59e0026 --- /dev/null +++ b/scala/private/resources.bzl @@ -0,0 +1,49 @@ +load("@bazel_skylib//lib:paths.bzl", _paths = "paths") + +def paths(resources, resource_strip_prefix): + """Return a list of path tuples (target, source) where: + target - is a path in the archive (with given prefix stripped off) + source - is an absolute path of the resource file + + Tuple ordering is aligned with zipper format ie zip_path=file + + Args: + resources: list of file objects + resource_strip_prefix: string to strip from resource path + """ + return [(_target_path(resource, resource_strip_prefix), resource.path) for resource in resources] + +def _target_path(resource, resource_strip_prefix): + path = _target_path_by_strip_prefix(resource, resource_strip_prefix) if resource_strip_prefix else _target_path_by_default_prefixes(resource.path) + return _strip_prefix(path, "/") + +def _target_path_by_strip_prefix(resource, resource_strip_prefix): + # Start from absolute resource path and then strip roots so we get to correct short path + # resource.short_path sometimes give weird results ie '../' prefix + path = resource.path + path = _strip_prefix(path, resource.owner.workspace_root + "/") + path = _strip_prefix(path, resource.root.path + "/") + + # proto_library translates strip_import_prefix to proto_source_root which includes root so we have to strip it + prefix = _strip_prefix(resource_strip_prefix, resource.root.path + "/") + if not path.startswith(prefix): + fail("Resource file %s is not under the specified prefix %s to strip" % (path, prefix)) + return path[len(prefix):] + +def _target_path_by_default_prefixes(path): + # Here we are looking to find out the offset of this resource inside + # any resources folder. We want to return the root to the resources folder + # and then the sub path inside it + dir_1, dir_2, rel_path = path.partition("resources") + if rel_path: + return rel_path + + # The same as the above but just looking for java + (dir_1, dir_2, rel_path) = path.partition("java") + if rel_path: + return rel_path + + return path + +def _strip_prefix(path, prefix): + return path[len(prefix):] if path.startswith(prefix) else path diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 15625830c..b4d0539c7 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -21,21 +21,7 @@ load( ":common.bzl", _collect_plugin_paths = "collect_plugin_paths", ) - -def adjust_resources_path_by_default_prefixes(path): - # Here we are looking to find out the offset of this resource inside - # any resources folder. We want to return the root to the resources folder - # and then the sub path inside it - dir_1, dir_2, rel_path = path.partition("resources") - if rel_path: - return dir_1 + dir_2, rel_path - - # The same as the above but just looking for java - (dir_1, dir_2, rel_path) = path.partition("java") - if rel_path: - return dir_1 + dir_2, rel_path - - return "", path +load(":resources.bzl", _resource_paths = "paths") def expand_location(ctx, flags): if hasattr(ctx.attr, "data"): @@ -150,6 +136,7 @@ CurrentTarget: {current_target} toolchain = ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"] scalacopts = [ctx.expand_location(v, input_plugins) for v in toolchain.scalacopts + in_scalacopts] + resource_paths = _resource_paths(resources, resource_strip_prefix) scalac_args = """ Classpath: {cp} @@ -160,11 +147,9 @@ Manifest: {manifest} Plugins: {plugin_arg} PrintCompileTime: {print_compile_time} ExpectJavaOutput: {expect_java_output} -ResourceDests: {resource_dest} +ResourceTargets: {resource_targets} +ResourceSources: {resource_sources} ResourceJars: {resource_jars} -ResourceSrcs: {resource_src} -ResourceShortPaths: {resource_short_paths} -ResourceStripPrefix: {resource_strip_prefix} ScalacOpts: {scala_opts} SourceJars: {srcjars} DependencyAnalyzerMode: {dependency_analyzer_mode} @@ -182,13 +167,8 @@ StatsfileOutput: {statsfile_output} files = _join_path(sources), srcjars = _join_path(all_srcjars.to_list()), # the resource paths need to be aligned in order - resource_src = ",".join([f.path for f in resources]), - resource_short_paths = ",".join([f.short_path for f in resources]), - resource_dest = ",".join([ - adjust_resources_path_by_default_prefixes(f.short_path)[1] - for f in resources - ]), - resource_strip_prefix = resource_strip_prefix, + resource_targets = ",".join([p[0] for p in resource_paths]), + resource_sources = ",".join([p[1] for p in resource_paths]), resource_jars = _join_path(resource_jars), dependency_analyzer_mode = dependency_analyzer_mode, unused_dependency_checker_mode = unused_dependency_checker_mode, diff --git a/scala_proto/private/scalapb_aspect.bzl b/scala_proto/private/scalapb_aspect.bzl index 32b8f6469..0d51cfb65 100644 --- a/scala_proto/private/scalapb_aspect.bzl +++ b/scala_proto/private/scalapb_aspect.bzl @@ -51,7 +51,9 @@ def _compile_scala( output, scalapb_jar, deps_java_info, - implicit_deps): + implicit_deps, + resources, + resource_strip_prefix): manifest = ctx.actions.declare_file( label.name + "_MANIFEST.MF", sibling = scalapb_jar, @@ -78,8 +80,8 @@ def _compile_scala( all_srcjars = depset([scalapb_jar]), transitive_compile_jars = merged_deps.transitive_compile_time_jars, plugins = [], - resource_strip_prefix = "", - resources = [], + resource_strip_prefix = resource_strip_prefix, + resources = resources, resource_jars = [], labels = {}, in_scalacopts = [], @@ -193,6 +195,8 @@ def _scalapb_aspect_impl(target, ctx): scalapb_file, deps, imps, + compile_protos, + "" if target_ti.proto_source_root == "." else target_ti.proto_source_root, ) else: # this target is only an aggregation target diff --git a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java b/src/java/io/bazel/rulesscala/scalac/CompileOptions.java index 471dc12d6..464c159bb 100644 --- a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java +++ b/src/java/io/bazel/rulesscala/scalac/CompileOptions.java @@ -1,5 +1,6 @@ package io.bazel.rulesscala.scalac; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -15,8 +16,7 @@ public class CompileOptions { public final String[] files; public final String[] sourceJars; public final String[] javaFiles; - public final Map resourceFiles; - public final String resourceStripPrefix; + public final List resourceFiles; public final String[] resourceJars; public final String[] classpathResourceFiles; public final String[] directJars; @@ -50,7 +50,6 @@ public CompileOptions(List args) { sourceJars = getCommaList(argMap, "SourceJars"); resourceFiles = getResources(argMap); - resourceStripPrefix = getOrEmpty(argMap, "ResourceStripPrefix"); resourceJars = getCommaList(argMap, "ResourceJars"); classpathResourceFiles = getCommaList(argMap, "ClasspathResourceSrcs"); @@ -67,29 +66,21 @@ public CompileOptions(List args) { statsfile = getOrError(argMap, "StatsfileOutput", "Missing required arg StatsfileOutput"); } - private static Map getResources(Map args) { - String[] keys = getCommaList(args, "ResourceSrcs"); - String[] dests = getCommaList(args, "ResourceDests"); - String[] shortPaths = getCommaList(args, "ResourceShortPaths"); + private static List getResources(Map args) { + String[] targets = getCommaList(args, "ResourceTargets"); + String[] sources = getCommaList(args, "ResourceSources"); - if (keys.length != dests.length) + if (targets.length != sources.length) throw new RuntimeException( String.format( - "mismatch in resources: keys: %s dests: %s", - getOrEmpty(args, "ResourceSrcs"), getOrEmpty(args, "ResourceDests"))); + "mismatch in resources: targets: %s sources: %s", + getOrEmpty(args, "ResourceTargets"), getOrEmpty(args, "ResourceSources"))); - if (keys.length != shortPaths.length) - throw new RuntimeException( - String.format( - "mismatch in resources: keys: %s shortPaths: %s", - getOrEmpty(args, "ResourceSrcs"), getOrEmpty(args, "ResourceShortPaths"))); - - HashMap res = new HashMap(); - for (int idx = 0; idx < keys.length; idx++) { - Resource resource = new Resource(dests[idx], shortPaths[idx]); - res.put(keys[idx], resource); + List resources = new ArrayList(); + for (int idx = 0; idx < targets.length; idx++) { + resources.add(new Resource(targets[idx], sources[idx])); } - return res; + return resources; } private static HashMap buildArgMap(List lines) { diff --git a/src/java/io/bazel/rulesscala/scalac/Resource.java b/src/java/io/bazel/rulesscala/scalac/Resource.java index a8db9dbc3..6c0b68cc4 100644 --- a/src/java/io/bazel/rulesscala/scalac/Resource.java +++ b/src/java/io/bazel/rulesscala/scalac/Resource.java @@ -1,11 +1,11 @@ package io.bazel.rulesscala.scalac; public class Resource { - public final String destination; - public final String shortPath; + public final String target; + public final String source; - public Resource(String destination, String shortPath) { - this.destination = destination; - this.shortPath = shortPath; + public Resource(String target, String source) { + this.target = target; + this.source = source; } } diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index 9e43ce639..76f06c06a 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -70,7 +70,7 @@ public void processRequest(List args) throws Exception { } /** Copy the resources */ - copyResources(ops.resourceFiles, ops.resourceStripPrefix, tmpPath); + copyResources(ops.resourceFiles, tmpPath); /** Extract and copy resources from resource jars */ copyResourceJars(ops.resourceJars, tmpPath); @@ -268,44 +268,11 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) } } - private static void copyResources( - Map resources, String resourceStripPrefix, Path dest) throws IOException { - for (Entry e : resources.entrySet()) { - Path source = Paths.get(e.getKey()); - Resource resource = e.getValue(); - Path shortPath = Paths.get(resource.shortPath); - String dstr; - // Check if we need to modify resource destination path - if (!"".equals(resourceStripPrefix)) { - /** - * NOTE: We are not using the Resource Hash Value as the destination path when - * `resource_strip_prefix` present. The path in the hash value is computed by the - * `_adjust_resources_path` in `scala.bzl`. These are the default paths, ie, path that are - * automatically computed when there is no `resource_strip_prefix` present. But when - * `resource_strip_prefix` is present, we need to strip the prefix from the Source Path and - * use that as the new destination path Refer Bazel -> BazelJavaRuleClasses.java#L227 for - * details - */ - dstr = getResourcePath(shortPath, resourceStripPrefix); - } else { - dstr = resource.destination; - } - - if (dstr.charAt(0) == '/') { - // we don't want to copy to an absolute destination - dstr = dstr.substring(1); - } - if (dstr.startsWith("../")) { - // paths to external repositories, for some reason, start with a leading ../ - // we don't want to copy the resource out of our temporary directory, so - // instead we replace ../ with external/ - // since "external" is a bit of reserved directory in bazel for these kinds - // of purposes, we don't expect a collision in the paths. - dstr = "external" + dstr.substring(2); - } - Path target = dest.resolve(dstr); - File tfile = target.getParent().toFile(); - tfile.mkdirs(); + private static void copyResources(List resources, Path dest) throws IOException { + for (Resource r : resources) { + Path source = Paths.get(r.source); + Path target = dest.resolve(r.target); + target.getParent().toFile().mkdirs(); Files.copy(source, target); } } @@ -328,24 +295,6 @@ private static void copyClasspathResourcesToRoot(String[] classpathResourceFiles } } - private static String getResourcePath(Path source, String resourceStripPrefix) - throws RuntimeException { - String sourcePath = source.toString(); - // convert strip prefix to a Path first and back to handle different file systems - String resourceStripPrefixPath = Paths.get(resourceStripPrefix).toString(); - // check if the Resource file is under the specified prefix to strip - if (!sourcePath.startsWith(resourceStripPrefixPath)) { - // Resource File is not under the specified prefix to strip - throw new RuntimeException( - "Resource File " - + sourcePath - + " is not under the specified strip prefix " - + resourceStripPrefix); - } - String newResPath = sourcePath.substring(resourceStripPrefix.length()); - return newResPath; - } - private static void copyResourceJars(String[] resourceJars, Path dest) throws IOException { for (String jarPath : resourceJars) { extractJar(jarPath, dest.toString(), null); diff --git a/test/proto/BUILD b/test/proto/BUILD index a4614bb57..b8d41b41c 100644 --- a/test/proto/BUILD +++ b/test/proto/BUILD @@ -167,3 +167,79 @@ scala_test( ":test_external_dep", ], ) + +proto_library( + name = "standalone_proto", + srcs = ["standalone.proto"], +) + +proto_library( + name = "standalone_proto_strip_import_prefix_partial", + srcs = ["standalone.proto"], + strip_import_prefix = "/test", +) + +proto_library( + name = "standalone_proto_strip_import_prefix_package", + srcs = ["standalone.proto"], + strip_import_prefix = "/" + package_name(), +) + +proto_library( + name = "standalone_proto_with_import_prefix", + srcs = ["standalone.proto"], + import_prefix = "prefix", +) + +proto_library( + name = "standalone_proto_with_custom_prefix", + srcs = ["standalone.proto"], + import_prefix = "prefix", + strip_import_prefix = "/test", +) + +proto_library( + name = "nested_proto", + srcs = ["some/path/nested.proto"], +) + +proto_library( + name = "nested_proto_strip_import_prefix_relative", + srcs = ["some/path/nested.proto"], + strip_import_prefix = "some", +) + +proto_library( + name = "nested_proto_with_import_prefix", + srcs = ["some/path/nested.proto"], + import_prefix = "prefix", +) + +proto_library( + name = "nested_proto_with_custom_prefix", + srcs = ["some/path/nested.proto"], + import_prefix = "prefix", + strip_import_prefix = "some", +) + +scala_proto_library( + name = "pack_protos_lib", + deps = [ + ":nested_proto", + ":nested_proto_strip_import_prefix_relative", + ":nested_proto_with_custom_prefix", + ":nested_proto_with_import_prefix", + ":standalone_proto", + ":standalone_proto_strip_import_prefix_package", + ":standalone_proto_strip_import_prefix_partial", + ":standalone_proto_with_custom_prefix", + ":standalone_proto_with_import_prefix", + ], +) + +scala_test( + name = "test_pack_protos", + srcs = ["PackProtosTest.scala"], + unused_dependency_checker_mode = "off", + deps = [":pack_protos_lib"], +) diff --git a/test/proto/PackProtosTest.scala b/test/proto/PackProtosTest.scala new file mode 100644 index 000000000..f75084177 --- /dev/null +++ b/test/proto/PackProtosTest.scala @@ -0,0 +1,13 @@ +class PackProtosTest extends org.scalatest.FlatSpec { + "scala_proto_library" should "pack input proto next to generated code" in { + assert(getClass.getResource("test/proto/standalone.proto") != null) + assert(getClass.getResource("proto/standalone.proto") != null) + assert(getClass.getResource("standalone.proto") != null) + assert(getClass.getResource("prefix/test/proto/standalone.proto") != null) + assert(getClass.getResource("prefix/proto/standalone.proto") != null) + assert(getClass.getResource("test/proto/some/path/nested.proto") != null) + assert(getClass.getResource("path/nested.proto") != null) + assert(getClass.getResource("prefix/test/proto/some/path/nested.proto") != null) + assert(getClass.getResource("prefix/path/nested.proto") != null) + } +} diff --git a/test/proto/some/path/nested.proto b/test/proto/some/path/nested.proto new file mode 100644 index 000000000..24afdf24c --- /dev/null +++ b/test/proto/some/path/nested.proto @@ -0,0 +1,4 @@ +syntax = "proto3"; + +message Message { +} diff --git a/test/proto/standalone.proto b/test/proto/standalone.proto new file mode 100644 index 000000000..24afdf24c --- /dev/null +++ b/test/proto/standalone.proto @@ -0,0 +1,4 @@ +syntax = "proto3"; + +message Message { +}