makes: rewrite to new build system

phoenix-rtos / phoenix-rtos-hostutils

Phoenix-RTOS supporting daemon

BSD 3-Clause "New" or "Revised" License

1 stars 5 forks source link

makes: rewrite to new build system #51

Closed nalajcie closed 1 year ago

nalajcie commented 1 year ago

CI to pass: https://github.com/phoenix-rtos/phoenix-rtos-project/pull/815

Description

remove tangled dependencies
fix compilation warnings

please note that currently sanitizers will be enabled on host utils - it can be easily changed (set NOSAN := 1 in Makefile), I think it's healthy to have them enabled for some time to maybe discover new errors

Motivation and Context

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

How Has This Been Tested?

[ ] Already covered by automatic testing.
[ ] New test added: (add PR link here).
[ ] Tested by hand on: (list targets here).

Checklist:

[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[ ] I have added tests to cover my changes.
[ ] All new and existing linter checks and tests passed.
[ ] My changes generate no new compilation warnings for any of the targets.

Special treatment

[ ] This PR needs additional PRs to work (list the PRs, preferably in merge-order).
[ ] I will merge this PR by myself when appropriate.

nalajcie commented 1 year ago

please note that currently sanitizers will be enabled on host utils - it can be easily changed (set NOSAN := 1 in Makefile), I think it's healthy to have them enabled for some time to maybe discover new errors

nalajcie commented 1 year ago

LGTM

I think that sanitizers should be enabled for developing host utils/test builds but not for hostutils releases (which we should do and use in normal builds)

I've added explicit info in the hostutils Makefile:

I think right now I'll keep compiling sanitizer-enabled tools by default in phoenix-rtos-project (if many developers would use them we might find more errors) but will build with NOSAN=1 while preparing docker images ("releases" used in CI).

nalajcie commented 1 year ago

Hehh, turns out building with NOSAN=1 didn't work due to magically changed order of linkage.

Introduced LOCAL_LDLIBS in binary.mk to circumvent that.

nalajcie commented 1 year ago

Heeh, first time I see that sanitizers covers some problems instead of exposing it.

Yeah, I think -fsanitize while linking by gcc is translated to linking against libasan.so (which needs to be the very first dynamic lib - so it results in dynamic linking order rearrangement).

So LOCAL_LDLIBS is designed to be used only in hostutils build (as in other cases we have everything in our _build root.

for anything in host-generic-pc as only there we're using dynamic linking / external host libraries (eg. adding -lm in tests could/should be done by LOCAL_LDLIBS)

Approving, but please do changes in phoenix-rtos-build before merge.

Yep, I'm waiting for test PR to pass before merging anything.