`makemigrations --check` fails with error code 1 if system checks identify warnings

[James Addison]

In working through the upgrade steps from a Django 3.2 based system to Django 4.2, I encountered behaviour that breaks our github actions continuous integration (it is intended to fail if there are missing migrations needed).

In this case, there are no missing migration files identified, but the ./venv/bin/python manage.py makemigrations --check command errors out with 1, presumably due to CICharField W905 and CITextField W907 warnings:

# ./venv/bin/python manage.py makemigrations --check
System check identified some issues:

WARNINGS:
accounts.ComplianceRegion.name: (fields.W905) django.contrib.postgres.fields.CICharField is deprecated. Support for it (except in historical migrations) will be removed in Django 5.1.
        HINT: Use CharField(db_collation="…") with a case-insensitive non-deterministic collation instead.
accounts.ComplianceRegion.sub_region: (fields.W905) django.contrib.postgres.fields.CICharField is deprecated. Support for it (except in historical migrations) will be removed in Django 5.1.
        HINT: Use CharField(db_collation="…") with a case-insensitive non-deterministic collation instead.
information.Degree.degree: (fields.W907) django.contrib.postgres.fields.CITextField is deprecated. Support for it (except in historical migrations) will be removed in Django 5.1.
        HINT: Use TextField(db_collation="…") with a case-insensitive non-deterministic collation instead.

System check identified 3 issues (5 silenced).

# echo $?
1


# /venv/bin/python manage.py check
System check identified some issues:

WARNINGS:
accounts.ComplianceRegion.name: (fields.W905) django.contrib.postgres.fields.CICharField is deprecated. Support for it (except in historical migrations) will be removed in Django 5.1.
        HINT: Use CharField(db_collation="…") with a case-insensitive non-deterministic collation instead.
accounts.ComplianceRegion.sub_region: (fields.W905) django.contrib.postgres.fields.CICharField is deprecated. Support for it (except in historical migrations) will be removed in Django 5.1.
        HINT: Use CharField(db_collation="…") with a case-insensitive non-deterministic collation instead.
information.Degree.degree: (fields.W907) django.contrib.postgres.fields.CITextField is deprecated. Support for it (except in historical migrations) will be removed in Django 5.1.
        HINT: Use TextField(db_collation="…") with a case-insensitive non-deterministic collation instead.

System check identified 3 issues (5 silenced).

# echo $?
0

Note that both manage.py makemigrations --check and manage.py check are provided above for clarity.

These are deprecation warnings (to be removed in Django 5.1). I think this management command should not be erroring out in relation to these at this point in time.

I personally have time to address these CITextField and CICharField warnings properly by moving to the preferred db_collation=... collation approach, but wanted to raise this as a bug.

[Mariusz Felisiak]

Replying to James Addison:

Note that both manage.py makemigrations --check and manage.py check are provided above for clarity.

These are deprecation warnings (to be removed in Django 5.1). I think this management command should not be erroring out in relation to these at this point in time.

check cannot succeed when warnings are raised, so a non-zero return code is expected. You can always use the ​SILENCED_SYSTEM_CHECKS setting to ignore them.

[James Addison]

I think this should be reopened - I suspect I didn't convey the meaning correctly?

• manage.py check is correctly not failing (see the 0 exit code in my example)
  • nothing is currently (until 5.1 is released) in a broken check state
• manage.py makemigrations --check is incorrectly failing (see 1 exit code in my example above)
  • the 'check' for missing migrations yielded no missing migrations, so should not return an error
  • the fact that those warning are raised have nothing to do with migrations currently at risk (again, until 5.1 is released)

[Mariusz Felisiak]

check and makemigrations --check are completely different commands:

• check is to run system checks
• makemigrations --check exits with a non-zero status when model changes without migrations are detected, so the --check option has nothing to do with system check.

the 'check' for missing migrations yielded no missing migrations, so should not return an error.

makemigrations --check no longer creates missing migrations files in Django 4.2+ (see 80d38de52bb2721a7b44fce4057bcff571afc23a and #34051).

[James Addison]

To close this off - while I don't think we were talking the same language in the conversation back and forth here, this was very much a red herring/rabbit hole. For the benefit of others who might encounter this behaviour, it's worthwhile adding the following.

manage.py makemigrations --check was correctly returning an exit code of 1 because of the ​migration autodetector changes in Django 4.0.

This manifested somewhat indirectly in a ​3rd party app, which is where the missing migration (although a no-op migration!) needs to be added. Until that is done in 3rd party apps, continuous integration relying on makemigrations --check will continue to fail.