cpu/esp32: use module newlib_nano by gschorcht · Pull Request #13812 · RIOT-OS/RIOT

gschorcht · 2020-04-03T22:55:09Z

Contribution description

This PR fixes the compilation and execution problem of tests/libc_newlib for ESP32. The new toolchain version uses the newlibc-nano. Module newlib_nano has to be used therefore.

Testing procedure

Compilation and execution of tests/libc_newlib should work with any ESP32 board, for example

make BOARD=esp32-wroom-32 -C tests/libc_newlib flash test

Issues/PRs references

Depends on #13814

gschorcht · 2020-04-03T22:56:54Z

@kaspar030 The merge of this PR has to be synchronized with the update of the workers with the new toolchain version

benpicco

Just like the other architectures with a newlib_nano toolchain :)
Thank you for the quick fix!

benpicco · 2020-04-03T23:04:19Z

@gschorcht Why do you think only some workers were updated?
It's been consistently failing - maybe the esp32 RTT PR was already queued for build before the update.

Let's run some more Murdock jobs and see if it stays consistent. (Feel free to kick them any time)

benpicco · 2020-04-03T23:08:55Z

@gschorcht Why do you think only some workers were updated?

nvm, I see…

/tests/libc_newlib/main.c:41:2:error: error newlib-nano is enabled but the header is not visible by the build system
 #error newlib-nano is enabled but the header is not visible by the build system

gschorcht · 2020-04-03T23:12:59Z

@gschorcht Why do you think only some workers were updated?
It's been consistently failing - maybe the esp32 RTT PR was already queued for build before the update.

No, the compilation of PR #13749 failed in an earlier run but did not fail in the last run. Furthermore, according to https://ci.riot-os.org/ a different number of boards failed for different runs:

PR #13734: tests: add test to test RDNSS option handling
PR #13806: sys/net/dhcpv6: fix requesting multiple prefixes

According to the logs, all tests failed on mobi9.inet.haw-hamburg.de.

cgundogan · 2020-04-04T06:18:44Z

@gschorcht I enabled a coupld of more workers in mobi9. This usually pulls in the latest riotdocker image. Do you think the problem is with the latest image?

gschorcht · 2020-04-04T06:45:53Z

This usually pulls in the latest riotdocker image. Do you think the problem is with the latest image?

Ah, I see. The docker image seems to have already the new version of the toolchain with the newlibc-nano. This is usually no problem, except for tests/libc_newlib. The new version of the toolchain requires this PR for a successful compilation of this test.

What are the options? If we don't merge, compilation of tests/libc_newlib will fail on mobi9. If we merge this PR, the test will fail on all other nodes.

We could blacklist ESP32 boards for this test until all Murdock nodes are updated.

cgundogan · 2020-04-04T10:33:19Z

@gschorcht then let's get this in as soon as possible. I updated all the mobis to use the latest riotdocker image. I also triggered an update to haw-icc. It takes a bit time until all pods in that cluster react to the command, though. I guess @kaspar030 is then updating the image on breeze and @aabadie on riotbuild?

benpicco · 2020-04-04T11:12:33Z

Looks like Murdock is happy now.

cgundogan · 2020-04-04T11:21:08Z

Looks like Murdock is happy now.

I think that's luck. I am not sure whether breeze and riotbuild have yet been updated. haw-icc still take a lot of time to update all the docker images.

ESP32 boards must be blacklisted for `tests/libc_newlib` to avoid CI compilation errors until the toolchain is updated on all nodes.

This reverts commit c7dd22e.

kaspar030 · 2020-04-14T09:31:06Z

So the question arises whether it is really worth to use newlib-nano-formatted-io on a platform that has enough space in the flash memory.

Good question. I guess if there's megabytes of flash, it would be less surprising to have actually working long long prints in stdio.

