Improve handling of git operations (#890)

Our handling of git operations in the tool-suite is a mess; some functions use pygit2, others use command-line git and all have different interfaces and conventions and I am very annoyed that I always have the incorrect parameters at hand when I need to use some git functionality.

This PR aims to improve this situation by introducing a lightweight RepositoryHandle class that allows easy and uniform access to both, pygit2 and command-line git`. Since git is ubiquitous in the tool-suite, there are a lot of changes, but most of them are very minor.

As a result of the refactoring, git_util no longer depends on project_util, but the other way round.
In my opinion this makes more sense since projects make use of git, while git can happily be used without projects.

One open point that is remains is to sort out what belongs in git_util vs. git_commands since that is currently not very strict and at least one function in git_commands uses a function in git_util, which imo it should not do.

.pre-commit-config.yaml

@@ -2,7 +2,7 @@
 # See https://pre-commit.com/hooks.html for more hooks
 repos:
 -   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.1.0
+    rev: v4.6.0
     hooks:
     -   id: trailing-whitespace
     -   id: end-of-file-fixer
@@ -13,17 +13,17 @@ repos:
         args: ['--django']
         exclude: 'tests/helper_utils.py'
 -   repo: https://github.com/timothycrosley/isort.git
-    rev: 5.11.5
+    rev: 5.13.2
     hooks:
     - id: isort
       args: ['--nis']
 -   repo: https://github.com/myint/docformatter.git
-    rev: v1.4
+    rev: v1.7.5
     hooks:
     - id: docformatter
       args: ['--in-place', '--wrap-summaries=80', '--wrap-descriptions=80', '--pre-summary-newline']
 -   repo: https://github.com/MarcoGorelli/auto-walrus
-    rev: v0.2.2
+    rev: 0.3.4
     hooks:
     -   id: auto-walrus
 -   repo: https://github.com/pre-commit/mirrors-yapf

tests/data/test_commit_report.py

@@ -16,7 +16,7 @@
     generate_interactions,
 )
 from varats.mapping.commit_map import CommitMap
-from varats.project.project_util import get_local_project_git_path
+from varats.project.project_util import get_local_project_repo
 from varats.projects.discover_projects import initialize_projects
 from varats.report.report import FileStatusExtension, ReportFilename
 from varats.utils.git_util import FullCommitHash, ShortCommitHash
@@ -355,9 +355,9 @@ def testing_gen_commit_map() -> CommitMap:
     """Generate a local commit map for testing."""
 
     initialize_projects()
-    xz_repo_path = get_local_project_git_path("xz")
+    xz_repo = get_local_project_repo("xz")
     return CommitMap(
-        xz_repo_path,
+        xz_repo,
         start="923bf96b55e5216a6c8df9d8331934f54784390e",
         end="80a1a8bb838842a2be343bd88ad1462c21c5e2c9"
     )

tests/paper_mgmt/test_case_study.py

@@ -17,7 +17,7 @@
 from varats.paper.paper_config import get_paper_config, load_paper_config
 from varats.plot.plots import PlotConfig
 from varats.plots.case_study_overview import CaseStudyOverviewPlot
-from varats.project.project_util import get_local_project_git_path
+from varats.project.project_util import get_local_project_repo
 from varats.projects.discover_projects import initialize_projects
 from varats.report.report import FileStatusExtension, ReportFilename
 from varats.utils.git_util import FullCommitHash, ShortCommitHash
@@ -342,12 +342,12 @@ def test_extend_with_revs_per_year(self) -> None:
         random.seed(42)
 
         cs = CaseStudy("xz", 0)
-        git_path = get_local_project_git_path("xz")
+        repo = get_local_project_repo("xz")
         cmap = get_commit_map(
             "xz", end="c5c7ceb08a011b97d261798033e2c39613a69eb7"
         )
 
-        MCS.extend_with_revs_per_year(cs, cmap, 0, True, str(git_path), 2, True)
+        MCS.extend_with_revs_per_year(cs, cmap, 0, True, repo, 2, True)
         self.assertEqual(cs.num_stages, 17)
         self.assertEqual(len(cs.revisions), 31)
         self.assertEqual(

tests/provider/test_bug_provider.py

@@ -23,6 +23,7 @@
     _filter_commit_message_bugs,
 )
 from varats.provider.bug.bug_provider import BugProvider
+from varats.utils.git_util import RepositoryHandle
 
 
 class DummyIssueData:
@@ -164,8 +165,10 @@ def get(commit_id: str) -> pygit2.Commit:
             return mock_commit
 
         # pygit2 dummy repo
-        self.mock_repo = mock.create_autospec(pygit2.Repository)
-        self.mock_repo.get = get
+        self.mock_pygit = mock.create_autospec(pygit2.Repository)
+        self.mock_pygit.get = get
+        self.mock_repo_handle = mock.create_autospec(RepositoryHandle)
+        self.mock_repo_handle.pygit_repo = self.mock_pygit
 
     def test_issue_events_closing_bug(self) -> None:
         """Test identifying issue events that close a bug related issue, with
@@ -257,17 +260,17 @@ def test_pygit_bug_creation(self, mock_pydriller_git) -> None:
         mock_pydriller_git.return_value = DummyPydrillerRepo("")
 
         pybug = _create_corresponding_bug(
-            self.mock_repo.get(issue_event.commit_id), self.mock_repo,
+            self.mock_pygit.get(issue_event.commit_id), self.mock_pygit,
             issue_event.issue.number
         )
 
         self.assertEqual(issue_event.commit_id, str(pybug.fixing_commit.id))
         self.assertEqual(issue_event.issue.number, pybug.issue_id)
 
     @mock.patch('varats.provider.bug.bug.pydriller.Git')
-    @mock.patch('varats.provider.bug.bug.get_local_project_git')
+    @mock.patch('varats.provider.bug.bug.get_local_project_repo')
     def test_filter_issue_bugs(
-        self, mock_get_local_project_git, mock_pydriller_git
+        self, mock_get_local_project_repo, mock_pydriller_git
     ) -> None:
         """Test on a set of IssueEvents whether the corresponding set of bugs is
         created correctly."""
@@ -302,7 +305,7 @@ def test_filter_issue_bugs(
             event_close_second, event_close_first
         ]
 
-        mock_get_local_project_git.return_value = self.mock_repo
+        mock_get_local_project_repo.return_value = self.mock_repo_handle
         mock_pydriller_git.return_value = DummyPydrillerRepo("")
 
         # issue filter method for pygit bugs
@@ -346,9 +349,9 @@ def accept_pybugs(
         self.assertEqual(expected_second_bug_intro_ids, intro_second_bug)
 
     @mock.patch('varats.provider.bug.bug.pydriller.Git')
-    @mock.patch('varats.provider.bug.bug.get_local_project_git')
+    @mock.patch('varats.provider.bug.bug.get_local_project_repo')
     def test_filter_commit_message_bugs(
-        self, mock_get_local_project_git, mock_pydriller_git
+        self, mock_get_local_project_repo, mock_pydriller_git
     ) -> None:
         """Test on the commit history of a project whether the corresponding set
         of bugs is created correctly."""
@@ -377,18 +380,18 @@ def mock_walk(_start_id: str, _sort_mode: int):
             ])
 
         # customize walk method of mock repo
-        self.mock_repo.walk = mock.create_autospec(
+        self.mock_pygit.walk = mock.create_autospec(
             pygit2.Repository.walk, side_effect=mock_walk
         )
 
-        mock_get_local_project_git.return_value = self.mock_repo
+        mock_get_local_project_repo.return_value = self.mock_repo_handle
         mock_pydriller_git.return_value = DummyPydrillerRepo("")
 
         # commit filter method for pygit bugs
         def accept_pybugs(repo: pygit2.Repository,
                           commit: pygit2.Commit) -> tp.Optional[PygitBug]:
             if _is_closing_message(commit.message):
-                return _create_corresponding_bug(commit, self.mock_repo)
+                return _create_corresponding_bug(commit, self.mock_pygit)
             return None
 
         pybug_ids = set(

tests/provider/test_patch_provider.py

@@ -4,7 +4,6 @@
 
 import benchbuild as bb
 from benchbuild.source.base import target_prefix
-from benchbuild.utils.revision_ranges import _get_git_for_path
 
 from tests.helper_utils import TEST_INPUTS_DIR
 from varats.projects.perf_tests.feature_perf_cs_collection import (
@@ -15,6 +14,7 @@
     ShortCommitHash,
     get_all_revisions_between,
     get_initial_commit,
+    RepositoryHandle,
 )
 
 
@@ -76,12 +76,14 @@ def setUpClass(cls) -> None:
 
         project_git_source.fetch()
 
-        repo_git_path = Path(target_prefix() + "/FeaturePerfCSCollection")
+        repo = RepositoryHandle(
+            Path(target_prefix() + "/FeaturePerfCSCollection")
+        )
 
         cls.all_revisions = set(
             get_all_revisions_between(
-                get_initial_commit(repo_git_path).hash, "", ShortCommitHash,
-                repo_git_path
+                repo,
+                get_initial_commit(repo).hash, "", ShortCommitHash
             )
         )
 

tests/tools/test_driver_gen_benchbuild_config.py

@@ -14,7 +14,7 @@ class TestDriverGenBenchbuildConfig(unittest.TestCase):
 
     @run_in_test_environment()
     def test_gen_bbconfig(self):
-        """basic tests for the `gen-bbconfig` command."""
+        """Basic tests for the `gen-bbconfig` command."""
         runner = CliRunner()
         Path(vara_cfg()["benchbuild_root"].value + "/.benchbuild.yml").unlink()
         runner.invoke(driver_gen_benchbuild_config.main, [])