Skip to content

fix: prevent false success for unsupported system backend - #65

Open
muhammadburhandevv-hub wants to merge 1 commit into
embeddedos-org:masterfrom
muhammadburhandevv-hub:fix/system-backend-false-success
Open

fix: prevent false success for unsupported system backend#65
muhammadburhandevv-hub wants to merge 1 commit into
embeddedos-org:masterfrom
muhammadburhandevv-hub:fix/system-backend-false-success

Conversation

@muhammadburhandevv-hub

Copy link
Copy Markdown

Summary

Fixes an issue where eBuild could report a successful build even though no
supported build backend ran. A system: section is now kept separate from
backend configuration, and unsupported backends fail with a clear error.

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

  • Keep system: settings separate from compilation backend configuration.
  • Preserve explicit backend selection and normal backend auto-detection.
  • Raise BackendError when the external dispatcher cannot handle a backend.
  • Add configuration, dispatcher, and CLI regression tests.
  • Reconcile the fix with the latest upstream dispatcher changes.

Testing

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

Focused Python regression tests:

47 passed

Additional validation:

  • Python compilation passed.
  • git diff --check passed.
  • Full pytest suite: 198 passed, 11 failed, 1 skipped.

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 <type>(<scope>): <description> convention
  • Branch is rebased on latest master

Related Issues

No related issue number.

Screenshots / Logs

============================= 47 passed in 0.33s =============================

Additional Notes

  • This change does not implement a new system compilation backend.
  • The CTest command was not run because this contribution changes the Python
    eBuild implementation and its pytest suite.
  • The full pytest failures are outside this change. Three require cpio, which
    is unavailable in the Windows test environment. The remaining failures come
    from an existing missing NinjaBackend._object_path method.

Signed-off-by: muhammadburhandevv-hub <muhammadburhandevv@gmail.com>
@srpatcha

Copy link
Copy Markdown
Member

Same defect as #66, reached from a different direction — needs a rebase

The problem you are fixing is real and it is the same one #66 addresses: BackendDispatcher accepting a backend it does not implement and letting the caller report "Build completed successfully" when nothing ran.

Both PRs edit ebuild/build/dispatch.py and they conflict with each other. #66 is further along — it also repairs three defects that stop master running at all:

$ ebuild new hi && cd hi && ebuild build
  File ".../ebuild/build/dispatch.py", line 133
    else:
    ^^^^
SyntaxError: invalid syntax

$ pytest
2 errors during collection

So I have approved #66 to land first. Verified locally: this branch on current master is 9 failed / 201 passed, and it conflicts with #66 in dispatch.py.

What is yours alone, and worth keeping

Two things in here are not in #66, and I would not want them lost in the rebase:

Suggested rebase

Once #66 lands, drop your dispatch.py hunks in favour of its version — it raises RuntimeError from configure(), build() and clean() uniformly, and removes "ninja" from the list of backends that need no configure step, which is the specific hole that let configure("ninja") succeed silently. Then keep the core/config.py change and the CLI test on top. That should be a small branch and I will merge it.

One question worth settling while you are in there: #66 standardised on RuntimeError and this PR raises BackendError. A dedicated exception type is arguably better, but the tree currently has two test files that already disagree about which type to expect, so whichever wins should win everywhere rather than adding a third. If you would rather introduce BackendError properly, that is fine by me — just make configure, build and clean all raise it and update both tests/ebuild/test_dispatch.py and tests/unit/test_dispatch.py.

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.

2 participants