fix(ninja): compile shared libraries with PIC

[Abd-ullah2001]

Summary

• Default shared-library compile commands to -fPIC.
• Preserve an explicit PIC policy while ensuring a later PIE flag cannot
  replace PIC for a shared-library target.
• Cover both the Ninja manifest and compilation database output with regression
  tests.

Why

The Ninja backend already links shared_library targets with the platform's
shared-library rule, but their source objects could be compiled without
position-independent code. On ELF and common cross toolchains, that can make
the final link fail with relocation errors asking for the objects to be rebuilt
with -fPIC.

PIC and PIE are also not interchangeable here: PIE output is intended for
executables. The resolver now considers flag order and makes sure the last
effective policy for a shared-library compile is PIC, unless the project has
explicitly selected a PIC policy itself.

Validation

• python -m pytest tests/ebuild/test_ninja_backend.py -q --tb=short — 13 passed, 1 skipped
• python -m pytest tests/unit/test_ninja_backend.py -q --tb=short — 16 passed
• ruff check ebuild/build/ninja_backend.py tests/ebuild/test_ninja_backend.py — passed
• Changed-file mypy check — no issues in 2 source files
• python -m build — sdist and wheel built successfully

The full suite currently finishes with 9 failures, 681 passes, and 1 skip. All
nine failures reproduce on upstream master and come from the existing missing
PackageRecipe.to_dict() method in index synchronization; that separate issue
is being addressed in #119. Full-repository Ruff also reports four existing,
unrelated findings being addressed in #122.

[srpatcha]

Review — ebuild#131 "fix(ninja): compile shared libraries with PIC"

head: c16d363 author: Abd-ullah2001 ci: none reported (mergeStateStatus: BLOCKED)

Verdict: Correct, and it fixes a bug that had a half-written fix sitting in the file since before this PR. _PIC_FLAGS existed at ninja_backend.py:29 on master with a comment describing exactly this behaviour — and was never referenced anywhere, so no shared library ever got -fPIC. This PR makes the constant real and splits PIE out of it, which is the right distinction. I exercised six cases against the generated compile_commands.json and all six match the stated intent. The open question is Windows.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	ebuild/build/ninja_backend.py _resolve_target_cflags, cflags.append("-fPIC")	-fPIC is appended for every shared_library target on every platform, and this backend does build shared libraries on Windows — the extension selector picks .dll when sys.platform == "win32", and the CI matrix has three windows-2022 legs. On mingw/MSYS targets gcc emits warning: -fPIC ignored for target (all code is position independent) for each compile; a project carrying -Werror in its toolchain cflags (which .github/PULL_REQUEST_TEMPLATE.md asks contributors to build with) turns that into a hard failure on a configuration that builds today. I did not measure this — no Windows host here — so treat it as a concern with a named mechanism, not a demonstrated break.	Gate the append on the target platform: skip it when sys.platform == "win32" (or, better, when the resolved toolchain targets PE/COFF), the same way _exe_suffix() at :33 already special-cases Windows. Add a Windows-path unit test alongside the four existing parametrized ones — the suite already fakes platform state elsewhere in this repo.
2	Low	ebuild/build/ninja_backend.py _resolve_target_cflags	-fno-pic on a shared_library is honoured silently. Verified: target cflags ["-fno-pic"] produce -fno-pic with no -fPIC. Honouring an explicit policy is the right default — the user asked for it — but this is the one configuration guaranteed to produce the relocation error the PR exists to prevent, and it produces it at link time with a message about rebuilding objects, far from the build.yaml line that caused it.	Keep the behaviour, add a diagnostic. One logger.warning when a shared_library resolves to -fno-pic/-fno-PIC, naming the target, converts a confusing linker error into an answer. §9.2 calls for "actionable diagnostics with remediation guidance"; this is a cheap instance of it.
3	Low	PR body	No ## Type of Change section, no Pre-Submission checklist, and no linked issue. The body's technical content is good — the Validation section is specific and honestly separates this PR's results from the pre-existing failures, correctly attributing them to #119 and #122. But master gained a Linked issue policy workflow in 52e1f94 that runs on pull_request_target for opened, edited, reopened, synchronize. No check has run on this branch yet; the next push will trigger it and it will fail, exactly as it does on #127.	Add Closes #NNN against a tracking issue now, before the next push, rather than discovering it as a red check later.
4	Low	CHANGELOG.md	Merge conflict with #125. Both PRs insert a new bullet at the top of the same ### Fixed list. Verified with git merge-tree: merging this head with #125's conflicts on CHANGELOG.md only — ebuild/build/ninja_backend.py auto-merges cleanly, because #125 edits the ldflags block around :283 and this edits _resolve_target_cflags around :169. Against #133 (fix: avoid ninja object path collisions, which edits _object_path at :178) the merge is fully clean.	Nothing to change now — a one-line CHANGELOG resolution for whichever lands second. Flagging it so it is expected rather than surprising.

