Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The *.mo translation files have been recompiled #6

Merged
merged 2 commits into from Jun 22, 2018
Merged

Conversation

ale-rt
Copy link
Member

@ale-rt ale-rt commented Jun 1, 2018

Closes #5

@ale-rt
Copy link
Member Author

ale-rt commented Jun 1, 2018

I recompiled the po files using zest.pocompile (another beer for @mauritsvanrees).

[ale@emily zope.app.locales]$ pocompile 
INFO: Building .mo for ./src/zope/app/locales/hu/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/pt_BR/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/it/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/he/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/zh_CN/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/ru/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/en/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/nl/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/pl/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/zh_TW/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/es/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/tr/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/ja/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/de/LC_MESSAGES/zope.po
INFO: Building .mo for ./src/zope/app/locales/fr/LC_MESSAGES/zope.po

@mgedmin
Copy link
Member

mgedmin commented Jun 4, 2018

This breaks tests? How come this breaks tests?

(Also I'm not very happy with compilation artefacts being committed in the repo, and I would like some reassurances that the binary .mo format is platform-independent.)

@vincentfretin
Copy link
Member

The mo files should be removed from the repo, and only included in the releases (with zest.releaser+zest.pocompile) like I do with plone.app.locales.

@mgedmin
Copy link
Member

mgedmin commented Jun 4, 2018

Is there a way to make zest.releaser do that automatically, and most importantly abort if zest.pocompile is not available? Because if I ever need to make a release, I will run fullrelease without remembering any special requirements of any specific package.

@mauritsvanrees
Copy link
Member

Theoretically, you could add zest.pocompile to some extras_require in setup.py, but that does not install it in the same place where you have already installed zest.releaser.

A future option might be to have something in setup.cfg like this:

[zest.releaser]
requires = zest.pocompile

Then zest.releaser could try to import zest.pocompile and abort if that doesn't work. But that is just an idea.

No way comes to mind that currently works, except installing zest.pocompile in the same virtualenv (or similar) where you have zest.releaser. That is what I have, and it should be fine for all projects. (I guess there can be exceptions.)

@mgedmin
Copy link
Member

mgedmin commented Jun 5, 2018

For safety reasons we could add a check in setup.py that looks at the timestamps of *.mo and aborts if they're older than the timestamps of *.po. I think it would make sense to do that check only when __version__ does not contain dev. Also, we'd have to remove the *.mo from Git so git checkout wouldn't accidentally set new timestamps on old *.mo files.

@ale-rt
Copy link
Member Author

ale-rt commented Jun 10, 2018

I fixed the build by removing fuzzy in the tr and pl po files are rerunning pocompile.
I would definitely ask some Polish and Turkish people to have a look at the status of their translation files.

I am also +1 to remove the *.mo files from the repo, but this PR is focused on fixing the state of the art of the package and I opened #7 to discuss that.

Copy link
Member

@icemac icemac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes the current problem so I am fine with it. For the future it would be nice to get rid of the .mo files from the repos.

In my projects I create the .mo files at runtime by setting the following environment variables:

zope_i18n_allowed_languages=de,en,es,fr
zope_i18n_compile_mo_files=True

This instructs zope.i18n to create the .mo files at runtime, see https://github.com/zopefoundation/zope.i18n/blob/0df6ea60c8cd9f2efa98bf1c8e5ae4c8d7a34f76/src/zope/i18n/zcml.py#L92

@mauritsvanrees
Copy link
Member

zope_i18n_allowed_languages=de,en,es,fr
zope_i18n_compile_mo_files=True

I do this too in all my Plone projects. But when you build the site as one user and run it as a different user with less permissions, that user may not have permission to write the .mo files, so you could get an error on startup, or you simply miss translations, also when you don't have those lines in your config. So it is always nice to include the .mo files in the distribution. (But indeed preferably not in source control.)

@icemac
Copy link
Member

icemac commented Jun 15, 2018

@ale-rt Feel free to merge. I can create a release afterwards.

@pbauer pbauer merged commit e3ca2d9 into master Jun 22, 2018
@icemac icemac deleted the compile-mo branch June 29, 2018 06:25
@icemac
Copy link
Member

icemac commented Jun 29, 2018

I just released https://pypi.org/project/zope.app.locales/4.0.1/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants