Install as a kconfiglib package, not loose modules - #55
Merged
Merged
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Installing dropped fifteen unprefixed modules straight into site- packages: kconfiglib.py next to menuconfig.py, rawterm.py, and a defconfig.py and setconfig.py that any other distribution is free to want for itself. A port maintainer packaging this for FreeBSD hit it. The sources stay flat in the checkout, so running python menuconfig.py from a clone keeps working and the README links still point at files that exist. build_py maps them into the package at build time instead. Globbing the repository root would have swept up setup.py and lint.py, so the module list is explicit. Editable installs default to strict mode, because the lenient one maps the package name at the root where there is no __init__.py to find, but an explicit editable_mode is left alone. This breaks importing menuconfig from an installed Kconfiglib, so the version goes to 15.0.0. Importing it from the kconfiglib package replaces that. Nothing changes for code that only imports kconfiglib itself. One wart comes with keeping the layout: the root kconfiglib.py shadows the installed package whenever the checkout is the working directory, which the README now says and the CI check works around by testing from elsewhere. The sdist also grows the whole tests tree. It shipped the test modules but not conftest.py, the helpers or the Kconfig fixtures, so the suite could not run from the tarball at all, which is what a packager building from it needs.
menuconfig.py and guiconfig.py share 38 function names, and 17 of those had byte-identical bodies: the whole expression-formatting chain that renders every expression either tool prints, plus the range, include path and choice symbol blurbs from the info dialog. Two copies means a fix applied to one silently leaves the other wrong, and the 21 names that are not identical show that has already been happening. Eleven are pure functions of a MenuNode or an expression, with no terminal and no Tk in them, so they move to uicommon.py unchanged. The remaining six read module globals belonging to one tool or the other and stay put for now. Both tools import the module rather than aliasing each name, so adding a helper is a one-place edit and the call site says where the answer comes from. Together the two shrink from 6653 lines to 6334. Putting them in kconfiglib.py instead would have avoided the import dance that guiconfig.py now needs, but the core library is already the densest file here and these are presentation helpers, not configuration ones. rawterm.py set the precedent for extracting a module the interfaces share. Three tests were parametrized over both tools to check the two agreed, which is a tautology once there is one copy, and the guiconfig half dragged in tkinter to re-run the same function. They call uicommon directly now, as do two of the range tests. The checks that validate an entered value keep their pair, being still a real copy in each tool.
Twenty module globals, assigned through 64 global statements scattered across the file, with the selected index and the scroll offset written from a dozen places each. Nothing could be called before the main loop had set them up, so the cursor and scrolling logic had no tests at all: reaching it meant starting a terminal. That is most of why menuconfig.py sits at 11 percent coverage while the library it drives is at 75. They become fields on a _State that the entry point builds fresh per run. The rewrite was driven off the syntax tree rather than the text, after checking that no function binds any of the twenty names locally, so every one of the 314 references was unambiguously the global it looked like. Behavior is unchanged: the row renderer, the shown-node walk and the info dialog over five test Kconfigs in four display modes produce output identical to before, byte for byte. Building it fresh also fixes something. The old globals survived between runs, so a second run in one process inherited the first one's scroll position, dialog state and show-all mode. The search caches needed an explicit clear at entry for the same reason, so that goes away. tests/test_uinav.py is what this was for. It builds a _State, hands it a window with a height, and drives entering and leaving menus, the four selection commands and the scroll offset directly. One case pins the invariant the whole offset dance exists for, that the selection never leaves the window; another pins the clamp that catches a terminal shrinking while the user is inside a submenu. Building the fixture's node list has to come after installing the state, not before, because the walk reads show-all off the module rather than off anything passed in. Getting that backwards silently dropped the promptless node the fixture exists to show.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
A help, prompt, type, def_bool, transitional, option or visible if written under a menu or a comment escaped as an AttributeError traceback instead of a parse error. Menu and comment nodes carry a plain constant as their item rather than a symbol or a choice, and they leave the help and visibility slots unset, so the handlers reached for attributes that were not there. A typo in a Kconfig file produced a Python stack trace with no file or line in it. Alongside those, the modules property raised TypeError on any symbol that is not MODULES, because its warning passed a filename and a line number to a helper that takes a single location. Which node kinds may carry which property is a table now, checked once at the top of the property loop, rather than nine hand-written kind tests scattered through the dispatch. The table is complete because it is a table: the chain only ever covered the cases somebody remembered. It also reuses the symbol-or-choice frozenset that already existed. Visibility stays a separate check, since menu and comment items are both plain constants and telling them apart is an identity test, not a class one. Nothing that parsed before parses differently. Every input newly rejected raised AttributeError or TypeError before, which is not acceptance. tests/test_fuzz.py found all of them. Generating token soup was tried first and is useless here, because every case dies on line one and the block and property parsers are never reached. So it builds a structurally valid Kconfig tree and then corrupts it, which puts about half of each run past the end of the parser. The contract it asserts is the only one worth asserting on random input: the parser either accepts the file or raises its own error, never anything else. A blown Python stack is not acceptance either, so it is only forgiven for the one shape known to cause it, where a config listed by two separate choice blocks sends visibility evaluation around a cycle. That defect reproduces on the base branch and is left for its own change. Matching the shape rather than a seed number means changing the generator cannot quietly widen the exemption.
scripts/benchmark.py has been sitting in the tree unused. The collector and probe cache work that went in recently came with specific numbers attached, twenty small parses dropping from 0.81s to 0.01s among them, and nothing was watching whether they held. The selftest job now runs it and keeps the JSON as an artifact, so a later regression in the parse or redraw paths can be attributed instead of merely noticed. Once per job, not twice: asking for JSON returns before the table is printed, so a second run would re-measure everything and the table in the log would not be the numbers in the artifact. Still no pass/fail threshold, and the numbers want reading with care. Phases on this fixture move by around 7 percent run to run on an idle machine and around 30 on a busy one, and the spread is cross-process fixed cost rather than the timing loop, so a shared runner is the busy case. That attributes a large regression and nothing subtler. Pointing it at a tree big enough for the phases to run in milliseconds would tighten it. The coverage floor goes from 35 to 38 against a total that is now 41, and uicommon.py joins the measured set. The comment explaining how to reproduce the runner figure locally stays, because it is easy to take a number from a plain local run and set a floor that fails every job here.
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
1 participant
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.
Installing Kconfiglib dropped fifteen unprefixed modules straight into site-packages, so a shared site-packages ended up holding this project's
defconfig.pyandsetconfig.pyunder names any other distribution is free to want for itself. A FreeBSD port maintainer packaging this hit it and filed #53. The sources stay flat in the checkout, sopython menuconfig.pykeeps working from a clone and the README links still point at files that exist;build_pymaps them into akconfiglibpackage at build time instead. That breaksimport menuconfigagainst an installed copy, so this goes out as 15.0.0, withfrom kconfiglib import menuconfigreplacing it. Nothing changes for code that only importskconfiglibitself.Four adjacent changes ride along. The presentation helpers that
menuconfig.pyandguiconfig.pyheld byte-identical copies of move intouicommon.py. The terminal interface's twenty module globals become one_Statebuilt fresh per run, which is what finally made the cursor and scrolling logic reachable from a test, and which also stops a second run in one process inheriting the first one's scroll position. Ahelp,prompt, type orvisible ifwritten under amenuorcommentused to escape as anAttributeErrortraceback rather than a parse error, so a typo produced a Python stack trace with no file or line in it; five properties were affected and they share one guard now. The benchmark script that had been sitting in the tree unused now runs in CI.Verified on Python 3.14 on macOS: 470 tests pass, up from 306. Each commit was exported with
git archiveand tested standalone, so the series bisects. The two refactors are behavior-identical rather than merely test-passing, with_node_str,_shown_nodesand_info_strover five test Kconfigs in four display modes producing output byte-identical to the base branch. The parser guards came out of the new fuzz tests, which found all five crashes. One further crash turned up that is not this branch's: aconfiglisted by two separatechoiceblocks sends visibility evaluation around a cycle until the Python stack runs out, which reproduces unchanged onmain, so it is recorded with a minimized reproducer and left for a separate change rather than fixed here. The wheel builds with 16 package members and no loose files, installs into a clean venv, and all 13 console scripts resolve.Left out deliberately: the kernel conformance suite, which needs a Linux tree and the C kconfig tools and is covered by its own CI lane, and
guiconfig.py's own module globals, which want the same treatment asmenuconfig.py's and are now tracked separately.Closes #53