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

newlib.mk: fix regressions [backport 2018.07] #9689

Merged

Conversation

cladmi
Copy link
Contributor

@cladmi cladmi commented Aug 2, 2018

Backport of #9515

Contribution description

Fixes:

  • clean the output of gcc to get compiler includes
  • Non indented comments
  • Patterns not taken into account
  • The need to have a trailing slash in NEWLIB_INCLUDE_DIR

Testing

Can be tested by compiling tests/libc_newlib:

make -C tests/libc_newlib/ BOARD=samr21-xpro clean all
make -C tests/libc_newlib/ BOARD=samr21-xpro clean all TOOLCHAIN=llvm # this should also work.

Issues/PRs references

Fixes for #9394

Split from #9512

cladmi added 6 commits August 2, 2018 18:47
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.

(cherry picked from commit a946c2c)
As NEWLIB_INCLUDE_DIR has already been set here, with an empty value, it is not
overwriting it because of the '?='.

(cherry picked from commit 18a4ccf)
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

(cherry picked from commit 3a4538e)
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
(cherry picked from commit 37a92c4)
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.

(cherry picked from commit 154d64e)
Comments inside an if are usually also indented.

(cherry picked from commit 3226918)
@cladmi cladmi added Process: release backport Integration Process: The PR is a release backport of a change previously provided to master 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 16:48
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.

Looks sensible and is the same as before

@miri64 miri64 merged commit 48f04cd into RIOT-OS:2018.07-branch Aug 2, 2018
@cladmi cladmi deleted the 2018.07/pr/make/newlib/fixandstuff branch August 2, 2018 17:19
@cladmi cladmi added this to the Release 2018.07 milestone Aug 11, 2018
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 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants