diff --git a/library_generation/model/config_change.py b/library_generation/model/config_change.py index bfb83a87e6..8a5e813244 100644 --- a/library_generation/model/config_change.py +++ b/library_generation/model/config_change.py @@ -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 @@ -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: @@ -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 @@ -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 diff --git a/library_generation/model/gapic_inputs.py b/library_generation/model/gapic_inputs.py index 992e4c2e3c..5c07777965 100644 --- a/library_generation/model/gapic_inputs.py +++ b/library_generation/model/gapic_inputs.py @@ -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" @@ -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: diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py index 6207c2100a..147753ec99 100644 --- a/library_generation/test/model/config_change_unit_tests.py +++ b/library_generation/test/model/config_change_unit_tests.py @@ -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( @@ -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