So this https://github.com/mike-holmes-linaro/odp/tree/helpers-os is what I
was thinking

It moves the Linux specifics to helper/os/linux and selects the helper os
to be built in the configure step
It renames the linux.h helper include to threads.h which is what it
predominately is
It also removes the unused Linux specific  APIs since no odp test or
example uses them and they break having an OS agnostic helper suite.

On 8 December 2016 at 13:44, Mike Holmes <mike.hol...@linaro.org> wrote:

>
>
> On 7 December 2016 at 01:54, Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
> wrote:
>
>>
>>
>> Le 12/06/2016 à 09:37 PM, Mike Holmes a écrit :
>> > On 6 December 2016 at 12:08, Nicolas Morey-Chaisemartin
>> > <nmo...@kalray.eu> wrote:
>> >> Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
>> >> ---
>> >>  helper/Makefile.am      | 6 +++---
>> >>  helper/test/Makefile.am | 6 +++---
>> >>  2 files changed, 6 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/helper/Makefile.am b/helper/Makefile.am
>> >> index d09d900..c75db00 100644
>> >> --- a/helper/Makefile.am
>> >> +++ b/helper/Makefile.am
>> >> @@ -1,7 +1,7 @@
>> >>  include $(top_srcdir)/platform/@with_platform@/Makefile.inc
>> >>
>> >>  pkgconfigdir = $(libdir)/pkgconfig
>> >> -pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper-linux.pc
>> >> +pkgconfig_DATA = $(top_builddir)/pkgconfig/libo
>> dphelper@ODP_LIB_FLAVOR@.pc
>> > Why would the helpers be tied to the implementation being built ? They
>> > should be agnostic and depend on the OS
>> I agree that they should not depend on the implementation of ODP. But as
>> for libodp, helper might not be ABI compatible.
>> The issue is that there's no evident place in the connfigure to select
>> which implementation of the helper should be used.
>> platform configure.m4 are the easiest place to add this which ties this
>> to the platform.
>> But I could add an ODPH_LIB_IMPL. So several platforms could use the same
>> helper, while keeping the possibility of using non ABI compatible versions
>> in other platforms.
>>
>> > I think the helpers are broken or we need an alternative to linux.c in
>> > there, perhaps a odp/helpers/platform/linux/linux.c and
>> > odp/helpers/platform/mpaa/linux.c
>> I changed it in our git tree. helper/linux.c got moved to
>> helper/os/linux/linux.c
>> And we added helper/os/nodeos/linux.c and helper/os/mos/linux.c (we have
>> 2 OS flavors usable with ODP)
>> The platform configure.m4 is in charge of selecting the OS depending on
>> flags & compiler selected.
>>
>
> I would be keen to do the same thing master, if it is OS differences then
> that is much more in keeping with the intent.
> I will play with a patch to move linux.c under and OS directory called
> linux
>
>
>> >
>> >>  LIB   = $(top_builddir)/lib
>> >>  AM_CFLAGS  = -I$(srcdir)/include
>> >> @@ -31,7 +31,7 @@ noinst_HEADERS = \
>> >>                  $(srcdir)/odph_lineartable.h \
>> >>                  $(srcdir)/odph_list_internal.h
>> >>
>> >> -__LIB__libodphelper_linux_la_SOURCES = \
>> >> +__LIB__libodphelper@ODP_LIB_FLAVOR@_la_SOURCES = \
>> >>                                         eth.c \
>> >>                                         ip.c \
>> >>                                         chksum.c \
>> >> @@ -39,4 +39,4 @@ __LIB__libodphelper_linux_la_SOURCES = \
>> >>                                         hashtable.c \
>> >>                                         lineartable.c
>> >>
>> >> -lib_LTLIBRARIES = $(LIB)/libodphelper-linux.la
>> >> +lib_LTLIBRARIES = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la
>> >> diff --git a/helper/test/Makefile.am b/helper/test/Makefile.am
>> >> index 545db73..02c260c 100644
>> >> --- a/helper/test/Makefile.am
>> >> +++ b/helper/test/Makefile.am
>> >> @@ -28,10 +28,10 @@ EXTRA_DIST = odpthreads_as_processes
>> odpthreads_as_pthreads
>> >>
>> >>  dist_chksum_SOURCES = chksum.c
>> >>  dist_odpthreads_SOURCES = odpthreads.c
>> >> -odpthreads_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/
>> libodp-linux.la
>> >> +odpthreads_LDADD = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la
>> $(LIB)/libodp@ODP_LIB_FLAVOR@.la
>> >>  dist_thread_SOURCES = thread.c
>> >> -thread_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
>> >> +thread_LDADD = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la
>> $(LIB)/libodp@ODP_LIB_FLAVOR@.la
>> >>  dist_process_SOURCES = process.c
>> >>  dist_parse_SOURCES = parse.c
>> >> -process_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
>> >> +process_LDADD = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la
>> $(LIB)/libodp@ODP_LIB_FLAVOR@.la
>> >>  dist_table_SOURCES = table.c
>> >> --
>> >> 2.10.1.4.g0ffc436
>> >>
>> >>
>> >
>> >
>>
>>
>
>
> --
> Mike Holmes
> Program Manager - Linaro Networking Group
> Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs
> "Work should be fun and collaborative, the rest follows"
>
>
>


-- 
Mike Holmes
Program Manager - Linaro Networking Group
Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs
"Work should be fun and collaborative, the rest follows"

Reply via email to