Repository: get an instance of MergeFileResult from git_merge_file_from_index

In Repository.merge_file_from_index, pygit2 is only returning
the resulting content out of the merge, however other values might be
relevant like the filemode.

Add a new class named MergeFileResult that maps to libgit2's
git_merge_file_result.

Add a flag called "use_deprecated" in Repository.merge_file_from_index
to control whether the caller wants to use the deprecated functionality
receiving an string. In case use_deprecated==False, an instance of
MergeFileResult will be used, instead.

The default value for use_deprecated is True so not to break existing
callers all of the sudden. Later, giving enough time for callers to
notice the deprecation warning and adjust their calls to use
use_deprecated=False, the default value can be switched to False. And even
more time later the flag can disappear completely from the method once
all callers have removed the parameter because they are already using
the non-deprecated version by removing the parameter completely from
their calls.

Add a warning when using the deprecated implementation.

Author: eantoranz

pygit2/index.py

@@ -23,6 +23,7 @@
 # the Free Software Foundation, 51 Franklin Street, Fifth Floor,
 # Boston, MA 02110-1301, USA.
 
+import typing
 import warnings
 import weakref
 
@@ -349,6 +350,65 @@ def conflicts(self):
         return self._conflicts()
 
 
+class MergeFileResult:
+    automergeable: bool
+    'True if the output was automerged, false if the output contains conflict markers'
+
+    path: typing.Union[str, None]
+    'The path that the resultant merge file should use, or None if a filename conflict would occur'
+
+    mode: FileMode
+    'The mode that the resultant merge file should use'
+
+    contents: str
+    'Contents of the file, which might include conflict markers'
+
+    def __init__(
+        self,
+        automergeable: int,
+        path: typing.Union[str, None],
+        mode: FileMode,
+        contents: str,
+    ):
+        self.automergeable = automergeable
+        self.path = path
+        self.mode = mode
+        self.contents = contents
+
+    def __str__(self):
+        # FIXME Should I include contents over here?
+        return f'<automergeable={self.automergeable} path={self.path} mode={self.mode} content={len(self.contents)}>'
+
+    def __repr__(self):
+        t = type(self)
+        # FIXME should I include contents over here?
+        return f'<{t.__module__}.{t.__qualname__} automergeable={self.automergeable} path={self.path} mode={self.mode}>'
+
+    def __eq__(self, other):
+        if self is other:
+            return True
+        if not isinstance(other, MergeFileResult):
+            return NotImplemented
+        return (
+            self.automergeable == other.automergeable
+            and self.path == other.path
+            and self.mode == other.mode
+            and self.contents == other.contents
+        )
+
+    @classmethod
+    def _from_c(cls, centry):
+        if centry == ffi.NULL:
+            return None
+
+        automergeable = centry.automergeable != 0
+        path = to_str(ffi.string(centry.path)) if centry.path else None
+        mode = FileMode(centry.mode)
+        contents = ffi.string(centry.ptr, centry.len).decode('utf-8')
+
+        return MergeFileResult(automergeable, path, mode, contents)
+
+
 class IndexEntry:
     path: str
     'The path of this entry'

pygit2/repository.py

@@ -56,7 +56,7 @@
 )
 from .errors import check_error
 from .ffi import ffi, C
-from .index import Index, IndexEntry
+from .index import Index, IndexEntry, MergeFileResult
 from .packbuilder import PackBuilder
 from .references import References
 from .remotes import RemoteCollection
@@ -682,9 +682,13 @@ def merge_file_from_index(
         ancestor: typing.Union[None, IndexEntry],
         ours: typing.Union[None, IndexEntry],
         theirs: typing.Union[None, IndexEntry],
-    ) -> str:
-        """Merge files from index. Return a string with the merge result
-        containing possible conflicts.
+        use_deprecated: bool = True,
+    ) -> typing.Union[str, typing.Union[MergeFileResult, None]]:
+        """Merge files from index.
+
+        Returns: A string with the content of the file containing
+        possible conflicts if use_deprecated==True.
+        If use_deprecated==False then it returns an instance of MergeFileResult.
 
         ancestor
             The index entry which will be used as a common
