On 9 December 2016 at 04:41, Nicolas Morey-Chaisemartin <nmo...@kalray.eu> wrote:
> I'm OK with this. The only issue I have with this is that it moves back > platform/OS selection back to configure.ac while I've been trying to move > it out (https://github.com/nmorey/odp/tree/dev/generic-platforms). > Shouldn't each platform select the right OS ? > I mean linux-generic will probably always use the linux helper while the > mppa implementation will use the right one for us (depending on the > compilation flags). > I would actually like to make the helpers much more independent so I am in line with your thinking. How about we just do exactly that and have helpers build completely independently with its own configure.ac ? > > Also I think the lib should be renamed to libodphelper instead of > libodphelper-linux. > agree > > Any plans to get these patches in soon? > with your help asap > Should I wait for your patch to get in master, or get them in my patch > series? > Will work with you, let me make your suggested change to the lib and circle back with you on your first point. > > > Nicolas > > > > Le 12/08/2016 à 09:47 PM, Mike Holmes a écrit : > > 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" > > > -- 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"
