From bc0cf9f40f784d936bb42214b2cde2b6605e136c Mon Sep 17 00:00:00 2001 From: Edmundo Carmona Antoranz Date: Sun, 22 Jun 2025 14:53:54 +0200 Subject: [PATCH] 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. --- pygit2/index.py | 42 ++++++++++++ pygit2/repository.py | 29 ++++++-- test/test_repository.py | 145 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 209 insertions(+), 7 deletions(-) diff --git a/pygit2/index.py b/pygit2/index.py index b1096056..8f7bb0a9 100644 --- a/pygit2/index.py +++ b/pygit2/index.py @@ -23,8 +23,10 @@ # the Free Software Foundation, 51 Franklin Street, Fifth Floor, # Boston, MA 02110-1301, USA. +import typing import warnings import weakref +from dataclasses import dataclass # Import from pygit2 from ._pygit2 import Oid, Tree, Diff @@ -349,6 +351,46 @@ def conflicts(self): return self._conflicts() +@dataclass +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 __repr__(self): + t = type(self) + contents = ( + self.contents if len(self.contents) <= 20 else f'{self.contents[:20]}...' + ) + return ( + f'<{t.__module__}.{t.__qualname__} "' + f'automergeable={self.automergeable} "' + f'path={self.path} ' + f'mode={self.mode} ' + f'contents={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' diff --git a/pygit2/repository.py b/pygit2/repository.py index 34712bb3..37a6d2c5 100644 --- a/pygit2/repository.py +++ b/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, diff --git a/test/test_repository.py b/test/test_repository.py index d8c8efa1..d48aa7ac 100644 --- a/test/test_repository.py +++ b/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,7 +42,9 @@ RepositoryState, ResetMode, StashApplyProgress, + FileMode, ) +from pygit2.index import MergeFileResult from . import utils @@ -985,3 +987,144 @@ 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) + + # 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 == MergeFileResult( + True, hello_txt.path, hello_txt.mode, 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 == MergeFileResult( + True, + hello_txt.path, + hello_txt_executable.mode, + 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 == MergeFileResult( + True, + hello_txt.path, + hello_txt_executable.mode, + 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 == MergeFileResult( + True, + hello_txt.path, + hello_txt_executable.mode, + 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 == MergeFileResult( + True, hello_world.path, hello_txt.mode, 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 == MergeFileResult( + True, hello_world.path, hello_txt.mode, 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 == MergeFileResult( + True, None, hello_txt.mode, 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 == MergeFileResult( + True, + hello_world.path, + hello_txt_executable.mode, + 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 == MergeFileResult( + True, + hello_world.path, + hello_txt_executable.mode, + testrepo.get(hello_txt.id).data.decode(), + )