-
Notifications
You must be signed in to change notification settings - Fork 697
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
Don't register trigger on Python for securedrop-app-code package #6231
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #6231 +/- ##
===========================================
- Coverage 85.13% 85.12% -0.01%
===========================================
Files 59 59
Lines 4090 4088 -2
Branches 487 487
===========================================
- Hits 3482 3480 -2
Misses 491 491
Partials 117 117
Continue to review full report at Codecov.
|
504bef7
to
258c40a
Compare
We currently register a trigger with the Python interpreter so dh_virtualenv can fix up the interpreter's symlinks in case something changes as recommended in their docs[1]. However, our packages are designed for a single Ubuntu release, which keeps the same version of Python for its entire lifetime, so the interpreter in the venv will always be a symlink to `/usr/bin/python3` and the dh_virtualenv postinst code will always be a no-op because it skips all symlinks. This was noticed because our custom postinst was not handling the "triggered" state, which is valid per deb-postinst(5). Add that in for future proofing even though we don't expect it to be called. [1] https://dh-virtualenv.readthedocs.io/en/latest/tutorial.html?highlight=trigger#step-2-set-up-packaging-for-your-project Fixes #6230.
258c40a
to
b64f7a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for the detailed comments discussing the approach!
FYI, I grepped through the repo and found some lingering references to the trigger file in the build logic, and snipped those out, squashing into your commit. The per-PR CI didn't complain, but the full nightly CI runs would have. Let's keep an eye on the nightly builds to make sure we didn't miss anything.
@conorsch Thanks! Looks like the nightly CI passed fine. Is there any reason we don't run the package building CI parts when those files are modified in a PR? |
Last I checked, CircleCI doesn't support triggering jobs based on subpaths within a repo. We could rig up our own bash script to compare branches with |
Status
Ready for review, but untested
Description of Changes
Don't register trigger on Python for securedrop-app-code package
We currently register a trigger with the Python
interpreter so dh_virtualenv can fix up the interpreter's symlinks
in case something changes as recommended in their docs[1].
However, our packages are designed for a single Ubuntu release, which
keeps the same version of Python for its entire lifetime, so the
interpreter in the venv will always be a symlink to
/usr/bin/python3
and the dh_virtualenv postinst code will always be a no-op because it
skips all symlinks.
This was noticed because our custom postinst was not handling the
"triggered" state, which is valid per deb-postinst(5). Add that in for
future proofing even though we don't expect it to be called
[1] https://dh-virtualenv.readthedocs.io/en/latest/tutorial.html?highlight=trigger#step-2-set-up-packaging-for-your-project
Fixes #6230.
Changes proposed in this pull request:
Testing
Alternatively I think you can directly run the trigger with something like
dpkg-trigger --by-package python3.8 /usr/bin/python3.8
Deployment
This will remove the trigger behavior entirely, which should be a no-op because it was broken until now.
Checklist
make lint
) and tests (make test
) pass in the development container