compilemessages reports errors only once

[Balazs Endresz]

When calling compilemessages for a locale that has invalid entries it prints out the errors correctly first.
But any subsequent calls just say that it's already compiled and up to date.
This is because msgfmt updates the last modified date of the mo file even if it's invalid, and then django will skip that locale afterwards:
​https://github.com/django/django/blob/main/django/core/management/commands/compilemessages.py#L153

This can reproduced by calling compilemessages twice in django/tests/i18n/test_compilation.py:

class CompilationErrorHandling(MessageCompilationTests):
    def test_error_reported_by_msgfmt(self):
        # po file contains wrong po formatting.
        with self.assertRaises(CommandError):
            call_command("compilemessages", locale=["ja"], verbosity=0)

        # subsequent calls should still fail
        with self.assertRaises(CommandError):
            call_command("compilemessages", locale=["ja"], verbosity=0)  # currently this doesn't raise CommandError

One way of fixing this is perhaps doing the validation separately first, e.g. with mo_path = "NUL" if os.name == "nt" else "/dev/null" and then calling msgfmt again with the actual mo_path only if that passes.

[Claude Paroz]

Good catch! I think your suggested resolution strategy makes sense.

[amansharma612]

I don't think that the error is due to msgfmt updating the last modified date, as I found that to not be the case when reproducing the bug.

The following is mentioned in the comments for ​is_writable

    # Known side effect: updating file access/modified time to current time if
    # it is writable.

Checking is_writable on 'mo_path modifies the date. Nonetheless, the suggested fix seems reasonable, or there could be a way to implement is_writable` without modifying the date.

[Claude Paroz]

I cannot remember if I didn't use os.access(file_path, os.W_OK) on purpose or if I didn't know that method at the time. To be tested.

[amansharma612]

open(mo_path, "a") has the semantics of creating a new file as long as the path is accessible (if accessible, is_writable never returns false). In such a scenario, using os.access leads to change in semantics and failing test cases.

I will proceed with the first fix mentioned above to avoid breaking that behaviour.

[Balazs Endresz]

But if we need to run msgfmt regardless of the last modified date then doesn't that make the optimisation to skip it based on the last modified date pointless? So I think that optimisation just has to be removed and always run msgfmt with the same args as we do now. I think that would be the behaviour most people would expect anyway.

[amansharma612]

Makes sense. I have opened a PR for the same ​here.

[Claude Paroz]

Note that your patch is reverting an optimization done to fix #31692.

I would vote for trying a different method to test writability, maybe by writing a temporary file in the parent directory?

I experimented that approach in ​that PR.