Merged
Conversation
vac
approved these changes
Jul 7, 2018
Contributor
Author
|
@vac Nice to hear that. To have some more information on your windows setup, could you give me the values of diff --git a/makefiles/libc/newlib.mk b/makefiles/libc/newlib.mk
index e29b240..5335580 100644
--- a/makefiles/libc/newlib.mk
+++ b/makefiles/libc/newlib.mk
@@ -31,6 +31,8 @@ ifeq (,$(NEWLIB_INCLUDE_DIR))
NEWLIB_INCLUDE_DIR := $(firstword $(abspath $(dir $(wildcard $(addsuffix /newlib.h, $(COMPILER_INCLUDE_PATHS))))))
endif
+$(info 1 - NEWLIB_INCLUDE_DIR: $(NEWLIB_INCLUDE_DIR))
+
ifeq (,$(NEWLIB_INCLUDE_DIR))
# Since Clang is not installed as a separate instance for each crossdev target
# we need to tell it where to look for platform specific includes (Newlib
@@ -59,12 +61,16 @@ ifeq (,$(NEWLIB_INCLUDE_DIR))
NEWLIB_INCLUDE_DIR := $(firstword $(abspath $(dir $(wildcard $(addsuffix /newlib.h, $(NEWLIB_INCLUDE_PATTERNS))))))
endif
+$(info 2 - NEWLIB_INCLUDE_DIR: $(NEWLIB_INCLUDE_DIR))
+
# If nothing was found we will try to fall back to searching for a cross-gcc in
# the current PATH and use a relative path for the includes
ifeq (,$(NEWLIB_INCLUDE_DIR))
NEWLIB_INCLUDE_DIR := $(abspath $(wildcard $(dir $(shell command -v $(PREFIX)gcc 2>/dev/null))/../$(TARGET_ARCH)/include))
endif
+$(info 3 - NEWLIB_INCLUDE_DIR: $(NEWLIB_INCLUDE_DIR))
+
ifeq ($(TOOLCHAIN),llvm)
# A cross GCC already knows the target libc include path (build-in at compile time)
# but Clang, when cross-compiling, needs to be told where to find the headersI am interested to know what is used for you. |
|
@cladmi, As I write you yesterday your first commit fixes issues in my case. When I've applied all of your commits (+debug logs) it is not working and the output is: I think that the problem came with When I've applied all of your commits + revert changes from 4th commit it works and the output is different than in first case: I hope that it would be somehow helpful. |
Contributor
Author
|
That is really helpful. Having them in different drives is perfectly fine and shows more problems :) Could you also tell me which commit produced a value for the first debug output, and what was the output in that case. Working or not does not matter. When searching for zephyrproject-rtos/zephyr#2061 (comment) Which basically is the problem faced here. Unfortunately the link is not working but it looks like it is this zephyrproject-rtos/zephyr@941059c by using I could also just remove |
Contributor
Author
|
If it is the commit that sanitize the output, I would also like to have the output of both |
cladmi
added
CI: needs squashing
Contributor
Author
|
I pushed changes to use |
|
this is the minimal change that fixes compilation on Windows, in my case: Index: newlib.mk
===================================================================
--- newlib.mk (revision 503)
+++ newlib.mk (revision 504)
@@ -72,9 +72,9 @@
endif
ifeq (1,$(USE_NEWLIB_NANO))
- NEWLIB_NANO_INCLUDE_DIR ?= $(firstword $(wildcard $(NEWLIB_INCLUDE_DIR)newlib-nano \
- $(NEWLIB_INCLUDE_DIR)newlib/nano \
- $(NEWLIB_INCLUDE_DIR)nano))
+ NEWLIB_NANO_INCLUDE_DIR ?= $(firstword $(wildcard $(NEWLIB_INCLUDE_DIR)/newlib-nano \
+ $(NEWLIB_INCLUDE_DIR)/newlib/nano \
+ $(NEWLIB_INCLUDE_DIR)/nano))
# newlib-nano overrides newlib.h and its include dir should therefore go before
# the regular system include dirs.
INCLUDES := -isystem $(NEWLIB_NANO_INCLUDE_DIR) $(INCLUDES)I was about to make such pull request when I found yours - which have exactly the same commit: 79dad7c So 79dad7c is the change that you asking for - first output. |
|
Here is the output of commands that you asked for: And the second one: ...yes, the output of second one is empty |
|
I can confirm that changing abspath to realpath fixes my Windows issues:) |
Contributor
Author
|
@vac For the output, it could be an issue with the It's great that |
cladmi
force-pushed
the
pr/make/newlib/fixandstuff
branch
from
7a6057d to
62b463f
Compare
Member
|
re-introduces bug on macOS, build fails for stuff using newlib with: INCLUDES from and from master: |
Contributor
Author
|
@smlng Thanks for your feedback. Could you give the output with this diff applied: #9515 (comment) And give which commit broke it ? Because on |
This was referenced Jul 16, 2018
Contributor
Author
|
I added commits that write debug output. Can you post the result of a compilation with them: |
Contributor
I'm using ArchLinux with the following toolchain packages installed: |
Contributor
Author
|
Can you list where or similar |
Contributor
Author
|
It looks like it has been fixed on arm-none-eabi-newlib 3.0 so a newer version than yours https://bugs.archlinux.org/task/50481 |
Contributor
|
This is my output on OS X: HEAD is now at 447066ee5 fixup! fixup! REMOVE ME: add warnings for debug
snake:RIOT facosta$ make -C examples/hello-world/ BOARD=samr21-xpro
make: Entering directory '/Users/facosta/git/RIOT-OS/RIOT/examples/hello-world'
Warning: no PORT set!
Using built-in specs.
COLLECT_GCC=arm-none-eabi-gcc
Target: arm-none-eabi
Configured with: /Users/build/work/GCC-7-build/src/gcc/configure --target=arm-none-eabi --prefix=/Users/build/work/GCC-7-build/install-native --libexecdir=/Users/build/work/GCC-7-build/install-native/lib --infodir=/Users/build/work/GCC-7-build/install-native/share/doc/gcc-arm-none-eabi/info --mandir=/Users/build/work/GCC-7-build/install-native/share/doc/gcc-arm-none-eabi/man --htmldir=/Users/build/work/GCC-7-build/install-native/share/doc/gcc-arm-none-eabi/html --pdfdir=/Users/build/work/GCC-7-build/install-native/share/doc/gcc-arm-none-eabi/pdf --enable-languages=c,c++ --enable-plugins --disable-decimal-float --disable-libffi --disable-libgomp --disable-libmudflap --disable-libquadmath --disable-libssp --disable-libstdcxx-pch --disable-nls --disable-shared --disable-threads --disable-tls --with-gnu-as --with-gnu-ld --with-newlib --with-headers=yes --with-python-dir=share/gcc-arm-none-eabi --with-sysroot=/Users/build/work/GCC-7-build/install-native/arm-none-eabi --build=x86_64-apple-darwin10 --host=x86_64-apple-darwin10 --with-gmp=/Users/build/work/GCC-7-build/build-native/host-libs/usr --with-mpfr=/Users/build/work/GCC-7-build/build-native/host-libs/usr --with-mpc=/Users/build/work/GCC-7-build/build-native/host-libs/usr --with-isl=/Users/build/work/GCC-7-build/build-native/host-libs/usr --with-libelf=/Users/build/work/GCC-7-build/build-native/host-libs/usr --with-host-libstdcxx='-static-libgcc -Wl,-lstdc++ -lm' --with-pkgversion='GNU Tools for Arm Embedded Processors 7-2018-q2-update' --with-multilib-list=rmprofile
Thread model: single
gcc version 7.3.1 20180622 (release) [ARM/embedded-7-branch revision 261907] (GNU Tools for Arm Embedded Processors 7-2018-q2-update)
COLLECT_GCC_OPTIONS='-v' '-E'
/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/cc1 -E -quiet -v -iprefix /usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/ -isysroot /usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../arm-none-eabi -D__USES_INITFINI__ /dev/null
ignoring duplicate directory "/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/../../lib/gcc/arm-none-eabi/7.3.1/include"
ignoring nonexistent directory "/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../arm-none-eabi/usr/local/include"
ignoring duplicate directory "/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/../../lib/gcc/arm-none-eabi/7.3.1/include-fixed"
ignoring duplicate directory "/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/../../lib/gcc/arm-none-eabi/7.3.1/../../../../arm-none-eabi/include"
ignoring nonexistent directory "/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../arm-none-eabi/usr/include"
#include "..." search starts here:
#include <...> search starts here:
/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/include
/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/include-fixed
/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/../../../../arm-none-eabi/include
End of search list.
COMPILER_PATH=/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/:/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/:/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/../../../../arm-none-eabi/bin/
LIBRARY_PATH=/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/:/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/:/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/../../../../arm-none-eabi/lib/:/usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../arm-none-eabi/lib/
COLLECT_GCC_OPTIONS='-v' '-E'
/Users/facosta/git/RIOT-OS/RIOT/makefiles/libc/newlib.mk:42: 0 - COMPILER_INCLUDE_PATHS
/Users/facosta/git/RIOT-OS/RIOT/makefiles/libc/newlib.mk:43: 0 - /usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/include:
/Users/facosta/git/RIOT-OS/RIOT/makefiles/libc/newlib.mk:43: 0 - /usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/include-fixed:
/Users/facosta/git/RIOT-OS/RIOT/makefiles/libc/newlib.mk:43: 0 - /usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/../../../../arm-none-eabi/include: /usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/bin/../lib/gcc/arm-none-eabi/7.3.1/../../../../arm-none-eabi/include/newlib.h
/Users/facosta/git/RIOT-OS/RIOT/makefiles/libc/newlib.mk:44: 1 - NEWLIB_INCLUDE_DIR: /usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/arm-none-eabi/include
/Users/facosta/git/RIOT-OS/RIOT/makefiles/libc/newlib.mk:74: 2 - NEWLIB_INCLUDE_DIR: /usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/arm-none-eabi/include
/Users/facosta/git/RIOT-OS/RIOT/makefiles/libc/newlib.mk:82: 3 - NEWLIB_INCLUDE_DIR: /usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/arm-none-eabi/include
/Users/facosta/git/RIOT-OS/RIOT/makefiles/libc/newlib.mk:98: 4 - NEWLIB_NANO_INCLUDE_DIR: /usr/local/Cellar/arm-none-eabi-gcc/7-2018-q2-update/gcc/arm-none-eabi/include/newlib-nano
Building application "hello-world" for "samr21-xpro" with MCU "samd21".
"make" -C /Users/facosta/git/RIOT-OS/RIOT/boards/samr21-xpro
"make" -C /Users/facosta/git/RIOT-OS/RIOT/core
"make" -C /Users/facosta/git/RIOT-OS/RIOT/cpu/samd21
"make" -C /Users/facosta/git/RIOT-OS/RIOT/cpu/cortexm_common
"make" -C /Users/facosta/git/RIOT-OS/RIOT/cpu/cortexm_common/periph
"make" -C /Users/facosta/git/RIOT-OS/RIOT/cpu/sam0_common
"make" -C /Users/facosta/git/RIOT-OS/RIOT/cpu/sam0_common/periph
"make" -C /Users/facosta/git/RIOT-OS/RIOT/cpu/samd21/periph
"make" -C /Users/facosta/git/RIOT-OS/RIOT/drivers
"make" -C /Users/facosta/git/RIOT-OS/RIOT/drivers/periph_common
"make" -C /Users/facosta/git/RIOT-OS/RIOT/sys
"make" -C /Users/facosta/git/RIOT-OS/RIOT/sys/auto_init
"make" -C /Users/facosta/git/RIOT-OS/RIOT/sys/isrpipe
"make" -C /Users/facosta/git/RIOT-OS/RIOT/sys/newlib_syscalls_default
"make" -C /Users/facosta/git/RIOT-OS/RIOT/sys/pm_layered
"make" -C /Users/facosta/git/RIOT-OS/RIOT/sys/tsrb
"make" -C /Users/facosta/git/RIOT-OS/RIOT/sys/uart_stdio
text data bss dec hex filename
8624 140 2740 11504 2cf0 /Users/facosta/git/RIOT-OS/RIOT/examples/hello-world/bin/samr21-xpro/hello-world.elf
make: Leaving directory '/Users/facosta/git/RIOT-OS/RIOT/examples/hello-world'With this configuration: Installed compiler toolchains
-----------------------------
native gcc: Apple LLVM version 8.0.0 (clang-800.0.42.1)
arm-none-eabi-gcc: arm-none-eabi-gcc (GNU Tools for Arm Embedded Processors 7-2018-q2-update) 7.3.1 20180622 (release) [ARM/embedded-7-branch revision 261907]
avr-gcc: avr-gcc (GCC) 8.1.0
mips-mti-elf-gcc: missing
msp430-gcc: msp430-gcc (GCC) 4.7.0 20120322 (mspgcc dev 20120911)
riscv-none-embed-gcc: missing
clang: Apple LLVM version 8.0.0 (clang-800.0.42.1)
Installed compiler libs
-----------------------
arm-none-eabi-newlib: "3.0.0"
mips-mti-elf-newlib: missing
riscv-none-embed-newlib: missing
avr-libc: "2.0.0" ("20150208")
Installed development tools
---------------------------
cmake: cmake version 3.11.4
cppcheck: missing
doxygen: missing
flake8: missing
git: git version 2.18.0
coccinelle: missing |
Contributor
|
It's at |
Contributor
Author
|
@pyropeter can you try upgrading |
Contributor
Author
Member
|
TL;DR: doesn't work on macOS |
Contributor
Author
|
It was working on @kYc0o mac, so there must be something else. |
cladmi
force-pushed
the
pr/make/newlib/fixandstuff
branch
from
f0ee69b to
1257134
Compare
Contributor
Author
|
Found it, it was replacing I tried a fixup to remove the leading spaces. Can you please retry @smlng ? I also rebased on |
Member
|
better now: and compiling |
Contributor
Author
|
Thanks. I will squash and remove the debug commit for final reviews. |
cladmi
force-pushed
the
pr/make/newlib/fixandstuff
branch
from
1257134 to
2c19189
Compare
cladmi
removed
the
CI: needs squashing
Contributor
Author
|
I rebased and verified that the diff with the previous version is only removing debug messages. So ready for reviewing. |
Contributor
Author
|
@vac If you are here could you reverify on windows too, just to be sure ? |
When NEWLIB_INCLUDE_DIR is set from other parts than 'COMPILER_INCLUDE_PATHS' it does not have a trailing slash. Also, it makes it more problematic when supplying it from the command line. And anyway having two '/' does not break anything.
As NEWLIB_INCLUDE_DIR has already been set here, with an empty value, it is not overwriting it because of the '?='.
Only keep lines that are indeed include path.
It also keeps newlines as they do not matter.
It fixes Mingw32 support where `grep '^\s'` is not working the same way.
It also handles some mac `sed` that do not support `\s`.
Ouput tested with:
make -C examples/hello-world BOARD=samr21-xpro info-debug-variable-COMPILER_INCLUDE_PATHS
# by also putting newlines for readability
Now:
/usr/bin/../lib/gcc/arm-none-eabi/7.2.1/include
/usr/bin/../lib/gcc/arm-none-eabi/7.2.1/include-fixed
/usr/bin/../lib/gcc/arm-none-eabi/7.2.1/../../../../arm-none-eabi/include
Before:
/usr/bin/../lib/gcc/arm-none-eabi/7.2.1/cc1 -E -quiet -v -iprefix /usr/bin/../lib/gcc/arm-none-eabi/7.2.1/ -isysroot /usr/bin/../arm-none-eabi -D__USES_INITFINI__ /dev/null
/usr/bin/../lib/gcc/arm-none-eabi/7.2.1/include
/usr/bin/../lib/gcc/arm-none-eabi/7.2.1/include-fixed
/usr/bin/../lib/gcc/arm-none-eabi/7.2.1/../../../../arm-none-eabi/include
Some versions of Mingw32 abspath implementation has trouble working with
windows formatted path.
$(abspath "C:/A/B") returns "/C/CUR/DIR/C:/A/B" instead of "/C/A/B"
relpath does not have this problem, it does additional symlink resolution but is
not a problem.
Note: on windows it does not remove the trailing `/`.
zephyrproject-rtos/zephyr#2061 (comment)
Patched in
zephyrproject-rtos/zephyr@941059c
It replaces
make BOARD=iotlab-m3 info-debug-variable-NEWLIB_INCLUDE_DIR
/usr/bin/../lib/gcc/arm-none-eabi/7.2.1/../../../../arm-none-eabi/include/
with
make BOARD=iotlab-m3 info-debug-variable-NEWLIB_INCLUDE_DIR
/usr/arm-none-eabi/include
Without trailing slash and without relative '..' everywhere.
It also uses `realpath` instead of `abspath` to support Mingw32.
Comments inside an if are usually also indented.
cladmi
force-pushed
the
pr/make/newlib/fixandstuff
branch
from
2c19189 to
3226918
Compare
Contributor
|
Tested in Arch Linux, gcc and clang, for samr21-xpro. |
jcarrano
suggested changes
Aug 2, 2018
Contributor
jcarrano
left a comment
jcarrano
left a comment
There was a problem hiding this comment.
Good, but the script assumes GCC is present.
|
|
||
| # Try to search for newlib in the standard search path of the compiler for includes | ||
| ifeq (,$(NEWLIB_INCLUDE_DIR)) | ||
| COMPILER_INCLUDE_PATHS := $(shell $(PREFIX)gcc -v -x c -E /dev/null 2>&1 | grep '^\s' | tr -d '\n') |
Contributor
There was a problem hiding this comment.
Why you assume GCC is installed?
Contributor
Author
There was a problem hiding this comment.
It was already the case before.
Also it is not possible to compile for llvm without gcc.
https://github.com/RIOT-OS/RIOT/blob/master/makefiles/toolchain/llvm.inc.mk
https://github.com/RIOT-OS/RIOT/blob/master/makefiles/libc/newlib.mk#L65
jcarrano
added
the
CI: ready for build
Member
|
Please provide a backport. |
Contributor
Author
|
Backport provided in #9689 |
|
I can confirm that it works fine on windows. |
Contributor
Author
|
@vac Thank you. |
cladmi
added
the
OS: Windows
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 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
Fixes:
Testing
Can be tested by compiling
tests/libc_newlib:Issues/PRs references
Fixes for #9394
Split from #9512