newlib.mk: fix regressions #9515
Merged
Conversation
This file contains 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
vac
approved these changes
Jul 7, 2018
|
@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. |
|
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 |
|
If it is the commit that sanitize the output, I would also like to have the output of both |
cladmi
added
CI: needs squashing
|
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:) |
|
@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
|
re-introduces bug on macOS, build fails for stuff using newlib with: INCLUDES from and from master: |
|
@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
|
I added commits that write debug output. Can you post the result of a compilation with them: |
I'm using ArchLinux with the following toolchain packages installed: |
|
Can you list where or similar |
|
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 |
|
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 |
|
It's at |
|
@pyropeter can you try upgrading |
|
TL;DR: doesn't work on macOS |
|
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
|
Found it, it was replacing I tried a fixup to remove the leading spaces. Can you please retry @smlng ? I also rebased on |
|
better now: and compiling |
|
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
|
I rebased and verified that the diff with the previous version is only removing debug messages. So ready for reviewing. |
|
@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
|
Tested in Arch Linux, gcc and clang, for samr21-xpro. |
jcarrano
suggested changes
Aug 2, 2018
| # Search for Newlib include directories | ||
|
|
||
| # 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') |
There was a problem hiding this comment.
Why you assume GCC is installed?
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
|
Please provide a backport. |
|
Backport provided in #9689 |
|
I can confirm that it works fine on windows. |
|
@vac Thank you. |
cladmi
added
the
OS: Windows
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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