Defect: PO-Revision-Date being removed causes validation errors in 1.x release

[justinhynes]

We recently updated the edx-i18n-tools package in the Credentials IDA and it appears to be causing issues. We have temporarily pinned the version to the 0.9.x release (which resolved our CI issues).

It looks like the PO-Revision-Date attribute in the file (headers?) are being removed now. When validation is run using this package, it is flagging the missing PO-Revision-Date data as a warning, which causes the validation to fail with an error. Since the validation exits with an error code, it is causing issues in the Credentials CI.

I have a PR demonstrating the issue here: https://github.com/openedx/credentials/pull/2085/files.

The failing CI checks can be seen here: https://github.com/openedx/credentials/actions/runs/5588178826/jobs/10214553458?pr=2085

Example logs from the failing validate:

INFO:i18n.validate:

WARNING:i18n.validate:
en/LC_MESSAGES/djangojs-partial.po:7: warning: header field 'PO-Revision-Date' missing in header

INFO:i18n.execute:msgfmt -c -o /dev/null en/LC_MESSAGES/django.po
INFO:i18n.validate:

WARNING:i18n.validate:
en/LC_MESSAGES/django.po:7: warning: header field 'PO-Revision-Date' missing in header

INFO:i18n.execute:msgfmt -c -o /dev/null en/LC_MESSAGES/djangojs.po
INFO:i18n.validate:

WARNING:i18n.validate:
en/LC_MESSAGES/djangojs.po:7: warning: header field 'PO-Revision-Date' missing in header

INFO:i18n.execute:msgfmt -c -o /dev/null en/LC_MESSAGES/django-partial.po
INFO:i18n.validate:

WARNING:i18n.validate:
en/LC_MESSAGES/django-partial.po:7: warning: header field 'PO-Revision-Date' missing in header

INFO:i18n.execute:msgfmt -c -o /dev/null ro/LC_MESSAGES/django.po
INFO:i18n.validate:No problems found in /home/runner/work/credentials/credentials/credentials/conf/locale/ro/LC_MESSAGES/django.po
INFO:i18n.execute:msgfmt -c -o /dev/null ro/LC_MESSAGES/djangojs.po
INFO:i18n.validate:No problems found in /home/runner/work/credentials/credentials/credentials/conf/locale/ro/LC_MESSAGES/djangojs.po
INFO:i18n.execute:msgfmt -c -o /dev/null nb/LC_MESSAGES/django.po
INFO:i18n.validate:No problems found in /home/runner/work/credentials/credentials/credentials/conf/locale/nb/LC_MESSAGES/django.po
INFO:i18n.execute:msgfmt -c -o /dev/null nb/LC_MESSAGES/djangojs.po
INFO:i18n.validate:No problems found in /home/runner/work/credentials/credentials/credentials/conf/locale/nb/LC_MESSAGES/djangojs.po
INFO:i18n.execute:msgfmt -c -o /dev/null fa_IR/LC_MESSAGES/django.po
INFO:i18n.validate:No problems found in /home/runner/work/credentials/credentials/credentials/conf/locale/fa_IR/LC_MESSAGES/django.po
INFO:i18n.execute:msgfmt -c -o /dev/null fa_IR/LC_MESSAGES/djangojs.po
INFO:i18n.validate:No problems found in /home/runner/work/credentials/credentials/credentials/conf/locale/fa_IR/LC_MESSAGES/djangojs.po
INFO:i18n.execute:msgfmt -c -o /dev/null pl/LC_MESSAGES/django.po
INFO:i18n.validate:No problems found in /home/runner/work/credentials/credentials/credentials/conf/locale/pl/LC_MESSAGES/django.po
INFO:i18n.execute:msgfmt -c -o /dev/null pl/LC_MESSAGES/djangojs.po
INFO:i18n.validate:No problems found in /home/runner/work/credentials/credentials/credentials/conf/locale/pl/LC_MESSAGES/djangojs.po
make: *** [Makefile:168: validate_translations] Error 1
Error: Process completed with exit code 2.

[justinhynes]

Update as this came up again very recently.

The validate functionality in this library calls out to msgfmt:

i18n-tools/i18n/validate.py

Line 76 in 851c02f

out, err = call(f'msgfmt -c -o /dev/null {rfile}', working_directory=locale_dir)

.

Specifically, the -c flag ensures that the format, headers, and domain directives pass. I think this is where we are running into issues. We're using msgfmt to check the headers, which I think expects PO-Revision-Date to be present?

[brian-smith-tcril]

#129 seems to be the culprit. @shadinaif @OmarIthawi any thoughts?

[OmarIthawi]

@justinhynes Sorry that it broke the CI. It was an unintended consequence of our change. Otherwise, we could have bumped a major version for the edx-i18n-tools package.

To give some context, we removed it because PO-Revision-Date triggers a lot of diffs and commits in the openedx/openedx-translations. I still thinks this is a good change because PO-Revision-Date has practically minimal use of in the en source translations.

Translations is being refactored as part of OEP-58 which will eventually remove the translations from individual repositories. Credentials deprecation pull requests haven't been done but here's an example from another repo:

• feat!: remove Transifex calls for FC-0012 xblock-drag-and-drop-v2#352

I'd like to suggest a couple of options in the following order:

1. Turn off the -c flag for the English translations in the credentials repository CI.
2. Add static value for PO-Revision-Date for all i18n_tool extract files e.g. PO-Revision-Date: 1970-01-01 10:30+0000 so it passes the -c flag.
3. Use an earlier version of i18n_tool until further notice.

Please let me know which option makes more sense to proceed with and we'll try to fix the issues.

[justinhynes]

Thanks for the extra eyes and your suggestions on this, @OmarIthawi.

Unfortunately, we can't follow your first suggestion -- the -c flag is coming from this library itself, not specified in Credentials.

The reason this is turning into a problem now is that we were told that upgrading to 1.x of i18n-tools is required for the Django 4.2 upgrade in the Open edX IDAs. We have Credentials pinned to a release of i18n-tools < 1.x, but now we're blocking the Django upgrade now. I

We can try and look at suggestion #2, but I expect that the i18n tool will just strip it out again?

[OmarIthawi]

We can try and look at suggestion no. 2, but I expect that the i18n tool will just strip it out again?

@justinhynes This behavior will be added to i18n_tool extract. That should pass the -c, we'll add a test case to ensure that whatever the i18n_tool extract generate passes the validate command. It makes sense that both commands are "on the same page" so to speak.

Again, apologies for breaking the workflow. no. 2 seems to be the way to go. So if others agree we can move forward and document this decision as soon as possible.

cc: @shadinaif and @brian-smith-tcril what do you think?

[shadinaif]

Thank you for the details @justinhynes . I was able to reproduce the error locally. The fix is on the way. I tried it and now quality_and_translations_tests job is passing successfully on my local machine

cc: @OmarIthawi @brian-smith-tcril

[OmarIthawi]

@justinhynes We've completed the fix, would you mind testing it before we cut a new release?

• feat: restore PO-Revision-Date but with fixed value #140

[justinhynes]

Hey @OmarIthawi,

I'll try to get to this today. Sorry for the slow response, I was out of commission for a few days due to COVID.

[OmarIthawi]

@justinhynes thanks for the heads up! Get well and fully recovered soon!

[justinhynes]

I was able to try the changes first thing this morning and it looks good. No issues locally. Thanks!

[OmarIthawi]

Closed by #140 and released in v1.3.0.