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

mechanizes continuous-localization workflow #1282

Merged
merged 9 commits into from
Sep 22, 2021
Merged

Conversation

cfm
Copy link
Member

@cfm cfm commented Aug 26, 2021

Description

Closes #1266 by providing the following mechanisms for integrating with Weblate's workflow for continuous localization, contrasted below with their equivalents in securedrop's phased workflow. An explicit goal of #1266 is to avoid replicating the complexity of securedrop's i18n_tool.py.

Step Implemented here as Implemented in securedrop as
Update strings to be translated Nightly CI workflow[1] for make update-translation-catalogs make translatei18n_tool.py translate-messages
Merge developmain with Weblate fork Weblate pulls automatically Localization Manager merges manually
Merge translations back to developmain Weblate opens[3] pull request automatically Localization Manager merges manually
—Compile translations Runtime[2] make compile-translation-catalogs Manual i18n_tool.py translate-messages --compile

Notes:

  1. This is implemented as a GitHub Actions workflow for ease of pushing changes back into this repository. If this is undesirable tool-sprawl, it will be trivial to port this workflow into CircleCI à la https://github.com/freedomofpress/infrastructure/issues/1904#issuecomment-503796888, at the cost of being somewhat less easy to trigger manually in addition to on a schedule.
    • NB. We should carefully consider the security implications of this, since securedrop-client unlike securedrop-debian-packaging runs CI on pull requests including from forks.
  2. In development only, pending packaging should include compiled gettext MOs #1283.
  3. Think of this as an upsert: Weblate will open a new pull request weblate-fpf/securedrop-client@weblate-securedrop-securedrop-clientfreedomofpress/securedrop-client@main only if it isn't already open.

Overview

Weblate's generic continuous-localization workflow

Adapted from their documentation and Graphivz:

Workflow implemented in this pull request

Test Plan

I can also arrange (or record) a demo of the end-to-end workflow.

  1. On 1266-continuous-l10n (and inside the virtual environment):

     $ make update-translation-catalogs
     $ git push -f origin 1266-continuous-l10n:i18n-testing
    
    • This simulates the l10n workflow that will run every night on main.
  2. https://weblate.securedrop.org/projects/securedrop/securedrop-client/#repository: Go to Maintenance and select Reset.

  3. https://weblate.securedrop.org/projects/securedrop/securedrop-client/: Start a New Translation for es.

  4. Observe 2 new commits (after 1266-continuous-l10n) in weblate-fpf/securedrop-client@weblate-securedrop-securedrop-client.

  5. https://weblate.securedrop.org/translate/securedrop/securedrop-client/es/?q=: Search for the string username and suggest Nombre de Usuario.

    • WARNING. Be careful to enter only correct translations when testing at the production weblate.securedrop.org. Weblate will happily sync translations of identical source strings between securedrop and securedrop-client, which can pollute the translation (or at least the Git history) of the former.
  6. https://weblate.securedrop.org/projects/securedrop/securedrop-client/#repository: Select Commit.

  7. Observe 3 commits (after 1266-continuous-l10n) in weblate-fpf/securedrop-client@weblate-securedrop-securedrop-client.

  8. In a securedrop-client development environment (i.e., with the virtual environment activated):

     $ git remote add weblate-fpf https://github.com/weblate-fpf/securedrop-client
     $ git fetch weblate-fpf
     $ git checkout weblate-fpf/weblate-securedrop-securedrop-client
     $ LANG=es_ES ./run.sh
    
  9. Observe that the Username label on the login screen has been translated.

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:

  • I have tested these changes in the appropriate Qubes environment
  • I do not have an appropriate Qubes OS workstation set up (the reviewer will need to test these changes)
  • These changes should not need testing in Qubes

If these changes add or remove files other than client code, the AppArmor profile may need to be updated. Please check as applicable:

  • I have updated the AppArmor profile
  • No update to the AppArmor profile is required for these changes
  • I don't know and would appreciate guidance

If these changes modify the database schema, you should include a database migration. Please check as applicable:

  • I have written a migration and upgraded a test database based on main and confirmed that the migration applies cleanly
  • I have written a migration but have not upgraded a test database based on main and would like the reviewer to do so
  • I need help writing a database migration
  • No database schema changes are needed

Pre-merge steps

  1. Remove "restricted" status from securedrop-client Weblate component
  2. Remove FIXME in supported-languages Make target

Post-merge steps → #1302

  1. https://weblate.securedrop.org/settings/securedrop/securedrop-client/#vcs: Change Repository Branch to main
  2. Monitor daily l10n GitHub Actions workflow for successful update-translation-catalogs

