fix(build): link boot log into eboot_core and make CI run the tests - #41
Open
Divyanshu-2907 wants to merge 1 commit into
Open
fix(build): link boot log into eboot_core and make CI run the tests#41Divyanshu-2907 wants to merge 1 commit into
Divyanshu-2907 wants to merge 1 commit into
Conversation
Divyanshu-2907
requested review from
hshanmug12,
maheshmunnangi and
srpatcha
as code owners
August 26, 2026 08:17
core/recovery.c calls eos_boot_log_append(), eos_boot_log_read() and
eos_boot_log_get_head(), which were defined in stage1/boot_log.c and
compiled only into eboot_stage1. tests/test_recovery links eboot_core
alone, so those symbols never resolved and the host test build failed:
undefined reference to `eos_boot_log_append'
collect2: error: ld returned 1 exit status
The boot log is a core service now that core/recovery.c depends on it,
so move the translation unit into eboot_core. This removes an undeclared
reverse dependency from core up into stage1. eboot_stage1 already depends
on eboot_core, so stage-1 callers still resolve. The file moves unchanged;
no code was edited.
Two CI defects hid the break:
* ci.yml passed -DBUILD_TESTS=ON, but the option is EBLDR_BUILD_TESTS.
CMake warned the variable was unused, tests were never built, and
ctest then passed with 0 tests registered.
* ci.yml triggered on main/develop only. This repository's default
branch is master and no main branch exists, so it never ran.
Also drops -DENABLE_COVERAGE=ON, which is not an option in this project
and was silently ignored.
Verified on Ubuntu 24.04 / gcc 13.3.0 / cmake 3.28.3: clean configure and
build from an empty build directory, ctest 27/27 passing (14 unit + 13
valgrind). Before this change: 25/27, test_recovery failing to link.
Divyanshu-2907
force-pushed
the
fix/host-test-build-and-ci
branch
from
August 26, 2026 08:35
8f37c7e to
7120546
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The host test build does not link on
master. Building with tests enabled fails:core/recovery.cis compiled intoeboot_core, but the three boot-log functions itcalls were defined in
stage1/boot_log.c, which is compiled only intoeboot_stage1.tests/test_recoverylinkseboot_corealone, so the symbols never resolve. The sameapplies to any consumer that links
eboot_corewithouteboot_stage1.The calls were introduced in d3ae185 ("Implement authenticated recovery log retrieval",
#31). Two independent CI defects meant nothing caught it.
Root cause of the CI gap
1. The test flag name is wrong.
ci.ymlpassed-DBUILD_TESTS=ON, but the optiondeclared in
CMakeLists.txtisEBLDR_BUILD_TESTS. CMake warns that the variable isunused, tests are never built, and the
cteststep then succeeds with zero tests:2. The workflow never fired.
ci.ymltriggered onpush: [main, develop]andpull_request: [main]. This repository's default branch ismaster, and nomainbranch exists, so the workflow did not run at all.
Approach
boot_log.cfromeboot_stage1intoeboot_core. The boot log is a coreservice now that
core/recovery.cdepends on it, so the library boundary was whatwas wrong, not the call.
eboot_stage1already linkseboot_corePUBLIC, so stage-1callers still resolve. The file moves unchanged — no code was edited.
BUILD_TESTStoEBLDR_BUILD_TESTSinci.yml.masterto theci.ymltriggers, matching the convention already used bybuild.yml. Keptmain/developso the workflow still behaves correctly if thedefault branch is renamed.
-DENABLE_COVERAGE=ON, which is likewise not an option in this project and wassilently ignored.
Validation
Ubuntu 24.04, gcc 13.3.0, cmake 3.28.3, valgrind 3.22.0. Clean configure and build
from an empty build directory, using the same flags CI now uses.
test_recoverylinkctestresultAll 13 valgrind leak-check targets pass, including
valgrind_test_recovery, whichpreviously could not run because the binary did not exist.
Verifying the stage-1 side
The host test suite cannot prove this half:
eboot_stage1is only linked into anexecutable by the
eboot_firmwaretarget, which exists solely in the cross-compileboard build. In a host build it is produced as a static archive and never linked, so
its symbols are never required to resolve.
Checked directly against the built archives instead:
Stage-1's undefined set is exactly a subset of what
eboot_corenow defines. Confirmedby linking a probe
main()against the archives in the order CMake generates from thedeclared dependencies:
The first case reproduces the exact failure the move could have introduced; the second
shows the declared
eboot_stage1 -> eboot_core -> eboot_haldependency chain resolvesit. A full ARM cross-compile additionally exercises the board library and linker script,
neither of which this change touches.
Scope and limitations
Deliberately not addressed here, to keep this change reviewable:
ENABLE_COVERAGEflag had no implementation behind it; the
codecovupload only ever received thePython
coverage.xmlfrom pytest. Wiring up real C coverage is a feature, not a fix.include/eos_boot_log.hdoes not match the implementation. It declaresint eos_boot_log_init(void)andint eos_boot_log_read(eos_boot_log_entry_t *, uint32_t),while the implementation provides
void eos_boot_log_init(uint32_t)andint eos_boot_log_read(uint32_t, eos_boot_log_entry_t *)— note the reversedparameter order. Nothing in
core/orstage1/includes the header; they use localexterndeclarations instead, which bypasses type checking. Reconciling the headerwith the implementation is an API decision worth its own PR.
tests/unit/test_boot_log.cdoes not test the shipped boot log. It includeseos_boot_log.hand defines its own local implementations ofeos_boot_log_init,_append,_readand_clear, so it exercises stubs in the test file rather thanboot_log.c. This is why it passed while the real implementation was unreachable.No behavioural change to the bootloader itself: the only non-CI change is which static
library a translation unit is compiled into.