Not a finding, recorded as context: the shared_library branch ignores target.depends entirely, while the executable/test branch collects dep_archives from static_library dependencies. So a static library cannot currently be linked into a shared library by this backend, which is why the absence of -fPIC on static_library targets (verified: none is added) is not a hole today. If that asymmetry is ever closed, the PIC decision has to follow the consumer, not the target type. Out of scope here.

Architecture conformance

Conforms. §21 places ebuild in Tier 1 — Foundation, and ebuild/build/ninja_backend.py is build-graph generation, the core of what §9 assigns to eBuild. §5.1 is untouched: no import, link line or manifest entry moves, and nothing becomes a runtime dependency — this only changes the text of a generated build.ninja and compile_commands.json.

The change reads correctly against §9.1, which places Toolchains and Targets on the same level feeding one dependency graph: the resolver now takes toolchain cflags and target cflags together and decides the effective policy from their combined order, rather than letting either silently win. That is the right shape. It also fixes the compile_commands.json output in the same pass, which matters for §9.2's "One source of truth for CLI, VS Code and EoStudio" — an IDE reading a compile database that disagrees with the actual compile line is a second source of truth.

No API or wire-format change (brief item 8): _resolve_target_cflags is private, its signature is unchanged, and _PIC_FLAGS — the one public-ish name whose meaning narrows — was unreferenced anywhere in the repository, so nothing can be depending on the old membership. No hot path is affected (brief item 9): the new scan is a single reversed() walk over one target's cflags, bounded by flag count, inside a loop that already iterates every target. No duplication (brief item 10): this is the only place PIC policy is decided. Documentation changed with behaviour (brief item 11): CHANGELOG entry present, _resolve_target_cflags's docstring updated, and the _PIC_FLAGS comment rewritten to describe what the code now actually does.

Proposed changes

1. Decide finding 1. Either gate the append on non-Windows targets, or establish by measurement that mingw's warning is not promoted anywhere in this project's supported configurations and say so in the PR body. The second is a legitimate answer; the current state is that nobody has checked.
2. Add the logger.warning from finding 2.
3. Link an issue (finding 3).

Findings 1-3 are all small. Nothing here needs restructuring.

Not checked

• pytest — NOT RUN. pytest is not importable on this host, and the local ebuild clone has a dirty working tree, left untouched per the rules of engagement. The body's 13 passed, 1 skipped and 16 passed are unverified as pytest runs; I verified the underlying behaviour by driving NinjaBackend directly, below.
• Windows — NOT RUN. Finding 1 is unverified. No Windows host, no mingw toolchain, and no cross-compile was attempted. I did not measure whether -fPIC produces a warning, whether that warning is promoted under this repo's configurations, or whether any consumer builds a .dll through this backend today.
• No compiler was invoked at all. Every result below is the text of the generated compile database. That the emitted flags produce a correctly position-independent object, and that the last-wins ordering assumption matches the real GCC and Clang drivers, is read from documented behaviour, not measured.
• mypy — NOT RUN. Not installed. yamllint — NOT RUN. Not installed; no YAML in the diff. python -m build — NOT RUN.
• CI — no checks reported on Abdullah-assessment-branch. GitHub returns no check runs for this branch, so none of this has CI evidence behind it. mergeStateStatus is BLOCKED.
• I did not confirm the body's attribution of the nine test_index_sync.py failures to #119; I did independently confirm the four ruff findings it attributes to #122 are real and present on origin/master.

Verified locally, against a git archive export of head c16d3639:

