Repository navigation
fix(ninja): the backend crashed on every target, and emitted an undefined rule - #101
Closed
Kartikey1306 wants to merge 2 commits into
Closed
Kartikey1306 wants to merge 2 commits into
Kartikey1306 wants to merge 2 commits into
Conversation
master does not parse. `ebuild` cannot start and pytest collects nothing:
$ python -c "import ebuild.cli.commands"
File "ebuild/cli/commands.py", line 855
"""ebuild — A unified embedded OS build system."""
^
SyntaxError: invalid character '—' (U+2014)
Line 855 is a normal docstring. The real damage is 300 lines earlier:
_board_config() lost its body and its closing quotes to a copy of
_resolve_backend_request()'s last line, spliced in during a conflict
resolution:
def _board_config() -> Optional[Dict[str, Any]]:
"""The project's own board description, if it ships one.
return resolved_backend, backend_config
Its docstring never closes, so every subsequent triple-quote toggles the
wrong way and the first em-dash the tokenizer reaches outside a string is
reported instead -- at line 855, in a function that is fine.
_resolve_backend_request itself still has its own return, so the line is
a duplicate rather than a move.
Restores the body from f5209ac, the last commit where this file parsed.
The same resolution also dropped the whole of embeddedos-org#89 -- _NO_TESTS_MARKERS,
_TEST_COUNT_PATTERNS, _ran_no_tests, _parse_test_counts and both call
sites -- leaving its tests behind and `test` back on:
log.success("All tests passed.")
reached unconditionally on a zero exit. ctest exits 0 when it finds
nothing to run, so a project with enable_testing() and no add_test()
reported a pass having executed nothing. Restored from the same commit.
Collection goes from "cannot import" to 474 passing. The 51 that fail are
pre-existing and were invisible while the module would not parse; they
are not all one cause and are listed in the PR description.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ined rule
Generating a build file fails outright on master:
NameError: name '_ninja_path' is not defined
_ninja_path was renamed escape_ninja_path, and three of its call sites
were not updated -- the executable edge, the static_library edge and the
shared_library edge. Between them that is every target type the backend
emits, so `configure`/`build` on the ninja backend produce no build.ninja
at all.
Those three calls were also double-escaping: `out`, `obj_files` and
`dep_archives` are each already escaped where they are built, so the
wrapper is not just undefined but redundant. Dropped rather than renamed.
With that fixed the generated file is still not usable for a
shared_library, because the edge names a rule the file never defines:
build build/libmylib.dylib: link_shared build/obj/mylib/lib.o
$ ninja
ninja: error: unknown rule 'link_shared'
Adds the rule, carrying the shared-object flag so no edge has to repeat
it, and _shared_flag() to pick it: -dynamiclib on macOS, -shared on ELF.
Emitting the ELF spelling on macOS is what the two tests in this area
were failing on.
End to end, a shared_library project now builds:
[1/2] CC build/obj/mylib/lib.o
[2/2] LINK_SHARED build/libmylib.dylib
$ file build/libmylib.dylib
Mach-O 64-bit dynamically linked shared library arm64
tests/unit/test_ninja_backend.py referenced the pre-rename name in five
assertions; updated to the exported one. 16/16 in the two ninja modules,
and the suite goes from 474 to 498 passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Kartikey1306
force-pushed
the
fix/ninja-path-rename-tests
branch
from
September 1, 2026 11:30
c26eb95 to
fcc97e6
Compare
Contributor
Author
Contributor
Author
|
Closing in favour of #102, which contains both of this branch's commits ( This PR cannot go green on its own.
The 27 that remain here are the same class the last three commits fix — implementations discarded in conflict resolution while their tests landed. Adding them to this branch would just reproduce #102. The ninja work in |
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.
Generating a build file fails outright
_ninja_pathwas renamedescape_ninja_path, and three call sites were not updated — the executable edge (line 223), the static_library edge (241) and the shared_library edge (257). Between them that is every target type the backend emits, soconfigure/buildon the ninja backend produce nobuild.ninjaat all.Those three were also double-escaping:
out,obj_filesanddep_archivesare each already escaped where they are built (lines 191, 217, 221, 238). So the wrapper is not merely undefined, it is redundant — dropped rather than renamed.Even fixed, a shared_library was unbuildable
The edge names a rule the generated file never defines. Only
cc,linkandar_ruleare emitted:The comment at line 246 already assumed the rule existed ("the flag itself lives in the
link_sharedrule, so it must not be repeated here") — the rule was simply never written.This adds it, carrying the shared-object flag so no edge repeats it, plus
_shared_flag()to choose it:-dynamiclibon macOS,-sharedon ELF. Emitting the ELF spelling on macOS is what the two tests in this area were failing on; both name_shared_flag()in their comments.End to end
On master the same project never gets as far as a build file.
Tests
tests/unit/test_ninja_backend.pyused the pre-rename name in five assertions (its import line already had the new one); updated.