Makefile.include: fully define BASELIBS before using its value#9451
Conversation
cladmi
added
Type: bug
cladmi
force-pushed
the
pr/make/makefile_include/define_baselibs
branch
2 times, most recently
from
a814ac8 to
0dc40ef
Compare
|
I moved the 'test' commit earlier in the history to help demonstrate the fixes. You can execute these tests on each commit and see the fixes applied: Each commit currently fixes one problem so it is easy to verify. |
smlng
left a comment
smlng
left a comment
There was a problem hiding this comment.
tested ACK, first commit fails as expected with BASELIB change detected, following commit fix that as promised 😉
|
Is there a reason to keep the BASELIBS change detection or should I remove as I planned to ? |
Build targets were using the immediate value of '$(BASELIBS)' before it was actually set. bindist.inc.mk should also be processed before as it is adding `BIN_USEMODULE` to `USEMODULE`. This fixes use before define problems for: * `make -C examples/hello-world` * `make -C examples/bindist`
They can define PSEUDOMODULES which is used to populate BASELIBS. It means they must be processed before using BASELIBS immediate value. This moved the processing of all `Makefile.include` before using `BASELIBS`. It is also moved before setting `BASELIBS` in `modules.inc.mk to keep the order logical (it would work anyway thanks to deferred variables evaluation). This fixes the problems for `make -C tests/unittests`
mhm, maybe its good to keep this (and maybe add other) checks in the build system - but could be moved into a distinct file like |
cladmi
force-pushed
the
pr/make/makefile_include/define_baselibs
branch
from
0dc40ef to
90fdd09
Compare
|
I moved the commit as last so there is no "broken" intermediate state. Refactoring with a separate check file could be done in a separate PR and would need specific thoughts as this one needs to be init at the right place. |
Remove debug '$(info)' added by RIOT-OS#9451
Remove debug '$(info)' added by RIOT-OS#9451
Remove debug '$(info)' added by RIOT-OS#9451
2 participants
Contribution description
As described in #8913 some places are using the immediate value of
BASELIBSbefore it is actually set (== use of non initialized variables…). The problem was mitigated because ELFFILE is always rebuild even if files in$(BASELIBS)did not change.This PR addresses this by moving the definition of
BASELIBSsooner inMakefile.include.This required:
modules.inc.mkearlierpkg/*/Makefile.includeaslibcosedefinesPSEUDOMODULESwhich modifiesBASELIBSRFC: SetPSEUDOMODULESin packagesMakefile.include#8984 and pseudomodules: allow defining them in pkg/PKG/Makefile.include #9003I also added a debug commit that asserts that
BASELIBSwas not changed during build. It can be removed before merging.Issues/PRs references
Issue #8913
This was problematic when trying to reference
$(BASELIBS)as a target dependency in the application Makefile: #9351