I'm a bit worried about portability issues. One of the biggest selling points of RIOT is that applications written for one MCU can be ported easily to another, usually be just re-compiling. That promise is broken if the application is using %llu, as it then doesn't work on any board with nano stdio (which means, every cortexm and soon msp430. also AVR doesn't do %llu). Worse, it'll break horribly at runtime. Thus I'd rather make users figure out the alternatives early.

gschorcht · 2020-04-14T09:46:43Z

I'm a bit worried about portability issues. One of the biggest selling points of RIOT is that applications written for one MCU can be ported easily to another, usually be just re-compiling. That promise is broken if the application is using %llu, as it then doesn't work on any board with nano stdio (which means, every cortexm and soon msp430. also AVR doesn't do %llu). Worse, it'll break horribly at runtime. Thus I'd rather make users figure out the alternatives early.

Ah ok, I see, it makes sense.

I have encountered this problem with DEBUG outputs in platform implementations where uint64_t and %llu are used for 48-bit and 64-bit hardware timers. Would it make more sense to change these DEBUG outputs and use the newlib-nano-formatted-io from your point of view?

kaspar030 · 2020-04-21T08:43:23Z

Would it make more sense to change these DEBUG outputs and use the newlib-nano-formatted-io from your point of view?

How many are there? :)

kaspar030 · 2020-04-21T08:44:25Z

How many are there? :)

But the DEBUG outputs are per-file. Is it easy to turn on full stdio (when debugging esp32)?

gschorcht · 2020-04-21T08:50:19Z

Would it make more sense to change these DEBUG outputs and use the newlib-nano-formatted-io from your point of view?

How many are there? :)

There is only one file periph/rtc with five such DEBUG outputs.

gschorcht · 2020-04-21T08:52:35Z

How many are there? :)

But the DEBUG outputs are per-file. Is it easy to turn on full stdio (when debugging esp32)?

How could it be controlled to use newlib instead of newlib-nano-formatted-io?

kaspar030 · 2020-04-21T09:15:34Z

How could it be controlled to use newlib instead of newlib-nano-formatted-io?

If the toolchain supports both (like ARM), nano is selected by the "newlib-nano" module. Without that, the full newlib is used. "newlib-nano" should be a default module, then it can be disabled case-by-case.

gschorcht · 2020-04-21T09:23:00Z

How could it be controlled to use newlib instead of newlib-nano-formatted-io?

If the toolchain supports both (like ARM), nano is selected by the "newlib-nano" module. Without that, the full newlib is used. "newlib-nano" should be a default module, then it can be disabled case-by-case.

It should be possible to use the ARM approach. The newlib could be compiled once with --enable-nano-formatted-io and once without --disable-nano-formatted-io. I will try but it will take some time.

kaspar030 · 2020-04-21T09:26:37Z

It should be possible to use the ARM approach. The newlib could be compiled once with --enable-nano-formatted-io and once without --disable-nano-formatted-io. I will try but it will take some time.

Hm, maybe this is overkill. I'm happy with this PR for now. Should we just fix the %llu in rtc as a followup?

gschorcht · 2020-05-02T15:55:41Z

@benpicco Thanks for merging the PR. Obviously, you had simply luck with Murdock 😉 It seems that there are workers that are still not updated. The compilation of your PR #13965 failed in tests/libc_newlib. breeze is complaining that newlib-nano is enabled but the header is not visible by the build system 😟

benpicco · 2020-05-02T16:34:34Z

Gnarly, I would have thought after a month the update would have trickled down to all the workers 😕

cgundogan · 2020-05-04T14:20:45Z

breeze is complaining that newlib-nano is enabled but the header is not visible by the build system

ping @kaspar030

kaspar030 · 2020-05-04T14:29:09Z

breeze should be updated now.

benpicco · 2020-05-05T12:27:59Z

I still get the occasional

-- running on worker breeze thread 12, build number 301935.
make: Entering directory '/tmp/dwq.0.11409646860483635/da8ef86477e10da72afc1594dede9071/tests/libc_newlib'
ESP32_SDK_DIR should be defined as /path/to/esp-idf directory
ESP32_SDK_DIR is set by default to /opt/esp/esp-idf
Building application "tests_libc_newlib" for "esp32-mh-et-live-minikit" with MCU "esp32".

