RFC: Riscv bare metal CI job

[sedited]

This adds a CI job for building the static consensus library and linking it to an executable. It uses newlib-cygwin as a C library for the final linking step. This ensure compatibility with this target going forward and can serve as a starting point for enabling bare metal builds for the entire kernel library. This would have also caught the error fixed in #31365.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31425.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	laanwj, hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #33810 (ci: Add IWYU job by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[DrahtBot]

🚧 At least one of the CI tasks failed. _{Debug: https://github.com/bitcoin/bitcoin/runs/33960779294}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

• Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

• A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

• An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

[maflcko]

can serve as a starting point for enabling bare metal builds for the entire kernel library.

Interesting. Do you think this is possible at all, given that the kernel library links leveldb, which I'd presume is not bare-metal ready?

So I guess this mostly serves as a check that users can ship their own-brewed libbitcoinconsensus (or so, or subset of it)?

[sedited]

Interesting. Do you think this is possible at all, given that the kernel library links leveldb, which I'd presume is not bare-metal ready?

Yes, definitely not ready. I think the main hurdle is the background compaction, which I am not sure how to tackle. Maybe we'll find a solution for it eventually though, either by patching it, or allowing the user to bring their own utxos.

So I guess this mostly serves as a check that users can ship their own-brewed libbitcoinconsensus (or so, or subset of it)?

Yes, that is the goal for now.

[laanwj]

It uses newlib-cygwin as a C library for the final linking step.

Mentioning this because i had to look it up to be sure: newlib-cygwin has nothing to do with Windows whatsoever. It's simply a minimalist libc.

I think the main hurdle is the background compaction, which I am not sure how to tackle.

Could be a periodic foreground task, if threads aren't available? But yes, this would imply patching leveldb, there is no such API right now.

[sedited]

Updated 4910ae5d0d9954b8b811d7c52e3539979ea46016 -> f27760012b3e236933ea5d1b39a6ba74884df1a2 (bare_metal_support_0 -> bare_metal_support_1, compare)

• Addressed @maflcko's comment, removing sources after install step.
• Addressed @maflcko's comment, wrapped cd step into a subshell
• Addressed @laanwj's comment, use the installed lib
• Addressed @laanwj's comment, removing a bunch of unneeded/superfluous flags
• Added a start section to the binary for the global pointer and returning an exit code. Though not needed, since we are only checking that it links, I feel like this makes the example a bit clearer.

[laanwj]

Concept ACK, this looks like the minimum required to sanity check that libconsensus.a can be compiled for, and linked for bare-metal RISC-V. It is not enough to test that it actually works (this would require a lot more, quite nasty low-level code), but maybe that's out of scope for this project. At the least it is for this PR.

[hebasto]

Concept ACK.

[DrahtBot]

CI failure:

[12:50:33.743] gmake: *** No rule to make target 'install'.  Stop.

[maflcko]

Coming from https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897032758, I noticed that https://www.sourceware.org/newlib/libc.html doesn't list ftruncate, so I guess the build will fail once more code is compiled/linked?

[fanquake]

I think the docs might be incomplete / outdated? See https://github.com/bminor/newlib/blob/b39b510c1ce68757e79410585262ca2cd48da839/newlib/libc/include/sys/unistd.h#L287.

[sedited]

Rebased f27760012b3e236933ea5d1b39a6ba74884df1a2 -> ec7c86c732c995d2c38e2a3f93ad55a451a8cf81 (bare_metal_support_1 -> bare_metal_support_2, compare)

• Fixed a bunch of silent and proper merge conflicts.

[sedited]

Rebased ec7c86c732c995d2c38e2a3f93ad55a451a8cf81 -> a577bbe5df766a4e0e5a36fc997b7880eec45c0e (bare_metal_support_2 -> bare_metal_support_3, compare)

• Fixed conflict with #32989

[sedited]

Rebased a577bbe5df766a4e0e5a36fc997b7880eec45c0e -> e254de76ac0a390569bc5b561f61babb6d021dcb (bare_metal_support_3 -> bare_metal_support_4, compare)