From 7b48cda400b3ed3e9835d840f49c654655ddddba Mon Sep 17 00:00:00 2001 From: Dmitry Sleptsov Date: Tue, 23 May 2023 13:28:01 +0200 Subject: [PATCH 1/4] Move find_migration_names to Command class --- .../management/commands/rebase_migration.py | 21 +++++++++---------- tests/test_rebase_migration.py | 10 ++++----- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/django_linear_migrations/management/commands/rebase_migration.py b/src/django_linear_migrations/management/commands/rebase_migration.py index 6292e9a..0019ce9 100644 --- a/src/django_linear_migrations/management/commands/rebase_migration.py +++ b/src/django_linear_migrations/management/commands/rebase_migration.py @@ -50,7 +50,7 @@ def handle(self, *args: Any, app_label: str, **options: Any) -> None: if not max_migration_txt.exists(): raise CommandError(f"{app_label} does not have a max_migration.txt.") - migration_names = find_migration_names( + migration_names = self.find_migration_names( max_migration_txt.read_text().splitlines() ) if migration_names is None: @@ -175,16 +175,15 @@ def handle(self, *args: Any, app_label: str, **options: Any) -> None: + " updated its dependencies, and updated max_migration.txt." ) - -def find_migration_names(max_migration_lines: list[str]) -> tuple[str, str] | None: - lines = max_migration_lines - if len(lines) <= 1: - return None - if not lines[0].startswith("<<<<<<<"): - return None - if not lines[-1].startswith(">>>>>>>"): - return None - return lines[1].strip(), lines[-2].strip() + def find_migration_names(self, max_migration_lines: list[str]) -> tuple[str, str] | None: + lines = max_migration_lines + if len(lines) <= 1: + return None + if not lines[0].startswith("<<<<<<<"): + return None + if not lines[-1].startswith(">>>>>>>"): + return None + return lines[1].strip(), lines[-2].strip() def migration_applied(app_label: str, migration_name: str) -> bool: diff --git a/tests/test_rebase_migration.py b/tests/test_rebase_migration.py index 8066c39..991a64d 100644 --- a/tests/test_rebase_migration.py +++ b/tests/test_rebase_migration.py @@ -427,19 +427,19 @@ class Migration(migrations.Migration): class FindMigrationNamesTests(SimpleTestCase): def test_none_when_no_lines(self): - result = module.find_migration_names([]) + result = module.Command().find_migration_names([]) assert result is None def test_none_when_no_first_marker(self): - result = module.find_migration_names(["not_a_marker", "0002_author_nicknames"]) + result = module.Command().find_migration_names(["not_a_marker", "0002_author_nicknames"]) assert result is None def test_none_when_no_second_marker(self): - result = module.find_migration_names(["<<<<<<<", "0002_author_nicknames"]) + result = module.Command().find_migration_names(["<<<<<<<", "0002_author_nicknames"]) assert result is None def test_works_with_two_way_merge(self): - result = module.find_migration_names( + result = module.Command().find_migration_names( [ "<<<<<<<", "0002_author_nicknames", @@ -451,7 +451,7 @@ def test_works_with_two_way_merge(self): assert result == ("0002_author_nicknames", "0002_longer_titles") def test_works_with_three_way_merge(self): - result = module.find_migration_names( + result = module.Command().find_migration_names( [ "<<<<<<<", "0002_author_nicknames", From 15539e6e078bfabeb84da65d30f67a7eb5ca0233 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 23 May 2023 11:39:03 +0000 Subject: [PATCH 2/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../management/commands/rebase_migration.py | 4 +++- tests/test_rebase_migration.py | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/django_linear_migrations/management/commands/rebase_migration.py b/src/django_linear_migrations/management/commands/rebase_migration.py index 0019ce9..b67f038 100644 --- a/src/django_linear_migrations/management/commands/rebase_migration.py +++ b/src/django_linear_migrations/management/commands/rebase_migration.py @@ -175,7 +175,9 @@ def handle(self, *args: Any, app_label: str, **options: Any) -> None: + " updated its dependencies, and updated max_migration.txt." ) - def find_migration_names(self, max_migration_lines: list[str]) -> tuple[str, str] | None: + def find_migration_names( + self, max_migration_lines: list[str] + ) -> tuple[str, str] | None: lines = max_migration_lines if len(lines) <= 1: return None diff --git a/tests/test_rebase_migration.py b/tests/test_rebase_migration.py index 991a64d..46dfd97 100644 --- a/tests/test_rebase_migration.py +++ b/tests/test_rebase_migration.py @@ -431,11 +431,15 @@ def test_none_when_no_lines(self): assert result is None def test_none_when_no_first_marker(self): - result = module.Command().find_migration_names(["not_a_marker", "0002_author_nicknames"]) + result = module.Command().find_migration_names( + ["not_a_marker", "0002_author_nicknames"] + ) assert result is None def test_none_when_no_second_marker(self): - result = module.Command().find_migration_names(["<<<<<<<", "0002_author_nicknames"]) + result = module.Command().find_migration_names( + ["<<<<<<<", "0002_author_nicknames"] + ) assert result is None def test_works_with_two_way_merge(self): From 343f069e97535990c1aae38df14a14c7be921f3d Mon Sep 17 00:00:00 2001 From: Dmitry Sleptsov Date: Fri, 26 May 2023 18:01:06 +0200 Subject: [PATCH 3/4] Detect if merge is in progress --- .../management/commands/rebase_migration.py | 43 +++++-- tests/test_rebase_migration.py | 117 ++++++++++++++++-- 2 files changed, 137 insertions(+), 23 deletions(-) diff --git a/src/django_linear_migrations/management/commands/rebase_migration.py b/src/django_linear_migrations/management/commands/rebase_migration.py index b67f038..89480e3 100644 --- a/src/django_linear_migrations/management/commands/rebase_migration.py +++ b/src/django_linear_migrations/management/commands/rebase_migration.py @@ -50,7 +50,7 @@ def handle(self, *args: Any, app_label: str, **options: Any) -> None: if not max_migration_txt.exists(): raise CommandError(f"{app_label} does not have a max_migration.txt.") - migration_names = self.find_migration_names( + migration_names = find_migration_names( max_migration_txt.read_text().splitlines() ) if migration_names is None: @@ -175,17 +175,36 @@ def handle(self, *args: Any, app_label: str, **options: Any) -> None: + " updated its dependencies, and updated max_migration.txt." ) - def find_migration_names( - self, max_migration_lines: list[str] - ) -> tuple[str, str] | None: - lines = max_migration_lines - if len(lines) <= 1: - return None - if not lines[0].startswith("<<<<<<<"): - return None - if not lines[-1].startswith(">>>>>>>"): - return None - return lines[1].strip(), lines[-2].strip() + +def find_migration_names(max_migration_lines: list[str]) -> tuple[str, str] | None: + lines = max_migration_lines + if len(lines) <= 1: + return None + if not lines[0].startswith("<<<<<<<"): + return None + if not lines[-1].startswith(">>>>>>>"): + return None + migration_names = (lines[1].strip(), lines[-2].strip()) + if is_merge_in_progress(): + # During the merge 'ours' and 'theirs' are swapped in comparison with rebase + migration_names = (migration_names[1], migration_names[0]) + return migration_names + + +def is_merge_in_progress() -> bool: + try: + result = subprocess.run( + ["git", "rev-parse", "--git-dir"], + capture_output=True, + check=True, + text=True, + ) + except (FileNotFoundError, subprocess.SubprocessError): + # Either `git` is not available or there is no git repository + return False + + git_dir = result.stdout.strip() + return Path(git_dir).joinpath("MERGE_HEAD").exists() def migration_applied(app_label: str, migration_name: str) -> bool: diff --git a/tests/test_rebase_migration.py b/tests/test_rebase_migration.py index 46dfd97..6a75157 100644 --- a/tests/test_rebase_migration.py +++ b/tests/test_rebase_migration.py @@ -1,5 +1,6 @@ from __future__ import annotations +import subprocess import sys import time from functools import partial @@ -427,23 +428,19 @@ class Migration(migrations.Migration): class FindMigrationNamesTests(SimpleTestCase): def test_none_when_no_lines(self): - result = module.Command().find_migration_names([]) + result = module.find_migration_names([]) assert result is None def test_none_when_no_first_marker(self): - result = module.Command().find_migration_names( - ["not_a_marker", "0002_author_nicknames"] - ) + result = module.find_migration_names(["not_a_marker", "0002_author_nicknames"]) assert result is None def test_none_when_no_second_marker(self): - result = module.Command().find_migration_names( - ["<<<<<<<", "0002_author_nicknames"] - ) + result = module.find_migration_names(["<<<<<<<", "0002_author_nicknames"]) assert result is None - def test_works_with_two_way_merge(self): - result = module.Command().find_migration_names( + def test_works_with_two_way_merge_during_rebase(self): + result = module.find_migration_names( [ "<<<<<<<", "0002_author_nicknames", @@ -454,8 +451,8 @@ def test_works_with_two_way_merge(self): ) assert result == ("0002_author_nicknames", "0002_longer_titles") - def test_works_with_three_way_merge(self): - result = module.Command().find_migration_names( + def test_works_with_three_way_merge_during_rebase(self): + result = module.find_migration_names( [ "<<<<<<<", "0002_author_nicknames", @@ -468,6 +465,104 @@ def test_works_with_three_way_merge(self): ) assert result == ("0002_author_nicknames", "0002_longer_titles") + def test_works_with_two_way_merge_during_merge(self): + with mock.patch.object(module, "is_merge_in_progress", return_value=True): + result = module.find_migration_names( + [ + "<<<<<<<", + "0002_longer_titles", + "=======", + "0002_author_nicknames", + ">>>>>>>", + ] + ) + assert result == ("0002_author_nicknames", "0002_longer_titles") + + def test_works_with_three_way_merge_during_merge(self): + with mock.patch.object(module, "is_merge_in_progress", return_value=True): + result = module.find_migration_names( + [ + "<<<<<<<", + "0002_longer_titles", + "|||||||", + "0001_initial", + "=======", + "0002_author_nicknames", + ">>>>>>>", + ] + ) + assert result == ("0002_author_nicknames", "0002_longer_titles") + + +class IsMergeInProgressTests(SimpleTestCase): + git_command = ["git", "rev-parse", "--git-dir"] + + def setUp(self) -> None: + subprocess_run_patch = mock.patch( + "django_linear_migrations.management.commands.rebase_migration" + ".subprocess.run" + ) + self.mock_subprocess_run = subprocess_run_patch.start() + + self.addCleanup(subprocess_run_patch.stop) + + @pytest.fixture(autouse=True) + def tmp_path_fixture(self, tmp_path): + git_dir_name = ".git" + str(time.time()).replace(".", "") + self.git_dir_path = tmp_path / git_dir_name + self.git_dir_path.mkdir() + + def test_true_when_git_repository_exists_and_merge_in_progress(self): + with open(self.git_dir_path.joinpath("MERGE_HEAD"), "w"): + pass + self.mock_subprocess_run.return_value = subprocess.CompletedProcess( + args=self.git_command, + returncode=0, + stdout=str(self.git_dir_path), + ) + + result = module.is_merge_in_progress() + + assert result is True + self.mock_subprocess_run.assert_called_once_with( + self.git_command, + capture_output=True, + check=True, + text=True, + ) + + def test_false_when_git_repository_exists_and_not_merge_in_progress(self): + self.mock_subprocess_run.return_value = subprocess.CompletedProcess( + args=self.git_command, + returncode=0, + stdout=str(self.git_dir_path), + ) + + result = module.is_merge_in_progress() + + assert result is False + + def test_false_when_repository_not_exists(self): + # subprocess.run raises SubprocessError with 128 code when there is + # no git repository + self.mock_subprocess_run.side_effect = subprocess.SubprocessError( + f"Command '{self.git_command}' returned non-zero exit status 128" + ) + + result = module.is_merge_in_progress() + + assert result is False + + def test_false_when_git_command_is_not_available(self): + # subprocess.run raises FailNotFound error when `git` command is not found + self.mock_subprocess_run.side_effect = FileNotFoundError( + "No such file or directory: 'git'" + ) + + result = module.is_merge_in_progress() + + assert result is False + class MigrationAppliedTests(TestCase): def test_table_does_not_exist(self): From 8156d931f233a1b7435136f13fb205c00327f705 Mon Sep 17 00:00:00 2001 From: Dmitry Sleptsov Date: Sun, 28 May 2023 12:19:38 +0200 Subject: [PATCH 4/4] Update comment, README and CHANGELOG --- CHANGELOG.rst | 5 +++++ README.rst | 3 ++- .../management/commands/rebase_migration.py | 3 ++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 75fcca1..0618dbb 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,11 @@ Changelog ========= +2.8.0 (2023-05-28) +------------------ + +* Improve `rebase_migration` command to handle both rebasing and merging of the base branch + 2.7.0 (2023-02-25) ------------------ diff --git a/README.rst b/README.rst index 6185d7d..5d50c96 100644 --- a/README.rst +++ b/README.rst @@ -150,7 +150,8 @@ Following a conflicted “rebase” operation in Git, run it with the name of th $ python manage.py rebase_migration The command will use the conflict information in the ``max_migration.txt`` file to determine which migration to rebase. -It will then rename the migration, edit it to depend on the new migration in your main branch, and update ``max_migration.txt``. +It will automatically detect whether it's a merge or rebase operation by checking for the existence of the ``MERGE_HEAD`` file in the ``.git`` directory. +The command then rename the migration, edit it to depend on the new migration in your main branch, and update ``max_migration.txt``. If Black is installed, it will format the updated migration file with it, like Django’s built-in migration commands (from version 4.1+). See below for some examples and caveats. diff --git a/src/django_linear_migrations/management/commands/rebase_migration.py b/src/django_linear_migrations/management/commands/rebase_migration.py index 89480e3..707f0c9 100644 --- a/src/django_linear_migrations/management/commands/rebase_migration.py +++ b/src/django_linear_migrations/management/commands/rebase_migration.py @@ -200,7 +200,8 @@ def is_merge_in_progress() -> bool: text=True, ) except (FileNotFoundError, subprocess.SubprocessError): - # Either `git` is not available or there is no git repository + # Either `git` is not available or there is no git repository, fall back to + # default behaviour return False git_dir = result.stdout.strip()