Opened 3 years ago

Closed 2 months ago

#24384 closed Bug (fixed)

compilemessages shouldn't return with exit code 0 when it fails

Reported by: Aymeric Augustin Owned by: Claude Paroz
Component: Internationalization Version: master
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

When compilemessages is run on a read-only location, it displays a messages to standard error and terminates with exit code 0. It should have exit code 1.

Patch:

diff --git a/django/core/management/commands/compilemessages.py b/django/core/management/commands/compilemessages.py
index dbadac0..75f3c57 100644
--- a/django/core/management/commands/compilemessages.py
+++ b/django/core/management/commands/compilemessages.py
@@ -108,9 +108,9 @@ class Command(BaseCommand):
 
             # Check writability on first location
             if i == 0 and not is_writable(npath(base_path + '.mo')):
-                self.stderr.write("The po files under %s are in a seemingly not writable location. "
-                                  "mo files will not be updated/created." % dirpath)
-                return
+                raise CommandError("The po files under %s are in a seemingly "
+                                   "not writable location. mo files will not "
+                                   "be updated/created." % dirpath)
 
             args = [self.program] + self.program_options + ['-o',
                     npath(base_path + '.mo'), npath(base_path + '.po')]

With this change, the output is exactly the same, but the exit code reflects the failure and the --traceback option works.

I don't quite have the motivation to write tests for this change :-/

Change History (8)

comment:1 Changed 3 years ago by Claude Paroz

The reason of the current behaviour is that compilemessages run compile_messages for all paths of settings.LOCALE_PATHS. The idea was to not stop running the command if only one path is read-only but not the others. When #24159 will be fixed, it's probable that compile_messages will be launched for all apps in INSTALLED_APPS. Then we don't want to fail when some app is read-only (maybe a third-party app in a non-readable location).

comment:2 Changed 3 years ago by Aymeric Augustin

We have a problem akin to test discovery here. I think the right solution is to ignore anything outside the current working directory and error if a non-writable location is encountered.

comment:3 Changed 3 years ago by Claude Paroz

See #21732 for the initial use case about non-writable paths.

Ignoring paths outside current directory could be a solution, but might be backwards incompatible.

comment:4 Changed 3 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:5 Changed 3 months ago by Claude Paroz

Owner: changed from nobody to Claude Paroz
Status: newassigned

comment:6 Changed 3 months ago by Claude Paroz

Has patch: set

comment:7 Changed 2 months ago by Carlton Gibson

Triage Stage: AcceptedReady for checkin

comment:8 Changed 2 months ago by Claude Paroz <claude@…>

Resolution: fixed
Status: assignedclosed

In a77f2188:

Fixed #24384 -- Allowed compilemessages to continue running after nonfatal errors.

Thanks Aymeric Augustin for the report and Carlton Gibson and Tim Graham for
the reviews.

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