Skip to content

Commit

Permalink
feat: parse BUILD.bzel to determine whether a commit that only chan…
Browse files Browse the repository at this point in the history
…ged `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.
  • Loading branch information
JoeWang1127 authored Jun 26, 2024
1 parent 47506e3 commit 502f801
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 8 deletions.
53 changes: 49 additions & 4 deletions library_generation/model/config_change.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,16 @@
from enum import Enum
from typing import Optional
from git import Commit, Repo

from library_generation.model.gapic_inputs import parse_build_str
from library_generation.model.generation_config import GenerationConfig
from library_generation.model.library_config import LibraryConfig
from library_generation.utils.utilities import sh_util
from library_generation.utils.proto_path_utils import find_versioned_proto_path

INSERTIONS = "insertions"
LINES = "lines"


class ChangeType(Enum):
GOOGLEAPIS_COMMIT = 1
Expand Down Expand Up @@ -77,6 +82,7 @@ def get_changed_libraries(self) -> Optional[list[str]]:
Returns a unique, sorted list of library name of changed libraries.
None if there is a repository level change, which means all libraries
in the current_config will be generated.
:return: library names of change libraries.
"""
if ChangeType.REPO_LEVEL_CHANGE in self.change_to_libraries:
Expand Down Expand Up @@ -143,11 +149,23 @@ def __create_qualified_commit(
:return: qualified commits.
"""
libraries = set()
for file in commit.stats.files.keys():
if file.endswith("BUILD.bazel"):
continue
versioned_proto_path = find_versioned_proto_path(file)
for file_path, changes in commit.stats.files.items():
versioned_proto_path = find_versioned_proto_path(file_path)
if versioned_proto_path in proto_paths:
if (
file_path.endswith("BUILD.bazel")
# Qualify a commit if the commit only added BUILD.bazel
# because it's very unlikely that a commit added BUILD.bazel
# without adding proto files. Therefore, the commit is
# qualified duo to the proto change eventually.
and (not ConfigChange.__is_added(changes))
and (
not ConfigChange.__is_qualified_build_change(
commit=commit, build_file_path=file_path
)
)
):
continue
# Even though a commit usually only changes one
# library, we don't want to miss generating a
# library because the commit may change multiple
Expand All @@ -156,3 +174,30 @@ def __create_qualified_commit(
if len(libraries) == 0:
return None
return QualifiedCommit(commit=commit, libraries=libraries)

@staticmethod
def __is_added(changes: dict[str, int]) -> bool:
return changes[INSERTIONS] == changes[LINES]

@staticmethod
def __is_qualified_build_change(commit: Commit, build_file_path: str) -> bool:
"""
Checks if the given commit containing a BUILD.bazel change is a
qualified commit.
The commit is a qualified commit if the
:class:`library_generation.model.gapic_inputs.GapicInputs` objects
parsed from the commit and its parent are different, since there are
changes in fields that used in library generation.
:param commit: a GitHub commit object.
:param build_file_path: the path of the BUILD.bazel
:return: True if the commit is a qualified commit; False otherwise.
"""
versioned_proto_path = find_versioned_proto_path(build_file_path)
build = str((commit.tree / build_file_path).data_stream.read())
parent_commit = commit.parents[0]
parent_build = str((parent_commit.tree / build_file_path).data_stream.read())
inputs = parse_build_str(build, versioned_proto_path)
parent_inputs = parse_build_str(parent_build, versioned_proto_path)
return inputs != parent_inputs
23 changes: 20 additions & 3 deletions library_generation/model/gapic_inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,20 @@ def __init__(
self.service_yaml = service_yaml
self.include_samples = include_samples

def __eq__(self, other):
if not isinstance(other, GapicInputs):
return False
return (
self.proto_only == other.proto_only
and self.additional_protos == other.additional_protos
and self.transport == other.transport
and self.rest_numeric_enum == other.rest_numeric_enum
and self.gapic_yaml == other.gapic_yaml
and self.service_config == other.service_config
and self.service_yaml == other.service_yaml
and self.include_samples == other.include_samples
)


def parse(
build_path: Path, versioned_path: str, build_file_name: str = "BUILD.bazel"
Expand All @@ -86,16 +100,19 @@ def parse(
"""
with open(f"{build_path}/{build_file_name}") as build:
content = build.read()
return parse_build_str(build_str=content, versioned_path=versioned_path)


def parse_build_str(build_str: str, versioned_path: str) -> GapicInputs:
proto_library_target = re.compile(
proto_library_pattern, re.DOTALL | re.VERBOSE
).findall(content)
).findall(build_str)
additional_protos = ""
if len(proto_library_target) > 0:
additional_protos = __parse_additional_protos(proto_library_target[0])
gapic_target = re.compile(gapic_pattern, re.DOTALL | re.VERBOSE).findall(content)
gapic_target = re.compile(gapic_pattern, re.DOTALL | re.VERBOSE).findall(build_str)
assembly_target = re.compile(assembly_pattern, re.DOTALL | re.VERBOSE).findall(
content
build_str
)
include_samples = "false"
if len(assembly_target) > 0:
Expand Down
56 changes: 55 additions & 1 deletion library_generation/test/model/config_change_unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,34 @@ def test_get_qualified_commits_success(self):
qualified_commits[2].commit.hexsha,
)

def test_get_qualified_commits_build_only_commit_returns_empty_list(self):
def test_get_qualified_commits_rest_numeric_enum_change_returns_a_qualified_commit(
self,
):
baseline_commit = "45694d2bad602c52170096072d325aa644d550e5"
latest_commit = "758f0d1217d9c7fe398aa5efb1057ce4b6409e55"
config_change = ConfigChange(
change_to_libraries={},
baseline_config=ConfigChangeTest.__get_a_gen_config(
googleapis_commitish=baseline_commit
),
current_config=ConfigChangeTest.__get_a_gen_config(
googleapis_commitish=latest_commit,
libraries=[
ConfigChangeTest.__get_a_library_config(
library_name="container",
gapic_configs=[GapicConfig(proto_path="google/container/v1")],
)
],
),
)
# one commit between latest_commit and baseline_commit which only
# changed BUILD.bazel.
# this commit changed `rest_numeric_enums`.
self.assertTrue(len(config_change.get_qualified_commits()) == 1)

def test_get_qualified_commits_irrelevant_build_field_change_returns_empty_list(
self,
):
baseline_commit = "bdda0174f68a738518ec311e05e6fd9bbe19cd78"
latest_commit = "c9a5050ef225b0011603e1109cf53ab1de0a8e53"
config_change = ConfigChange(
Expand All @@ -228,8 +255,35 @@ def test_get_qualified_commits_build_only_commit_returns_empty_list(self):
)
# one commit between latest_commit and baseline_commit which only
# changed BUILD.bazel.
# this commit didn't change fields used in library generation.
self.assertTrue(len(config_change.get_qualified_commits()) == 0)

def test_get_qualified_commits_add_build_file_returns_a_qualified_commit(self):
baseline_commit = "d007ca1b3cc820651530d44d5388533047ae1414"
latest_commit = "05d889e7dfe087fc2ddc9de9579f01d4e1c2f35e"
config_change = ConfigChange(
change_to_libraries={},
baseline_config=ConfigChangeTest.__get_a_gen_config(
googleapis_commitish=baseline_commit
),
current_config=ConfigChangeTest.__get_a_gen_config(
googleapis_commitish=latest_commit,
libraries=[
ConfigChangeTest.__get_a_library_config(
library_name="cloudcontrolspartner",
gapic_configs=[
GapicConfig(
proto_path="google/cloud/cloudcontrolspartner/v1"
)
],
)
],
),
)
# one commit between latest_commit and baseline_commit which added
# google/cloud/cloudcontrolspartner/v1.
self.assertTrue(len(config_change.get_qualified_commits()) == 1)

@staticmethod
def __get_a_gen_config(
googleapis_commitish="", libraries: list[LibraryConfig] = None
Expand Down

0 comments on commit 502f801

Please sign in to comment.