fix(build): unbreak master — SUPERSEDED BY #66 - #70
Conversation
… project
`master` cannot run. Not a stale badge — reproduced from a clean checkout:
$ ebuild new hi && cd hi && ebuild build
File ".../ebuild/build/dispatch.py", line 133
else:
^^^^
SyntaxError: invalid syntax
$ pytest
2 errors during collection
Four defects, each found by running the tool rather than reading it. Every one
of them reached master because the file it lives in is imported lazily, so
nothing on the merge path executed it.
1. `dispatch.py` had two consecutive `else:` blocks in `configure()` — one
raising ValueError, one RuntimeError. Python does not parse a module until
something imports it, so this sat on master reachable only by the command
that touches it, and it also made `tests/unit` uncollectable. The
RuntimeError arm is kept: `tests/unit/test_dispatch.py` documents the
intent, which is that an unhandled backend fails loudly rather than
silently doing nothing and letting the caller report a false success.
2. `configure()` then listed "ninja" among the backends that need no configure
step, so `configure("ninja")` silently did nothing — precisely the
regression that test guards against. ebuild's own ninja backend is
generated and invoked by the CLI, never dispatched here, so arriving with
it is a routing mistake and is now reported.
3. `NinjaBackend._object_path` was called from two places and defined in
neither: AttributeError on every ninja build, immediately after the
SyntaxError was cleared. Objects are namespaced by target name, because a
source shared by two targets must produce two distinct objects — ninja
rejects two edges writing one output, and the targets may use different
cflags. The path is flattened rather than mirrored so that a source from
outside the project cannot place its object outside the build directory,
where `clean` would not find it.
4. ninja was invoked as `python -m ninja`, which only works with the PyPI
wheel installed, so a machine with a real ninja on PATH failed with
"No module named ninja". `ninja_command()` prefers the executable.
Also, `build()` and `clean()` raised ValueError for the same condition
`configure()` reports as RuntimeError, so no single `except` guarded the
dispatcher. All three now raise RuntimeError, and the two test files that
disagreed about which to expect agree.
On shared libraries: the `link_shared` rule was declared and never used —
edges went through the generic `link` rule with -shared pushed into ldflags.
That works, but leaves the rule dead and broke
`test_shared_library_uses_shared_link_rule`. Edges use `link_shared` now, and
the platform's flag lives in the rule, so darwin's -dynamiclib is decided in
one place instead of at the call site. `test_static_library_unaffected`
asserted "-shared" was absent from the whole file, which cannot hold while the
rule preamble declares it; it checks build edges instead.
before pytest: 2 errors during collection — suite cannot run
after 202 passed
`ebuild build` now reaches the compiler and reports a real compile error
instead of a Python traceback. The remaining first-run gap — templates
including <eos/hal.h> with no path to it — is #64's scope, not this fix's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Superseded by #66 — take that one insteadI wrote this before seeing #66. Both repair the same Verified on Comparing the two, #66 covers three of the four defects I found. The one it does not: ebuild/build/dispatch.py:244 [sys.executable, "-m", "ninja", "-C", ...]
ebuild/cli/commands.py:690 [sys.executable, "-m", "ninja", "-f", ...]
I have noted that on #66 as a follow-up. It is three lines and does not belong in a hotfix: def ninja_command() -> list:
exe = shutil.which("ninja")
return [exe] if exe else [sys.executable, "-m", "ninja"]Leaving this open rather than closing it, since that is your call — but nothing here needs to merge, and merging both would conflict. The three feature PRs that were branched on top of this one (#71 footprint, #72 doctor, #74 add/summary) are rebased onto plain |
srpatcha
changed the title
|
Superseded by #66, which has merged. |
1 participant
P0.
mastercannot run. Reproduced from a clean checkout:Four defects. Every one reached master because the file it lives in is imported lazily, so nothing on the merge path ever executed it.
else:blocks inconfigure()tests/unituncollectable"ninja"listed among backends needing no configureconfigure("ninja")silently succeeds — the exact regressiontest_dispatch.pyguards againstNinjaBackend._object_pathcalled twice, defined nowhereAttributeErroron every ninja buildpython -m ninjaKept the RuntimeError arm of #1 because
tests/unit/test_dispatch.pydocuments the intent — an unhandled backend must fail loudly rather than do nothing and let the caller report a false "Build completed successfully".Also unified the exception type:
build()andclean()raisedValueErrorfor the same conditionconfigure()reports asRuntimeError, so no singleexceptguarded the dispatcher.Shared libraries: the
link_sharedrule was declared and never used — edges went through the genericlinkrule with-sharedpushed into ldflags. Functional, but it left the rule dead and broketest_shared_library_uses_shared_link_rule. Edges uselink_sharednow, with the platform flag in the rule so darwin's-dynamiclibis decided in one place.Verification
pytestebuild buildebuild buildnow reports a real compile error instead of a traceback. The remaining first-run gap — templates including<eos/hal.h>with no path — is #64's scope, not this fix's.🤖 Generated with Claude Code