From 18f07eaf4c036e0b6193fbafae40481f7d9d61e3 Mon Sep 17 00:00:00 2001 From: Joe Wang <106995533+JoeWang1127@users.noreply.github.com> Date: Tue, 20 Feb 2024 15:28:24 +0000 Subject: [PATCH] fix: ignore comment in BUILD (#2492) In this PR: - Ignore comment (leading with `#`) when parsing BUILD.bazel - Add unit tests Context: There's a [BUILD](https://github.com/googleapis/googleapis/blob/master/google/cloud/resourcemanager/v3/BUILD.bazel#L52C10-L52C50) in googleapis has common_resources.proto comment out. We need to adjust the regex pattern to adjust this case. --- library_generation/model/gapic_inputs.py | 30 ++++++++++++------- .../misc/BUILD_comment_common_resources.bazel | 5 ++++ .../misc/BUILD_comment_iam_policy.bazel | 5 ++++ .../misc/BUILD_comment_locations.bazel | 5 ++++ .../misc/BUILD_common_resources.bazel | 5 ++++ .../BUILD_iam_locations.bazel | 4 +-- .../resources/misc/BUILD_iam_policy.bazel | 5 ++++ .../test/resources/misc/BUILD_locations.bazel | 5 ++++ .../misc/BUILD_no_additional_protos.bazel | 4 +++ .../BUILD_common_resources.bazel | 6 ---- .../BUILD_iam_policy.bazel | 7 ----- .../BUILD_locations.bazel | 7 ----- library_generation/test/unit_tests.py | 22 ++++++++++++++ 13 files changed, 77 insertions(+), 33 deletions(-) create mode 100644 library_generation/test/resources/misc/BUILD_comment_common_resources.bazel create mode 100644 library_generation/test/resources/misc/BUILD_comment_iam_policy.bazel create mode 100644 library_generation/test/resources/misc/BUILD_comment_locations.bazel create mode 100644 library_generation/test/resources/misc/BUILD_common_resources.bazel rename library_generation/test/resources/{search_additional_protos => misc}/BUILD_iam_locations.bazel (57%) create mode 100644 library_generation/test/resources/misc/BUILD_iam_policy.bazel create mode 100644 library_generation/test/resources/misc/BUILD_locations.bazel create mode 100644 library_generation/test/resources/misc/BUILD_no_additional_protos.bazel delete mode 100644 library_generation/test/resources/search_additional_protos/BUILD_common_resources.bazel delete mode 100644 library_generation/test/resources/search_additional_protos/BUILD_iam_policy.bazel delete mode 100644 library_generation/test/resources/search_additional_protos/BUILD_locations.bazel diff --git a/library_generation/model/gapic_inputs.py b/library_generation/model/gapic_inputs.py index b6500a6f3db..4bb9ce64f4a 100644 --- a/library_generation/model/gapic_inputs.py +++ b/library_generation/model/gapic_inputs.py @@ -31,9 +31,13 @@ (.*?) \) """ -resource_pattern = r"//google/cloud:common_resources_proto" -location_pattern = r"//google/cloud/location:location_proto" -iam_pattern = r"//google/iam/v1:iam_policy_proto" +# match a line which the first character is "#". +comment_pattern = r"^\s*\#+" +pattern_to_proto = { + r"//google/cloud:common_resources_proto": "google/cloud/common_resources.proto", + r"//google/cloud/location:location_proto": "google/cloud/location/locations.proto", + r"//google/iam/v1:iam_policy_proto": "google/iam/v1/iam_policy.proto", +} transport_pattern = r"transport = \"(.*?)\"" rest_pattern = r"rest_numeric_enums = True" gapic_yaml_pattern = r"gapic_yaml = \"(.*?)\"" @@ -97,7 +101,9 @@ def parse( if len(assembly_target) > 0: include_samples = __parse_include_samples(assembly_target[0]) if len(gapic_target) == 0: - return GapicInputs(include_samples=include_samples) + return GapicInputs( + additional_protos=additional_protos, include_samples=include_samples + ) transport = __parse_transport(gapic_target[0]) rest_numeric_enum = __parse_rest_numeric_enums(gapic_target[0]) @@ -119,12 +125,16 @@ def parse( def __parse_additional_protos(proto_library_target: str) -> str: res = [" "] - if len(re.findall(resource_pattern, proto_library_target)) != 0: - res.append("google/cloud/common_resources.proto") - if len(re.findall(location_pattern, proto_library_target)) != 0: - res.append("google/cloud/location/locations.proto") - if len(re.findall(iam_pattern, proto_library_target)) != 0: - res.append("google/iam/v1/iam_policy.proto") + lines = proto_library_target.split("\n") + for line in lines: + if len(re.findall(comment_pattern, line)) != 0: + # skip a line which the first charactor is "#" since it's + # a comment. + continue + for pattern in pattern_to_proto: + if len(re.findall(pattern, line)) == 0: + continue + res.append(pattern_to_proto[pattern]) return " ".join(res) diff --git a/library_generation/test/resources/misc/BUILD_comment_common_resources.bazel b/library_generation/test/resources/misc/BUILD_comment_common_resources.bazel new file mode 100644 index 00000000000..126ffdb7cae --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_comment_common_resources.bazel @@ -0,0 +1,5 @@ +proto_library_with_info( + deps = [ + #"//google/cloud:common_resources_proto", + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/misc/BUILD_comment_iam_policy.bazel b/library_generation/test/resources/misc/BUILD_comment_iam_policy.bazel new file mode 100644 index 00000000000..a9a2c1ca758 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_comment_iam_policy.bazel @@ -0,0 +1,5 @@ +proto_library_with_info( + deps = [ + # "//google/iam/v1:iam_policy_proto", + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/misc/BUILD_comment_locations.bazel b/library_generation/test/resources/misc/BUILD_comment_locations.bazel new file mode 100644 index 00000000000..8b96e3ab819 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_comment_locations.bazel @@ -0,0 +1,5 @@ +proto_library_with_info( + deps = [ + # "//google/cloud/location:location_proto", + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/misc/BUILD_common_resources.bazel b/library_generation/test/resources/misc/BUILD_common_resources.bazel new file mode 100644 index 00000000000..9b749e6ad5f --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_common_resources.bazel @@ -0,0 +1,5 @@ +proto_library_with_info( + deps = [ + "//google/cloud:common_resources_proto", + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/search_additional_protos/BUILD_iam_locations.bazel b/library_generation/test/resources/misc/BUILD_iam_locations.bazel similarity index 57% rename from library_generation/test/resources/search_additional_protos/BUILD_iam_locations.bazel rename to library_generation/test/resources/misc/BUILD_iam_locations.bazel index e8241995e2e..d0c971da7c7 100644 --- a/library_generation/test/resources/search_additional_protos/BUILD_iam_locations.bazel +++ b/library_generation/test/resources/misc/BUILD_iam_locations.bazel @@ -1,8 +1,6 @@ -# this file is only used in testing `get_gapic_additional_protos_from_BUILD` in utilities.sh - proto_library_with_info( deps = [ - "//google/iam/v1:iam_policy_proto", "//google/cloud/location:location_proto", + "//google/iam/v1:iam_policy_proto", ] ) \ No newline at end of file diff --git a/library_generation/test/resources/misc/BUILD_iam_policy.bazel b/library_generation/test/resources/misc/BUILD_iam_policy.bazel new file mode 100644 index 00000000000..af5d4a32f84 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_iam_policy.bazel @@ -0,0 +1,5 @@ +proto_library_with_info( + deps = [ + "//google/iam/v1:iam_policy_proto", + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/misc/BUILD_locations.bazel b/library_generation/test/resources/misc/BUILD_locations.bazel new file mode 100644 index 00000000000..29ee14fdba0 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_locations.bazel @@ -0,0 +1,5 @@ +proto_library_with_info( + deps = [ + "//google/cloud/location:location_proto", + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/misc/BUILD_no_additional_protos.bazel b/library_generation/test/resources/misc/BUILD_no_additional_protos.bazel new file mode 100644 index 00000000000..a22257cad49 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_no_additional_protos.bazel @@ -0,0 +1,4 @@ +proto_library_with_info( + deps = [ + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/search_additional_protos/BUILD_common_resources.bazel b/library_generation/test/resources/search_additional_protos/BUILD_common_resources.bazel deleted file mode 100644 index 45e3987adbf..00000000000 --- a/library_generation/test/resources/search_additional_protos/BUILD_common_resources.bazel +++ /dev/null @@ -1,6 +0,0 @@ -# this file is only used in testing `get_gapic_additional_protos_from_BUILD` in utilities.sh - -proto_library_with_info( - deps = [ - ] -) \ No newline at end of file diff --git a/library_generation/test/resources/search_additional_protos/BUILD_iam_policy.bazel b/library_generation/test/resources/search_additional_protos/BUILD_iam_policy.bazel deleted file mode 100644 index 81064a7eb46..00000000000 --- a/library_generation/test/resources/search_additional_protos/BUILD_iam_policy.bazel +++ /dev/null @@ -1,7 +0,0 @@ -# this file is only used in testing `get_gapic_additional_protos_from_BUILD` in utilities.sh - -proto_library_with_info( - deps = [ - "//google/iam/v1:iam_policy_proto", - ] -) \ No newline at end of file diff --git a/library_generation/test/resources/search_additional_protos/BUILD_locations.bazel b/library_generation/test/resources/search_additional_protos/BUILD_locations.bazel deleted file mode 100644 index 07faa4ac95a..00000000000 --- a/library_generation/test/resources/search_additional_protos/BUILD_locations.bazel +++ /dev/null @@ -1,7 +0,0 @@ -# this file is only used in testing `get_gapic_additional_protos_from_BUILD` in utilities.sh - -proto_library_with_info( - deps = [ - "//google/cloud/location:location_proto", - ] -) \ No newline at end of file diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index fc251e2472c..625aaf8ffdf 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -214,6 +214,28 @@ def test_from_yaml_succeeds(self): self.assertEqual("google/cloud/asset/v1p5beta1", gapics[3].proto_path) self.assertEqual("google/cloud/asset/v1p7beta1", gapics[4].proto_path) + @parameterized.expand( + [ + ("BUILD_no_additional_protos.bazel", " "), + ("BUILD_common_resources.bazel", " google/cloud/common_resources.proto"), + ("BUILD_comment_common_resources.bazel", " "), + ("BUILD_locations.bazel", " google/cloud/location/locations.proto"), + ("BUILD_comment_locations.bazel", " "), + ("BUILD_iam_policy.bazel", " google/iam/v1/iam_policy.proto"), + ("BUILD_comment_iam_policy.bazel", " "), + ( + "BUILD_iam_locations.bazel", + " google/cloud/location/locations.proto google/iam/v1/iam_policy.proto", + ), + ] + ) + def test_gapic_inputs_parse_additional_protos(self, build_name, expected): + parsed = parse_build_file(build_file, "", build_name) + self.assertEqual( + expected, + parsed.additional_protos, + ) + def test_gapic_inputs_parse_grpc_only_succeeds(self): parsed = parse_build_file(build_file, "", "BUILD_grpc.bazel") self.assertEqual("grpc", parsed.transport)