Opened 10 years ago

Closed 2 years ago

#22617 closed Cleanup/optimization (needsinfo)

Allow to fix translation -> Fix makemessages to not delete debug data and hide errors (and all similar modules using "msgmerge")

Reported by: Cezary.Wagner Owned by: nobody
Component: Core (Management commands) Version: dev
Severity: Normal Keywords:
Cc: Ad Timmering Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description (last modified by Tim Graham)

Please replace code in make messages to not delete *.po files if error occurred and show messages (fix exception later).

Replace this code:

            if errors:
                if status != STATUS_OK:
                    raise CommandError(
                        "errors happened while running msgmerge\n%s" % errors)
                elif self.verbosity > 0:
                    self.stdout.write(errors)

With this code (it shows error root cause and save/copy *.po need to solve this error):

            if errors:
                if status != STATUS_OK:
                    self.stdout.write(errors) # bug exception bellow not shows errors
                    import shutil
                    shutil.copyfile(pofile, '%s.debug' % pofile)
                    raise CommandError(
                        "errors happened while running msgmerge\n%s" % errors)
                elif self.verbosity > 0:
                    self.stdout.write(errors)

All modules using command line tools should report errors and not delete input if error occurred.

Change History (16)

comment:1 by Tim Graham, 10 years ago

Does the --keep-pot option not help?

comment:2 by Tim Graham, 10 years ago

This PR appears related.

comment:3 by Cezary.Wagner, 10 years ago

Not known about such option but should not help too much since many bugs is in code - see my pulls on github.

  1. Messages from msgmerge is not presented since bug - exception blocks print out.
  2. --keep-pot is not presented in --help I think so (not checked but never seen it before - o.k. since it from 1.6).

I think problem is with "blocktrans" on 60% - I gave up now - I was done very large project more than 1k strings.

comment:4 by Cezary.Wagner, 10 years ago

Push Request on github is related.

comment:5 by Tim Graham, 10 years ago

I will let someone more familiar with makemessages make a call on whether or not anything needs to be done here, but for some more context, I also closed #22615 and #22616 which are related.

I do see --keep-pot in the help for makemessages on 1.6 (I'm not sure if that's what you are saying in the above comment).

comment:6 by Tim Graham, 10 years ago

Resolution: needsinfo
Status: newclosed

comment:7 by anonymous, 10 years ago

You not understand this bug really - it is not problem of *.pot bit *.po file. As far as I remember there is not help if I use --keep-pot.

It not helps :)

Read it again "Messages from msgmerge is not presented since bug - exception blocks print out." - I will reopen until we will find some solution - I will not give up now ...

comment:8 by Cezary.Wagner, 10 years ago

It is not solve still "Messages from msgmerge is not presented since bug - exception blocks print out." is in force!

You need to known where in django template there is error - --keep-pot allow nothing in this case.

comment:9 by Cezary.Wagner, 10 years ago

Resolution: needsinfo
Status: closednew

comment:10 by Areski Belaid, 10 years ago

It seems there is some confusion with this ticket, to clarify:

  • keep-pot option ONLY prevent Django from deleting the temporary .pot
  • this patch ensure that we don't delete the original PO file when msgmerge fails.

I haven't be able to test it as I guess it's a bit tricky to make msgmerge fails.
The PR looks good but I would recommend to move 'import shutil' at the top of the file.

comment:11 by Tim Graham, 10 years ago

Component: UncategorizedCore (Management commands)
Description: modified (diff)
Type: UncategorizedCleanup/optimization

comment:12 by Tim Graham, 9 years ago

Has patch: set
Triage Stage: UnreviewedAccepted
Version: 1.6master

Let's see if we can get someone to review the patch. Is it feasible to add tests?

comment:13 by Berker Peksag, 9 years ago

Patch needs improvement: set

comment:14 by Mariusz Felisiak, 2 years ago

Cc: Ad Timmering added

comment:15 by Ad Timmering, 2 years ago

@Mariusz: Thanks for cc'ing me. Would be happy to add some tests, however have been unable to replicate the behavior.

Forcably letting msgmerge throw errors (by pausing the makemessages process and messing up the intermediate file on purpose), does for me
(a) give sensible error messages, and
(b) they are actually displayed together with the CommandError.
I searched SO etc. for other users with similar issues, but unfortunately couldn't find an example to build a test around.

I suggest closing as needsinfo, pending the original author or someone else to provide an example of a case where the error message given with the exception does not provide enough information.

Example of an intentionally broken run:

(env) awtimmering @ ~/djtest $ ./manage.py makemessages  --locale nl
processing locale nl
CommandError: errors happened while running msgmerge
/home/awtimmering/djtest/locale/django.pot:49: keyword "msgid_plasdural" unknown
/home/awtimmering/djtest/locale/django.pot:45: missing 'msgstr' section
/home/awtimmering/djtest/locale/django.pot:49:16: syntax error
msgmerge: found 3 fatal errors

comment:16 by Mariusz Felisiak, 2 years ago

Resolution: needsinfo
Status: newclosed
Triage Stage: AcceptedUnreviewed

Ad, Thanks for checking!

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