On 06/22/17 17:17, Brian Brooks wrote:
> On 06/22 11:13:57, Maxim Uvarov wrote:
>> On 22 June 2017 at 06:24, Brian Brooks <[email protected]> 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.
>>>


not clean why $(srcdir) work and $(ARCH_DIR) will not work.

I changed this in your patch and it works well:

-odpapiinclude_HEADERS += $(srcdir)/arch/x86/odp/api/cpu_arch.h
+odpapiinclude_HEADERS += $(srcdir)/arch/$(ARCH_DIR)/odp/api/cpu_arch.h

Maxim.


>>> 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 <[email protected]>
>>> ---
>>>  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