newlib.mk: llvm, fix newlib-nano header not used #9513
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cladmi
added
the
CI: needs squashing
cladmi
changed the title
cladmi
added
Type: bug
4 tasks
smlng
approved these changes
Jul 12, 2018
Member
smlng
left a comment
smlng
left a comment
There was a problem hiding this comment.
(still) works on macOS, but INCLUDES do not differ from master
INCLUDES:
-isystem
/usr/local/Caskroom/gcc-arm-embedded/7-2017-q4-major/gcc-arm-none-eabi-7-2017-q4-major/bin/../lib/gcc/arm-none-eabi/7.2.1/../../../../arm-none-eabi/include/newlib-nano
-I/Volumes/devel/github/smlng/RIOT/core/include
-I/Volumes/devel/github/smlng/RIOT/drivers/include
-I/Volumes/devel/github/smlng/RIOT/sys/include
-I/Volumes/devel/github/smlng/RIOT/boards/samr21-xpro/include
-I/Volumes/devel/github/smlng/RIOT/cpu/samd21/include
-I/Volumes/devel/github/smlng/RIOT/cpu/sam0_common/include
-I/Volumes/devel/github/smlng/RIOT/cpu/cortexm_common/include
-I/Volumes/devel/github/smlng/RIOT/cpu/cortexm_common/include/vendor
-I/Volumes/devel/github/smlng/RIOT/sys/libc/include
-I/Volumes/devel/github/smlng/RIOT/sys/net/gnrc/netif/include
-I/Volumes/devel/github/smlng/RIOT/drivers/at86rf2xx/include
Member
|
btw. used |
Contributor
Author
|
@smlng Can you try the test command in the description to force using |
This was referenced Jul 23, 2018
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.
cladmi
force-pushed
the
pr/make/llvm/nano
branch
from
37adfc3 to
f8e1419
Compare
Contributor
Author
|
I removed the sanity check commit now that #9599 is merged. |
cladmi
removed
the
CI: needs squashing
Contributor
Author
|
It is required for enabling |
cladmi
added
the
Process: needs backport
Contributor
Author
|
@smlng up, can you retry with TOOLCHAIN=llvm ? I would like to have it in the release |
Member
|
I retested on macOS but got several issues, used native macOSincludes but error when compiling setting However: fails on master, too 😟 samr21-xpro, macOSNote no toolchain set includes: compiling, all good: samr21-xpro, macOS, llvmincludes (note: linker is gcc): compiling |
Member
It's a known problem. Also irrelevant for this PR since |
Member
|
(the fix is provided within #9398, but I can cherry-pick it out of there if desired) |
Member
|
See #9513 |
Contributor
Author
|
@smlng Thank you for testing and the detailed output, it works as I expected.
The other native issue was unrelated and fixed. I still let you hit merge in case you had any other remarks. |
Member
@cladmi does it make sense to change the |
Contributor
Author
|
@miri64: for me it is on purpose to link with GCC according to the comment, so it should be investigated before changing it: |
Member
|
Ah... I did not read the comment ^^" |
Member
|
(yepp, linking doesn't work then for newlib-supported boards :-/) |
Contributor
Author
|
Then it could be only for non |
miri64
added a commit
to miri64/RIOT
that referenced
this pull request
Aug 1, 2018
cladmi
added
the
Community: help wanted
Member
|
I guess this can be merged?!? |
miri64
added
CI: ready for build
Member
Member
|
does work on macOS, I get a bit confused with all those newlib PRs open here 😕, the other is #9515 right? |
Member
|
I interpret this as a "yes" ;-). |
Contributor
Author
|
@smlng yes the other is a different one for the newly introduced issues. This ones fixed a loooooooong lasting bug. |
Contributor
Author
|
Backport provided in #9685 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Output
Without patch:
You can notive that 'newlib-nano' is not in front
With the fix, newlib-nano directory is in front.
Replaced by #9599
Issues/PRs references
Split from #9512