From 98b40ffe61260fca38230ad05d7f65a78e57ec66 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Mon, 1 Jun 2015 17:22:10 -0600 Subject: [PATCH] [1.8.x] Fixed #24895 -- Fixed loading a pair of squashed migrations with a dependency. Backport of 84522c0d165076d01cd034d7c381b75044daec8d from master. --- django/db/migrations/loader.py | 30 ++++++++++++++++++++++-------- docs/releases/1.8.3.txt | 4 ++++ tests/migrations/test_loader.py | 29 +++++++++++++++++++++++++++-- 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/django/db/migrations/loader.py b/django/db/migrations/loader.py index fccf7d3205..b10b84f6f8 100644 --- a/django/db/migrations/loader.py +++ b/django/db/migrations/loader.py @@ -222,15 +222,29 @@ class MigrationLoader(object): for child_key in reverse_dependencies.get(replaced, set()): if child_key in migration.replaces: continue - # child_key may appear in a replacement + # List of migrations whose dependency on `replaced` needs + # to be updated to a dependency on `key`. + to_update = [] + # Child key may itself be replaced, in which case it might + # not be in `normal` anymore (depending on whether we've + # processed its replacement yet). If it's present, we go + # ahead and update it; it may be deleted later on if it is + # replaced, but there's no harm in updating it regardless. + if child_key in normal: + to_update.append(normal[child_key]) + # If the child key is replaced, we update its replacement's + # dependencies too, if necessary. (We don't know if this + # replacement will actually take effect or not, but either + # way it's OK to update the replacing migration). if child_key in reverse_replacements: - for replaced_child_key in reverse_replacements[child_key]: - if replaced in replacing[replaced_child_key].dependencies: - replacing[replaced_child_key].dependencies.remove(replaced) - replacing[replaced_child_key].dependencies.append(key) - else: - normal[child_key].dependencies.remove(replaced) - normal[child_key].dependencies.append(key) + for replaces_child_key in reverse_replacements[child_key]: + if replaced in replacing[replaces_child_key].dependencies: + to_update.append(replacing[replaces_child_key]) + # Actually perform the dependency update on all migrations + # that require it. + for migration_needing_update in to_update: + migration_needing_update.dependencies.remove(replaced) + migration_needing_update.dependencies.append(key) normal[key] = migration # Mark the replacement as applied if all its replaced ones are if all(applied_statuses): diff --git a/docs/releases/1.8.3.txt b/docs/releases/1.8.3.txt index 2761f06271..2940623e55 100644 --- a/docs/releases/1.8.3.txt +++ b/docs/releases/1.8.3.txt @@ -43,3 +43,7 @@ Bugfixes * Allowed using ``choices`` longer than 1 day with ``DurationField`` (:ticket:`24897`). + +* Fixed a crash when loading squashed migrations from two apps with a + dependency between them, where the dependent app's replaced migrations are + partially applied (:ticket:`24895`). diff --git a/tests/migrations/test_loader.py b/tests/migrations/test_loader.py index 070ca2a86c..b8801ce783 100644 --- a/tests/migrations/test_loader.py +++ b/tests/migrations/test_loader.py @@ -259,12 +259,37 @@ class LoaderTests(TestCase): loader.build_graph() plan = set(loader.graph.forwards_plan(('app1', '4_auto'))) - expected_plan = set([ + expected_plan = { ('app1', '4_auto'), ('app1', '2_squashed_3'), ('app2', '1_squashed_2'), ('app1', '1_auto') - ]) + } + self.assertEqual(plan, expected_plan) + + @override_settings(MIGRATION_MODULES={ + "app1": "migrations.test_migrations_squashed_complex_multi_apps.app1", + "app2": "migrations.test_migrations_squashed_complex_multi_apps.app2", + }) + @modify_settings(INSTALLED_APPS={'append': [ + "migrations.test_migrations_squashed_complex_multi_apps.app1", + "migrations.test_migrations_squashed_complex_multi_apps.app2", + ]}) + def test_loading_squashed_complex_multi_apps_partially_applied(self): + loader = MigrationLoader(connection) + recorder = MigrationRecorder(connection) + recorder.record_applied('app1', '1_auto') + recorder.record_applied('app1', '2_auto') + loader.build_graph() + + plan = set(loader.graph.forwards_plan(('app1', '4_auto'))) + plan = plan - loader.applied_migrations + expected_plan = { + ('app1', '4_auto'), + ('app1', '3_auto'), + ('app2', '1_squashed_2'), + } + self.assertEqual(plan, expected_plan) @override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations_squashed_erroneous"})