Opened 10 months ago
Closed 10 months ago
#34952 closed Bug (fixed)
manage.py compilemessages may skip locale folders if ignore is used.
Reported by: | Andrew Cordery | Owned by: | Andrew Cordery |
---|---|---|---|
Component: | Internationalization | Version: | 4.2 |
Severity: | Normal | Keywords: | |
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
manage.py compilemessages uses os.walk and iterates through the same dirnames instance that it removes members from when the ignore option is used.
This will cause compilemessages to accidentally ignore a locale directory that directly follows a skipped directory in the list. Unfortunately it is not easy to predict when this will occur or which directories may be affected as os.walk provides the list of dirnames in 'arbitrary' order according to the python docs.
I've provided the miniscule patch to fix this as well as a much lengthier regression test, however I'm not positive that the test is capable of recreating the correct conditions on all operating systems. I've tested on OSX only. Regardless, the issue at hand is a basic language error (modifying a value that you are iterating through) and the fix is extremely simple (i.e. copy dirnames before iterating, in my patch I just wrap it in list()), so this should not be a controversial patch.
Note: this was originally mentioned as a comment on my other ticket https://code.djangoproject.com/ticket/34925, I'm separating it for clarity and I will attach the PR here.
Change History (5)
comment:1 by , 10 months ago
comment:2 by , 10 months ago
Component: | Uncategorized → Internationalization |
---|---|
Triage Stage: | Unreviewed → Accepted |
Thank you! Accepting.
comment:3 by , 10 months ago
Owner: | changed from | to
---|---|
Patch needs improvement: | set |
Status: | new → assigned |
comment:4 by , 10 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:5 by , 10 months ago
Patch needs improvement: | unset |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
Fixed by ad41f1c53aa9f2c938df32e4386d8a80138923fc.
Here is the PR: https://github.com/django/django/pull/17452