Merge pull request #562 from openlawlibrary/renatav/git-error-fix

Fix Git Errors and Resetting Validation

CHANGELOG.md

@@ -13,6 +13,19 @@ and this project adheres to [Semantic Versioning][semver].
 
 ### Fixed
 
+## [0.32.1] - 11/01/2024
+
+### Added
+
+### Changed
+
+### Fixed
+
+- Fix two git methods where `GitError` was not being instantiated correctly ([562])
+- Fix determination of auth commits to be validated when starting the update from the beginning ([562])
+
+[562]: https://github.com/openlawlibrary/taf/pull/562
+
 ## [0.32.0] - 10/23/2024
 
 ### Added
@@ -23,8 +36,8 @@ and this project adheres to [Semantic Versioning][semver].
 
 ### Fixed
 
-- Fix specification of pygit2 version depending on the Python version [(558)]
-- Fix validation and listing targets of an auth repo that does not contain `mirrors.json` [(558)]
+- Fix specification of pygit2 version depending on the Python version ([558])
+- Fix validation and listing targets of an auth repo that does not contain `mirrors.json` ([558])
 
 [558]: https://github.com/openlawlibrary/taf/pull/558
 
@@ -1319,7 +1332,8 @@ and this project adheres to [Semantic Versioning][semver].
 
 [keepachangelog]: https://keepachangelog.com/en/1.0.0/
 [semver]: https://semver.org/spec/v2.0.0.html
-[unreleased]: https://github.com/openlawlibrary/taf/compare/v0.31.2...HEAD
+[unreleased]: https://github.com/openlawlibrary/taf/compare/v0.32.1...HEAD
+[0.32.1]: https://github.com/openlawlibrary/taf/compare/v0.32.0...v0.32.1
 [0.32.0]: https://github.com/openlawlibrary/taf/compare/v0.31.2...v0.32.0
 [0.31.2]: https://github.com/openlawlibrary/taf/compare/v0.31.1...v0.31.2
 [0.31.1]: https://github.com/openlawlibrary/taf/compare/v0.31.0...v0.31.1

setup.py

@@ -1,7 +1,7 @@
 from setuptools import find_packages, setup
 
 PACKAGE_NAME = "taf"
-VERSION = "0.32.0"
+VERSION = "0.32.1"
 AUTHOR = "Open Law Library"
 AUTHOR_EMAIL = "info@openlawlib.org"
 DESCRIPTION = "Implementation of archival authentication"

taf/api/repository.py

@@ -1,3 +1,4 @@
+import json
 from logging import ERROR, INFO
 from typing import Optional
 import click
@@ -193,7 +194,7 @@ def taf_status(path: str, library_dir: Optional[str] = None, indent: int = 0) ->
     print(f"{indent_str}Something to commit: {auth_repo.something_to_commit()}")
     print(f"{indent_str}Target Repositories Status:")
     # Call the list_targets function
-    list_targets(path=path)
+    print(json.dumps(list_targets(path=path), indent=4))
 
     # Load dependencies using repositoriesdb.get_auth_repositories
     repositoriesdb.load_dependencies(auth_repo, library_dir=library_dir)

taf/git.py

@@ -723,7 +723,9 @@ def clone_from_disk(
         self.path.mkdir(parents=True, exist_ok=True)
         pygit2.clone_repository(local_path, self.path, bare=is_bare)
         if not self.is_git_repository:
-            raise GitError(f"Could not clone repository from local path {local_path}")
+            raise GitError(
+                self, message=f"Could not clone repository from local path {local_path}"
+            )
         repo = self.pygit_repo
 
         if not keep_remote:
@@ -1627,7 +1629,7 @@ def top_commit_of_remote_branch(self, branch, remote="origin"):
         remote_branch = f"{remote}/{branch}"
         if not self.branch_exists(remote_branch, include_remotes=True):
             raise GitError(
-                f"Branch {remote_branch} does not exist in the remote repository."
+                self, message=f"Remote branch {remote_branch} does not exist"
             )
         return self.top_commit_of_branch(remote_branch)
 

taf/tools/cli/__init__.py

@@ -12,13 +12,14 @@
 from taf.utils import is_run_from_python_executable, on_rm_error
 
 
-def catch_cli_exception(func=None, *, handle=TAFError, print_error=False, remove_dir_on_error=False):
+def catch_cli_exception(func=None, *, handle=TAFError, print_error=False, remove_dir_on_error=False, skip_cleanup=False):
     if not func:
         return partial(
             catch_cli_exception,
             handle=handle,
             print_error=print_error,
-            remove_dir_on_error=remove_dir_on_error
+            remove_dir_on_error=remove_dir_on_error,
+            skip_cleanup=skip_cleanup,
         )
 
     handle = tuple(handle) if isinstance(handle, (list, tuple)) else (handle,)
@@ -40,10 +41,10 @@ def wrapper(*args, **kwargs):
             else:
                 raise e
         finally:
-            if not successful and "path" in kwargs:
+            if not skip_cleanup and not successful and "path" in kwargs:
                 path = kwargs["path"]
                 repo = GitRepository(path=path)
-                if repo.is_git_repository:
+                if repo.is_git_repository and not repo.is_bare_repository:
                     repo.clean_and_reset()
                 if remove_dir_on_error:
                     shutil.rmtree(path, onerror=on_rm_error)

taf/tools/repo/__init__.py

@@ -166,7 +166,7 @@ def clone_repo_command():
         --strict, which will raise errors during the update if any warnings are found. By default, --strict
         is disabled.
         """)
-    @catch_cli_exception(handle=UpdateFailedError)
+    @catch_cli_exception(handle=UpdateFailedError, skip_cleanup=True)
     @click.argument("url")
     @common_update_options
     @click.option("--path", help="Authentication repository's location. If not specified, calculated by combining repository's name specified in info.json and library dir")
@@ -235,7 +235,7 @@ def update_repo_command():
         is disabled.
         """)
     @find_repository