Known issues

  1. CircleCI will run the securedrop_client_ci workflow even for each new head of Weblate's fork, because it sees the source branch weblate-securedrop-securedrop-client through the pull request opened by Weblate. (This looks to CircleCI like pull/1281; there's no other information about the pull request available to use in filtering logic.)

@cfm cfm requested a review from a team as a code owner August 26, 2021 22:50
@cfm cfm marked this pull request as draft September 1, 2021 17:30
@cfm
Copy link
Member Author

cfm commented Sep 1, 2021

Re: failing test-buster: These are linting failures caused by gettext.translation().install(). flake8 can be placated with --builtins; mypy is trickier. I've returned this pull request to draft status while I work on a fix.

@cfm
Copy link
Member Author

cfm commented Sep 1, 2021

#1282 (comment):

Re: failing test-buster: These are linting failures caused by gettext.translation().install(). flake8 can be placated with --builtins; mypy is trickier.

This is thorny, as it turns out, and I regret not catching this earlier when it transitioned from "tests are flaking because of something in main" to "linting is failing here". Tracking issue forthcoming.

@cfm cfm force-pushed the 1266-continuous-l10n branch 3 times, most recently from e8af45a to daac963 Compare September 2, 2021 00:12
@cfm
Copy link
Member Author

cfm commented Sep 2, 2021

#1293 is resolved, crankily, in 35d8a16. Re-marking as ready for review.

@cfm cfm marked this pull request as ready for review September 2, 2021 00:31
@cfm cfm force-pushed the 1266-continuous-l10n branch from b62af0a to c6ad713 Compare September 2, 2021 01:49
cfm added 6 commits September 2, 2021 11:26
"from gettext import gettext as _" in a given module ignores the locale
installed in securedrop.app.configure_locale_and_language().

See: https://docs.python.org/3/library/gettext.html#gettext.install
…text API

Fixes #1293 by using gettext.[bind]textdomain(), which will allow

    from gettext import gettext as _

to use the globally-configured locale without running afoul of mypy.

This reverts commit 4dd4d13.
@cfm cfm force-pushed the 1266-continuous-l10n branch 2 times, most recently from 9bed725 to 15be76f Compare September 2, 2021 18:37
Copy link
Contributor

@sssoleileraaa sssoleileraaa left a comment

Choose a reason for hiding this comment

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

I think you need to allow members of the sd-maintainers group perms for https://weblate.securedrop.org/projects/securedrop/securedrop-client/#repository

@cfm
Copy link
Member Author

cfm commented Sep 13, 2021

#1282 (review):

I think you need to allow members of the sd-maintainers group perms for https://weblate.securedrop.org/projects/securedrop/securedrop-client/#repository

Ah; I'd assumed that you would have more access than I. :-) I've unmarked the "SecureDrop Client" component as "restricted", which should let you access it just like the other Weblate components.

@cfm cfm requested a review from sssoleileraaa September 13, 2021 18:20
sssoleileraaa
sssoleileraaa previously approved these changes Sep 21, 2021
Copy link
Contributor

@sssoleileraaa sssoleileraaa left a comment

Choose a reason for hiding this comment

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

@cfm This lgtm. You can see the three expected commits here: weblate-fpf@9395363 and when I run the client with the selected language as Spanish, I see the username translation. Are we ready to move this from securedrop to securedrop-client (main...1266-continuous-l10n#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R158) and merge? Anything else you'd like to do before a merge here?

@cfm
Copy link
Member Author

cfm commented Sep 21, 2021

@creviera in #1282 (review):

@cfm This lgtm. You can see the three expected commits here: weblate-fpf@9395363 and when I run the client with the selected language as Spanish, I see the username translation.

As expected! 🎉 Thanks for running through it from start to finish.

Are we ready to move this from securedrop to securedrop-client (main...1266-continuous-l10ndiff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R158) and merge? Anything else you'd like to do before a merge here?

This is done in c86325e. I've also tacked on 9d09d24 to fix a bug in my jq logic in the supported-languages target that I didn't catch while it was still pointing to securedrop rather than securedrop-client.

Otherwise, I think this is ready to go! I can take care of the post-merge steps in Weblate once this is in main.

Copy link
Contributor

@sssoleileraaa sssoleileraaa left a comment

Choose a reason for hiding this comment

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

most recent changes lgtm - tests pass locally and in ci + talked more with cfm about why we had to switch to using gettext.[bind]textdomain (see issue #1293)

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.

script continuous-localization workflow between GitHub repository and Weblate component
3 participants