Repository navigation
Bundle Python 3.10 for older Ubuntu support - #157
Conversation
7f2be83 to
c8faa92
Compare
rtibblesbot
left a comment
There was a problem hiding this comment.
Well-structured approach to bundling Python 3.10 for older Ubuntu. The layered resolution (user config → bundled → system), SHA256 verification at both build and install time, and the kolibri-wrapper.sh replacing the setuptools entry point are all solid design choices.
CI passing across Ubuntu 16.04–24.04. No UI files changed — visual inspection skipped.
- suggestion:
install-python.sharchitecture handling could be more graceful on unsupported arches - suggestion:
python3-pkg-resourcesremoval — confirm no runtime dependency - praise: Checksum verification, Python resolution chain, and
add_statereordering are all well done
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Reviewed the pull request diff checking for:
- Correctness: bugs, edge cases, undocumented behavior, resource leaks, hardcoded values
- Design: unnecessary complexity, naming, readability, comment accuracy, redundant state
- Architecture: duplicated concerns, minimal interfaces, composition over inheritance
- Testing: behavior-based assertions, mocks only at hard boundaries, accurate coverage
- Completeness: missing dependencies, unupdated usages, i18n, accessibility, security
- Principles: DRY (same reason to change), SRP, Rule of Three (no premature abstraction)
- Checked CI status and linked issue acceptance criteria
- For UI changes: inspected screenshots for layout, visual completeness, and consistency
bjester
left a comment
There was a problem hiding this comment.
Overall, looks complete. Some areas for improvement:
- have the download script ensure it has checksums to check the download with before downloading
- use
sourceinstead of.in scripts - consider the tradeoffs of using checksum files and
sha256sum -cinstead-- may not be of benefit because of the rich error logging
23d3a63 to
33fea47
Compare
rtibblesbot
left a comment
There was a problem hiding this comment.
Prior findings
Resolved:
- Exit 1 on unknown arch in
debian/install-python.sh(suggestion) — code now exits 0 with "Falling back to system Python."
Acknowledged (not re-raised):
python3-pkg-resourcesremoval from Depends (suggestion) — author replied: "Kolibri already bundles this internally when needed."
2/2 prior findings resolved or acknowledged. 0 re-raised.
CI passing. No UI files changed.
- praise: see inline on
build_tools/manage_python.py - suggestion: see inline on
build_tools/manage_python.py
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibblesbot
left a comment
There was a problem hiding this comment.
Prior findings
All 1 prior finding resolved: cmd_update double-download suggestion — code now writes directly to build_src/ using tarball_filename(full_version, arch), matching cmd_download's naming. 0 re-raised.
Clean additions — smoke tests and KOLIBRI_PYTHON resolution are both well-structured. CI passing. No UI files changed.
- praise: see inline on
install_test.ymlanddebian/startup/kolibri.default - nitpick: see inline on
install_test.yml
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
🟡 Waiting for changesLast updated: 2026-10-01 20:31 UTC |
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #157 — All prior findings resolved or acknowledged; the runtime path (init → wrapper → bundled/system Python) is coherent and covered by the new 16.04–24.04 smoke test. CI passing. Two new findings on the new build_tools/manage_python.py, both inline.
- important —
manage_python.pyships with no tests despite the siblinggenerate_changelog.py/tests/convention. Its pure functions (tarball_filename,tarball_url,read_config/write_config,find_release) are prime unit-test candidates. - suggestion — unanchored version-prefix match in
find_releasecould resolve3.1to a 3.10/3.11 tarball.
No UI files changed — Phase 3 (UI verification / manual QA) not applicable.
Prior-finding status
RESOLVED — debian/install-python.sh — exit 1 on unknown arch (now falls back to system Python)
ACKNOWLEDGED — debian/control:22 — python3-pkg-resources removal (Kolibri bundles internally)
RESOLVED — debian/startup/kolibri.default:70 — resolution-chain documentation
RESOLVED — build_tools/download_python.sh — checksum/source/.env concerns (rewritten to manage_python.py)
RESOLVED — debian/kolibri.scripts-common:285 — add_state ordering
RESOLVED — debian/kolibri-wrapper.sh:9 — wrapper re-sources /etc/default/kolibri under su
RESOLVED — build_tools/manage_python.py:143 — fail-fast before network request
RESOLVED — build_tools/manage_python.py — cmd_update double-download (now writes to build_src/)
RESOLVED — .github/workflows/install_test.yml:16 — fail-fast: false
RESOLVED — .github/workflows/install_test.yml — retry-wait wording (now "after 60 attempts")
RESOLVED — debian/startup/kolibri.default:81 — PYTHONPATH prepend handling
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Ran the same phased review passes as a first review (core, frontend/backend lenses, manual QA when required)
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #157 — 13 of 13 prior findings resolved; 5 new findings (1 blocking, 4 suggestions).
CI passing, but it only exercises dpkg-buildpackage -A; the blocking finding is on the -S source path that PR CI never runs. No UI files — Phase 3 not applicable.
- blocking build_tools/build.sh:127 — bundled tarballs at the source root break the
3.0 (quilt)source build used by the PPA release path. - suggestion debian/kolibri-wrapper.sh:10 — the wrapper exports the whole service environment, not just
KOLIBRI_PYTHON(this reverses my "harmless" call last round). - suggestion debian/kolibri.postrm:15 — extracted interpreter survives
apt remove. - suggestion debian/control:21 — unguarded
py3cleanin prerm breaks removal on a system with nopython3. - suggestion debian/rules:45 — answer to the open question on the 56 MB payload.
Prior-finding status
RESOLVED — build_tools/manage_python.py:1 — module ships with no tests
RESOLVED — build_tools/manage_python.py:185 — version prefix not anchored on version boundary
RESOLVED — build_tools/manage_python.py:229 — cmd_update discards downloaded tarballs
RESOLVED — build_tools/manage_python.py:143 — defensive checksum-present check (praise)
RESOLVED — debian/install-python.sh — exit 1 on unsupported architectures
RESOLVED — debian/control:22 — python3-pkg-resources fully removed
RESOLVED — debian/kolibri-wrapper.sh:9 — wrapper re-sources /etc/default/kolibri
RESOLVED — debian/startup/kolibri.default:70 — Python resolution chain (praise)
RESOLVED — debian/startup/kolibri.default:81 — ${PYTHONPATH:+:$PYTHONPATH} handling (praise)
RESOLVED — debian/kolibri.scripts-common:285 — add_state before service registration (praise)
RESOLVED — build_tools/download_python.sh:57 — checksum verified on cache hits and fresh downloads (praise)
RESOLVED — .github/workflows/install_test.yml:16 — fail-fast: false (praise)
RESOLVED — .github/workflows/install_test.yml:64 — failure message overstated the wait budget
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
497f025 to
db25056
Compare
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #157 — 1 blocking, 4 suggestions inline. CI green.
Prior-finding status
RESOLVED — build_tools/build.sh — orig-tree
RESOLVED — debian/kolibri-wrapper.sh — sourcing
RESOLVED — debian/kolibri.postrm — cleanup
RESOLVED — debian/control:21 — python3
ACKNOWLEDGED — debian/rules — payload
RESOLVED — debian/install-python.sh — exit 1
RESOLVED — debian/control:22 — python3-pkg-resources
RESOLVED — debian/startup/kolibri.default:70 — KOLIBRI_PYTHON
RESOLVED — debian/startup/kolibri.default:81 — PYTHONPATH
RESOLVED — build_tools/download_python.sh — checksums
RESOLVED — debian/kolibri.scripts-common:285 — after_postinst (withdrawn: unread)
RESOLVED — build_tools/manage_python.py:143 — fail-fast
RESOLVED — build_tools/manage_python.py — temp-dir
RESOLVED — build_tools/manage_python.py — prefix
RESOLVED — build_tools/manage_python.py:1 — tests/test_manage_python.py
RESOLVED — .github/workflows/install_test.yml:16 — fail-fast: false
RESOLVED — .github/workflows/install_test.yml — timeout
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
db25056 to
58cce56
Compare
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #157 — 19 of 23 prior findings resolved; 4 still open (see below).
CI passing, no UI files. Four prior suggestions re-raised unchanged, six new, nothing blocking.
Prior-finding status
RESOLVED — build_tools/build.sh — quilt source build
RESOLVED — build_tools/manage_python.py:1 — no tests
RESOLVED — build_tools/manage_python.py — temp-dir download
RESOLVED — build_tools/manage_python.py — unanchored match
RESOLVED — build_tools/manage_python.py:143 — praise
RESOLVED — build_tools/download_python.sh — praise
RESOLVED — debian/control — other arches
RESOLVED — debian/control:21 — Recommends
RESOLVED — debian/control:22 — pkg-resources
RESOLVED — debian/install-python.sh — arch exit 1
RESOLVED — debian/kolibri-wrapper.sh — KOLIBRI_PYTHON source
RESOLVED — debian/kolibri-wrapper.sh — dual-sourcing
RESOLVED — debian/kolibri.postrm — untracked tree
RESOLVED — debian/rules — both arches bundled
RESOLVED — debian/startup/kolibri.default:70 — praise
RESOLVED — debian/startup/kolibri.default:81 — praise
RESOLVED — debian/kolibri.scripts-common:285 — praise, retracted
RESOLVED — .github/workflows/install_test.yml:16 — praise
RESOLVED — .github/workflows/install_test.yml — wait message
UNADDRESSED — .github/workflows/install_test.yml:47 — masked failures
UNADDRESSED — debian/install-python.sh:78 — unstaged extract
UNADDRESSED — debian/rules:42 — env without tarballs
UNADDRESSED — build_tools/manage_python.py:238 — bad hash pinned
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
58cce56 to
914e9e8
Compare
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #157 — 28 of 30 prior findings resolved or withdrawn; 2 still open (re-raised inline).
dpkg-source blocker fixed; the 3.10.21+20260901 pin verified live. CI passing; no UI files. Suggestions only.
Prior-finding status
RESOLVED — build_tools/build.sh:128 — dpkg-source -b
RESOLVED — build_tools/build.sh:131 — every tarball shipped
RESOLVED — build_tools/manage_python.py:1 — no tests
RESOLVED — build_tools/manage_python.py:195 — prefix match
RESOLVED — build_tools/manage_python.py:157 — missing checksum
RESOLVED — build_tools/manage_python.py:266 — truncated file
RESOLVED — build_tools/manage_python.py:226 — version floor
RESOLVED — build_tools/python_versions.env:8 — stale pin
RESOLVED — build_tools/download_python.sh:57 — cache hits
RESOLVED — debian/control:9 — non-amd64 arches
RESOLVED — debian/control:21 — Recommends: python3
RESOLVED — debian/control:22 — python3-pkg-resources
RESOLVED — debian/rules:44 — arch-all payload
RESOLVED — debian/install-python.sh:41 — stale $INSTALL_DIR
RESOLVED — debian/install-python.sh:57 — other arches
RESOLVED — debian/install-python.sh:72 — checksum first
RESOLVED — debian/install-python.sh:82 — staged extraction
RESOLVED — debian/kolibri-wrapper.sh:9 — env sourcing
RESOLVED — debian/kolibri-wrapper.sh:13 — subshell set -e
RESOLVED — debian/kolibri-wrapper.sh:15 — dual-sourcing
RESOLVED — debian/kolibri.postrm:11 — untracked tree
ACKNOWLEDGED — debian/kolibri.scripts-common:307 — praise retracted
RESOLVED — debian/kolibri.scripts-common:424 — py3clean guard
RESOLVED — debian/startup/kolibri.default:70 — resolution chain
RESOLVED — debian/startup/kolibri.default:81 — PYTHONPATH
RESOLVED — .github/workflows/install_test.yml:16 — fail-fast
RESOLVED — .github/workflows/install_test.yml:73 — wait message
RESOLVED — .github/workflows/install_test.yml:26 — no-python leg
UNADDRESSED — .github/workflows/install_test.yml:49 — no interpreter assertion
UNADDRESSED — debian/rules:42 — env file, no tarball
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
914e9e8 to
1d8f7b0
Compare
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #157 — 43 of 44 prior findings resolved, 1 open. CI passing.
Prior-finding status
RESOLVED — install-python.sh — unsupported arch
RESOLVED — install-python.sh:42 — stale tree
RESOLVED — install-python.sh:72 — checksum first
RESOLVED — install-python.sh:93 — partial tree
RESOLVED — install-python.sh:93 — re-raise
RESOLVED — install-python.sh — --same-owner
RESOLVED — control:21 — Recommends: python3
RESOLVED — control:22 — python3-pkg-resources
RESOLVED — control — arm64/armhf/i386
UNADDRESSED — control:22 — dist-packages shadowing
RESOLVED — rules — Architecture: all payload
RESOLVED — rules — env file without tarball
RESOLVED — rules — re-raise
RESOLVED — rules — make get-python
RESOLVED — kolibri-wrapper.sh — sourcing
RESOLVED — kolibri-wrapper.sh — dual-sourcing
RESOLVED — kolibri-wrapper.sh:14 — subshell errors
RESOLVED — kolibri-wrapper.sh — conffile skip
RESOLVED — kolibri-wrapper.sh — -m and CWD
RESOLVED — kolibri.postrm — untracked tree
RESOLVED — scripts-common — state ordering
RESOLVED — scripts-common — praise retracted
RESOLVED — kolibri.default:70 — resolution chain
RESOLVED — kolibri.default:81 — colon handling
RESOLVED — manage_python.py:1 — no tests
RESOLVED — manage_python.py:162 — fail fast
RESOLVED — manage_python.py:201 — prereleases
RESOLVED — manage_python.py:277 — truncated hash
RESOLVED — manage_python.py:277 — re-raise
RESOLVED — manage_python.py:69 — metadata validation
RESOLVED — manage_python.py — temp-dir download
RESOLVED — manage_python.py — startswith match
RESOLVED — manage_python.py — supported floor
RESOLVED — build.sh — dpkg-source breakage
RESOLVED — build.sh — every cpython tarball
RESOLVED — download_python.sh — cache checksums
RESOLVED — python_versions.env — stale pin
RESOLVED — install_test.yml:16 — fail-fast
RESOLVED — install_test.yml — wait message
RESOLVED — install_test.yml — python3 on every leg
RESOLVED — install_test.yml — no assertion
RESOLVED — install_test.yml — re-raise
RESOLVED — install_test.yml — silent fallback
RESOLVED — Makefile:14 — narrowed glob
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
1d8f7b0 to
504f672
Compare
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #157: 40 of 42 prior findings resolved. The other 2 are acknowledged and deferred to the maintainer. Nothing blocking; CI pending at review time.
Seven new suggestions, all inline:
manage_python.py:237— nothing keeps the bundled interpreter at or above the floor it is checked against. Add a guard inupdated_config.kolibri-wrapper.sh:34— barepython3drops the/usr/bin/python3pin the old console script carried. Restore the absolute path here and atkolibri.default:74.kolibri.postinst:33— the interpreter tree can be replaced under a running server on thedpkg-reconfigurepath. Stop the service first.build.sh:165— the-Ssource-package path the PPA ships from is never built in CI. Addmake kolibri.changesto the PR build.Makefile:67—get-tarwipes the cpython download cache before every CI build. Narrowclean-taror move the cache.install_test.yml:18— arm64 and upgrade installs have no test leg. Add one for each.test_manage_python.py:248—cmd_download, the only path the build runs, is untested. Make its paths injectable and cover it.
Prior-finding status
RESOLVED — debian/install-python.sh — non-amd64/arm64 exit 1
RESOLVED — debian/install-python.sh:43 — system-Python exit never touched $INSTALL_DIR
RESOLVED — debian/install-python.sh:81 — checksum verified before the rm -rf
RESOLVED — debian/install-python.sh:88 — no version comparison before extracting
RESOLVED — debian/install-python.sh:103 — failed extract left a partial tree winning
RESOLVED — debian/install-python.sh — root tar --same-owner/--same-permissions
RESOLVED — debian/install-python.sh — source over .
RESOLVED — debian/control:21 — python3 dropped to Recommends
RESOLVED — debian/control:22 — python3-pkg-resources fully removed
RESOLVED — debian/control — arm64, armhf, i386 lose apt install kolibri
RESOLVED — debian/kolibri-wrapper.sh — sources /etc/default/kolibri under the init script
RESOLVED — debian/kolibri-wrapper.sh — dual-sourcing beyond KOLIBRI_PYTHON
RESOLVED — debian/kolibri-wrapper.sh — kept conffile never receives the resolution block
RESOLVED — debian/kolibri-wrapper.sh — -m puts the caller's CWD on sys.path
RESOLVED — debian/kolibri-wrapper.sh:14 — set -e inherited into the sourcing subshell
RESOLVED — debian/kolibri.postrm — /opt/kolibri/python untracked by dpkg
RESOLVED — debian/kolibri.scripts-common — add_state "after_postinst" ordering
RESOLVED — debian/kolibri.scripts-common — after_postinst never read
RESOLVED — debian/rules — both arch tarballs in an Architecture: all package
RESOLVED — debian/rules — env file without tarball aborts postinst
RESOLVED — debian/rules — make get-python runs where there is no Makefile
RESOLVED — debian/startup/kolibri.default:70 — resolution chain
RESOLVED — debian/startup/kolibri.default:81 — ${PYTHONPATH:+:$PYTHONPATH}
RESOLVED — build_tools/manage_python.py:1 — module shipped with no tests
RESOLVED — build_tools/manage_python.py:69 — tag and full_version unvalidated
RESOLVED — build_tools/manage_python.py:162 — fails fast before the network request
RESOLVED — build_tools/manage_python.py:201 — no draft/prerelease check
RESOLVED — build_tools/manage_python.py:277 — interrupted downloads pinned a truncated hash
RESOLVED — build_tools/manage_python.py — unanchored startswith(f"cpython-{target}") match
RESOLVED — build_tools/manage_python.py — cmd_update discarded ~56 MB to a temp directory
RESOLVED — build_tools/manage_python.py — supported floor tracked VERSION=
RESOLVED — build_tools/build.sh — non-debian/ files broke dpkg-source -b
RESOLVED — build_tools/build.sh — shipped every cpython-* tarball present
RESOLVED — build_tools/download_python.sh — checksums on cache hits and fresh downloads
RESOLVED — build_tools/python_versions.env — four months stale
RESOLVED — .github/workflows/install_test.yml:16 — fail-fast: false
RESOLVED — .github/workflows/install_test.yml — "within 120 seconds" vs ~420 s actual
RESOLVED — .github/workflows/install_test.yml — hid 22.04/24.04 failures, postrm uncovered
RESOLVED — .github/workflows/install_test.yml — no system-python-free leg
RESOLVED — .github/workflows/install_test.yml — fallback dropped python3, 2>/dev/null hid why
ACKNOWLEDGED — debian/control:22 — python3-cryptography unloadable under the bundled interpreter
ACKNOWLEDGED — build_tools/python_versions.env:7 — 3.10 EOL 2026-10-31; which minor to bundle is a maintainer call
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
Add scripts and configuration to download and manage Python 3.10 from python-build-standalone for bundling in the .deb: - python_versions.conf: pins version, release tag, and SHA256 checksums - download_python.sh: downloads and verifies tarballs for x86_64/aarch64 - update_python_version.sh: developer tool to bump Python version - Makefile: get-python and update-python targets, get-python as prerequisite for deb and changes builds, fix globs to not match cpython tarballs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
504f672 to
6f19337
Compare
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #157: the new build_source CI job fails; please fix before merge.
All prior findings are resolved or acknowledged. One new suggestion is inline.
Prior-finding status
RESOLVED — debian/install-python.sh — non-amd64/arm64 exit 1 with old system Python
RESOLVED — debian/control:22 — python3-pkg-resources removed
RESOLVED — debian/startup/kolibri.default:70 — resolution chain (praise)
RESOLVED — build_tools/download_python.sh — checksum on cache hits (praise)
RESOLVED — debian/kolibri.scripts-common — add_state ordering (praise)
RESOLVED — debian/kolibri-wrapper.sh — sourcing /etc/default/kolibri under init
RESOLVED — build_tools/manage_python.py:172 — fail fast before network (praise)
RESOLVED — build_tools/manage_python.py — cmd_update downloads tarballs just for checksums
RESOLVED — .github/workflows/install_test.yml:16 — fail-fast: false (praise)
RESOLVED — .github/workflows/install_test.yml — "120 seconds" message mismatch
RESOLVED — debian/startup/kolibri.default:81 — ${PYTHONPATH:+…} (praise)
RESOLVED — build_tools/manage_python.py:1 — no tests
RESOLVED — build_tools/manage_python.py — startswith version match
RESOLVED — build_tools/build.sh — non-debian/ files break dpkg-source -b
RESOLVED — debian/kolibri-wrapper.sh — dual-sourcing leaks more than KOLIBRI_PYTHON
RESOLVED — debian/kolibri.postrm — /opt/kolibri/python untracked by dpkg
RESOLVED — debian/control:21 — python3 to Recommends allows no-Python installs
RESOLVED — debian/rules — both arch tarballs in Architecture: all
RESOLVED — debian/control — arm64/armhf/i386 lose apt install kolibri
RESOLVED — debian/install-python.sh:101 — partial extract wins resolution
RESOLVED — debian/rules — env file without tarball aborts postinst
RESOLVED — build_tools/manage_python.py:292 — interrupted downloads pin truncated hash
RESOLVED — .github/workflows/install_test.yml — hides 22.04/24.04 failures
RESOLVED — .github/workflows/install_test.yml — re-raise, unchanged
RESOLVED — .github/workflows/install_test.yml — every leg installs python3
RESOLVED — build_tools/manage_python.py:292 — re-raise, unchanged
RESOLVED — build_tools/manage_python.py — supported floor tracks bundled VERSION=
RESOLVED — build_tools/build.sh — ships every cpython-* tarball present
RESOLVED — build_tools/python_versions.env — pin stale
RESOLVED — debian/install-python.sh:43 — exit leaves stale $INSTALL_DIR preferred
RESOLVED — debian/install-python.sh:101 — re-raise, unchanged
RESOLVED — debian/install-python.sh:79 — checked before rm -rf (praise)
RESOLVED — debian/rules — re-raise, unchanged
RESOLVED — debian/kolibri-wrapper.sh:14 — subshell error handling under set -e
RESOLVED — debian/kolibri.scripts-common — after_postinst never checked
RESOLVED — .github/workflows/install_test.yml — fallback drops python3 silently
RESOLVED — build_tools/manage_python.py:211 — no draft/prerelease check
RESOLVED — build_tools/manage_python.py:70 — unvalidated upstream metadata sourced
RESOLVED — debian/kolibri-wrapper.sh — conffile keep-local skips resolution
RESOLVED — debian/kolibri-wrapper.sh — -m puts caller's CWD on sys.path
RESOLVED — debian/install-python.sh — root tar keeps upstream uid/gid
RESOLVED — debian/rules — make get-python in dist/ has no Makefile
RESOLVED — debian/control:22 — python3-cryptography ABI via dist-packages
RESOLVED — debian/install-python.sh:86 — re-extracts on every configure
RESOLVED — debian/startup/kolibri.default:81 — old dist-packages ahead of bundled stdlib
ACKNOWLEDGED — build_tools/python_versions.env:7 — 3.10 EOL / per_page=20 (maintainer deferred)
RESOLVED — build_tools/manage_python.py:247 — bundled Python not checked against floor
RESOLVED — debian/kolibri-wrapper.sh — bare python3 drops interpreter pin
RESOLVED — debian/kolibri.postinst:33 — interpreter replaced under running server
RESOLVED — build_tools/build.sh:165 — no CI coverage of source build
RESOLVED — Makefile:67 — get-tar deletes interpreter cache
RESOLVED — .github/workflows/install_test.yml:18 — two paths with no test leg
RESOLVED — tests/test_manage_python.py — cmd_download untested
RESOLVED — Makefile:14 — tarball glob ordering (praise)
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
6f19337 to
1d6380a
Compare
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #157: all 56 prior findings resolved. No new findings. CI passing.
Prior-finding status
RESOLVED — debian/install-python.sh — non-amd64/arm64 exit 1
RESOLVED — debian/control:22 — python3-pkg-resources dropped
RESOLVED — debian/startup/kolibri.default:70 — resolution chain (praise)
RESOLVED — build_tools/download_python.sh — checksum on cache hits (praise)
RESOLVED — debian/kolibri.scripts-common — after_postinst ordering (praise)
RESOLVED — debian/kolibri-wrapper.sh — init-script sourcing of /etc/default/kolibri
RESOLVED — build_tools/manage_python.py:172 — fail-fast check (praise)
RESOLVED — build_tools/manage_python.py — cmd_update downloads tarballs to hash
RESOLVED — .github/workflows/install_test.yml:16 — fail-fast: false (praise)
RESOLVED — .github/workflows/install_test.yml — "within 120 seconds" message
RESOLVED — debian/startup/kolibri.default:81 — ${PYTHONPATH:+:…} (praise)
RESOLVED — build_tools/manage_python.py:1 — no tests
RESOLVED — build_tools/manage_python.py — startswith version match
RESOLVED — build_tools/build.sh — non-debian/ files break dpkg-source -b
RESOLVED — debian/kolibri-wrapper.sh — dual-sourcing leaks service env
RESOLVED — debian/kolibri.postrm — untracked /opt/kolibri/python
RESOLVED — debian/control:21 — python3 to Recommends
RESOLVED — debian/rules — both arch tarballs in Architecture: all
RESOLVED — debian/control — arm64/armhf/i386 lose apt install kolibri
RESOLVED — debian/install-python.sh:101 — partial extract wins
RESOLVED — debian/rules — env file without tarball
RESOLVED — build_tools/manage_python.py:292 — truncated-download hash
RESOLVED — .github/workflows/install_test.yml — hides 22.04/24.04 failures
RESOLVED — .github/workflows/install_test.yml — re-raise
RESOLVED — .github/workflows/install_test.yml — no no-system-Python leg
RESOLVED — build_tools/manage_python.py:292 — re-raise
RESOLVED — build_tools/manage_python.py — floor tracks bundled minor
RESOLVED — build_tools/build.sh — ships every cpython-* tarball
RESOLVED — build_tools/python_versions.env — stale pin
RESOLVED — debian/install-python.sh:43 — exit leaves stale $INSTALL_DIR
RESOLVED — debian/install-python.sh:101 — re-raise
RESOLVED — debian/install-python.sh:79 — checksum before rm -rf (praise)
RESOLVED — debian/rules — re-raise
RESOLVED — debian/kolibri-wrapper.sh:14 — subshell set -e
RESOLVED — debian/kolibri.scripts-common — after_postinst never read
RESOLVED — .github/workflows/install_test.yml — fallback drops python3
RESOLVED — build_tools/manage_python.py:211 — no draft/prerelease check
RESOLVED — build_tools/manage_python.py:70 — unvalidated metadata sourced
RESOLVED — debian/kolibri-wrapper.sh — conffile keep-local-version
RESOLVED — debian/kolibri-wrapper.sh — -m puts CWD on sys.path
RESOLVED — debian/install-python.sh — root tar ownership/perms
RESOLVED — debian/rules — make get-python outside repo root
RESOLVED — debian/control:22 — python3-cryptography on PYTHONPATH
RESOLVED — debian/install-python.sh:86 — no version comparison
RESOLVED — debian/startup/kolibri.default:81 — dist-packages ahead of stdlib
RESOLVED — build_tools/python_versions.env:7 — 3.10 EOL
RESOLVED — build_tools/manage_python.py:247 — no floor check
RESOLVED — debian/kolibri-wrapper.sh — bare python3 drops pin
RESOLVED — debian/kolibri.postinst:33 — tree replaced under running server
RESOLVED — build_tools/build.sh:163 — unreached in CI
RESOLVED — Makefile:71 — get-tar deletes download cache
RESOLVED — .github/workflows/install_test.yml:18 — bespoke paths without a test leg
RESOLVED — tests/test_manage_python.py — cmd_download untested
RESOLVED — Makefile:15 — load-bearing sort (praise)
RESOLVED — .github/workflows/pr_build.yml:54 — changelog drops 0.20.0a1, build_source fails
RESOLVED — tests/install_smoke_test.sh — interpreter not asserted
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
bjester
left a comment
There was a problem hiding this comment.
No major concerns, just a couple comments. Seems tidied up and ready to go
| # python-build-standalone ships every entry group-writable (0664/0775), which | ||
| # root's default --same-permissions would carry into the service interpreter. | ||
| if ! tar -xzf "$TARBALL" -C "$STAGING_DIR" --strip-components=1 \ | ||
| --no-same-owner --no-same-permissions; then |
There was a problem hiding this comment.
With --no-same-permissions, have you double checked that the permissions on the extracted contents make sense? I think it would depend on the umask
There was a problem hiding this comment.
Yes, I think we can add a test to cover this scenario. Will add, and fix.
| check_server_interpreter() { | ||
| exe=$(su -s /bin/sh "$KOLIBRI_USER" -c 'readlink "/proc/$(head -1 ~/.kolibri/server.pid)/exe"' || true) | ||
| if [ -n "$EXPECT_BUNDLED" ]; then | ||
| expected="/opt/kolibri/python/bin/python3.*" |
There was a problem hiding this comment.
Should it check that the executable can actually run?
There was a problem hiding this comment.
The check of whether Kolibri serves implicitly does that - but good to add an execution test here to localize any failure point too.
Package now includes python-build-standalone tarballs for x86_64 and aarch64. At install time, if system Python is below 3.10, the matching tarball is extracted to /opt/kolibri/python/. - install-python.sh: install-time script to verify and extract Python - debian/rules: installs bundled artifacts to /opt/kolibri/bundled/ - build.sh: copies tarballs and config into source tree during build - postinst: calls install-python.sh during configure - postrm: cleans up /opt/kolibri/ on purge - control: move python3 to Recommends (>= 3.10) since we bundle our own Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
At service start, resolve the Python interpreter: 1. KOLIBRI_PYTHON from daemon.conf (user override) 2. /opt/kolibri/python/bin/python3 (package-installed) 3. system python3 (fallback) When using a non-system Python, the init script overrides KOLIBRI_COMMAND to use '$KOLIBRI_PYTHON -m kolibri' instead of the system 'kolibri' entry point. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Install smoke test on Ubuntu 16.04-24.04, no-system-python, arm64 and upgrade-from-0.19.5 legs; asserts which interpreter postinst chose, that dpkg-reconfigure stops the server before replacing its interpreter, and purge cleanup. - Build the source package on PRs and check the bundled tarballs survive the dpkg-source round-trip. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGjrFvYX54KSBVbqZ9KPEz
The sdist VERSION file spells prereleases PEP 440 style (0.20.0a1), tags do not (v0.20.0-alpha1): - filter_new_releases dropped the current prerelease, so the changelog stayed at the previous stable version and dpkg-source found no orig tarball for it. - The orig tarball was named from VERSION verbatim, not the Debian upstream version the changelog uses (0.20.0~alpha1). version_to_debian now accepts both spellings, and the Makefile writes dist/DEB_VERSION for the orig name, build.sh and prerelease.yml. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGjrFvYX54KSBVbqZ9KPEz
1d6380a to
b86af87
Compare
Summary
Bundles Python 3.10 from python-build-standalone in the .deb so Kolibri can run on Ubuntu 16.04–20.04 without depending on the system Python version.
Both x86_64 and aarch64 tarballs are included (~56 MB total). At install time, if system Python is already >= 3.10, the bundled tarballs are ignored.
Tested installation on Ubuntu 16.04 (Python 3.5) via distrobox — bundled Python 3.10.20 extracted and verified working, Kolibri service resolves to the correct interpreter, purge cleans up completely.
References
In support of learningequality/kolibri#15504 (Python availability only; kolibri-server merge is a separate effort)
Reviewer guidance
make get-pythonwhich downloads ~56 MB of Python tarballs. This runs automatically as a prerequisite ofmake kolibri.debandmake kolibri.changes.python3moved from Depends to Recommends (>= 3.10). The package no longer requires system Python to install.debian/startup/kolibri.init:44-47— when bundled Python is active,KOLIBRI_COMMANDis overridden to$KOLIBRI_PYTHON -m kolibri. Worth verifying this works correctly on a real system with systemd.debian/install-python.sh— the install-time extraction script. Check the arch detection and SHA256 verification logic.Open questions:
AI usage
Built collaboratively with Claude Code (Opus 4.6). Design brainstormed interactively, then implemented task-by-task. All code reviewed, tested via distrobox on Ubuntu 16.04, and commit history cleaned up manually.