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.

Sign up for GitHub

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

Jump to bottom

newlib.mk: llvm, fix newlib-nano header not used [backport 2018.07] #9685

Merged
merged 1 commit into from
Aug 2, 2018
Merged

newlib.mk: llvm, fix newlib-nano header not used [backport 2018.07] #9685

merged 1 commit into from
Aug 2, 2018

Conversation

cladmi
Copy link
Contributor

@cladmi cladmi commented Aug 2, 2018

Backport of #9513

Contribution description

This fixes building with llvm where newlib-nano header is not used.

Basically, the default newlib directory was put before newlib-nano include directory.

Test command

The first commit can be tested to show it is broken and that the second fixes it

make TOOLCHAIN=llvm BOARD=samr21-xpro ASSERT_NEWLIB_NANO_HEADER=1 -C examples/hello-world/

Output

Without patch:

make TOOLCHAIN=llvm BOARD=samr21-xpro -C examples/hello-world/ info-debug-variable-INCLUDES

-isystem /usr/bin/../lib/gcc/arm-none-eabi/7.2.1/../../../../arm-none-eabi/include/ -nostdinc -isystem /usr/bin/../lib/gcc/arm-none-eabi/7.2.1/../../../../arm-none-eabi/include/newlib-nano -I/home/harter/work/git/RIOT/core/include -I/home/harter/work/git/RIOT/drivers/include -I/home/harter/work/git/RIOT/sys/include -I/home/harter/work/git/RIOT/boards/samr21-xpro/include -I/home/harter/work/git/RIOT/cpu/samd21/include -I/home/harter/work/git/RIOT/cpu/sam0_common/include -I/home/harter/work/git/RIOT/cpu/cortexm_common/include -I/home/harter/work/git/RIOT/cpu/cortexm_common/include/vendor -isystem /usr/bin/../lib/gcc/arm-none-eabi/7.2.1/include -isystem /usr/bin/../lib/gcc/arm-none-eabi/7.2.1/include-fixed -I/home/harter/work/git/RIOT/sys/libc/include

You can notive that 'newlib-nano' is not in front

With the fix, newlib-nano directory is in front.

-isystem /usr/bin/../lib/gcc/arm-none-eabi/7.2.1/../../../../arm-none-eabi/include/newlib-nano -isystem /usr/bin/../lib/gcc/arm-none-eabi/7.2.1/../../../../arm-none-eabi/include/ -nostdinc -I/home/harter/work/git/RIOT/core/include -I/home/harter/work/git/RIOT/drivers/include -I/home/harter/work/git/RIOT/sys/include -I/home/harter/work/git/RIOT/boards/samr21-xpro/include -I/home/harter/work/git/RIOT/cpu/samd21/include -I/home/harter/work/git/RIOT/cpu/sam0_common/include -I/home/harter/work/git/RIOT/cpu/cortexm_common/include -I/home/harter/work/git/RIOT/cpu/cortexm_common/include/vendor -isystem /usr/bin/../lib/gcc/arm-none-eabi/7.2.1/include -isystem /usr/bin/../lib/gcc/arm-none-eabi/7.2.1/include-fixed -I/home/harter/work/git/RIOT/sys/libc/include

Remove first commit before merge

Rebase before merging: This PR includes an arbitrary test for newlib-nano that I do not > want to include in this PR at the end but is required for testing.

Replaced by #9599

Issues/PRs references

Split from #9512

@cladmi
newlib.mk: llvm, fix newlib-nano header not used
270a0fe 
In the previous state, with llvm and arm for example, newlib-nano include dir
NEWLIB_NANO_INCLUDE_DIR is placed after NEWLIB_INCLUDES and so the default
'newlib.h' is used instead of the nano version.

(cherry picked from commit f8e1419)
@cladmi cladmi added Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Community: help wanted The contributors require help from other members of the community 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 labels Aug 2, 2018
@cladmi cladmi requested a review from miri64 August 2, 2018 15:04
@cladmi cladmi mentioned this pull request Aug 2, 2018
Merged
@cladmi cladmi added this to the Release 2018.07 milestone Aug 2, 2018
miri64
miri64 approved these changes Aug 2, 2018
View reviewed changes
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

ACK, behavior is described as in description and change is the same as #9513.

@cladmi cladmi merged commit 8b9231c into RIOT-OS:2018.07-branch Aug 2, 2018
@cladmi cladmi deleted the 2018.07/pr/make/llvm/nano branch August 2, 2018 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miri64 miri64 miri64 approved these changes

Assignees
No one assigned
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 Community: help wanted The contributors require help from other members of the community Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2018.07
Development

Successfully merging this pull request may close these issues.
2 participants
@cladmi @miri64