On 22 June 2017 at 06:24, Brian Brooks <brian.bro...@arm.com> wrote:
> Explicitly add all arch/<arch>/* files to respective _SOURCES
> variables instead of using @ARCH_DIR@ substitution.
>
> This patch fixes the broken build for ARM, PPC, and MIPS
> introduced by [1] and the similar issue reported while
> testing [2].
>
> From the Autoconf manual [3]:
>
> You can't put a configure substitution (e.g., '@FOO@' or
> '$(FOO)' where FOO is defined via AC_SUBST) into a _SOURCES
> variable. The reason for this is a bit hard to explain, but
> suffice to say that it simply won't work.
>
> Here be dragons..
>
> [1] https://lists.linaro.org/pipermail/lng-odp/2017-April/030324.html
> [2] https://lists.linaro.org/pipermail/lng-odp/2017-June/031598.html
> [3] https://www.gnu.org/software/automake/manual/html_node/
> Conditional-Sources.html
>
> Signed-off-by: Brian Brooks <brian.bro...@arm.com>
> ---
> configure.ac | 3 +++
> platform/linux-generic/Makefile.am | 40 ++++++++++++++++++++++++++++++
> --------
> 2 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 46c7bbd2..45812f66 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -225,6 +225,9 @@ AM_CONDITIONAL([HAVE_DOXYGEN], [test "x${DOXYGEN}" =
> "xdoxygen"])
> AM_CONDITIONAL([user_guide], [test "x${user_guides}" = "xyes" ])
> AM_CONDITIONAL([HAVE_MSCGEN], [test "x${MSCGEN}" = "xmscgen"])
> AM_CONDITIONAL([helper_linux], [test x$helper_linux = xyes ])
> +AM_CONDITIONAL([ARCH_IS_ARM], [test "x${ARCH_DIR}" = "xarm"])
> +AM_CONDITIONAL([ARCH_IS_MIPS64], [test "x${ARCH_DIR}" = "xmips64"])
> +AM_CONDITIONAL([ARCH_IS_POWERPC], [test "x${ARCH_DIR}" = "xpowerpc"])
> AM_CONDITIONAL([ARCH_IS_X86], [test "x${ARCH_DIR}" = "xx86"])
>
> ############################################################
> ##############
> diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/
> Makefile.am
> index 8dcdebd2..385c5304 100644
> --- a/platform/linux-generic/Makefile.am
> +++ b/platform/linux-generic/Makefile.am
> @@ -63,8 +63,20 @@ odpapiinclude_HEADERS = \
> $(srcdir)/include/odp/api/time.h \
> $(srcdir)/include/odp/api/timer.h \
> $(srcdir)/include/odp/api/traffic_mngr.h \
> - $(srcdir)/include/odp/api/version.h \
> - $(srcdir)/arch/@ARCH_DIR@/odp/api/cpu_arch.h
> + $(srcdir)/include/odp/api/version.h
> +
> +if ARCH_IS_ARM
> +odpapiinclude_HEADERS += $(srcdir)/arch/arm/odp/api/cpu_arch.h
> +endif
> +if ARCH_IS_MIPS64
> +odpapiinclude_HEADERS += $(srcdir)/arch/mips64/odp/api/cpu_arch.h
> +endif
> +if ARCH_IS_POWERPC
> +odpapiinclude_HEADERS += $(srcdir)/arch/powerpc/odp/api/cpu_arch.h
> +endif
> +if ARCH_IS_X86
> +odpapiinclude_HEADERS += $(srcdir)/arch/x86/odp/api/cpu_arch.h
> +endif
>
>
If - else is better to be here. Something like:
ifeq ($ARCH_IS_ARM), 1)
..
else ifeq ($ARCH_IS_MIPS64, 1)
else
unsupported
endif
It will be more nice if it would be:
ifeq ($ARCH, arm)
..
else ifeq ($ARCH, mips)
odpapiplatincludedir= $(includedir)/odp/api/plat
> odpapiplatinclude_HEADERS = \
> @@ -217,20 +229,32 @@ __LIB__libodp_linux_la_SOURCES = \
> odp_timer_wheel.c \
> odp_traffic_mngr.c \
> odp_version.c \
> - odp_weak.c \
> - arch/@ARCH_DIR@/odp_cpu_arch.c \
> - arch/@ARCH_DIR@/odp_sysinfo_parse.c
> -
> -__LIB__libodp_linux_la_LIBADD = $(ATOMIC_LIBS)
> + odp_weak.c
>
> +if ARCH_IS_ARM
> +__LIB__libodp_linux_la_SOURCES += arch/arm/odp_cpu_arch.c \
> + arch/arm/odp_sysinfo_parse.c
> +endif
> +if ARCH_IS_MIPS64
> +__LIB__libodp_linux_la_SOURCES += arch/mips64/odp_cpu_arch.c \
> + arch/mips64/odp_sysinfo_parse.c
> +endif
> +if ARCH_IS_POWERPC
> +__LIB__libodp_linux_la_SOURCES += arch/powerpc/odp_cpu_arch.c \
> + arch/powerpc/odp_sysinfo_parse.c
> +endif
> if ARCH_IS_X86
> -__LIB__libodp_linux_la_SOURCES += arch/@ARCH_DIR@/cpu_flags.c
> +__LIB__libodp_linux_la_SOURCES += arch/x86/odp_cpu_arch.c \
> + arch/x86/odp_sysinfo_parse.c \
> + arch/x86/cpu_flags.c
>
sort this.
> endif
>
> if HAVE_PCAP
> __LIB__libodp_linux_la_SOURCES += pktio/pcap.c
> endif
>
> +__LIB__libodp_linux_la_LIBADD = $(ATOMIC_LIBS)
> +
> # Create symlink for ABI header files. Application does not need to use
> the arch
> # specific include path for installed files.
> install-data-hook:
> --
> 2.13.1
>
>
