qemu/Makefile: Do not mount ROMFS partition on non-test targets.#19109
qemu/Makefile: Do not mount ROMFS partition on non-test targets.#19109agatti wants to merge 1 commit intomicropython:masterfrom
Conversation
There was a problem hiding this comment.
This seems a bit hacky. Eg it'd be nice in the future to add the test//% target that unix and webassembly have (I find that very useful, eg make test//float).
How about this:
--- a/ports/qemu/Makefile
+++ b/ports/qemu/Makefile
@@ -220,17 +220,15 @@ CFLAGS += $(SPECS_FRAGMENT)
LDFLAGS += $(SPECS_FRAGMENT)
endif
-# Mount the ROMFS test image only if no other ROMFS partition has been mounted on slot 0.
+# Mount the ROMFS test image specifically for test runs (if there is a slot 0).
ROMFS_TEST_IMAGE = assets/random_romfs.bin
ifneq ($(MICROPY_HW_ROMFS_PART0_START),)
-ifeq ($(QEMU_ROMFS_IMG0),)
-QEMU_ARGS += -device loader,file=$(ROMFS_TEST_IMAGE),addr=$(MICROPY_HW_ROMFS_PART0_START),force-raw=on
-endif
+QEMU_TEST_ARGS = -device loader,file=$(ROMFS_TEST_IMAGE),addr=$(MICROPY_HW_ROMFS_PART0_START),force-raw=on
endif
-RUN_TESTS_FULL_ARGS = -t execpty:"$(QEMU_SYSTEM) $(QEMU_ARGS) -serial pty -kernel ../ports/qemu/$<" $(RUN_TESTS_ARGS)
+RUN_TESTS_FULL_ARGS = -t execpty:"$(QEMU_SYSTEM) $(QEMU_ARGS) $(QEMU_TEST_ARGS) -serial pty -kernel ../ports/qemu/$<" $(RUN_TESTS_ARGS)
################################################################################
# Source files and librariesThere was a problem hiding this comment.
I didn't know that test syntax existed. For those situations I've always used ./run-tests.py -d ... -i ..., good to know though.
Anyway, I've tested your change and it did work! Looking at this a bit more let me spot another small issue in the makefile, test_full doesn't add $(RUN_TESTS_EXTRA) to the test runner command line(s).
Probably test_full is only run by CI these days but maybe it'd be nice to eventually make it work there too. Yet another first time (human) contributors task :)
That said, I'll update the PR - thanks for the suggestion.
There was a problem hiding this comment.
Great, thanks for updating.
Looking at this a bit more let me spot another small issue in the makefile,
test_fulldoesn't add$(RUN_TESTS_EXTRA)to the test runner command line(s).
Yes indeed...
This commit fixes a mistake introduced in micropython#19051, where the test ROMFS partition would get mounted even on targets that do not need it to function (ie. `repl`). Now the ROMFS image is mounted only for test targets: `test`, `test_full`, and `test_natmod`, and only if there's no explicit ROMFS partition being chosen to be mounted via the command line. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
agatti
force-pushed
the
qemu-romfs-target-fix
branch
from
9c84737 to
c567937
Compare
2 participants
Summary
This commit fixes a mistake introduced in #19051, where the test ROMFS partition would get mounted even on targets that do not need it to function (ie.
repl).Now the ROMFS image is mounted only for test targets:
test,test_full, andtest_natmod, and only if there's no explicit ROMFS partition being chosen to be mounted via the command line.Testing
The interpreter for the
VIRT_RV32board was built and executed both for Make targets that require ROMFS and targets that don't, making sure the process would proceed as expected.This was done using the version of Make that comes with Ubuntu 24.04, so hopefully this should work on the CI machine.
Generative AI
I did not use generative AI tools when creating this PR.