�[01m�[K/tmp/dwq.0.11409646860483635/da8ef86477e10da72afc1594dede9071/tests/libc_newlib/main.c:41:2:�[m�[K �[01;31m�[Kerror: �[m�[K#error newlib-nano is enabled but the header is not visible by the build system
 #error newlib-nano is enabled but the header is not visible by the build system
�[01;32m�[K  ^�[m�[K
/tmp/dwq.0.11409646860483635/da8ef86477e10da72afc1594dede9071/Makefile.base:108: recipe for target '/tmp/dwq.0.11409646860483635/da8ef86477e10da72afc1594dede9071/build/application_tests_libc_newlib/main.o' failed
make[1]: *** [/tmp/dwq.0.11409646860483635/da8ef86477e10da72afc1594dede9071/build/application_tests_libc_newlib/main.o] Error 1
/tmp/dwq.0.11409646860483635/da8ef86477e10da72afc1594dede9071/tests/libc_newlib/../../Makefile.include:568: recipe for target '/tmp/dwq.0.11409646860483635/da8ef86477e10da72afc1594dede9071/build/application_tests_libc_newlib.a' failed
make: *** [/tmp/dwq.0.11409646860483635/da8ef86477e10da72afc1594dede9071/build/application_tests_libc_newlib.a] Error 2
make: Leaving directory '/tmp/dwq.0.11409646860483635/da8ef86477e10da72afc1594dede9071/tests/libc_newlib'
-- build directory size: 2.2M

gschorcht requested review from benpicco and kaspar030 April 3, 2020 22:55

benpicco approved these changes Apr 3, 2020

View reviewed changes

cgundogan added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 4, 2020

tests/libc_newlib: blacklist esp32 boards temporarily

c7dd22e

ESP32 boards must be blacklisted for `tests/libc_newlib` to avoid CI compilation errors until the toolchain is updated on all nodes.

gschorcht mentioned this pull request Apr 4, 2020

tests/libc_newlib: blacklist esp32 boards temporarily #13814

Merged

gschorcht added 2 commits April 4, 2020 13:37

cpu/esp32: use module newlib_nano

f2e776b

Revert "tests/libc_newlib: blacklist esp32 boards temporarily"

ec2fed4

This reverts commit c7dd22e.

gschorcht force-pushed the cpu/esp32/fix_newlib_nano branch from 0092f4f to ec2fed4 Compare April 4, 2020 11:38

benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 16, 2020

cgundogan added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 17, 2020

gschorcht mentioned this pull request Apr 18, 2020

cpu/esp32: replace RTC implementation by RTT implementation #13749

Merged

kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 21, 2020

gschorcht mentioned this pull request May 1, 2020

tests/pkg_ubasic: failing on esp32-wroom-32 #13984

Closed

benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels May 1, 2020

benpicco merged commit 99e8b04 into RIOT-OS:master May 1, 2020

miri64 added this to the Release 2020.07 milestone Jun 24, 2020

gschorcht deleted the cpu/esp32/fix_newlib_nano branch June 25, 2020 11:32

Conversation

gschorcht commented Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

gschorcht commented Apr 3, 2020

Uh oh!

benpicco left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benpicco commented Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benpicco commented Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gschorcht commented Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cgundogan commented Apr 4, 2020

Uh oh!

gschorcht commented Apr 4, 2020

Uh oh!

cgundogan commented Apr 4, 2020

Uh oh!

benpicco commented Apr 4, 2020

Uh oh!

cgundogan commented Apr 4, 2020

Uh oh!

kaspar030 commented Apr 14, 2020

Uh oh!

gschorcht commented Apr 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaspar030 commented Apr 21, 2020 • edited by gschorcht Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaspar030 commented Apr 21, 2020

Uh oh!

gschorcht commented Apr 21, 2020

Uh oh!

gschorcht commented Apr 21, 2020

Uh oh!

kaspar030 commented Apr 21, 2020

Uh oh!

gschorcht commented Apr 21, 2020

Uh oh!

kaspar030 commented Apr 21, 2020

Uh oh!

gschorcht commented May 2, 2020

Uh oh!

benpicco commented May 2, 2020

Uh oh!

cgundogan commented May 4, 2020

Uh oh!

kaspar030 commented May 4, 2020

Uh oh!

benpicco commented May 5, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gschorcht commented Apr 3, 2020 •

edited

Loading

benpicco left a comment •

edited

Loading

benpicco commented Apr 3, 2020 •

edited

Loading

benpicco commented Apr 3, 2020 •

edited

Loading

gschorcht commented Apr 3, 2020 •

edited

Loading

gschorcht commented Apr 14, 2020 •

edited

Loading

kaspar030 commented Apr 21, 2020 •

edited by gschorcht

Loading