From 75d9249cebf639b986ec9032576cef70c515fdbd Mon Sep 17 00:00:00 2001 From: S Date: Fri, 11 Sep 2026 00:41:06 +0530 Subject: [PATCH 1/2] fix(ebuild): preserve toolchain linker flags for shared libraries Reuse toolchain linker flags and sysroot when linking shared libraries, matching executable targets. Add manifest and real CLI regression tests. Implemented and tested with assistance from OpenAI Codex. Signed-off-by: Siya Gupta --- CHANGELOG.md | 3 + TASKS.md | 56 +++++++++++++ ebuild/build/ninja_backend.py | 2 +- tests/unit/test_shared_library_toolchain.py | 88 +++++++++++++++++++++ 4 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_shared_library_toolchain.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ba0175..a4f48a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,9 @@ `cjson` (v1.7.18), `nanopb` (v0.4.9.1), `lvgl` (v9.2.2), `tinyusb` (v0.18.0), and `unity` (v2.6.1). ### Fixed +- Shared-library Ninja link commands now preserve toolchain `extra_ldflags` + and `sysroot`, before target linker flags and package library paths, matching + executable targets. Previously those toolchain settings were silently ignored. - **`ebuild test` now finds Windows test binaries.** The Ninja edge for a native `type: test` target already carried the platform suffix (`_exe_suffix()` names it `.exe` on Windows), but `ebuild test` diff --git a/TASKS.md b/TASKS.md index 09f6bcb..a363787 100644 --- a/TASKS.md +++ b/TASKS.md @@ -9,6 +9,62 @@ Status is one of: `todo`, `in-progress`, `blocked`, `review`, `done`. ## Active +### T-119 — Preserve toolchain linker settings for shared libraries + +Owner: backend / testing +Mode: Verification +Status: review +Depends on: none + +Goal: shared-library link commands honor the same toolchain settings as executables. + +Acceptance criteria: +- Shared-library link flags include toolchain ldflags and sysroot before target ldflags and package library paths. +- Generating multiple shared libraries does not mutate or leak flags between targets or into the toolchain. +- A real Linux shared-library build honors toolchain `-Wl,--no-undefined`: a resolved source builds successfully and an unresolved source fails to link. + +Design: reuse the already-computed `_get_toolchain_ldflags()` result with list +concatenation, matching executable handling. No API or dependency changes. +Files in scope: `ebuild/build/ninja_backend.py`, +`tests/unit/test_shared_library_toolchain.py`, `CHANGELOG.md`, `TASKS.md`. +Other shared-library features and unrelated defects are out of scope. +Risk: previously ignored linker options may now correctly reject invalid builds. +Verification: new tests before/after the fix, full pytest suite, Ruff, mypy, +Python package build, and the repository's CMake/CTest check. + +Handoff (planning/architecture -> implementation/testing): discovery found the +shared-library branch initializes flags from the target alone. The executable +branch already includes toolchain flags. Acceptance criteria are the three +bullets above; all checks are NOT RUN. Next: write failing regressions, then fix. + +Verification results (Linux, Python 3.12): +- PASS: all three new regression cases failed against the original backend, + then passed with the fix (`pytest tests/unit/test_shared_library_toolchain.py -q`). +- PASS: independent reviewer repeated the focused tests (3 passed), found no + blocking issues, and confirmed all three acceptance criteria. +- PASS: Ruff on both changed Python files; mypy on `ninja_backend.py`; + `python -m build --no-isolation` (sdist and wheel); `git diff --check`; + `yamllint .` (no YAML changes). +- FAIL (pre-existing): full suite: `9 failed, 672 passed, 3 skipped in 51.88s`. + All nine failures are in `tests/unit/test_index_sync.py`: + `AttributeError: 'PackageRecipe' object has no attribute 'to_dict'`. + Reproduced the same nine failures in a detached baseline worktree at + `8b623d5` (9 failed, 13 passed in that module). +- FAIL (pre-existing): repository-wide mypy reports the same missing + `PackageRecipe.to_dict` at `ebuild/packages/index_sync.py:354`. +- FAIL (pre-existing): repository-wide Ruff reports F811 in + `tests/ebuild/test_build_dir_resolution.py`, W292 in + `tests/ebuild/test_package_recipe.py`, and two E402 findings in + `tests/unit/test_ci_gate.py`. These files are unchanged. +- NOT RUN: CMake/CTest; `cmake: command not found` in this environment. +- NOT RUN: real Windows/macOS linker execution and cross-compilation with an SDK. + Portable manifest tests cover sysroot emission; the compiler regression is + explicitly Linux/GCC-only. + +Handoff (verification -> maintainer): focused change independently reviewed; +submission is for review, not a release. Remaining work: maintainer review/CI, +with the unrelated baseline failures above tracked here for separate fixes. + | ID | Task | Owner | Mode | Status | Depends on | |----|------|-------|------|--------|------------| | T-002 | Fix Windows Ninja test-target path parsing | backend | Maintenance | review | none | diff --git a/ebuild/build/ninja_backend.py b/ebuild/build/ninja_backend.py index de417bd..72c5482 100644 --- a/ebuild/build/ninja_backend.py +++ b/ebuild/build/ninja_backend.py @@ -280,7 +280,7 @@ def _write_ninja(self) -> None: # get, which the rule preamble alone does not supply. The # "build a shared object" flag itself lives in the # link_shared rule, so it must not be repeated here. - ldflags = list(target.ldflags) + ldflags = toolchain_ldflags + list(target.ldflags) libs = [] for pkg_name in target.uses: pkg = self.package_paths.get(pkg_name) diff --git a/tests/unit/test_shared_library_toolchain.py b/tests/unit/test_shared_library_toolchain.py new file mode 100644 index 0000000..69da3fc --- /dev/null +++ b/tests/unit/test_shared_library_toolchain.py @@ -0,0 +1,88 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""Regression tests for toolchain settings in shared-library link commands.""" + +import shutil +import sys +from pathlib import Path + +import pytest +from click.testing import CliRunner + +from ebuild.build.ninja_backend import NinjaBackend, PackagePaths +from ebuild.build.toolchain import ResolvedToolchain +from ebuild.cli.commands import cli +from ebuild.core.config import ProjectConfig, TargetConfig + + +@pytest.mark.parametrize("sysroot", [None, "/opt/target-sysroot"]) +def test_shared_link_preserves_toolchain_flags_without_mutation(tmp_path, sysroot): + toolchain = ResolvedToolchain(ldflags=["-Wl,--no-undefined"], sysroot=sysroot) + targets = [ + TargetConfig( + name=name, + target_type="shared_library", + sources=[f"{name}.c"], + ldflags=[f"-Wl,-soname,lib{name}.so"], + uses=[name], + ) + for name in ("first", "second") + ] + packages = { + name: PackagePaths(lib_dirs=[Path(f"vendor/{name}")], libraries=[name]) + for name in ("first", "second") + } + config = ProjectConfig(name="libs", version="1.0", targets=targets) + NinjaBackend(config, tmp_path, toolchain, packages).generate() + manifest = (tmp_path / "build.ninja").read_text(encoding="utf-8") + + for target in targets: + edge = next( + block for block in manifest.split("\n\n") + if ": link_shared " in block and f"lib{target.name}." in block + ) + expected = ["-Wl,--no-undefined"] + if sysroot: + expected.append(f"--sysroot={sysroot}") + expected += target.ldflags + [f"-L{Path('vendor') / target.name}"] + assert edge.splitlines()[1:] == [ + f" ldflags = {' '.join(expected)}", + f" libs = -l{target.name}", + ] + assert target.ldflags == [f"-Wl,-soname,lib{target.name}.so"] + assert toolchain.ldflags == ["-Wl,--no-undefined"] + + +@pytest.mark.skipif( + not sys.platform.startswith("linux") or shutil.which("gcc") is None, + reason="requires Linux and GCC for --no-undefined shared-library linking", +) +def test_cli_shared_library_honors_toolchain_no_undefined(tmp_path, monkeypatch): + pytest.importorskip("ninja", reason="ninja Python package not installed") + monkeypatch.chdir(tmp_path) + (tmp_path / "build.yaml").write_text( + "project:\n name: shared-link-check\n version: '1.0'\n" + "toolchain:\n compiler: gcc\n" + " extra_ldflags: ['-Wl,--no-undefined']\n" + "targets:\n - name: example\n type: shared_library\n" + " sources: [example.c]\n cflags: [-fPIC]\n", + encoding="utf-8", + ) + source = tmp_path / "example.c" + source.write_text("int example(void) { return 42; }\n", encoding="utf-8") + runner = CliRunner() + good = runner.invoke(cli, ["build", "--backend", "ninja", "--build-dir", "good"]) + assert good.exit_code == 0, good.output + assert (tmp_path / "good" / "libexample.so").is_file() + + source.write_text( + "extern int missing_symbol(void);\n" + "int example(void) { return missing_symbol(); }\n", + encoding="utf-8", + ) + # A separate output directory avoids timestamp-based incremental decisions. + bad = runner.invoke(cli, ["build", "--backend", "ninja", "--build-dir", "bad"]) + assert bad.exit_code != 0, bad.output + assert "undefined reference" in bad.output, bad.output + assert "missing_symbol" in bad.output, bad.output From ed3a2eed4ff2d614f042db532700df1c217b5d4b Mon Sep 17 00:00:00 2001 From: S Date: Mon, 14 Sep 2026 12:44:43 +0530 Subject: [PATCH 2/2] docs(ebuild): condense shared-library task ledger entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the verbose T-119 block with a T-006 row in the Active table, following the review of PR #125. Code and tests are unchanged. Historical verification from the removed block is preserved below because the GitHub integration cannot update the upstream PR description (HTTP 403). ## Summary Fix shared-library Ninja link commands silently dropping toolchain `extra_ldflags` and the link-time `sysroot`. Reuse the already-computed toolchain flags, matching executable/test targets. ## Type of Change - [x] fix — Bug fix - [x] test — Add regression tests ## Changes - Include toolchain linker flags and sysroot before target flags and package library paths, without mutating flag lists. - Add two test functions producing three cases: manifest checks with/without sysroot across two distinct targets, and a Linux/GCC CLI regression proving `-Wl,--no-undefined` reaches the linker. - Document the behavior in CHANGELOG.md. - Reduce the task ledger entry to one row, as requested in review. Keep the detailed verification here. ## Testing Recorded on Linux/Python 3.12 for code commit `75d9249`; the follow-up is documentation-only. These are local results, not GitHub CI results. | Check | Command / evidence | Result | |---|---|---| | New regression tests | `python -m pytest tests/unit/test_shared_library_toolchain.py -q` | PASS — 3 failed before the fix; 3 passed after | | Independent focused review | Same focused pytest command | PASS — 3 passed; all acceptance criteria met | | Changed-file lint | `ruff check ebuild/build/ninja_backend.py tests/unit/test_shared_library_toolchain.py` | PASS | | Backend type check | `mypy ebuild/build/ninja_backend.py --ignore-missing-imports --no-strict-optional` | PASS | | Python package | `python -m build --no-isolation` | PASS — sdist and wheel built | | Whitespace | `git diff --check` | PASS | | YAML lint | `yamllint .` | PASS — no YAML changes | | Full Python suite | `python -m pytest tests/ -q` | FAIL (pre-existing) — 9 failed, 672 passed, 3 skipped | | Repository-wide mypy | `mypy . --ignore-missing-imports --no-strict-optional --exclude '^(layers\|core\|promo)/'` | FAIL (pre-existing) — missing `PackageRecipe.to_dict` at `ebuild/packages/index_sync.py:354` | | Repository-wide Ruff | `ruff check .` | FAIL (pre-existing) — four findings in unchanged test files | | CMake/CTest | CMake unavailable in the local environment | NOT RUN | | Windows/macOS linking; cross-compilation with an SDK | Not exercised locally | NOT RUN | | GitHub CI | No checks reported at the time of the review follow-up | UNKNOWN | All nine full-suite failures are in `tests/unit/test_index_sync.py`, caused by `AttributeError: 'PackageRecipe' object has no attribute 'to_dict'`. The same nine failures were reproduced in a detached baseline worktree at `8b623d5` (9 failed, 13 passed for that module). The four baseline Ruff findings are F811 in `tests/ebuild/test_build_dir_resolution.py`, W292 in `tests/ebuild/test_package_recipe.py`, and two E402 findings in `tests/unit/test_ci_gate.py`. ## Pre-Submission Checklist - [x] Regression tests added and observed failing before the fix - [x] Focused tests and changed-file checks pass - [x] Changelog updated - [x] Commit includes DCO sign-off - [ ] All existing tests pass — baseline failures documented above - [ ] GitHub CI passes — no results reported ## Related Issues No matching bug issue found. Creating the requested upstream tracking issue through the connected GitHub integration returned HTTP 403, "Resource not accessible by integration". A tracking issue still needs to be created and linked; no unrelated issue is claimed as fixed. ## Additional Notes The compiler regression explicitly requires Linux/GCC; portable manifest tests cover sysroot emission and flag isolation. A real cross-compilation sysroot has not been tested. Retain explicit `-fPIC` in the compiler fixture while #131 remains unmerged; its removal was optional in review. Implemented and tested with assistance from OpenAI Codex. Signed-off-by: Siya Gupta --- TASKS.md | 57 +------------------------------------------------------- 1 file changed, 1 insertion(+), 56 deletions(-) diff --git a/TASKS.md b/TASKS.md index a363787..a681074 100644 --- a/TASKS.md +++ b/TASKS.md @@ -9,64 +9,9 @@ Status is one of: `todo`, `in-progress`, `blocked`, `review`, `done`. ## Active -### T-119 — Preserve toolchain linker settings for shared libraries - -Owner: backend / testing -Mode: Verification -Status: review -Depends on: none - -Goal: shared-library link commands honor the same toolchain settings as executables. - -Acceptance criteria: -- Shared-library link flags include toolchain ldflags and sysroot before target ldflags and package library paths. -- Generating multiple shared libraries does not mutate or leak flags between targets or into the toolchain. -- A real Linux shared-library build honors toolchain `-Wl,--no-undefined`: a resolved source builds successfully and an unresolved source fails to link. - -Design: reuse the already-computed `_get_toolchain_ldflags()` result with list -concatenation, matching executable handling. No API or dependency changes. -Files in scope: `ebuild/build/ninja_backend.py`, -`tests/unit/test_shared_library_toolchain.py`, `CHANGELOG.md`, `TASKS.md`. -Other shared-library features and unrelated defects are out of scope. -Risk: previously ignored linker options may now correctly reject invalid builds. -Verification: new tests before/after the fix, full pytest suite, Ruff, mypy, -Python package build, and the repository's CMake/CTest check. - -Handoff (planning/architecture -> implementation/testing): discovery found the -shared-library branch initializes flags from the target alone. The executable -branch already includes toolchain flags. Acceptance criteria are the three -bullets above; all checks are NOT RUN. Next: write failing regressions, then fix. - -Verification results (Linux, Python 3.12): -- PASS: all three new regression cases failed against the original backend, - then passed with the fix (`pytest tests/unit/test_shared_library_toolchain.py -q`). -- PASS: independent reviewer repeated the focused tests (3 passed), found no - blocking issues, and confirmed all three acceptance criteria. -- PASS: Ruff on both changed Python files; mypy on `ninja_backend.py`; - `python -m build --no-isolation` (sdist and wheel); `git diff --check`; - `yamllint .` (no YAML changes). -- FAIL (pre-existing): full suite: `9 failed, 672 passed, 3 skipped in 51.88s`. - All nine failures are in `tests/unit/test_index_sync.py`: - `AttributeError: 'PackageRecipe' object has no attribute 'to_dict'`. - Reproduced the same nine failures in a detached baseline worktree at - `8b623d5` (9 failed, 13 passed in that module). -- FAIL (pre-existing): repository-wide mypy reports the same missing - `PackageRecipe.to_dict` at `ebuild/packages/index_sync.py:354`. -- FAIL (pre-existing): repository-wide Ruff reports F811 in - `tests/ebuild/test_build_dir_resolution.py`, W292 in - `tests/ebuild/test_package_recipe.py`, and two E402 findings in - `tests/unit/test_ci_gate.py`. These files are unchanged. -- NOT RUN: CMake/CTest; `cmake: command not found` in this environment. -- NOT RUN: real Windows/macOS linker execution and cross-compilation with an SDK. - Portable manifest tests cover sysroot emission; the compiler regression is - explicitly Linux/GCC-only. - -Handoff (verification -> maintainer): focused change independently reviewed; -submission is for review, not a release. Remaining work: maintainer review/CI, -with the unrelated baseline failures above tracked here for separate fixes. - | ID | Task | Owner | Mode | Status | Depends on | |----|------|-------|------|--------|------------| +| T-006 | Preserve toolchain linker settings for shared libraries ([#125](https://github.com/embeddedos-org/ebuild/pull/125)) | backend | Verification | review | none | | T-002 | Fix Windows Ninja test-target path parsing | backend | Maintenance | review | none | | T-003 | `ebuild package` looks for the unsuffixed binary on Windows (`_build/app` rather than `_build/app.exe`) | backend | Maintenance | review | none | | T-004 | `_report_footprint` (the flash/RAM report `ebuild build` prints) looks for the unsuffixed binary on Windows, and fails silently rather than logging why | backend | Maintenance | review | none |