Opened 5 years ago

Closed 4 years ago

#26429 closed Bug (fixed)

Name clash of merge migrations after squashing

Reported by: xgenadam Owned by: Raphael Gaschignard
Component: Migrations Version: 1.9
Severity: Normal Keywords: makemigrations merge clash
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Change History (12)

comment:1 Changed 5 years ago by Tim Graham

For clarity, could you give the specific steps that led to the clash?

comment:2 Changed 5 years ago by Tim Graham

Resolution: needsinfo
Status: newclosed

comment:3 Changed 5 years ago by Raphael Gaschignard

I just ran into a similar problem, going to detail what happened:

  • Over the course of development, do a merge migration. In our case, it was the 21st migration so got named 0021_merge.py
  • Squash our migrations, deleting 0021_merge.py
  • Continue development, and through sheer luck, our next 2st migration is also a merge commit. It gets named 0021_merge.py
  • When running migrations, because the original migration was run, this new 0021_merge.py commit is not run

In my case, this was the dependencies of 0021_merge.py (the new one):

    dependencies = [
        ('document', '0020_auto_20160413_1834'),
        ('document', '0019_auto_20160414_1150'),
    ]

Because the merge commit wasn't run, some people only had the 0020 mainline migrations migrated, some people only had the 0019 branch migrations run, and some people (who hadn't run migrate in a while) had the following happen:

  • migrate detects 0021_merge.py as the leaf node
  • there was record of the (old) 0021_merge.py migration
  • the system was like "oh, guess you're up to date!" and then doesn't run either branch of the merge commit

Like the OP, I believe the simplest solution is to timestamp the merge migrations.

comment:4 Changed 5 years ago by Raphael Gaschignard

Resolution: needsinfo
Status: closednew

comment:5 Changed 5 years ago by Raphael Gaschignard

Did a bit more digging into this, I believe that the fix is changing [(https://github.com/django/django/blob/master/django/core/management/commands/makemigrations.py#L286)], from :

new_migration = subclass("%04i_merge" % (biggest_number + 1), app_label) 

to :

timestamp = datetime.datetime.now().strftime("%Y%m%d_%H%M")
new_migration = subclass("%04i_merge_%s" % (biggest_number + 1, timestamp), app_label) 

I'm not quite sure how to write tests for this, though. You could write some models, generate migrations, squash, generate new ones and then confirm that everything got run? But I feel like the current squash migration tests also cover this, and that there isn't really much in term of name conflict tests.

comment:6 Changed 5 years ago by Daniel Izquierdo

Description: modified (diff)

The docs in https://github.com/django/django/blob/2cd2d188516475ddf256e6267cd82c495fb5c430/django/db/migrations/autodetector.py#L1129 say that names are not guaranteed to be unique and the timestamp is just a best effort to avoid conflicts. So I think rtpg's suggested fix works well for merge migrations, and a test to ensure names don't conflict is not necessary.

comment:7 Changed 5 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:8 Changed 5 years ago by Raphael Gaschignard

Owner: changed from nobody to Raphael Gaschignard
Status: newassigned

comment:9 Changed 5 years ago by Tim Graham

Has patch: set

comment:10 Changed 5 years ago by Tim Graham

Patch needs improvement: set

Tests need to be fixed on the PR.

comment:11 Changed 4 years ago by Tim Graham

Patch needs improvement: unset

comment:12 Changed 4 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 8f6a1a15:

Fixed #26429 -- Added a timestamp to merge migration names.

This reduces the possibility of a naming conflict, especially after
squashing migrations.

Note: See TracTickets for help on using tickets.
Back to Top