Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpu: make newlib_nano a DEFAULT_MODULE #16397

Merged
merged 1 commit into from May 4, 2021

Conversation

benpicco
Copy link
Contributor

Contribution description

This allows to disable nanospecs with

DISABLE_MODULE += newlib_nano

if a full-features version of newlib is desired.

Testing procedure

examples/default

   text	   data	    bss	    dec	    hex	filename
  43144	    144	  18040	  61328	   ef90	/home/benpicco/dev/RIOT/examples/default/bin/same54-xpro/default.elf

examples/default with DISABLE_MODULE += newlib_nano

   text	   data	    bss	    dec	    hex	filename
  70748	   2520	  18084	  91352	  164d8	/home/benpicco/dev/RIOT/examples/default/bin/same54-xpro/default.elf

Issues/PRs references

@benpicco benpicco added Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Apr 26, 2021
@kfessel
Copy link
Contributor

kfessel commented Apr 26, 2021

When i read the title i thought it was newlib (no nano) before and now it is newlib_nano by default.

But this is: it was always newlib_nano before, now it can be disabled

@benpicco
Copy link
Contributor Author

How do I interpret this static test failure?

BOARDS_FEATURES_MISSING= "arduino-duemilanove newlib" "arduino-nano newlib" "arduino-uno newlib" "atmega1284p newlib" "derfmega128 newlib" "mega-xplained newlib" "microduino-corerf newlib" "native newlib" "waspmote-pro newlib" "zigduino newlib"
Makefile:23: *** The CI will never build for the following boards:  arduino-duemilanove arduino-nano arduino-uno atmega1284p derfmega128 mega-xplained microduino-corerf native waspmote-pro zigduino.  Stop.

I also wonder if we shouldn't just always select newlib_nano as a DEFAULT_MODULE.
Since it's a pseudomodule it won't do anything unless newlib is also selected, but it would reduce code duplication.

@maribu
Copy link
Member

maribu commented Apr 26, 2021

That test checks whether dependency tracking is able to determine that every board is buildable for hello word. With this PR, the AVR8 are detected to no longer be build-able, as the newlib feature is not provided by them.

I think that make info-boards-supported does not reset DEFAULT_MODULES after each board. Previously, that list was independent of the boards used. Since this no longer is the case, this Makefiles need to be fixed.

I'm pretty happy that we have that build system check now :-)

@benpicco
Copy link
Contributor Author

benpicco commented May 3, 2021

Hm do you know where DEFAULT_MODULES should be reset?

@maribu
Copy link
Member

maribu commented May 4, 2021

Hm do you know where DEFAULT_MODULES should be reset?

Here:

DEFAULT_MODULE := $(DEFAULT_MODULE_GLOBAL)

But as you see, this is already done. Maybe here is the issue:

DEFAULT_MODULE_GLOBAL := $(DEFAULT_MODULE)

If DEFAULT_MODULE at the time no specific board is looked into already contains board-specific entries, things will break.

@maribu
Copy link
Member

maribu commented May 4, 2021

Out of curiosity: What is the use case for the non-nano version of newlib?

Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK.

@fjmolinas
Copy link
Contributor

ACK.

Will need #16435 to pass.

@fjmolinas fjmolinas added this to Backlog / Proposals in RIOT Sprint Day via automation May 4, 2021
@maribu
Copy link
Member

maribu commented May 4, 2021

If I recall correctly, the static tests need a rebase to include upstream changes.

This allows to disable nanospecs with

    DISABLE_MODULE += newlib_nano

if a full-features version of newlib is desired.
@benpicco benpicco merged commit 4c38f69 into RIOT-OS:master May 4, 2021
RIOT Sprint Day automation moved this from Backlog / Proposals to Done May 4, 2021
@benpicco benpicco deleted the newlib_nano-default branch May 4, 2021 15:15
@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants