postinst error in securedrop-app-code package install
#6230
Comments
|
Took a quick look at this today. My understanding is that we're registering a trigger for changes to the python3.8 interpreter in the |
|
I posted the fully generated postinst at https://gist.github.com/legoktm/0542388d8639e2a4bacb072ec5649c41 The problem is that we inject the autogenerated dh-virtualenv snippet, which correctly handles "triggered", after we bail out for not recognizing "triggered". I think we either teach our postinst about "triggered", OR not erroring out on an invalid postinst command. I'll codesearch the Debian archive to see which pattern is more popular. I considered for a minute putting the |
|
The codesearch wasn't that helpful, mostly because I can't easily search for the absence of "triggered". So if we do go ahead and add it to the known list, it does mean there will be a behavior change in that we will now run the It seems safe and future proof to add "triggered" in either way, so I'll put up PRs for that shortly. Per https://manpages.debian.org/bullseye/dpkg-dev/deb-postinst.5.en.html triggered is the only one we're missing. |
Per deb-postinst(5), the postinst is called with "triggered" after the package was triggered. 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 custom postinst snippet was running before that and erroring out on any invocation for "triggered", so it never ran. Now "triggered" will fall through and the dh-virtualenv inserted code will run whenever Python is upgraded. I am a bit skeptical of the need for the trigger and what dh-virtualenv wants to do (especially given it hasn't run up until now), but having "triggered" here seems like good future-proofing in general. [1] https://dh-virtualenv.readthedocs.io/en/latest/tutorial.html?highlight=trigger#step-2-set-up-packaging-for-your-project Fixes #6230.
|
I re-reviewed the code for dh_venv_safe_interpreter_update() {
# get Python version used
local pythonX_Y=$(cd "$dh_venv_install_dir/lib" && ls -1d python[2-9].*[0-9] | tail -n1)
local i
for i in python ${pythonX_Y%.*} ${pythonX_Y}; do
local interpreter_path="$dh_venv_install_dir/bin/$i"
# skip any symlinks, and make sure we have an existing target
test ! -L "$interpreter_path" || continue
test -x "$interpreter_path" || continue
# skip if already identical
if cmp "/usr/bin/$pythonX_Y" "$interpreter_path" >/dev/null 2>&1; then
continue
fi
...Stepping through that code, the Looking in Except both of those are symlinks, so they get skipped by the "skip any symlinks" step. So it really does nothing. After git-blaming a bit, I found spotify/dh-virtualenv#48 which describes the issue it's trying to solve: "virtualenvs tend to break horribly after certain host Python updates – those that change internal, statically linked modules, or make symlinked .so modules incompatible to the old "python" binary contained in the virtualenv." Which isn't an issue for us because there is no "python" binary in the virtualenv, it's all symlinks back to the system This is the first time I'm looking at the packaging, so let me know if I'm missing something. |
|
Debian/Ubuntu will not update to a new major Python version within a release, means the ABI will be the same and the virtualenv should be just functioning. Our virtualenv is already having compiled code in it, means if we update the OS and get a new Python ABI, then we should have a new virtualenv via a new package. |
|
@kushaldas Thanks for chiming in! My takeaway is that we should remove the trigger altogether, no longer registering an "interest" in the python interpreter. That's a slightly different approach from what's in #6231, but would be a similarly minimal diff, with the added advantage that the functionality wouldn't change: triggers would continue not to run! Do you agree? cc @legoktm |
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.
|
Sounds good to me, I reworked #6231 in that direction. |
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.
Seen on my test instance and one other:
I see this for upgrades going back to SecureDrop 2.0.0, so it does not appear to interfere with successful upgrades.
The text was updated successfully, but these errors were encountered: