From cc43b2831dee5a75a2b3d821a1884329751afcce Mon Sep 17 00:00:00 2001 From: sudolulo Date: Mon, 15 Jun 2026 04:07:16 +0000 Subject: [PATCH] Fix five quality findings from second re-review sitecustomize.py: when find_spec resolves real_spec as None (module absent after a TrueNAS update), record a FAIL status and mark the module done so hook_status.json is still written and cmd_verify shows a diagnostic FAIL instead of the ambiguous "no status file found". sitecustomize.py: the AttributeError fallback in the URL-fix wrapper now writes a WARNING to stderr before returning the unmodified result, making the unexpected ResticConfig type visible in journalctl. apply.sh: after falling back to bare python3, verify that python3 can also import middlewared; if not, emit a second warning so the operator knows the backend patch may be installed in the wrong site-packages directory. patch_ui.py: abort (return without writing) when FIND.subn produces a count other than 1, instead of committing a doubly-patched bundle and having subsequent runs silently accept it via the MARKER check. uninstall.sh: when a vendor sitecustomize.py backup exists, use mv to atomically overwrite our file rather than rm-then-mv; eliminates the window where a read-only /usr causes rm to fail under set -e, aborting before the backup is restored. --- patch/apply.sh | 3 +++ patch/create_task.py | 2 +- patch/patch_ui.py | 4 +++- patch/sitecustomize.py | 16 +++++++++++++++- uninstall.sh | 8 +++++--- 5 files changed, 27 insertions(+), 6 deletions(-) diff --git a/patch/apply.sh b/patch/apply.sh index 23b9e36..d63287e 100755 --- a/patch/apply.sh +++ b/patch/apply.sh @@ -63,6 +63,9 @@ find_mw_python() { if ! "$py" -c "import middlewared" 2>/dev/null; then echo "WARNING: '$py' cannot import middlewared; falling back to python3" >&2 py="python3" + if ! "$py" -c "import middlewared" 2>/dev/null; then + echo "WARNING: 'python3' also cannot import middlewared; sitecustomize.py may be installed in the wrong location" >&2 + fi fi echo "$py" diff --git a/patch/create_task.py b/patch/create_task.py index 6e26683..7cb8116 100755 --- a/patch/create_task.py +++ b/patch/create_task.py @@ -114,7 +114,7 @@ def cmd_verify(): print() if all_ok: - print("All patches active. B2 and S3 backups should work.") + print("All patches installed. Run a test backup to confirm end-to-end.") else: print("One or more patches failed to apply.") print("Check /data/truecloud-patch/apply.log and journalctl -u middlewared") diff --git a/patch/patch_ui.py b/patch/patch_ui.py index 8065b75..57618de 100644 --- a/patch/patch_ui.py +++ b/patch/patch_ui.py @@ -96,9 +96,11 @@ def main(): if count != 1: print( f"[truecloud-patch] WARNING: {count} replacement(s) in {path}; " - f"expected exactly 1 — file an issue at " + f"expected exactly 1 — skipping write to avoid corrupting the bundle.\n" + f"[truecloud-patch] File an issue at " f"https://github.com/sudolulo/truenas-truecloud-patch" ) + return tmp = path + ".tmp" try: diff --git a/patch/sitecustomize.py b/patch/sitecustomize.py index a760723..75f8b48 100644 --- a/patch/sitecustomize.py +++ b/patch/sitecustomize.py @@ -60,7 +60,13 @@ class _Finder: self._loading.discard(fullname) if real_spec is None: - return None # module doesn't exist; don't intercept + # Module absent in this Python installation (e.g. removed in a + # TrueNAS update). Record a FAIL so hook_status.json is still + # written and cmd_verify gives a diagnostic instead of "no file". + _record_status(fullname, ok=False, + detail="module not found in this Python installation") + self._mark_done(fullname) + return None return importlib.machinery.ModuleSpec( fullname, @@ -185,6 +191,10 @@ def _patch_restic(module): try: return result._replace(cmd=cmd) except AttributeError: + sys.stderr.write( + "[truecloud-patch] WARNING: cannot fix repo URL; " + f"unexpected ResticConfig type {type(result).__name__!r}\n" + ) return result break if i and cmd[i - 1] in ("-r", "--repo", "--repository"): @@ -197,6 +207,10 @@ def _patch_restic(module): try: return result._replace(cmd=cmd) except AttributeError: + sys.stderr.write( + "[truecloud-patch] WARNING: cannot fix repo URL; " + f"unexpected ResticConfig type {type(result).__name__!r}\n" + ) return result break return result diff --git a/uninstall.sh b/uninstall.sh index 93465a0..9a49b41 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -62,13 +62,15 @@ SITE_PKG=$("$PYTHON" -c "import site; print(site.getsitepackages()[0])" 2>/dev/n if [ -n "$SITE_PKG" ] && [ -f "$SITE_PKG/sitecustomize.py" ]; then if grep -q "truecloud-patch" "$SITE_PKG/sitecustomize.py" 2>/dev/null; then - rm "$SITE_PKG/sitecustomize.py" - echo " Removed $SITE_PKG/sitecustomize.py" - if [ -f "$SITE_PKG/sitecustomize.py.pre-truecloud-patch" ]; then + # mv atomically overwrites our file with the vendor original — + # safer than rm-then-mv if /usr is transiently read-only. mv "$SITE_PKG/sitecustomize.py.pre-truecloud-patch" \ "$SITE_PKG/sitecustomize.py" echo " Restored previous sitecustomize.py" + else + rm "$SITE_PKG/sitecustomize.py" + echo " Removed $SITE_PKG/sitecustomize.py" fi else echo " $SITE_PKG/sitecustomize.py is not ours; leaving it alone."