[Buildroot] [PATCH 1/3] Experimental addition of the newlib library
Thomas Petazzoni
thomas.petazzoni at free-electrons.com
Sun Sep 13 08:17:38 UTC 2015
Chris,
Thanks a lot! This patch looks pretty good. I have a couple of comments
below, but overall it looks nice.
On Sun, 13 Sep 2015 03:02:46 -0400, Chris Wardman wrote:
> This patch add support for a newlib library build of the gcc toolchain.
> This is designed to build arm-none-eabi- toolchain, and it was tested against an stm32f4discovery board.
>
> Hopefully this will help people build bare metal code for arm processors
A few comments on the commit log:
* The title should be something like:
toolchain: add support for the newlib library
* The commit log text should be line wrapped at ~80 columns.
* I believe the last sentence "Hopefully this will help people build
bare metal code for arm processors" is not really useful as part of
the commit log.
> diff --git a/package/Makefile.in b/package/Makefile.in
> index 545694f..7f4d74e 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -37,10 +37,16 @@ $(error BR2_TOOLCHAIN_BUILDROOT_VENDOR cannot be 'unknown'. \
> endif
>
> # Compute GNU_TARGET_NAME
> +ifeq ($(BR2_TOOLCHAIN_NO_VENDOR),y)
> +GNU_TARGET_NAME = $(ARCH)-$(TARGET_OS)-$(LIBC)$(ABI)
Is it really mandatory to *not* have a vendor part of the tuple?
> diff --git a/package/gcc/gcc.mk b/package/gcc/gcc.mk
> index 501fcea..62f6b80 100644
> --- a/package/gcc/gcc.mk
> +++ b/package/gcc/gcc.mk
> @@ -100,6 +100,13 @@ HOST_GCC_COMMON_CONF_OPTS = \
> HOST_GCC_COMMON_CONF_ENV = \
> MAKEINFO=missing
>
> +ifeq ($(BR2_TOOLCHAIN_BUILDROOT_NEWLIB),y)
> +HOST_GCC_COMMON_CONF_OPTS += --with-gnu-as
> +else
> +HOST_GCC_COMMON_CONF_OPTS += --disable-__cxa_atexit
> +endif
While I may understand that you're adding some specific options when
BR2_TOOLCHAIN_BUILDROOT_NEWLIB=y, could you explain why you're doing
something new in the "else" case, which should already be working today?
> +
> +
Only one empty new line is necessary here.
> GCC_COMMON_TARGET_CFLAGS = $(TARGET_CFLAGS)
> GCC_COMMON_TARGET_CXXFLAGS = $(TARGET_CXXFLAGS)
>
> diff --git a/package/newlib/Config.in b/package/newlib/Config.in
> new file mode 100644
> index 0000000..0f3e2d7
> --- /dev/null
> +++ b/package/newlib/Config.in
> @@ -0,0 +1,4 @@
> +config BR2_PACKAGE_NEWLIB
> + bool
> + depends on BR2_TOOLCHAIN_USES_NEWLIB
> + default y
Use tab for the indentation of bool / depends on / default.
> diff --git a/package/newlib/newlib-0001-configure-tooldir-path.patch b/package/newlib/newlib-0001-configure-tooldir-path.patch
> new file mode 100644
> index 0000000..c162678
> --- /dev/null
> +++ b/package/newlib/newlib-0001-configure-tooldir-path.patch
> @@ -0,0 +1,25 @@
> +This patch is required to fix how the newlib headers are installed.
> +
> +The cross compiler expects headers to be in
> +.../host/usr/arm-none-eabi/sysroot/usr/include/newlib.h
> +by default newlib installed the headers into
> +.../host/usr/arm-none-eabi/sysroot/arm-none-eabi/include/newlib.h
> +
> +${exec_prefix} provides the .../host/usr/arm-none-eabi/sysroot path
> +${target_noncanonical} provides an extra arm-none-eabi/ that must be removed.
> +
> +Signed-off-by: Chris Wardman <cjwfirmware at vxmdesign.com>
> +
> +
> +diff -uNr newlib-old/configure newlib-new/configure
> +--- newlib-old/configure 2014-07-05 17:09:07.000000000 -0400
> ++++ newlib-new/configure 2014-12-25 00:59:01.727549186 -0500
> +@@ -6985,7 +6985,7 @@
> +
> + # Some systems (e.g., one of the i386-aix systems the gas testers are
> + # using) don't handle "\$" correctly, so don't use it here.
> +-tooldir='${exec_prefix}'/${target_noncanonical}
> ++tooldir='${exec_prefix}'/usr
> + build_tooldir=${tooldir}
You're patching the configure script, which we generally don't like,
since it's generated from configure.ac. So you should either patch
configure.ac and use NEWLIB_AUTORECONF = YES, or alternatively don't
patch the code, and instead use a post install hook to move around the
headers.
> diff --git a/package/newlib/newlib.mk b/package/newlib/newlib.mk
> new file mode 100644
> index 0000000..02008e5
> --- /dev/null
> +++ b/package/newlib/newlib.mk
> @@ -0,0 +1,48 @@
> +################################################################################
> +#
> +# newlib
> +#
> +################################################################################
> +
> +NEWLIB_VERSION = 2.2.0
There's a 2.2.0-1 version, that was released after 2.2.0, so presumably
we may want to use this newer version?
> +NEWLIB_SITE = ftp://sourceware.org/pub/newlib
> +NEWLIB_LICENSE = MIT
> +NEWLIB_LICENSE_FILES = COPYRIGHT
> +
> +NEWLIB_DEPENDENCIES = host-gcc-initial
> +NEWLIB_ADD_TOOLCHAIN_DEPENDENCY = NO
> +NEWLIB_INSTALL_STAGING = YES
> +
> +define NEWLIB_CONFIGURE_CMDS
> + (cd $(@D); \
> + $(TARGET_MAKE_ENV) \
> + ./configure \
> + --target=$(GNU_TARGET_NAME) \
> + --host=$(GNU_HOST_NAME) \
> + --build=$(GNU_HOST_NAME) \
> + --prefix=$(STAGING_DIR) \
> + --includedir=$(STAGING_DIR)/usr/include \
> + --oldincludedir=$(STAGING_DIR)/usr/include \
> + --with-build-sysroot=$(STAGING_DIR) \
> + --enable-newlib-io-long-long \
> + --enable-newlib-register-fini \
> + --disable-newlib-supplied-syscalls \
> + --disable-nls)
Why don't you use the autotools-package infrastructure? newlib is using
autoconf, so I believe it does make sense to use the autotools-package
infrastructure.
> +define NEWLIB_APPLY_PATCHES
> + $(APPLY_PATCHES) $(@D) package/newlib \*.patch
> +endef
This does not have any effect, and is not necessary: patches in
package/<pkg>/ are automatically applied.
> +define NEWLIB_BUILD_CMDS
> + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
> +endef
Not needed if you use the autotools-package infrastructure.
> +define NEWLIB_INSTALL_STAGING_CMDS
> + mkdir -p $(HOST_DIR)/usr/$(GNU_TARGET_NAME)/lib
> + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) install
Except the mkdir, the install command is not needed if you use the
autotools-package infrastructure.
Thanks a lot!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
More information about the buildroot
mailing list