Opened 8 years ago

Closed 8 years ago

#26940 closed Bug (fixed)

makemessages incorrectly configured as not requiring settings

Reported by: Jorge Romero Owned by: nobody
Component: Internationalization Version: dev
Severity: Normal Keywords: translations
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

django-admin and manage.py are supposed to be interchangeable.

From the docs(https://docs.djangoproject.com/en/1.9/ref/django-admin/):

The command-line examples throughout this document use django-admin to be consistent, but any example can use manage.py or python -m django just as well.

However, this doesn't seem to be true for makemessages.

If I run manage.py makemessages from the top level of the project MEDIA_ROOT and STATIC_ROOT are ignored. This isn't precisely a bug because that's what the code is intended to do (https://github.com/django/django/blob/master/django/core/management/commands/makemessages.py#L448), but I find it strange specially for --domain djangojs because is very common for Javascript files to be put in STATIC_ROOT.

django-admin makemessages doesn't ignore MEDIA_ROOT or STATIC_ROOT and it traverses the entire source tree.

At least this should be explained in the documentation. At best both commands should do the exact same. It's very easy to be bitten by this as we don't always review the resulting *.po file.

Am I missing something here?

Change History (10)

comment:1 by Simon Charette, 8 years ago

Maybe the documentation should mention that --settings must be specified to django-admin and python -m django for them to be interchangeable with manage.py.

comment:2 by Claude Paroz, 8 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Currently, makemessages is explicitly whitelisted as not needing a configured settings file (see no_settings_commands in django.core.management.__init__.py.
I wonder now if we should revert that, considering the dependency of that command on settings.FILE_CHARSET, settings.LOCALE_PATHS, settings.MEDIA_ROOT, and settings.STATIC_ROOT.

comment:3 by Claude Paroz, 8 years ago

Has patch: set
Version: 1.9master

comment:4 by Tim Graham, 8 years ago

Summary: django-admin makemessages vs. manage.py makemessagesmakemessages incorrectly configured as not requiring settings
Triage Stage: AcceptedReady for checkin

comment:5 by Claude Paroz <claude@…>, 8 years ago

Resolution: fixed
Status: newclosed

In a6baada7:

Fixed #26940 -- Removed makemessages from no_settings_commands whitelist

As makemessages uses several settings for proper run (FILE_CHARSET,
LOCALE_PATHS, MEDIA_ROOT, and STATIC_ROOT), we should require settings
configuration for this command.

comment:6 by Claude Paroz, 8 years ago

Resolution: fixed
Status: closednew
Triage Stage: Ready for checkinUnreviewed

I'm considering reverting the fact that we require configured settings for makemessages.
I'm thinking about the use case of reusable apps, when you typically don't have/need STATIC_ROOT/MEDIA_ROOT/LOCALE_PATHS, and most of the time FILE_CHARSET in utf-8 is fine.
See this commit, above that PR.

comment:7 by Claude Paroz, 8 years ago

Here's the PR.

comment:8 by Tim Graham, 8 years ago

Triage Stage: UnreviewedReady for checkin

comment:9 by Claude Paroz <claude@…>, 8 years ago

In fa2f55c:

Refs #26940 -- Re-allowed makemessages without settings

Thanks Tim Graham for the review.

comment:10 by Claude Paroz, 8 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top