From 969f3d55ef1d4f07886d38d869749aad7572753a Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 5 Oct 2021 23:01:11 -0700 Subject: [PATCH] Check that grpc_package statements have the right name (#27608) * named-right * Update check_package_name.py * Automated change: Fix sanity tests Co-authored-by: ctiller --- test/core/client_channel/resolvers/BUILD | 2 +- test/core/resource_quota/BUILD | 2 +- test/core/uri/BUILD | 2 +- test/cpp/ext/filters/census/BUILD | 2 +- tools/run_tests/sanity/check_package_name.py | 74 ++++++++++++++++++++ tools/run_tests/sanity/sanity_tests.yaml | 1 + 6 files changed, 79 insertions(+), 4 deletions(-) create mode 100755 tools/run_tests/sanity/check_package_name.py diff --git a/test/core/client_channel/resolvers/BUILD b/test/core/client_channel/resolvers/BUILD index ac351fe4286..5707abe8ff9 100644 --- a/test/core/client_channel/resolvers/BUILD +++ b/test/core/client_channel/resolvers/BUILD @@ -14,7 +14,7 @@ load("//bazel:grpc_build_system.bzl", "grpc_cc_binary", "grpc_cc_library", "grpc_cc_test", "grpc_package") -grpc_package(name = "test/core/client_channel_resolvers") +grpc_package(name = "test/core/client_channel/resolvers") licenses(["notice"]) # Apache v2 diff --git a/test/core/resource_quota/BUILD b/test/core/resource_quota/BUILD index 9139eac904e..e14415fd9d8 100644 --- a/test/core/resource_quota/BUILD +++ b/test/core/resource_quota/BUILD @@ -16,7 +16,7 @@ load("//bazel:grpc_build_system.bzl", "grpc_cc_test", "grpc_package") licenses(["notice"]) -grpc_package(name = "test/core/promise") +grpc_package(name = "test/core/resource_quota") load("//test/core/util:grpc_fuzzer.bzl", "grpc_proto_fuzzer") diff --git a/test/core/uri/BUILD b/test/core/uri/BUILD index aaa9b68cd15..fd2990c52c9 100644 --- a/test/core/uri/BUILD +++ b/test/core/uri/BUILD @@ -14,7 +14,7 @@ load("//bazel:grpc_build_system.bzl", "grpc_cc_test", "grpc_package") -grpc_package(name = "test/core/client_channel") +grpc_package(name = "test/core/uri") licenses(["notice"]) diff --git a/test/cpp/ext/filters/census/BUILD b/test/cpp/ext/filters/census/BUILD index 0351330de00..62477a8498c 100644 --- a/test/cpp/ext/filters/census/BUILD +++ b/test/cpp/ext/filters/census/BUILD @@ -17,7 +17,7 @@ load("//bazel:cc_grpc_library.bzl", "cc_grpc_library") licenses(["notice"]) # Apache v2 -grpc_package(name = "test/core/ext/census") +grpc_package(name = "test/cpp/ext/filters/census") grpc_cc_test( name = "grpc_opencensus_plugin_test", diff --git a/tools/run_tests/sanity/check_package_name.py b/tools/run_tests/sanity/check_package_name.py new file mode 100755 index 00000000000..e0396ac7026 --- /dev/null +++ b/tools/run_tests/sanity/check_package_name.py @@ -0,0 +1,74 @@ +#!/usr/bin/env python3 + +# Copyright 2021 gRPC authors. +# +# 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. + +import os +import sys + +os.chdir(os.path.join(os.path.dirname(sys.argv[0]), '../../..')) + +# Allowance for overrides for specific files +EXPECTED_NAMES = { + 'src/proto/grpc/channelz': 'channelz', + 'src/proto/grpc/status': 'status', + 'src/proto/grpc/testing': 'testing', + 'src/proto/grpc/testing/duplicate': 'duplicate', + 'src/proto/grpc/lb/v1': 'lb', + 'src/proto/grpc/testing/xds': 'xds', + 'src/proto/grpc/testing/xds/v3': 'xds_v3', + 'src/proto/grpc/core': 'core', + 'src/proto/grpc/http_over_grpc': 'http_over_grpc', + 'src/proto/grpc/health/v1': 'health', + 'src/proto/grpc/reflection/v1alpha': 'reflection', +} + +errors = 0 +for root, dirs, files in os.walk('.'): + if root.startswith('./'): + root = root[len('./'):] + # don't check third party + if root.startswith('third_party/'): + continue + # only check BUILD files + if 'BUILD' not in files: + continue + text = open('%s/BUILD' % root).read() + # find a grpc_package clause + pkg_start = text.find('grpc_package(') + if pkg_start == -1: + continue + # parse it, taking into account nested parens + pkg_end = pkg_start + len('grpc_package(') + level = 1 + while level == 1: + if text[pkg_end] == ')': + level -= 1 + elif text[pkg_end] == '(': + level += 1 + pkg_end += 1 + # it's a python statement, so evaluate it to pull out the name of the package + name = eval(text[pkg_start:pkg_end], + {'grpc_package': lambda name, **kwargs: name}) + # the name should be the path within the source tree, excepting some special + # BUILD files (really we should normalize them too at some point) + # TODO(ctiller): normalize all package names + expected_name = EXPECTED_NAMES.get(root, root) + if name != expected_name: + print("%s/BUILD should define a grpc_package with name=%r, not %r" % + (root, expected_name, name)) + errors += 1 + +if errors != 0: + sys.exit(1) diff --git a/tools/run_tests/sanity/sanity_tests.yaml b/tools/run_tests/sanity/sanity_tests.yaml index 9d8d50ccab0..77826d6a29c 100644 --- a/tools/run_tests/sanity/sanity_tests.yaml +++ b/tools/run_tests/sanity/sanity_tests.yaml @@ -18,6 +18,7 @@ - script: tools/run_tests/sanity/core_banned_functions.py - script: tools/run_tests/sanity/core_untyped_structs.sh - script: tools/run_tests/sanity/cpp_banned_constructs.sh +- script: tools/run_tests/sanity/check_package_name.py - script: tools/buildgen/generate_projects.sh -j 3 cpu_cost: 3 - script: tools/distrib/check_copyright.py