@@ -693,6 +697,10 @@ def merge_file_from_index(
             The index entry to take as "ours" or base.
         theirs
             The index entry which will be merged into "ours"
+        use_deprecated
+            This controls what will be returned. If use_deprecated==True (default),
+            a string with the contents of the file will be returned.
+            An instance of MergeFileResult will be returned otherwise.
         """
         cmergeresult = ffi.new('git_merge_file_result *')
 
@@ -709,10 +717,19 @@ def merge_file_from_index(
         )
         check_error(err)
 
-        ret = ffi.string(cmergeresult.ptr, cmergeresult.len).decode('utf-8')
+        mergeFileResult = MergeFileResult._from_c(cmergeresult)
         C.git_merge_file_result_free(cmergeresult)
 
-        return ret
+        if use_deprecated:
+            warnings.warn(
+                'Getting an str from Repository.merge_file_from_index is deprecated. '
+                'The method will later return an instance of MergeFileResult by default, instead. '
+                'Check parameter use_deprecated.',
+                DeprecationWarning,
+            )
+            return mergeFileResult.contents if mergeFileResult else ''
+
+        return mergeFileResult
 
     def merge_commits(
         self,

test/test_repository.py

@@ -31,7 +31,7 @@
 
 # pygit2
 import pygit2
-from pygit2 import init_repository, clone_repository, discover_repository
+from pygit2 import init_repository, clone_repository, discover_repository, IndexEntry
 from pygit2 import Oid
 from pygit2.enums import (
     CheckoutNotify,
@@ -42,6 +42,7 @@
     RepositoryState,
     ResetMode,
     StashApplyProgress,
+    FileMode,
 )
 from . import utils
 
@@ -985,3 +986,139 @@ def test_repository_hashfile_filter(testrepo):
     testrepo.config['core.safecrlf'] = 'fail'
     with pytest.raises(pygit2.GitError):
         h = testrepo.hashfile('hello.txt')
+
+
+def test_merge_file_from_index_deprecated(testrepo):
+    hello_txt = testrepo.index['hello.txt']
+    hello_txt_executable = IndexEntry(
+        hello_txt.path, hello_txt.id, FileMode.BLOB_EXECUTABLE
+    )
+    hello_world = IndexEntry('hello_world.txt', hello_txt.id, hello_txt.mode)
+    other_file_blob = testrepo.create_blob('Data that will clash with hello.txt')
+
+    # no change
+    res = testrepo.merge_file_from_index(hello_txt, hello_txt, hello_txt)
+    assert res == testrepo.get(hello_txt.id).data.decode()
+
+    # executable switch on ours
+    res = testrepo.merge_file_from_index(hello_txt, hello_txt_executable, hello_txt)
+    assert res == testrepo.get(hello_txt.id).data.decode()
+
+    # executable switch on theirs
+    res = testrepo.merge_file_from_index(hello_txt, hello_txt, hello_txt_executable)
+    assert res == testrepo.get(hello_txt.id).data.decode()
+
+    # executable switch on both
+    res = testrepo.merge_file_from_index(
+        hello_txt, hello_txt_executable, hello_txt_executable
+    )
+    assert res == testrepo.get(hello_txt.id).data.decode()
+
+    # path switch on ours
+    res = testrepo.merge_file_from_index(hello_txt, hello_world, hello_txt)
+    assert res == testrepo.get(hello_txt.id).data.decode()
+
+    # path switch on theirs
+    res = testrepo.merge_file_from_index(hello_txt, hello_txt, hello_world)
+    assert res == testrepo.get(hello_txt.id).data.decode()
+
+    # path switch on both
+    res = testrepo.merge_file_from_index(hello_txt, hello_world, hello_world)
+    assert res == testrepo.get(hello_txt.id).data.decode()
+
+    # path switch on ours, executable flag switch on theirs
+    res = testrepo.merge_file_from_index(hello_txt, hello_world, hello_txt_executable)
+    assert res == testrepo.get(hello_txt.id).data.decode()
+
+    # path switch on theirs, executable flag switch on ours
+    res = testrepo.merge_file_from_index(hello_txt, hello_txt_executable, hello_world)
+    assert res == testrepo.get(hello_txt.id).data.decode()
+
+
+def test_merge_file_from_index_non_deprecated(testrepo):
+    hello_txt = testrepo.index['hello.txt']
+    hello_txt_executable = IndexEntry(
+        hello_txt.path, hello_txt.id, FileMode.BLOB_EXECUTABLE
+    )
+    hello_world = IndexEntry('hello_world.txt', hello_txt.id, hello_txt.mode)
+
+    # no change
+    res = testrepo.merge_file_from_index(
+        hello_txt, hello_txt, hello_txt, use_deprecated=False
+    )
+    assert res.automergeable
+    assert res.path == hello_txt.path
+    assert res.mode == hello_txt.mode
+    assert res.contents == testrepo.get(hello_txt.id).data.decode()
+
+    # executable switch on ours
+    res = testrepo.merge_file_from_index(
+        hello_txt, hello_txt_executable, hello_txt, use_deprecated=False
+    )
+    assert res.automergeable
+    assert res.path == hello_txt.path
+    assert res.mode == hello_txt_executable.mode
+    assert res.contents == testrepo.get(hello_txt.id).data.decode()
+
+    # executable switch on theirs
+    res = testrepo.merge_file_from_index(
+        hello_txt, hello_txt, hello_txt_executable, use_deprecated=False
+    )
+    assert res.automergeable
+    assert res.path == hello_txt.path
+    assert res.mode == hello_txt_executable.mode
+    assert res.contents == testrepo.get(hello_txt.id).data.decode()
+
+    # executable switch on both
+    res = testrepo.merge_file_from_index(
+        hello_txt, hello_txt_executable, hello_txt_executable, use_deprecated=False
+    )
+    assert res.automergeable
+    assert res.path == hello_txt.path
+    assert res.mode == hello_txt_executable.mode
+    assert res.contents == testrepo.get(hello_txt.id).data.decode()
+
+    # path switch on ours
+    res = testrepo.merge_file_from_index(
+        hello_txt, hello_world, hello_txt, use_deprecated=False
+    )
+    assert res.automergeable
+    assert res.path == hello_world.path
+    assert res.mode == hello_txt.mode
+    assert res.contents == testrepo.get(hello_txt.id).data.decode()
+
+    # path switch on theirs
+    res = testrepo.merge_file_from_index(
+        hello_txt, hello_txt, hello_world, use_deprecated=False
+    )
+    assert res.automergeable
+    assert res.path == hello_world.path
+    assert res.mode == hello_txt.mode
+    assert res.contents == testrepo.get(hello_txt.id).data.decode()
+
+    # path switch on both
+    res = testrepo.merge_file_from_index(
+        hello_txt, hello_world, hello_world, use_deprecated=False
+    )
+    assert res.automergeable
+    assert res.path is None
+    assert res.mode == hello_txt.mode
+    assert res.contents == testrepo.get(hello_txt.id).data.decode()
+
+    # path switch on ours, executable flag switch on theirs
+    res = testrepo.merge_file_from_index(
+        hello_txt, hello_world, hello_txt_executable, use_deprecated=False
+    )
+    assert res.automergeable
+    assert res.path == hello_world.path
+    assert res.mode == hello_txt_executable.mode
+    assert res.contents == testrepo.get(hello_txt.id).data.decode()
+
+    # path switch on theirs, executable flag switch on ours
+    res = testrepo.merge_file_from_index(
+        hello_txt, hello_txt_executable, hello_world, use_deprecated=False
+    )
+    assert res.automergeable
+    assert res.path == hello_world.path
+    assert res.mode == hello_txt_executable.mode
+    assert res.contents == testrepo.get(hello_txt.id).data.decode()