Re: [lng-odp] [PATCH] build: fix conditional compilation of sources

2017-06-22 Thread Maxim Uvarov 
On 06/22/17 19:54, Brian Brooks wrote:
> On 06/22 19:44:45, Maxim Uvarov wrote:
>> On 06/22/17 19:19, Brian Brooks wrote:
>>> On 06/22 19:06:01, Maxim Uvarov wrote:
 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  wrote:
>>
>>> Explicitly add all 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
>>>
>>> Tried it on ARM and it breaks. If you read the Autoconf manual (above) it
>>> explicitly states that you cannot use variable substitution in _SOURCES
>>> (obviously also _HEADERS). As you point out, this is probably also only
>>> for user-defined variables (e.g. configure.ac) instead of preset output
>>> variables (e.g. srcdir).
>>>
>>
>> ok thanks. then only one comment for alphabetical reorder.
> 
> They are in alphabetical order according to arch: A > M > P > X
> Do you see something else?
> 

cpu flags before odp_

+__LIB__libodp_linux_la_SOURCES += arch/x86/odp_cpu_arch.c \
+ arch/x86/odp_sysinfo_parse.c \
+ arch/x86/cpu_flags.c

>> I think we can try to add arm-qemu to travis to also capture such bugs.
> 
> I thought there were ARM machines in LNG CI? Is there a way for users
> to trigger a CI run over there before submitting a patch?
> 

it's possible to integrate it with github. And it has to do the same
thing which travis do now. I created ticket to automation team to do it.
But looks like it's very low priority for then. I can not do it because
it requires admin rights to Jenkins I think.

Maxim.


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

Re: [lng-odp] [PATCH] build: fix conditional compilation of sources

2017-06-22 Thread Brian Brooks 
On 06/22 19:44:45, Maxim Uvarov wrote:
> On 06/22/17 19:19, Brian Brooks wrote:
> > On 06/22 19:06:01, Maxim Uvarov wrote:
> >> 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  wrote:
> 
> > Explicitly add all 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
> > 
> > Tried it on ARM and it breaks. If you read the Autoconf manual (above) it
> > explicitly states that you cannot use variable substitution in _SOURCES
> > (obviously also _HEADERS). As you point out, this is probably also only
> > for user-defined variables (e.g. configure.ac) instead of preset output
> > variables (e.g. srcdir).
> > 
> 
> ok thanks. then only one comment for alphabetical reorder.

They are in alphabetical order according to arch: A > M > P > X
Do you see something else?

> I think we can try to add arm-qemu to travis to also capture such bugs.

I thought there were ARM machines in LNG CI? Is there a way for users
to trigger a CI run over there before submitting a patch?

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

Re: [lng-odp] [PATCH] build: fix conditional compilation of sources

2017-06-22 Thread Maxim Uvarov 
On 06/22/17 19:19, Brian Brooks wrote:
> On 06/22 19:06:01, Maxim Uvarov wrote:
>> 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  wrote:

> Explicitly add all 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
> 
> Tried it on ARM and it breaks. If you read the Autoconf manual (above) it
> explicitly states that you cannot use variable substitution in _SOURCES
> (obviously also _HEADERS). As you point out, this is probably also only
> for user-defined variables (e.g. configure.ac) instead of preset output
> variables (e.g. srcdir).
> 

ok thanks. then only one comment for alphabetical reorder.

I think we can try to add arm-qemu to travis to also capture such bugs.

Maxim.


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

Re: [lng-odp] [PATCH] build: fix conditional compilation of sources

2017-06-22 Thread Brian Brooks 
On 06/22 19:06:01, Maxim Uvarov wrote:
> 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  wrote:
> >>
> >>> Explicitly add all 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

Tried it on ARM and it breaks. If you read the Autoconf manual (above) it
explicitly states that you cannot use variable substitution in _SOURCES
(obviously also _HEADERS). As you point out, this is probably also only
for user-defined variables (e.g. configure.ac) instead of preset output
variables (e.g. srcdir).

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

Re: [lng-odp] [PATCH] build: fix conditional compilation of sources

2017-06-22 Thread Maxim Uvarov 
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  wrote:
>>
>>> Explicitly add all 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 
>>> ---
>>>  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 \
>>> +

Re: [lng-odp] [PATCH] build: fix conditional compilation of sources

2017-06-22 Thread Brian Brooks 
On 06/22 11:13:57, Maxim Uvarov wrote:
> On 22 June 2017 at 06:24, Brian Brooks  wrote:
> 
> > Explicitly add all 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 
> > ---
> >  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

Re: [lng-odp] [PATCH] build: fix conditional compilation of sources

2017-06-22 Thread Maxim Uvarov 
On 22 June 2017 at 06:24, Brian Brooks  wrote:

> Explicitly add all 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 
> ---
>  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
>
>

[lng-odp] [PATCH] build: fix conditional compilation of sources

2017-06-21 Thread Brian Brooks 
Explicitly add all 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 
---
 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
 
 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
 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