-    @catch_cli_exception(handle=UpdateFailedError)
+    @catch_cli_exception(handle=UpdateFailedError, skip_cleanup=True)
     @common_update_options
     @click.option("--path", default=".", help="Authentication repository's location. If not specified, set to the current directory")
     @click.option("--library-dir", default=None, help="Directory where target repositories and, optionally, authentication repository are located. If not specified, calculated based on the authentication repository's path")

taf/updater/updater_pipeline.py

@@ -1060,31 +1060,46 @@ def determine_start_commits(self):
             # we still expect the last validated target commit to exist
             # and the remaining commits will be validated afterwards
             # if the last validated target commit does not exist, start the validation from scratch
-            if self.state.last_validated_commit is not None:
-                for repository in self.state.temp_target_repositories.values():
-                    if repository.name not in self.state.targets_data_by_auth_commits:
-                        continue
-                    self.state.old_heads_per_target_repos_branches[repository.name] = {}
-                    last_validated_repository_commits_data = (
-                        self.state.targets_data_by_auth_commits[repository.name].get(
-                            self.state.last_validated_commit, {}
+            try:
+                if self.state.last_validated_commit is not None:
+                    for repository in self.state.temp_target_repositories.values():
+                        if (
+                            repository.name
+                            not in self.state.targets_data_by_auth_commits
+                        ):
+                            continue
+                        self.state.old_heads_per_target_repos_branches[
+                            repository.name
+                        ] = {}
+                        last_validated_repository_commits_data = (
+                            self.state.targets_data_by_auth_commits[
+                                repository.name
+                            ].get(self.state.last_validated_commit, {})
                         )
-                    )
 
-                    if last_validated_repository_commits_data:
-                        if repository.name in self.state.repos_not_on_disk:
-                            is_initial_state_in_sync = False
-                            break
-                        if not self._is_repository_in_sync(
-                            repository, last_validated_repository_commits_data
-                        ):
-                            is_initial_state_in_sync = False
-                            break
+                        if last_validated_repository_commits_data:
+                            if repository.name in self.state.repos_not_on_disk:
+                                is_initial_state_in_sync = False
+                                break
+                            if not self._is_repository_in_sync(
+                                repository, last_validated_repository_commits_data
+                            ):
+                                is_initial_state_in_sync = False
+                                break
+
+                if not is_initial_state_in_sync:
+                    taf_logger.log(
+                        "NOTICE",
+                        f"{self.state.users_auth_repo.name}: states of target repositories are not in sync with last validated commit. Starting the validation from the beginning",
+                    )
+            except Exception as e:
+                taf_logger.log(
+                    "NOTICE",
+                    f"{self.state.users_auth_repo.name}: could not determine if repos are in sync due to error. Starting the validation from the beginning. Error: {e}",
+                )
+                is_initial_state_in_sync = False
 
             if not is_initial_state_in_sync:
-                taf_logger.info(
-                    f"{self.state.users_auth_repo.name}: states of target repositories are not in sync with last validated commit. Starting the update from the beginning"
-                )
                 self._update_state_for_initial_sync()
                 self.reset_target_repositories()
 
@@ -1152,6 +1167,11 @@ def _update_state_for_initial_sync(self):
                 self.state.users_auth_repo.default_branch
             )
         )
+        # append fetched commits
+        if self.state.update_handler is not None and self.state.update_handler.commits:
+            self.state.auth_commits_since_last_validated.extend(
+                self.state.update_handler.commits[1:]
+            )
         self.state.targets_data_by_auth_commits = (
             self.state.users_auth_repo.targets_data_by_auth_commits(
                 self.state.auth_commits_since_last_validated