makemessages too eager to cleanup, makes it hard to debug errors

[Andy Terra]

./manage.py makemessages always removes .pot files when finished, but sometimes this just makes debugging harder. I propose an option be added to allow for prevent deletion of these files, even when errors occur in the function.

The need for this option arose of an error which was, even when at verbose level 3, resulting in nearly useless debugging information when running ./manage.py makemessages --locale=pt_BR -a -d djangojs -v3:

processing language pt_BR
ignoring file .gitignore in .
ignoring file .project in .
ignoring file .pydevproject in .
ignoring file .urls.py.swp in .
ignoring file urls.py~ in .
processing file athena.js in .\static\js
(...)
processing file superfish.js in .\static\js
processing file grappelli.js in .\static\js\grappelli
processing file jquery.grp_collapsible.js in .\static\js\grappelli
processing file jquery.grp_collapsible_group.js in .\static\js\grappelli
processing file jquery.grp_inline.js in .\static\js\grappelli
processing file jquery.grp_related_fk.js in .\static\js\grappelli
processing file jquery.grp_related_generic.js in .\static\js\grappelli
processing file jquery.grp_related_m2m.js in .\static\js\grappelli
processing file jquery.grp_timepicker.js in .\static\js\grappelli
Error: errors happened while running msguniq
C:\projects\portal\hermes\locale\pt_BR\LC_MESSAGES\djangojs.pot:57: context separator <EOT> within string
C:\projects\portal\hermes\locale\pt_BR\LC_MESSAGES\djangojs.pot:58: context separator <EOT> within string
msguniq: found 2 fatal errors [1]

As you can see, it's hard to tell which of these files caused the EOT within string error, but by keeping djangojs.pot and then reading lines 57 and 58, I was able to deduct that athena.js was the file to blame

#: .\static\js\athena.js.c:30 .\static\js\athena.js.c:38
msgid "<EOT>" [2]
msgid_plural "<EOT>" [2]
msgstr[0] ""
msgstr[1] ""

[1] this line translated from Portuguese in this post to help understanding.

[2] these lines had a EOT character rather than <EOT> that can't be displayed here.

I've attached a simple patch which provides the user with a --keep-pot option to help in edge cases like this one, which simply skips the os.unlink(potfile) call.

Cheers!

[Andy Terra]

Adds --keep-pot option to makemessages.py

[Aymeric Augustin]

This looks useful enough to justify adding an option to makemessages. Error messages with references to non-existing files aren't useful!

The patch is a bit inconsistent: sometimes it uses unlink_potfile, sometimes if not keep_pot. I suggest removing the unlink_potfile function and always using a conditional — it's more explicit.

It also needs tests.

The options of makemessages are documented, you must include a doc patch too (see ​https://docs.djangoproject.com/en/dev/ref/django-admin/#makemessages).

NB: .pot files are removed since r15506 (although the patch was written much earlier).

[Andy Terra]

I've improved the patch as requested, including the doc patch. I wasn't sure about the wording, and nobody replied in #django-dev, so I just wrote what I thought was relevant.

As for testing, I'm really not sure how to include this in tests/, as I couldn't spot where management commands tests are located. I have however tested it manually and it works as advertised even when errors in generating the .po occur.

If anyone could point me where in tests/ I should place the new test, I'd be glad to write it. Let me know if anything else is missing!

[Julien Phalip]

Thanks, the patch looks pretty good. The test could live in tests/regressiontests/i18n/commands/extraction.py.

[Andy Terra]

I'm attaching a patch with the requested tests. All tests are passing here using Cygwin. I wasn't sure I needed to subclass the tearDown method to remove the .pot file, but I did so anyway.

foo@bar /c/temp/download/django-django-439ed79
$ tests/runtests.py -v3 --settings=test_sqlite i18n

(...)
----------------------------------------------------------------------
Ran 94 tests in 5.844s

OK

Let me know if anything else is missing! (hopefully not :P)

Cheers,

AT

[Andy Terra]

docs/tests/patch in one single file

[Julien Phalip]

I've updated the patch to current trunk and did a little pep8-cleanup.

It'd be nice to add more comprehensive tests. The main purpose of the --keep-pot option is for debugging in case the makemessages command (or more precisely the underlying calls to xgettext, msgmerge and msguniq) crashes, yet the patch currently only tests when everything works.

I'm not entirely sure how to write tests for this yet. Maybe that could be done by monkey-patching the _popen() function.

[Ramiro Morales]

Patch from Julien updated to changes that happened in makemessages code

[Ramiro Morales]

See ​https://github.com/ramiro/django/compare/17008-keep-pot for a refreshed patch.

Thanks Jannis for remembering about this ticket.

[Ramiro Morales <cramm0@…>]

In eee865257aaa9005947a7b4994c475c2ad59d698:

Fixed #17008 -- Added makemessages option to not remove .pot files.

Thanks airstrike for the report and initial patch, Julien for an
enhanced patch and Jannis for reviewing.

[Simon Charette]

Maybe we could also add a release note about this new option.

[Julien Phalip <jphalip@…>]

In 28d3b33c04cc2e2250059039c5ebbab97869d525:

Added a note to the 1.6 release about the new --keep-pot option for makemessages.
Refs #17008.

[Julien Phalip <jphalip@…>]

In 59bf42f79e6f2fce2fc9128a86643cdb80b4b49a:

[1.6.x] Added a note to the 1.6 release about the new --keep-pot option for makemessages.
Refs #17008.
Backport of 28d3b33c04cc2 from master.