From 834b4765c27b044cedb0a24204004e2604da4c7d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 17 Oct 2022 10:42:13 -0400 Subject: [PATCH] Bump gitpython from 3.1.18 to 3.1.29 (#272) * Bump gitpython from 3.1.18 to 3.1.29 Bumps [gitpython](https://github.com/gitpython-developers/GitPython) from 3.1.18 to 3.1.29. - [Release notes](https://github.com/gitpython-developers/GitPython/releases) - [Changelog](https://github.com/gitpython-developers/GitPython/blob/main/CHANGES) - [Commits](https://github.com/gitpython-developers/GitPython/compare/3.1.18...3.1.29) --- updated-dependencies: - dependency-name: gitpython dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] * Handle git.Repo.working_dir being None with gitpython 3.1.29 Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Josh Co-authored-by: Josh Woodward --- CHANGELOG.md | 13 +++++++++++++ pygitops/_util.py | 29 ++++++++++++++++++++++++++--- pygitops/exceptions.py | 4 ++++ pygitops/operations.py | 3 ++- requirements.txt | 2 +- setup.cfg | 2 +- tests/test_operations.py | 32 ++++++++++++++++---------------- tests/test_util.py | 19 ++++++++++++++++++- 8 files changed, 81 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41f5134..3db0538 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.16.1] - 2022-10-16 + +### Added + +* Define new exception `PyGitOpsWorkingDirError` extending from `PyGitOpsError`. + * Exception raised in cases where `git.Repo.working_dir` is None (see below). + +### Changed + +* Adjust repo working_dir path operations to comply with `gitpython` type hint changes. + * Between `gitpython==3.1.18` -> `3.1.29`, `git.Repo.working_dir` was typed as `Optional`. + * Since many `os` and `pathlib` operations rely on `Repo.working_dir` and expect `Union[str, os.PathLike]`, code touching `repo.working_dir` was updated to comply with this difference. + ## [0.16.0] - 2022-07-27 ### Added diff --git a/pygitops/_util.py b/pygitops/_util.py index ab2d03a..83f3ffc 100644 --- a/pygitops/_util.py +++ b/pygitops/_util.py @@ -1,14 +1,15 @@ import logging import os from contextlib import contextmanager +from os import PathLike from pathlib import Path -from typing import Iterator +from typing import Iterator, Union from filelock import FileLock, Timeout from git import PushInfo, Repo from git.exc import InvalidGitRepositoryError -from pygitops.exceptions import PyGitOpsError +from pygitops.exceptions import PyGitOpsError, PyGitOpsWorkingDirError _logger = logging.getLogger(__name__) _lockfile_path = Path("lockfiles") @@ -24,7 +25,7 @@ def lock_repo(repo: Repo) -> Iterator[None]: :param repo: The repo to lock on. """ - repo_name = os.path.basename(os.path.normpath(repo.working_dir)) + repo_name = os.path.basename(os.path.normpath(repo_working_dir(repo))) lockfile_name = str(get_lockfile_path(repo_name)) lock = FileLock(lockfile_name) @@ -123,3 +124,25 @@ def is_git_repo(path: Path) -> bool: return True except InvalidGitRepositoryError: return False + + +def repo_working_dir(repo: Repo) -> Union[str, PathLike]: + """ + Between gitpython 3.1.18 and 3.1.29, `git.Repo.working_dir` is now typed as Optional. + + The `os.path` and `pathlib.Path` operations are not typed to support `Optional`. + Calling this function to access this property will handle cases where `repo.working_dir` is None, although it should never happen in practice. + + :param repo: The repo whose `working_dir` we are interested in. + :raises PyGitOpsWorkingDirError: Raise error if `working_dir` is unexpectedly None. + :return str: `working_dir` of the repo + """ + working_dir = repo.working_dir + + # Can replace with assignment operator when python 3.7 support is dropped + if working_dir is None: + raise PyGitOpsWorkingDirError( + f"The working_dir for repo {repo} is unexpectedly None" + ) + + return working_dir diff --git a/pygitops/exceptions.py b/pygitops/exceptions.py index 26381cf..716de5c 100644 --- a/pygitops/exceptions.py +++ b/pygitops/exceptions.py @@ -8,3 +8,7 @@ class PyGitOpsValueError(PyGitOpsError): class PyGitOpsStagedItemsError(PyGitOpsError): """There were no items to stage for commit.""" + + +class PyGitOpsWorkingDirError(PyGitOpsError): + """There was an error with the filesystem, namely `git.Repo.working_dir` is unexpectedly None.""" diff --git a/pygitops/operations.py b/pygitops/operations.py index 8c01490..742390b 100755 --- a/pygitops/operations.py +++ b/pygitops/operations.py @@ -12,6 +12,7 @@ from pygitops._util import is_git_repo as _is_git_repo from pygitops._util import lock_repo as _lock_repo from pygitops._util import push_error_present as _push_error_present +from pygitops._util import repo_working_dir as _repo_working_dir from pygitops.exceptions import PyGitOpsError, PyGitOpsStagedItemsError from pygitops.remote_git_utils import _scrub_github_auth from pygitops.types import PathOrStr @@ -43,7 +44,7 @@ def stage_commit_push_changes( :raises PyGitOpsError: There was an error staging, committing, or pushing code. """ index = repo.index - workdir_path = Path(repo.working_dir) + workdir_path = Path(_repo_working_dir(repo)) # We will determine items_to_stage if the parameter was not provided. if items_to_stage is None: diff --git a/requirements.txt b/requirements.txt index db14965..c0d66f4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,2 @@ filelock~=3.4 -GitPython==3.1.18 +GitPython==3.1.29 diff --git a/setup.cfg b/setup.cfg index f6bf5ec..d7a4a86 100644 --- a/setup.cfg +++ b/setup.cfg @@ -3,7 +3,7 @@ name = pygitops url = https://github.com/wayfair-incubator/pygitops author = Josh Woodward author_email = josh.woodward2693@gmail.com -version = 0.16.0 +version = 0.16.1 description = Wrapper for low-level git logic. Useful for systems automating git operations. long_description = file: README.md long_description_content_type = text/markdown diff --git a/tests/test_operations.py b/tests/test_operations.py index 1b58a82..eab1db7 100644 --- a/tests/test_operations.py +++ b/tests/test_operations.py @@ -7,7 +7,7 @@ from git import Actor, GitCommandError, GitError, Repo from pygitops._constants import GIT_BRANCH_MAIN, GIT_BRANCH_MASTER -from pygitops._util import checkout_pull_branch +from pygitops._util import checkout_pull_branch, repo_working_dir from pygitops.exceptions import PyGitOpsError, PyGitOpsStagedItemsError from pygitops.operations import ( feature_branch, @@ -68,7 +68,7 @@ def test_stage_commit_push_changes__push_failure__raises_pygitops_error( with feature_branch(cloned_repo, SOME_FEATURE_BRANCH): # Write to remote from the clone, then pull from local - test_file_path = Path(cloned_repo.working_dir) / SOME_CONTENT_FILENAME + test_file_path = Path(repo_working_dir(cloned_repo)) / SOME_CONTENT_FILENAME test_file_path.write_text(SOME_INITIAL_CONTENT) with pytest.raises(PyGitOpsError, match=SOME_FEATURE_BRANCH): @@ -78,17 +78,17 @@ def test_stage_commit_push_changes__push_failure__raises_pygitops_error( def _delete_existing_file(local_repo: Repo, filename: str) -> None: - test_second_file_path = Path(local_repo.working_dir) / filename + test_second_file_path = Path(repo_working_dir(local_repo)) / filename test_second_file_path.unlink() def _modify_existing_file(local_repo: Repo, filename: str, content: str) -> None: - test_file_path = Path(local_repo.working_dir) / filename + test_file_path = Path(repo_working_dir(local_repo)) / filename test_file_path.write_text(content) def _add_new_file(local_repo: Repo, filename: str) -> None: - test_other_file_path = Path(local_repo.working_dir) / filename + test_other_file_path = Path(repo_working_dir(local_repo)) / filename test_other_file_path.touch() @@ -188,7 +188,7 @@ def test_stage_commit_push_changes__add_new_file__change_persisted(tmp_path): with feature_branch(local_repo, SOME_FEATURE_BRANCH): - test_file_path = Path(local_repo.working_dir) / SOME_CONTENT_FILENAME + test_file_path = Path(repo_working_dir(local_repo)) / SOME_CONTENT_FILENAME test_file_path.write_text(SOME_INITIAL_CONTENT) # act, asserting expected state before and after operation @@ -217,7 +217,7 @@ def test_stage_commit_push_changes__remove_old_file__change_persisted(tmp_path): cloned_repo = repos.cloned_repo with feature_branch(cloned_repo, SOME_FEATURE_BRANCH): - test_file_path = Path(cloned_repo.working_dir) / SOME_CONTENT_FILENAME + test_file_path = Path(repo_working_dir(cloned_repo)) / SOME_CONTENT_FILENAME test_file_path.write_text(SOME_INITIAL_CONTENT) stage_commit_push_changes( @@ -231,7 +231,7 @@ def test_stage_commit_push_changes__remove_old_file__change_persisted(tmp_path): with _assert_branch_state_stage_commit_push_changes__file_removed( remote_repo, local_repo ): - removal_path = Path(local_repo.working_dir) / SOME_CONTENT_FILENAME + removal_path = Path(repo_working_dir(local_repo)) / SOME_CONTENT_FILENAME removal_path.unlink() # Commit the removal @@ -260,9 +260,9 @@ def test_stage_commit_push_changes__with_staged_files__adds_only_requested_file( with feature_branch(local_repo, SOME_FEATURE_BRANCH): - test_file_path = Path(local_repo.working_dir) / SOME_CONTENT_FILENAME + test_file_path = Path(repo_working_dir(local_repo)) / SOME_CONTENT_FILENAME test_file_path.write_text(SOME_INITIAL_CONTENT) - other_file_path = Path(local_repo.working_dir) / other_file + other_file_path = Path(repo_working_dir(local_repo)) / other_file other_file_path.write_text("other test content") # act, asserting expected state before and after operation @@ -300,7 +300,7 @@ def test_stage_commit_push_changes__force_push_flag__changes_pushed(tmp_path): repos = _initialize_multiple_empty_repos(tmp_path) local_repo = repos.local_repo with feature_branch(local_repo, SOME_FEATURE_BRANCH): - test_file_path = Path(local_repo.working_dir) / SOME_CONTENT_FILENAME + test_file_path = Path(repo_working_dir(local_repo)) / SOME_CONTENT_FILENAME test_file_path.write_text("content one") stage_commit_push_changes( local_repo, SOME_FEATURE_BRANCH, SOME_ACTOR, SOME_COMMIT_MESSAGE @@ -807,7 +807,7 @@ def _assert_get_updated_repo_state(repo): def _assert_branch_state_stage_commit_push_changes__file_added(remote_repo, local_repo): """Run assertions before and after staging, committing, and pushing for a given repo pair.""" - local_content_path = Path(local_repo.working_dir) / SOME_CONTENT_FILENAME + local_content_path = Path(repo_working_dir(local_repo)) / SOME_CONTENT_FILENAME # The file should exist only on local before pushing assert SOME_INITIAL_CONTENT in local_content_path.read_text() @@ -826,7 +826,7 @@ def _assert_branch_state_stage_commit_push_changes__file_removed( remote_repo, local_repo ): """Run assertions before and after staging, committing, and pushing for a given repo pair.""" - local_content_path = Path(local_repo.working_dir) / SOME_CONTENT_FILENAME + local_content_path = Path(repo_working_dir(local_repo)) / SOME_CONTENT_FILENAME # The file should exist only on local before pushing assert SOME_INITIAL_CONTENT in local_content_path.read_text() @@ -872,8 +872,8 @@ def _initialize_multiple_empty_repos( # Simplify things by assuring local copies are cloned from the remote. # Because they are clones, they will have 'origin' correctly configured - local_repo = Repo.clone_from(remote_repo.working_dir, local_path) - cloned_repo = Repo.clone_from(remote_repo.working_dir, clone_path) + local_repo = Repo.clone_from(repo_working_dir(remote_repo), local_path) + cloned_repo = Repo.clone_from(repo_working_dir(remote_repo), clone_path) return MultipleTestingRepos( remote_repo=remote_repo, local_repo=local_repo, cloned_repo=cloned_repo @@ -889,7 +889,7 @@ def _get_diff(repo: Repo) -> str: """ index = repo.index - workdir_path = Path(repo.working_dir) + workdir_path = Path(repo_working_dir(repo)) untracked_file_paths = [Path(f) for f in repo.untracked_files] items_to_stage = untracked_file_paths + [Path(f.a_path) for f in index.diff(None)] diff --git a/tests/test_util.py b/tests/test_util.py index 7395fe6..1d950ff 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -13,8 +13,9 @@ is_git_repo, lock_repo, push_error_present, + repo_working_dir, ) -from pygitops.exceptions import PyGitOpsError +from pygitops.exceptions import PyGitOpsError, PyGitOpsWorkingDirError SOME_REPO_NAME = "some-repo-name" @@ -193,3 +194,19 @@ def test_is_git_repo__not_a_git_repo__returns_false(tmp_path): def test_is_git_repo__is_a_git_repo__returns_true(tmp_path): Repo.init(tmp_path) assert is_git_repo(tmp_path) + + +def test_repo_working_dir__working_dir_none__raises_pygitops_working_dir_error(mocker): + + repo = mocker.Mock(working_dir=None) + with pytest.raises(PyGitOpsWorkingDirError): + repo_working_dir(repo) + + +def test_repo_working_dir__working_dir_present__expected_working_dir_returned(tmp_path): + + repo_path = tmp_path / SOME_REPO_NAME + repo_path.mkdir() + repo = Repo.init(repo_path) + + assert repo_working_dir(repo) == str(repo_path)