build: default CROSS=riscv64-elf + document toolchain variants in README - #1
Conversation
Makefile:
- Default CROSS changed from 'riscv64-unknown-elf' to 'riscv64-elf' — this
matches the mainstream bare-metal toolchain (xpack-riscv-none-elf-gcc,
RISC-V collaborations/riscv-gnu-toolchain) that most users have.
- Added comment block explaining the three variants:
* riscv64-elf- (preferred, default)
* riscv64-unknown-elf- (Debian's gcc-riscv64-unknown-elf package)
* riscv64-linux-gnu- (Arch's riscv64-linux-gnu-gcc — does NOT work for
bare-metal OnyxBoot linking)
README.md + README.ru.md:
- Quick start: 'make CROSS=riscv64-elf' → just 'make' (default is now correct)
- Add note explaining why the default is riscv64-elf and when to override
Reviewer's GuideSwitches the default RISC-V cross-compiler prefix to riscv64-elf and updates documentation to describe the new default and when/how to override it, including clarifying supported toolchain variants. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The Makefile recipe lines for
bootloader.elf,bootloader.bin,clean,test, andtest-allare now indented with spaces instead of tabs, which will breakmake; these should remain tab-indented shell commands. - Consider mirroring the new Makefile note about
riscv64-linux-gnu-being unsuitable for bare-metal in the README toolchain section so users who don’t inspect the Makefile still see that guidance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Makefile recipe lines for `bootloader.elf`, `bootloader.bin`, `clean`, `test`, and `test-all` are now indented with spaces instead of tabs, which will break `make`; these should remain tab-indented shell commands.
- Consider mirroring the new Makefile note about `riscv64-linux-gnu-` being unsuitable for bare-metal in the README toolchain section so users who don’t inspect the Makefile still see that guidance.
## Individual Comments
### Comment 1
<location path="Makefile" line_range="30-39" />
<code_context>
bootloader.elf: $(SRCS)
- $(CC) $(CFLAGS) $^ $(LDFLAGS) -o $@
+ $(CC) $(CFLAGS) $^ $(LDFLAGS) -o $@
bootloader.bin: bootloader.elf
- $(OBJCOPY) -O binary $< $@
+ $(OBJCOPY) -O binary $< $@
clean:
- rm -f bootloader.elf bootloader.bin
+ rm -f bootloader.elf bootloader.bin
test:
- ./test/run_qemu.sh
+ ./test/run_qemu.sh
test-all:
- ./test/test_all.sh
+ ./test/test_all.sh
.PHONY: all clean test test-all
</code_context>
<issue_to_address>
**issue (bug_risk):** Use real tab characters for Make recipes instead of spaces to avoid `make` errors.
The recipe lines for `bootloader.elf`, `bootloader.bin`, `clean`, `test`, and `test-all` are now indented with spaces instead of a literal tab, which will trigger `missing separator` errors in many `make` implementations and break the build. Please change these indentations back to actual tab characters to keep the Makefile functional.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| $(CC) $(CFLAGS) $^ $(LDFLAGS) -o $@ | ||
|
|
||
| bootloader.bin: bootloader.elf | ||
| $(OBJCOPY) -O binary $< $@ | ||
| $(OBJCOPY) -O binary $< $@ | ||
|
|
||
| clean: | ||
| rm -f bootloader.elf bootloader.bin | ||
| rm -f bootloader.elf bootloader.bin | ||
|
|
||
| test: | ||
| ./test/run_qemu.sh | ||
| ./test/run_qemu.sh |
There was a problem hiding this comment.
issue (bug_risk): Use real tab characters for Make recipes instead of spaces to avoid make errors.
The recipe lines for bootloader.elf, bootloader.bin, clean, test, and test-all are now indented with spaces instead of a literal tab, which will trigger missing separator errors in many make implementations and break the build. Please change these indentations back to actual tab characters to keep the Makefile functional.
Makefile default changed from
riscv64-unknown-elftoriscv64-elf(mainstream bare-metal toolchain). README.md and README.ru.md updated to reflect new default and document when to override.Summary by Sourcery
Adopt
riscv64-elfas the default bare-metal toolchain and document alternatives for building OnyxBoot.Enhancements:
riscv64-elfas the default bare-metal cross-compiler prefix while documenting supported toolchain variants and override guidance.Build:
Documentation:
riscv64-unknown-elf.