Skip to content

Deprecate passing str to Repository.merge (per #1348) #1349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions pygit2/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
# along with this program; see the file COPYING. If not, write to
# the Free Software Foundation, 51 Franklin Street, Fifth Floor,
# Boston, MA 02110-1301, USA.

import warnings
from io import BytesIO
from os import PathLike
from string import hexdigits
Expand All @@ -43,7 +43,6 @@
from .enums import (
AttrCheck,
BlameFlag,
BranchType,
CheckoutStrategy,
DescribeStrategy,
DiffOption,
Expand Down Expand Up @@ -879,15 +878,18 @@ def merge(
# Annotated commit from commit id
if isinstance(source, str):
# For backwards compatibility, parse a string as a partial commit hash
warnings.warn(
'Passing str to Repository.merge is deprecated. '
'Pass Commit, Oid, or a Reference (such as a Branch) instead.',
DeprecationWarning,
)
oid = self[source].peel(Commit).id
elif isinstance(source, Commit):
oid = source.id
elif isinstance(source, Oid):
oid = source
else:
raise TypeError(
'expected Reference, Commit, Oid, or commit hash string'
)
raise TypeError('expected Reference, Commit, or Oid')
c_id = ffi.new('git_oid *')
ffi.buffer(c_id)[:] = oid.raw[:]
commit_ptr = ffi.new('git_annotated_commit **')
Expand Down
68 changes: 39 additions & 29 deletions test/test_merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ def test_merge_invalid_type(mergerepo, id):
mergerepo.merge(id)


# TODO: Once Repository.merge drops support for str arguments,
# add an extra parameter to test_merge_invalid_type above
# to make sure we cover legacy code.
def test_merge_string_argument_deprecated(mergerepo):
branch_head_hex = '5ebeeebb320790caf276b9fc8b24546d63316533'

with pytest.warns(DeprecationWarning, match=r'Pass Commit.+instead'):
mergerepo.merge(branch_head_hex)


def test_merge_analysis_uptodate(mergerepo):
branch_head_hex = '5ebeeebb320790caf276b9fc8b24546d63316533'
branch_id = mergerepo.get(branch_head_hex).id
Expand Down Expand Up @@ -82,7 +92,10 @@ def test_merge_no_fastforward_no_conflicts(mergerepo):

def test_merge_invalid_hex(mergerepo):
branch_head_hex = '12345678'
with pytest.raises(KeyError):
with (
pytest.raises(KeyError),
pytest.warns(DeprecationWarning, match=r'Pass Commit.+instead'),
):
mergerepo.merge(branch_head_hex)


Expand Down Expand Up @@ -132,7 +145,7 @@ def test_merge_no_fastforward_conflicts(mergerepo):


def test_merge_remove_conflicts(mergerepo):
other_branch_tip = '1b2bae55ac95a4be3f8983b86cd579226d0eb247'
other_branch_tip = pygit2.Oid(hex='1b2bae55ac95a4be3f8983b86cd579226d0eb247')
mergerepo.merge(other_branch_tip)
idx = mergerepo.index
conflicts = idx.conflicts
Expand All @@ -158,30 +171,29 @@ def test_merge_remove_conflicts(mergerepo):
],
)
def test_merge_favor(mergerepo, favor):
branch_head_hex = '1b2bae55ac95a4be3f8983b86cd579226d0eb247'
mergerepo.merge(branch_head_hex, favor=favor)
branch_head = pygit2.Oid(hex='1b2bae55ac95a4be3f8983b86cd579226d0eb247')
mergerepo.merge(branch_head, favor=favor)

assert mergerepo.index.conflicts is None


def test_merge_fail_on_conflict(mergerepo):
branch_head_hex = '1b2bae55ac95a4be3f8983b86cd579226d0eb247'
branch_head = pygit2.Oid(hex='1b2bae55ac95a4be3f8983b86cd579226d0eb247')

with pytest.raises(pygit2.GitError):
with pytest.raises(pygit2.GitError, match=r'merge conflicts exist'):
mergerepo.merge(
branch_head_hex, flags=MergeFlag.FIND_RENAMES | MergeFlag.FAIL_ON_CONFLICT
branch_head, flags=MergeFlag.FIND_RENAMES | MergeFlag.FAIL_ON_CONFLICT
)


def test_merge_commits(mergerepo):
branch_head_hex = '03490f16b15a09913edb3a067a3dc67fbb8d41f1'
branch_id = mergerepo.get(branch_head_hex).id
branch_head = pygit2.Oid(hex='03490f16b15a09913edb3a067a3dc67fbb8d41f1')

merge_index = mergerepo.merge_commits(mergerepo.head.target, branch_head_hex)
merge_index = mergerepo.merge_commits(mergerepo.head.target, branch_head)
assert merge_index.conflicts is None
merge_commits_tree = merge_index.write_tree(mergerepo)

mergerepo.merge(branch_id)
mergerepo.merge(branch_head)
index = mergerepo.index
assert index.conflicts is None
merge_tree = index.write_tree()
Expand All @@ -190,26 +202,23 @@ def test_merge_commits(mergerepo):


def test_merge_commits_favor(mergerepo):
branch_head_hex = '1b2bae55ac95a4be3f8983b86cd579226d0eb247'
branch_head = pygit2.Oid(hex='1b2bae55ac95a4be3f8983b86cd579226d0eb247')

merge_index = mergerepo.merge_commits(
mergerepo.head.target, branch_head_hex, favor=MergeFavor.OURS
mergerepo.head.target, branch_head, favor=MergeFavor.OURS
)
assert merge_index.conflicts is None

# Incorrect favor value
with pytest.raises(TypeError):
mergerepo.merge_commits(mergerepo.head.target, branch_head_hex, favor='foo')
with pytest.raises(TypeError, match=r'favor argument must be MergeFavor'):
mergerepo.merge_commits(mergerepo.head.target, branch_head, favor='foo')


def test_merge_trees(mergerepo):
branch_head_hex = '03490f16b15a09913edb3a067a3dc67fbb8d41f1'
branch_id = mergerepo.get(branch_head_hex).id
branch_id = pygit2.Oid(hex='03490f16b15a09913edb3a067a3dc67fbb8d41f1')
ancestor_id = mergerepo.merge_base(mergerepo.head.target, branch_id)

merge_index = mergerepo.merge_trees(
ancestor_id, mergerepo.head.target, branch_head_hex
)
merge_index = mergerepo.merge_trees(ancestor_id, mergerepo.head.target, branch_id)
assert merge_index.conflicts is None
merge_commits_tree = merge_index.write_tree(mergerepo)

Expand Down Expand Up @@ -312,10 +321,10 @@ def test_merge_octopus(mergerepo):
def test_merge_mergeheads(mergerepo):
assert mergerepo.listall_mergeheads() == []

branch_head_hex = '1b2bae55ac95a4be3f8983b86cd579226d0eb247'
mergerepo.merge(branch_head_hex)
branch_head = pygit2.Oid(hex='1b2bae55ac95a4be3f8983b86cd579226d0eb247')
mergerepo.merge(branch_head)

assert mergerepo.listall_mergeheads() == [pygit2.Oid(hex=branch_head_hex)]
assert mergerepo.listall_mergeheads() == [branch_head]

mergerepo.state_cleanup()
assert mergerepo.listall_mergeheads() == [], (
Expand All @@ -327,27 +336,28 @@ def test_merge_message(mergerepo):
assert not mergerepo.message
assert not mergerepo.raw_message

branch_head_hex = '1b2bae55ac95a4be3f8983b86cd579226d0eb247'
mergerepo.merge(branch_head_hex)
branch_head = pygit2.Oid(hex='1b2bae55ac95a4be3f8983b86cd579226d0eb247')
mergerepo.merge(branch_head)

assert mergerepo.message.startswith(f"Merge commit '{branch_head_hex}'")
assert mergerepo.message.startswith(f"Merge commit '{branch_head}'")
assert mergerepo.message.encode('utf-8') == mergerepo.raw_message

mergerepo.state_cleanup()
assert not mergerepo.message


def test_merge_remove_message(mergerepo):
branch_head_hex = '1b2bae55ac95a4be3f8983b86cd579226d0eb247'
mergerepo.merge(branch_head_hex)
branch_head = pygit2.Oid(hex='1b2bae55ac95a4be3f8983b86cd579226d0eb247')
mergerepo.merge(branch_head)

assert mergerepo.message.startswith(f"Merge commit '{branch_head_hex}'")
assert mergerepo.message.startswith(f"Merge commit '{branch_head}'")
mergerepo.remove_message()
assert not mergerepo.message


def test_merge_commit(mergerepo):
commit = mergerepo['1b2bae55ac95a4be3f8983b86cd579226d0eb247']
assert isinstance(commit, pygit2.Commit)
mergerepo.merge(commit)

assert mergerepo.message.startswith(f"Merge commit '{str(commit.id)}'")
Expand Down
Loading