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

Stop using pkg_resources #1851

Merged
merged 1 commit into from
Feb 22, 2024
Merged

Stop using pkg_resources #1851

merged 1 commit into from
Feb 22, 2024

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented Feb 19, 2024

Status

Ready for review

Description

pkg_resources is deprecated and we're supposed to use
importlib.resources instead. Unfortunately it's very awkward
to use because it wants to support being able to read from
possibly compressed archives or other Python package distribution
formats.

Since our distribution mechanism is Debian packages, we can avoid
all of this entirely and just read the files off disk like any
other file.

Take the opportunity to simplify font loading: instead of hardcoding
each font, just load every TTF in the fonts/ directory by using a
recursive glob.

And then drop some tests that don't test anything useful

Fixes #1672.
Fixes #1836.

Test Plan

  • CI passes
  • Visual review, various modals style correctly and look correct.

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

@rocodes
Copy link
Contributor

rocodes commented Feb 22, 2024

(will need a rebase and unfortuantely there are a couple more places that introduced pkg_resources in the Veracrypt work, in export_wizard.py and export_wizard_page.py, didn't know this was on deck sorry)

@rocodes
Copy link
Contributor

rocodes commented Feb 22, 2024

(Rebased + removed pkg_resources in export_wizard and export_wizard_page)

Copy link
Member Author

@legoktm legoktm left a comment

Choose a reason for hiding this comment

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

Your changes LGTM!

@legoktm
Copy link
Member Author

legoktm commented Feb 22, 2024

oh there's now an isort conflict, one minute...

pkg_resources is deprecated and we're supposed to use
importlib.resources instead. Unfortunately it's very awkward
to use because it wants to support being able to read from
possibly compressed archives or other Python package distribution
formats.

Since our distribution mechanism is Debian packages, we can avoid
all of this entirely and just read the files off disk like any
other file. A new `load_relative_css` helper finds CSS files next to the
file invoking the function.

Take the opportunity to simplify font loading: instead of hardcoding
each font, just load every TTF in the fonts/ directory by using a
recursive glob.

And then drop some tests that don't test anything useful

Fixes #1672.
Fixes #1836.
@rocodes rocodes merged commit c27f367 into main Feb 22, 2024
72 checks passed
@rocodes rocodes deleted the pkg_resources branch February 22, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
2 participants