From f683d3ceb46c79ece884b9c49ca94aedb71444c2 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Thu, 10 Oct 2019 17:12:10 +0200 Subject: [PATCH] Add proto_common.direct_source_infos This change adds a helper to extract information about direct sources of `ProtoInfo` providers. Design doc: https://docs.google.com/document/d/1u95vlQ1lWeQNR4bUw5T4cMeHTGJla2_e1dHHx7v4Dvg/edit# Note that this was originally named `proto_common.protos_to_generate`, but was renamed because it applies more broadly. --- .bazelci/presubmit.yml | 20 +++- WORKSPACE | 4 + proto/defs.bzl | 7 +- proto/private/utils/BUILD | 1 + proto/private/utils/direct_source_infos.bzl | 72 ++++++++++++ tests/BUILD | 107 ++++++++++++++++++ tests/analysis/BUILD | 5 + ...oto_common_contains_native_fields_test.bzl | 45 ++++++++ tests/empty.proto | 0 tests/prefix/empty.proto | 17 +++ tests/utils/BUILD | 81 +++++++++++++ tests/utils/direct_source_infos_tests.bzl | 105 +++++++++++++++++ 12 files changed, 457 insertions(+), 7 deletions(-) create mode 100644 proto/private/utils/BUILD create mode 100644 proto/private/utils/direct_source_infos.bzl mode change 100644 => 100755 tests/BUILD create mode 100755 tests/analysis/BUILD create mode 100755 tests/analysis/proto_common_contains_native_fields_test.bzl mode change 100644 => 100755 tests/empty.proto create mode 100755 tests/prefix/empty.proto create mode 100755 tests/utils/BUILD create mode 100755 tests/utils/direct_source_infos_tests.bzl diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 33c0645..edc4131 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -5,20 +5,30 @@ buildifier: platforms: macos: build_targets: - - "..." + - "//..." + test_targets: + - "//..." rbe_ubuntu1604: build_targets: - - "..." + - "//..." + test_targets: + - "//..." ubuntu1604: build_targets: - - "..." + - "//..." + test_targets: + - "//..." ubuntu1804: build_targets: - - "..." + - "//..." + test_targets: + - "//..." windows: build_targets: - - "..." + - "//..." + test_targets: + - "//..." diff --git a/WORKSPACE b/WORKSPACE index 90172d2..213bfe8 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -7,6 +7,10 @@ rules_proto_dependencies() rules_proto_toolchains() +load("@bazel_skylib//lib:unittest.bzl", "register_unittest_toolchains") + +register_unittest_toolchains() + http_archive( name = "bazel_toolchains", sha256 = "1e16833a9f0e32b292568c0dfee7bd48133c2038605757d3a430551394310006", diff --git a/proto/defs.bzl b/proto/defs.bzl index 17f1337..3288620 100644 --- a/proto/defs.bzl +++ b/proto/defs.bzl @@ -14,7 +14,8 @@ """Starlark rules for building protocol buffers.""" -load("//proto/private:native.bzl", "NativeProtoInfo", "native_proto_common") +load("//proto/private:native.bzl", "NativeProtoInfo") +load("//proto/private/utils:direct_source_infos.bzl", "direct_source_infos") _MIGRATION_TAG = "__PROTO_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__" @@ -57,4 +58,6 @@ ProtoInfo = NativeProtoInfo # Utilities for protocol buffers. # # https://docs.bazel.build/versions/master/skylark/lib/proto_common.html -proto_common = native_proto_common +proto_common = struct( + direct_source_infos = direct_source_infos, +) diff --git a/proto/private/utils/BUILD b/proto/private/utils/BUILD new file mode 100644 index 0000000..5990249 --- /dev/null +++ b/proto/private/utils/BUILD @@ -0,0 +1 @@ +# Intentionally left empty (for now). diff --git a/proto/private/utils/direct_source_infos.bzl b/proto/private/utils/direct_source_infos.bzl new file mode 100644 index 0000000..ec26f57 --- /dev/null +++ b/proto/private/utils/direct_source_infos.bzl @@ -0,0 +1,72 @@ +# Copyright 2019 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Contains the implementation for `proto_common.direct_source_infos`.""" + +ProtoFileInfo = provider( + doc = "Encapsulates information about a single `.proto` file.", + fields = { + "file": "The source file.", + "import_path": "The import path of the `.proto` file.", + }, +) + +# TODO(yannic): Remove this suppression when +# https://github.com/bazelbuild/buildtools/issues/774 has been fixed. +# buildifier: disable=function-docstring-args +def direct_source_infos(proto_info, *, provided_sources = []): + """Returns sequence of `ProtoFileInfo` for `proto_info`'s direct sources. + + Files that are both in `proto_info`'s direct sources and in + `provided_sources` are skipped. This is useful, e.g., for well-known + protos that are already provided by the Protobuf runtime. + + Args: + proto_info: An instance of `ProtoInfo`. + provided_sources: Optional. A sequence of import paths to ignore. + + Returns: A sequence of `ProtoFileInfo` containing information about + `proto_info`'s direct sources. + """ + + files = [_info(f, proto_info) for f in proto_info.direct_sources] + return [f for f in files if f.import_path not in provided_sources] + +def _info(file, proto_info): + """Computes `ProtoFileInfo` for a single file. + + Args: + file: The proto file to generate `ProtoFileInfo` for. Must be in + `direct_sources` of `proto_info`. + proto_info: An instance of `ProtoInfo`. + + Returns: An instance of `ProtoFileInfo`. + """ + + if "." == proto_info.proto_source_root: + # The `proto_library` didn't specify `import_prefix` or + # `strip_import_prefix`, and `file` is a regular source file + # (i.e. not generated) in the main workspace. + + return ProtoFileInfo( + file = file, + import_path = file.path, + ) + + source_root = proto_info.proto_source_root + offset = len(source_root) + 1 # + '/'. + return ProtoFileInfo( + file = file, + import_path = file.path[offset:], + ) diff --git a/tests/BUILD b/tests/BUILD old mode 100644 new mode 100755 index 2381713..9703439 --- a/tests/BUILD +++ b/tests/BUILD @@ -1,8 +1,115 @@ load("//proto:defs.bzl", "proto_library") +WORKSPACE_PREFIX = "" # copybara-workspace-prefix(WORKSPACE_PREFIX) + +proto_library( + name = "multiple_srcs_proto", + srcs = [ + "empty.proto", + "prefix/empty.proto", + ], + visibility = [ + "//tests:__subpackages__", + ], +) + proto_library( name = "empty_proto", srcs = [ "empty.proto", ], + visibility = [ + "//tests:__subpackages__", + ], +) + +proto_library( + name = "empty_with_prefix_proto", + srcs = [ + "prefix/empty.proto", + ], + visibility = [ + "//tests:__subpackages__", + ], +) + +proto_library( + name = "empty_with_absolute_strip_prefix_proto", + srcs = [ + "empty.proto", + ], + strip_import_prefix = "/" + WORKSPACE_PREFIX + "tests", + visibility = [ + "//tests:__subpackages__", + ], +) + +proto_library( + name = "empty_with_relative_strip_prefix_proto", + srcs = [ + "prefix/empty.proto", + ], + strip_import_prefix = "prefix", + visibility = [ + "//tests:__subpackages__", + ], +) + +proto_library( + name = "empty_with_empty_strip_prefix_proto", + srcs = [ + "empty.proto", + ], + strip_import_prefix = "", + visibility = [ + "//tests:__subpackages__", + ], +) + +proto_library( + name = "empty_with_add_and_absolute_strip_prefix_proto", + srcs = [ + "empty.proto", + ], + import_prefix = "foo", + strip_import_prefix = "/" + WORKSPACE_PREFIX + "tests", + visibility = [ + "//tests:__subpackages__", + ], +) + +proto_library( + name = "empty_with_add_and_relative_strip_prefix_proto", + srcs = [ + "prefix/empty.proto", + ], + import_prefix = "foo", + strip_import_prefix = "prefix", + visibility = [ + "//tests:__subpackages__", + ], +) + +proto_library( + name = "empty_with_add_and_empty_strip_prefix_proto", + srcs = [ + "empty.proto", + ], + import_prefix = "foo", + strip_import_prefix = "", + visibility = [ + "//tests:__subpackages__", + ], +) + +proto_library( + name = "empty_with_add_and_empty_strip_prefix_proto_in_subdir", + srcs = [ + "prefix/empty.proto", + ], + import_prefix = "foo", + strip_import_prefix = "", + visibility = [ + "//tests:__subpackages__", + ], ) diff --git a/tests/analysis/BUILD b/tests/analysis/BUILD new file mode 100755 index 0000000..a13e073 --- /dev/null +++ b/tests/analysis/BUILD @@ -0,0 +1,5 @@ +load("//tests/analysis:proto_common_contains_native_fields_test.bzl", "proto_common_contains_native_fields_test") + +proto_common_contains_native_fields_test( + name = "proto_common_contains_native_fields_test", +) diff --git a/tests/analysis/proto_common_contains_native_fields_test.bzl b/tests/analysis/proto_common_contains_native_fields_test.bzl new file mode 100755 index 0000000..ee93bc2 --- /dev/null +++ b/tests/analysis/proto_common_contains_native_fields_test.bzl @@ -0,0 +1,45 @@ +# Copyright 2019 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Unit-test to verify that the Starlark `proto_common` shim contains all fields +of the native version of `proto_common`, excluding fields that are considered +to be an implementation detail (thus not part of the public API). +""" + +load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") +load("//proto:defs.bzl", "proto_common") +load("//proto/private:native.bzl", "native_proto_common") # buildifier: disable=bzl-visibility + +def _impl(ctx): + """Verifies that the Starlark `proto_common` contains all necessary fields. + + Args: + ctx: The rule context. + + Returns: A (not further specified) sequence of providers. + """ + + env = unittest.begin(ctx) + + for field in dir(native_proto_common): + # Starlark `proto_common` only exports fields that are part of the + # stable and supported API of native `proto_common`. + if not field.endswith("_do_not_use_or_we_will_break_you_without_mercy"): + asserts.true(env, hasattr(proto_common, field)) + else: + asserts.false(env, hasattr(proto_common, field)) + + return unittest.end(env) + +proto_common_contains_native_fields_test = unittest.make(_impl) diff --git a/tests/empty.proto b/tests/empty.proto old mode 100644 new mode 100755 diff --git a/tests/prefix/empty.proto b/tests/prefix/empty.proto new file mode 100755 index 0000000..0645554 --- /dev/null +++ b/tests/prefix/empty.proto @@ -0,0 +1,17 @@ +// Copyright 2019 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +syntax = "proto3"; + +package rules_proto.tests.prefix; diff --git a/tests/utils/BUILD b/tests/utils/BUILD new file mode 100755 index 0000000..329c224 --- /dev/null +++ b/tests/utils/BUILD @@ -0,0 +1,81 @@ +load( + "//tests/utils:direct_source_infos_tests.bzl", + "compute_import_path_test", + "exclude_provided_sources_test", +) + +WORKSPACE_PREFIX = "" # copybara-workspace-prefix(WORKSPACE_PREFIX) + +compute_import_path_test( + name = "compute_import_path_test", + deps = { + "//tests:empty_proto": WORKSPACE_PREFIX + "tests/empty.proto", + "//tests:empty_with_absolute_strip_prefix_proto": "empty.proto", + "//tests:empty_with_add_and_absolute_strip_prefix_proto": "foo/empty.proto", + "//tests:empty_with_add_and_empty_strip_prefix_proto": "foo/empty.proto", + "//tests:empty_with_add_and_empty_strip_prefix_proto_in_subdir": "foo/prefix/empty.proto", + "//tests:empty_with_add_and_relative_strip_prefix_proto": "foo/empty.proto", + "//tests:empty_with_empty_strip_prefix_proto": "empty.proto", + "//tests:empty_with_prefix_proto": WORKSPACE_PREFIX + "tests/prefix/empty.proto", + "//tests:empty_with_relative_strip_prefix_proto": "empty.proto", + # WKPs are not excluded by default, so their import path must be correct. + "@com_google_protobuf//:any_proto": "google/protobuf/any.proto", + "@com_google_protobuf//:compiler_plugin_proto": "google/protobuf/compiler/plugin.proto", + "@com_google_protobuf//:timestamp_proto": "google/protobuf/timestamp.proto", + "@com_google_protobuf//:type_proto": "google/protobuf/type.proto", + }, +) + +exclude_provided_sources_test( + name = "exclude_proto_test", + dep = "@com_google_protobuf//:any_proto", + # No direct sources expected. + import_path = [], + provided_sources = [ + "@com_google_protobuf//:any_proto", + ], +) + +exclude_provided_sources_test( + name = "exclude_some_direct_sources_test", + dep = "//tests:multiple_srcs_proto", + import_path = [ + WORKSPACE_PREFIX + "tests/prefix/empty.proto", + ], + provided_sources = [ + "//tests:empty_proto", + ], +) + +exclude_provided_sources_test( + name = "exclude_non_deps_test", + dep = "//tests:empty_proto", + import_path = [ + WORKSPACE_PREFIX + "tests/empty.proto", + ], + provided_sources = [ + "@com_google_protobuf//:any_proto", + ], +) + +exclude_provided_sources_test( + name = "exclude_direct_deps_test", + dep = "@com_google_protobuf//:type_proto", + import_path = [ + "google/protobuf/type.proto", + ], + provided_sources = [ + "@com_google_protobuf//:any_proto", + ], +) + +exclude_provided_sources_test( + name = "exclude_indirect_deps_test", + dep = "@com_google_protobuf//:api_proto", + import_path = [ + "google/protobuf/api.proto", + ], + provided_sources = [ + "@com_google_protobuf//:any_proto", + ], +) diff --git a/tests/utils/direct_source_infos_tests.bzl b/tests/utils/direct_source_infos_tests.bzl new file mode 100755 index 0000000..2745439 --- /dev/null +++ b/tests/utils/direct_source_infos_tests.bzl @@ -0,0 +1,105 @@ +# Copyright 2019 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Unit-tests for `proto_common.direct_source_infos`.""" + +load("@bazel_skylib//lib:new_sets.bzl", "sets") +load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") +load("//proto:defs.bzl", "ProtoInfo", "proto_common") + +def _compute_import_path_test_impl(ctx): + """Verifies that `direct_source_infos` computes the correct import paths. + + Args: + ctx: The rule context. + + Returns: A (not further specified) sequence of providers. + """ + + env = unittest.begin(ctx) + + for target, expected_import_path in ctx.attr.deps.items(): + proto_info = target[ProtoInfo] + asserts.equals( + env, + 1, + len(proto_info.direct_sources), + "Target under test must have exactly one direct source", + ) + + file_infos = proto_common.direct_source_infos(proto_info) + asserts.equals(env, 1, len(file_infos)) + file_info = file_infos[0] + + asserts.equals(env, proto_info.direct_sources[0], file_info.file) + asserts.equals(env, expected_import_path, file_info.import_path) + + return unittest.end(env) + +compute_import_path_test = unittest.make( + impl = _compute_import_path_test_impl, + attrs = { + "deps": attr.label_keyed_string_dict( + mandatory = True, + providers = [ProtoInfo], + ), + }, +) + +def _exclude_provided_sources_test_impl(ctx): + """Verifies that `direct_source_infos` excludes provided sources. + + Args: + ctx: The rule context. + + Returns: A (not further specified) sequence of providers. + """ + + env = unittest.begin(ctx) + + provided_sources = sets.make() + for target in ctx.attr.provided_sources: + info = target[ProtoInfo] + infos = [f.import_path for f in proto_common.direct_source_infos(info)] + provided_sources = sets.union(provided_sources, sets.make(infos)) + + file_infos = proto_common.direct_source_infos( + proto_info = ctx.attr.dep[ProtoInfo], + provided_sources = sets.to_list(provided_sources), + ) + asserts.equals(env, len(ctx.attr.import_path), len(file_infos)) + asserts.new_set_equals( + env, + sets.make(ctx.attr.import_path), + sets.make([info.import_path for info in file_infos]), + ) + + return unittest.end(env) + +exclude_provided_sources_test = unittest.make( + impl = _exclude_provided_sources_test_impl, + attrs = { + "dep": attr.label( + mandatory = True, + providers = [ProtoInfo], + ), + "import_path": attr.string_list( + mandatory = True, + ), + "provided_sources": attr.label_list( + mandatory = True, + providers = [ProtoInfo], + ), + }, +)