Skip to content

Commit 502f801

Browse files
authored
feat: parse BUILD.bzel to determine whether a commit that only changed BUILD.bazel is a qualified commit (#2937)
In this PR: - For a commit that only changed `BUILD.bzel`, parse the `BUILD.bazel` to determine whether this is a qualified commit. The commit is a qualified commit if the two `Gapic_Inputs` objects parsed from the commit and its parent are different.
1 parent 47506e3 commit 502f801

File tree

3 files changed

+124
-8
lines changed

3 files changed

+124
-8
lines changed

library_generation/model/config_change.py

+49-4
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,16 @@
1616
from enum import Enum
1717
from typing import Optional
1818
from git import Commit, Repo
19+
20+
from library_generation.model.gapic_inputs import parse_build_str
1921
from library_generation.model.generation_config import GenerationConfig
2022
from library_generation.model.library_config import LibraryConfig
2123
from library_generation.utils.utilities import sh_util
2224
from library_generation.utils.proto_path_utils import find_versioned_proto_path
2325

26+
INSERTIONS = "insertions"
27+
LINES = "lines"
28+
2429

2530
class ChangeType(Enum):
2631
GOOGLEAPIS_COMMIT = 1
@@ -77,6 +82,7 @@ def get_changed_libraries(self) -> Optional[list[str]]:
7782
Returns a unique, sorted list of library name of changed libraries.
7883
None if there is a repository level change, which means all libraries
7984
in the current_config will be generated.
85+
8086
:return: library names of change libraries.
8187
"""
8288
if ChangeType.REPO_LEVEL_CHANGE in self.change_to_libraries:
@@ -143,11 +149,23 @@ def __create_qualified_commit(
143149
:return: qualified commits.
144150
"""
145151
libraries = set()
146-
for file in commit.stats.files.keys():
147-
if file.endswith("BUILD.bazel"):
148-
continue
149-
versioned_proto_path = find_versioned_proto_path(file)
152+
for file_path, changes in commit.stats.files.items():
153+
versioned_proto_path = find_versioned_proto_path(file_path)
150154
if versioned_proto_path in proto_paths:
155+
if (
156+
file_path.endswith("BUILD.bazel")
157+
# Qualify a commit if the commit only added BUILD.bazel
158+
# because it's very unlikely that a commit added BUILD.bazel
159+
# without adding proto files. Therefore, the commit is
160+
# qualified duo to the proto change eventually.
161+
and (not ConfigChange.__is_added(changes))
162+
and (
163+
not ConfigChange.__is_qualified_build_change(
164+
commit=commit, build_file_path=file_path
165+
)
166+
)
167+
):
168+
continue
151169
# Even though a commit usually only changes one
152170
# library, we don't want to miss generating a
153171
# library because the commit may change multiple
@@ -156,3 +174,30 @@ def __create_qualified_commit(
156174
if len(libraries) == 0:
157175
return None
158176
return QualifiedCommit(commit=commit, libraries=libraries)
177+
178+
@staticmethod
179+
def __is_added(changes: dict[str, int]) -> bool:
180+
return changes[INSERTIONS] == changes[LINES]
181+
182+
@staticmethod
183+
def __is_qualified_build_change(commit: Commit, build_file_path: str) -> bool:
184+
"""
185+
Checks if the given commit containing a BUILD.bazel change is a
186+
qualified commit.
187+
188+
The commit is a qualified commit if the
189+
:class:`library_generation.model.gapic_inputs.GapicInputs` objects
190+
parsed from the commit and its parent are different, since there are
191+
changes in fields that used in library generation.
192+
193+
:param commit: a GitHub commit object.
194+
:param build_file_path: the path of the BUILD.bazel
195+
:return: True if the commit is a qualified commit; False otherwise.
196+
"""
197+
versioned_proto_path = find_versioned_proto_path(build_file_path)
198+
build = str((commit.tree / build_file_path).data_stream.read())
199+
parent_commit = commit.parents[0]
200+
parent_build = str((parent_commit.tree / build_file_path).data_stream.read())
201+
inputs = parse_build_str(build, versioned_proto_path)
202+
parent_inputs = parse_build_str(parent_build, versioned_proto_path)
203+
return inputs != parent_inputs

library_generation/model/gapic_inputs.py

+20-3
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,20 @@ def __init__(
7272
self.service_yaml = service_yaml
7373
self.include_samples = include_samples
7474

75+
def __eq__(self, other):
76+
if not isinstance(other, GapicInputs):
77+
return False
78+
return (
79+
self.proto_only == other.proto_only
80+
and self.additional_protos == other.additional_protos
81+
and self.transport == other.transport
82+
and self.rest_numeric_enum == other.rest_numeric_enum
83+
and self.gapic_yaml == other.gapic_yaml
84+
and self.service_config == other.service_config
85+
and self.service_yaml == other.service_yaml
86+
and self.include_samples == other.include_samples
87+
)
88+
7589

7690
def parse(
7791
build_path: Path, versioned_path: str, build_file_name: str = "BUILD.bazel"
@@ -86,16 +100,19 @@ def parse(
86100
"""
87101
with open(f"{build_path}/{build_file_name}") as build:
88102
content = build.read()
103+
return parse_build_str(build_str=content, versioned_path=versioned_path)
104+
89105

106+
def parse_build_str(build_str: str, versioned_path: str) -> GapicInputs:
90107
proto_library_target = re.compile(
91108
proto_library_pattern, re.DOTALL | re.VERBOSE
92-
).findall(content)
109+
).findall(build_str)
93110
additional_protos = ""
94111
if len(proto_library_target) > 0:
95112
additional_protos = __parse_additional_protos(proto_library_target[0])
96-
gapic_target = re.compile(gapic_pattern, re.DOTALL | re.VERBOSE).findall(content)
113+
gapic_target = re.compile(gapic_pattern, re.DOTALL | re.VERBOSE).findall(build_str)
97114
assembly_target = re.compile(assembly_pattern, re.DOTALL | re.VERBOSE).findall(
98-
content
115+
build_str
99116
)
100117
include_samples = "false"
101118
if len(assembly_target) > 0:

library_generation/test/model/config_change_unit_tests.py

+55-1
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,34 @@ def test_get_qualified_commits_success(self):
208208
qualified_commits[2].commit.hexsha,
209209
)
210210

211-
def test_get_qualified_commits_build_only_commit_returns_empty_list(self):
211+
def test_get_qualified_commits_rest_numeric_enum_change_returns_a_qualified_commit(
212+
self,
213+
):
214+
baseline_commit = "45694d2bad602c52170096072d325aa644d550e5"
215+
latest_commit = "758f0d1217d9c7fe398aa5efb1057ce4b6409e55"
216+
config_change = ConfigChange(
217+
change_to_libraries={},
218+
baseline_config=ConfigChangeTest.__get_a_gen_config(
219+
googleapis_commitish=baseline_commit
220+
),
221+
current_config=ConfigChangeTest.__get_a_gen_config(
222+
googleapis_commitish=latest_commit,
223+
libraries=[
224+
ConfigChangeTest.__get_a_library_config(
225+
library_name="container",
226+
gapic_configs=[GapicConfig(proto_path="google/container/v1")],
227+
)
228+
],
229+
),
230+
)
231+
# one commit between latest_commit and baseline_commit which only
232+
# changed BUILD.bazel.
233+
# this commit changed `rest_numeric_enums`.
234+
self.assertTrue(len(config_change.get_qualified_commits()) == 1)
235+
236+
def test_get_qualified_commits_irrelevant_build_field_change_returns_empty_list(
237+
self,
238+
):
212239
baseline_commit = "bdda0174f68a738518ec311e05e6fd9bbe19cd78"
213240
latest_commit = "c9a5050ef225b0011603e1109cf53ab1de0a8e53"
214241
config_change = ConfigChange(
@@ -228,8 +255,35 @@ def test_get_qualified_commits_build_only_commit_returns_empty_list(self):
228255
)
229256
# one commit between latest_commit and baseline_commit which only
230257
# changed BUILD.bazel.
258+
# this commit didn't change fields used in library generation.
231259
self.assertTrue(len(config_change.get_qualified_commits()) == 0)
232260

261+
def test_get_qualified_commits_add_build_file_returns_a_qualified_commit(self):
262+
baseline_commit = "d007ca1b3cc820651530d44d5388533047ae1414"
263+
latest_commit = "05d889e7dfe087fc2ddc9de9579f01d4e1c2f35e"
264+
config_change = ConfigChange(
265+
change_to_libraries={},
266+
baseline_config=ConfigChangeTest.__get_a_gen_config(
267+
googleapis_commitish=baseline_commit
268+
),
269+
current_config=ConfigChangeTest.__get_a_gen_config(
270+
googleapis_commitish=latest_commit,
271+
libraries=[
272+
ConfigChangeTest.__get_a_library_config(
273+
library_name="cloudcontrolspartner",
274+
gapic_configs=[
275+
GapicConfig(
276+
proto_path="google/cloud/cloudcontrolspartner/v1"
277+
)
278+
],
279+
)
280+
],
281+
),
282+
)
283+
# one commit between latest_commit and baseline_commit which added
284+
# google/cloud/cloudcontrolspartner/v1.
285+
self.assertTrue(len(config_change.get_qualified_commits()) == 1)
286+
233287
@staticmethod
234288
def __get_a_gen_config(
235289
googleapis_commitish="", libraries: list[LibraryConfig] = None

0 commit comments

Comments
 (0)