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

build: package only translations explicitly marked as supported #1497

Merged
merged 5 commits into from Jul 5, 2022

Conversation

cfm
Copy link
Member

@cfm cfm commented May 25, 2022

Description

Closes #1438 by adding in MANIFEST.in (rather than setup.py as originally proposed):

  1. a prune directive to exclude translation assets present in the tree by default; and
  2. instructions for adding graft directives for explicitly including translation assets only for languages considered supported.

Also:

  1. removes make supported-languages, obsoleted by this approach per Add list of supported languages; disable unsupported languages #1438 (comment);
  2. removes the Weblate CLI (wlc), which is no longer needed; and
  3. updates development dependencies along the way.
    • ebb5336 fixes an unrelated typing error caught by (4)'s update of mypy 0.940 → 0.960. Let me know if a stack of pull requests is preferable for separate review of (3)‒(4) and then these (1)‒(2).

Test Plan

  • By default, no translations are packaged:
    (.venv) user@sd-dev:~/securedrop-client$ python setup.py sdist 2> /dev/null | grep "locale"
    (.venv) user@sd-dev:~/securedrop-client$
  • Pick a language from securedrop_client/locale, e.g.:
    (.venv) user@sd-dev:~/securedrop-client$ export LANG=de
  • An unpackaged translation is still loadable:
    (.venv) user@sd-dev:~/securedrop-client$ python -m securedrop_client  # appears in $LANG
  • A language marked for inclusion is packaged:
    (.venv) user@sd-dev:~/securedrop-client$ echo "graft securedrop_client/locale/$LANG" >> MANIFEST.in
    (.venv) user@sd-dev:~/securedrop-client$ python setup.py sdist 2> /dev/null | grep "locale"
    creating securedrop-client-0.7.0/securedrop_client/locale
    creating securedrop-client-0.7.0/securedrop_client/locale/de
    creating securedrop-client-0.7.0/securedrop_client/locale/de/LC_MESSAGES
    copying securedrop_client/locale/de/LC_MESSAGES/messages.mo -> securedrop-client-0.7.0/securedrop_client/locale/de/LC_MESSAGES
    copying securedrop_client/locale/de/LC_MESSAGES/messages.po -> securedrop-client-0.7.0/securedrop_client/locale/de/LC_MESSAGES

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

These assets are covered by:

/opt/venvs/securedrop-client/** r,

  • 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

@cfm cfm added the i18n label May 25, 2022
@cfm cfm added this to In Development in SecureDrop Team Board May 25, 2022
@cfm cfm marked this pull request as ready for review May 25, 2022 18:47
@cfm cfm requested a review from a team as a code owner May 25, 2022 18:47
@cfm cfm moved this from In Development to Ready for Review in SecureDrop Team Board May 25, 2022
@cfm
Copy link
Member Author

cfm commented May 25, 2022

Marking as ready for review while I investigate why an unrelated test has started to fail in CI.

@cfm
Copy link
Member Author

cfm commented May 26, 2022

Once CI has been placated on main, I'll rebase this and see if anything's still unhappy here.

@gonzalo-bulnes
Copy link
Contributor

You should be good to go whenever you want now @cfm -CI is fixed | latest nightly build 🙂

@cfm cfm force-pushed the 1438-supported-languages branch 2 times, most recently from 5840426 to fb85e48 Compare May 27, 2022 06:22
@cfm cfm mentioned this pull request Jun 3, 2022
10 tasks
@gonzalo-bulnes gonzalo-bulnes self-requested a review June 8, 2022 05:59
@cfm cfm self-assigned this Jun 22, 2022
@cfm cfm moved this from Ready for Review to In Development in SecureDrop Team Board Jun 22, 2022
cfm added 3 commits June 22, 2022 14:31
Translations are excluded from the source distribution by default by
pruning the "locale" directory.  Translations for supported languages
must be explicitly grafted back in.

All translations present in the tree remain loadable during development
via (e.g.):

	$ LANG=es python -m securedrop_client
Per #1438, language support will be a maintainers' decision, rather than
an automated check based on translation coverage in Weblate.
wlc was only used by "make supported-languages", which we've removed.
@cfm cfm force-pushed the 1438-supported-languages branch from fb85e48 to 94b2a50 Compare June 22, 2022 21:33
@cfm
Copy link
Member Author

cfm commented Jun 22, 2022

Investigating unrelated failing check remaining after rebase from main:

securedrop_client/queue.py:95: error: Argument 1 to "append" of "list" has incompatible type "Optional[QueueJob]"; expected "QueueJob"  [arg-type]

@cfm cfm force-pushed the 1438-supported-languages branch from 2ed1bc4 to 57e9b40 Compare June 22, 2022 22:16
@cfm
Copy link
Member Author

cfm commented Jun 22, 2022

Ready for review with CI (finally) passing. Along the way, ebb5336 fixes an unrelated typing error caught by new mypy 0.940 → 0.960.

This is an unfortunate side effect of not pinning development dependencies. Removing a dependency X from a requirements.in still entails regenerating the requirements.txt for all dependencies X'. And new versions of mypy often break reveal preexisting bugs in things!

Let me know if a stack of pull requests is preferable for separate review.

@cfm cfm removed their assignment Jun 22, 2022
@cfm cfm moved this from In Development to Ready for Review in SecureDrop Team Board Jun 22, 2022
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.

lgtm - test plan checks out

#
# graft securedrop_client/locale/$LANG
#
# Please keep this list alphabetized.
Copy link
Contributor

Choose a reason for hiding this comment

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

<3

@@ -92,7 +92,8 @@ def _check_for_duplicate_jobs(self, job: QueueJob) -> bool:
stored on self.current_job. We check that the job to be added is not among them.
"""
in_progress_jobs = [in_progress_job for priority, in_progress_job in self.queue.queue]
in_progress_jobs.append(self.current_job)
if self.current_job is not None:
in_progress_jobs.append(self.current_job)
Copy link
Contributor

Choose a reason for hiding this comment

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

I see this was caught by mypy update to 0.960

@sssoleileraaa sssoleileraaa merged commit 69c3d7a into main Jul 5, 2022
SecureDrop Team Board automation moved this from Ready for Review to Done Jul 5, 2022
@sssoleileraaa sssoleileraaa deleted the 1438-supported-languages branch July 5, 2022 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Add list of supported languages; disable unsupported languages
3 participants