Skip to content

fix(ebuild): use ninja_command() in ebuild build - #160

Open
agayushh wants to merge 1 commit into
embeddedos-org:masterfrom
agayushh:fix/build-uses-ninja-command
Open

agayushh wants to merge 1 commit into
embeddedos-org:masterfrom
agayushh:fix/build-uses-ninja-command

Conversation

@agayushh

Copy link
Copy Markdown

ebuild test already preferred a PATH ninja binary and fell back to python -m ninja. ebuild build still hardcoded the module form, so a system ninja install was not enough for the main command.

Fixes #159

Summary

Wire ebuild build to ninja_command() (same helper ebuild test already
uses) and update README.md / demo.md, which still said the pip ninja
package was required because ebuild always invoked python -m ninja.

Type of Change

  • feat - New feature
  • fix - Bug fix
  • docs - Documentation only
  • style - Formatting, no code change
  • refactor - Code restructuring without behavior change
  • test - Add or fix tests
  • build - Build system or dependency changes
  • ci - CI/CD pipeline changes
  • perf - Performance improvement

Changes

  • ebuild/cli/commands.py: ebuild build now calls ninja_command() instead of [sys.executable, "-m", "ninja", ...]
  • README.md, demo.md: document PATH ninja first, pip module as fallback
  • CHANGELOG.md: record the fix
  • tests/unit/test_ninja_command.py: unit tests for the helper and for the ebuild build argv

Testing

  • Unit tests pass (ctest --test-dir build --output-on-failure)
  • Integration tests pass
  • Manual testing performed
  • New tests added for new functionality

Pre-Submission Checklist

  • Code compiles without warnings (-Wall -Wextra -Werror for C)
  • All existing tests pass
  • New tests added for new functionality
  • Documentation updated if API changed
  • Commit messages follow the <type>(<scope>): <description> convention
  • Branch is rebased on latest master
  • This PR links a real issue in this repository with a closing keyword

Related Issue

Fixes #159

Screenshots / Logs

python -m pytest tests/unit/test_ninja_command.py -v --tb=short
4 passed in 0.08s

python -m pytest tests/unit/test_dispatch.py tests/unit/test_doctor.py tests/unit/test_documented_commands_exist.py tests/ebuild/test_build_cli.py tests/unit/test_build_failure_output.py -v --tb=short
33 passed in 0.20s

python -m ruff check ebuild/cli/commands.py tests/unit/test_ninja_command.py
All checks passed

cd examples/hello_world
ebuild build
./_build/hello
Hello from EoS Build System!

Additional Notes

I did not rewrite commands.py line endings. That file is already CRLF in the repository; git diff --check flags \r on newly added lines for that reason.

ebuild test already preferred a PATH ninja binary and fell back to
python -m ninja. ebuild build still hardcoded the module form, so a
system ninja install was not enough for the main command.

Fixes embeddedos-org#159

Signed-off-by: agayushh <[email protected]>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: ebuild build ignores ninja_command() and still requires the pip ninja module

1 participant