fix(build): unbreak master — ebuild build raised a traceback on every project - #70
Open
srpatcha wants to merge 1 commit into
Open
fix(build): unbreak master — ebuild build raised a traceback on every project#70srpatcha wants to merge 1 commit into
srpatcha wants to merge 1 commit into
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>
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.
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