On 06/22 11:13:57, Maxim Uvarov wrote:
> 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)

Can't do this because ifeq, ifneq, ifdef, and ifndef are Makefile conditionals,
not Automake conditionals.

>  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
> >
> >

Reply via email to