• ruff 0.16.5 check . → 4 errors, all pre-existing on origin/master. This PR adds none.
• _PIC_FLAGS on origin/master is defined at :29 and referenced nowhere — confirmed by grep over the whole file. The fix it documented was never wired up.
• Driving NinjaBackend(...).generate() and reading compile_commands.json, PIC/PIE flags in the emitted compile line:

case	emitted
shared_library, no flags	-fPIC
shared_library, target -fno-pic	-fno-pic (no -fPIC)
shared_library, toolchain -fPIE	-fPIE -fPIC
shared_library, toolchain -fPIC + target -fpie	-fPIC -fpie -fPIC
executable	none
static_library	none

All six match the PR's stated intent, including the last-wins ordering the tests assert.

• git merge-tree against #125 → conflict in CHANGELOG.md only; against #133 → clean.

---

Automated architecture review of c16d36394970 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[srpatcha]

Thanks — this is a real regression: _PIC_FLAGS on master is a leftover from 0028d07, whose -fPIC logic and tests were lost in a later merge. Your version is an improvement on the original because it stops treating -fPIE as "PIC already requested", which was wrong for shared objects. I ran the branch locally: the new tests pass, the rest of the suite is unchanged, ruff/mypy are clean.

Small follow-ups, none blocking:

• ninja_backend.py:172-179 — because the scan covers merged toolchain+target flags, a toolchain-level -fno-pic now disables PIC for all shared libraries. I think that's the right call, but please state it in CHANGELOG.md:25 so nobody is surprised.
• • CHANGELOG.md:25 — worth noting this restores 0028d07's behaviour rather than introducing it; it helps anyone bisecting.
• • • tests/ebuild/test_ninja_backend.py:136 — the exact ["-fPIC", pie_flag, "-fPIC"] assertion pins the redundant trailing flag; consider asserting only that the last PIC-ish flag is -fPIC.
• • • • Please add Signed-off-by per CONTRIBUTING.md.
        Approving. Expect a trivial CHANGELOG.md conflict with #125/#126.

[Abd-ullah2001]

Hello, Thank you for your reply. I will look into the context and fix the minimal changes. Is my PR approved for now? As I got 2 emails with different wordings, I am a little confused. Kindly let me know Thank you, Abdullah Noor

…

On Mon, 14 Sept 2026, 10:35 am Srikanth Patchava, ***@***.***> wrote: ***@***.**** approved this pull request. Thanks — this is a real regression: _PIC_FLAGS on master is a leftover from 0028d07 <0028d07>, whose -fPIC logic and tests were lost in a later merge. Your version is an improvement on the original because it stops treating -fPIE as "PIC already requested", which was wrong for shared objects. I ran the branch locally: the new tests pass, the rest of the suite is unchanged, ruff/mypy are clean. Small follow-ups, none blocking: - *ninja_backend.py:172-179* — because the scan covers merged toolchain+target flags, a toolchain-level -fno-pic now disables PIC for all shared libraries. I think that's the right call, but please state it in CHANGELOG.md:25 so nobody is surprised. - - *CHANGELOG.md:25* — worth noting this restores 0028d07 <https://github.com/embeddedos-org/ebuild/commit/0028d074335fd5e32419d4cc0772a21fdb1e97eb>'s behaviour rather than introducing it; it helps anyone bisecting. - - - *tests/ebuild/test_ninja_backend.py:136* — the exact ["-fPIC", pie_flag, "-fPIC"] assertion pins the redundant trailing flag; consider asserting only that the last PIC-ish flag is -fPIC. - - - - Please add Signed-off-by per CONTRIBUTING.md. Approving. Expect a trivial CHANGELOG.md conflict with #125 <#125>/#126 <#126>. — Reply to this email directly, view it on GitHub <#131?email_source=notifications&email_token=A6TZMQX4HHBZHZEF3335KED5O57RVA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMJZGQZDCOJRGY2KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-5194219164>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/A6TZMQQOGQ66FM3DRG2MRU35O57RVAVCNFSNUABGKJSXA33TNF2G64TZHMYTCOJQGEZDKNZYGE5US43TOVSTWNJUGIYTKOJWGUZTRILWAI> . You are receiving this because you authored the thread.Message ID: ***@***.***>