Remove the vendored elftools tree and the unreachable GDB source-archive path - #249
Merged
Merged
Conversation
danielrmerskine
force-pushed
the
de/drop-vendored-elftools
branch
from
August 13, 2026 19:08
f730ef6 to
5895956
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #240.
What was wrong
cli/elftools/was vendored in without itsconstruct/lib/subpackage. The directory isabsent from the repo itself, so all 30 modules raised
ModuleNotFoundErrorin everyenvironment — pip installs and dev checkouts alike, not just the wheel:
Four separate things kept it invisible: the tree was excluded from ruff, omitted from
coverage, had no tests, and
compileallchecks syntax without resolving imports. Thepackaging gate's import walk was the first thing to actually try importing it.
Why deletion rather than repair
The issue offered three options. Investigating pushed the answer to its third one, harder than
expected:
cli/commands/development/debug/gdb.pyis the soleimporter, and
gdbappears in neitherlist_commandsnorget_commandof the debug group(
cli/commands/development/debug/commands.py:474-482). There is nolager debug gdbcommand and there has not been one. The only reference anywhere is a lazy re-export in
debug/__init__.py.pyelftools 0.27 with no local modifications (
grep -rni lager cli/elftools/is empty), andit has no DWARF5 line-program support —
dwarf/structs.pystill defines the DWARF≤4 fileentry form.
gdb.pycompounded it by hard-coding 1-indexed file/dir entries, which DWARF5made 0-indexed. GCC 11+ and clang 14+ emit DWARF5 by default.
pyelftoolsis not a declared dependency anywhere, and the boxside deliberately hand-rolls a minimal ELF32 reader specifically to avoid pulling it in
(
box/lager/debug/da1469x_loader.py:242).So this removes 48 files and 13,241 lines that could not run, under a command that could not
be invoked. If ELF/DWARF parsing is wanted later, the right move is
pyelftoolsfrom PyPI —recorded in
cli/vendor/__init__.pyso the next person doesn't re-vendor it.Also removed
The ruff
--excludeand the coverageOMITentry for*/elftools/*. Both existed only tohide this tree from static analysis; leaving them would let a future vendored tree inherit the
same blind spot silently.
Verified
python -m build→twine check(bothartifacts PASSED) → wheel installed into a fresh venv →
lager --version→ import walk.by the baseline and both belonging to cli.impl.box_config and cli.impl.power.enable_disable import lager.* from the box tree #241. It was 34 failures before. The two
cli.elftools.*and
cli.commands.development.debug.gdbbaseline entries are deleted here, and the baselineis two-sided, so a stale entry would have failed the gate.
gdb.py(147 entries total).unit (cli): 1179 passed, 2 xfailed — matches this branch'stest/COVERAGE.mdexactly, sothe deletion changed no test count.
tools/check_coverage_counts.pyagrees on every suiteexcept
unit (box), which reports 1556 (+7 skipped) here purely because 7 box tests aregated on
/procandflock(1)and this is macOS. Those tests are untouched by this changeand run on Linux CI; the darwin-awareness that reports them as
n/alanded in v0.36.2 and isnot on this branch yet.
ruff check --select E9,F63,F7,F82 --exclude cli/vendor cli/ box/ test/ tools/— all checkspassed with the narrowed exclusion.
Left alone deliberately
.github/workflows/packaging.yml's header comment still lists the vendored tree among thedefects the gate's first run found. That is a statement about that run, and it stays accurate.