Hi Mike,
I do like what's going on here...
I have to admit my experience in debian package creation is limited, so I
don't know whether I should mark it as reviewed or not when there are
obscure zones for me...
At least I have a few extra questions:
On 6 July 2015 at 01:08, Mike Holmes <mike.hol...@linaro.org> wrote:
> Remove the need to build helper source files into the linux-generic
> library by converting helpers to be their own library.
>
> This removes the need for all other platforms to also build in the
> helpers which are optional just to run the tests.
>
> Signed-off-by: Mike Holmes <mike.hol...@linaro.org>
> ---
> .gitignore | 2 +-
> Makefile.am | 3 ++-
> configure.ac | 1 +
> debian/libodphelper-dev.dirs | 2 ++
> debian/libodphelper-dev.install | 4 ++++
> debian/libodphelper.dirs | 1 +
> debian/libodphelper.install | 1 +
> example/Makefile.inc | 2 +-
> helper/Makefile.am | 30 +++++++++++++++++++++++++++++-
> helper/test/Makefile.am | 2 ++
> pkgconfig/libodphelper.pc.in | 11 +++++++++++
> platform/linux-generic/Makefile.am | 16 ----------------
> test/Makefile.inc | 2 +-
> test/validation/Makefile.inc | 2 +-
> 14 files changed, 57 insertions(+), 22 deletions(-)
> create mode 100644 debian/libodphelper-dev.dirs
> create mode 100644 debian/libodphelper-dev.install
> create mode 100644 debian/libodphelper.dirs
> create mode 100644 debian/libodphelper.install
> create mode 100644 pkgconfig/libodphelper.pc.in
>
> diff --git a/.gitignore b/.gitignore
> index 6222fd9..4dbf28e 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -24,7 +24,7 @@ missing
> config.log
> config.status
> libtool
> -pkgconfig/libodp.pc
> +pkgconfig/libodp*.pc
> .deps/
> cscope.out
> tags
> diff --git a/Makefile.am b/Makefile.am
> index 2c8a9d6..7ce3a3c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -3,9 +3,10 @@ AUTOMAKE_OPTIONS = foreign
>
> #@with_platform@ works alone in subdir but not as part of a path???
> SUBDIRS = @platform_with_platform@ \
> + helper \
> test \
> @platform_with_platform_test@ \
> - helper \
> + helper/test \
> doc \
> example \
> scripts
> diff --git a/configure.ac b/configure.ac
> index 28dad3b..29fcb18 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -296,6 +296,7 @@ AC_CONFIG_FILES([Makefile
> helper/Makefile
> helper/test/Makefile
> pkgconfig/libodp.pc
> + pkgconfig/libodphelper.pc
> platform/linux-generic/Makefile
> platform/linux-generic/test/pktio/Makefile
> scripts/Makefile
> diff --git a/debian/libodphelper-dev.dirs b/debian/libodphelper-dev.dirs
> new file mode 100644
> index 0000000..4418816
> --- /dev/null
> +++ b/debian/libodphelper-dev.dirs
> @@ -0,0 +1,2 @@
> +usr/lib
> +usr/include
> diff --git a/debian/libodphelper-dev.install
> b/debian/libodphelper-dev.install
> new file mode 100644
> index 0000000..b973af4
> --- /dev/null
> +++ b/debian/libodphelper-dev.install
> @@ -0,0 +1,4 @@
> +usr/include/*
> +usr/lib/*/lib*.so
> +usr/lib/*/lib*.a
> +usr/lib/*/pkgconfig/*
> diff --git a/debian/libodphelper.dirs b/debian/libodphelper.dirs
> new file mode 100644
> index 0000000..6845771
> --- /dev/null
> +++ b/debian/libodphelper.dirs
> @@ -0,0 +1 @@
> +usr/lib
> diff --git a/debian/libodphelper.install b/debian/libodphelper.install
> new file mode 100644
> index 0000000..3ddde58
> --- /dev/null
> +++ b/debian/libodphelper.install
> @@ -0,0 +1 @@
> +usr/lib/*/lib*.so.*
> diff --git a/example/Makefile.inc b/example/Makefile.inc
> index b3a9706..e1c1cb7 100644
> --- a/example/Makefile.inc
> +++ b/example/Makefile.inc
> @@ -1,7 +1,7 @@
> include $(top_srcdir)/Makefile.inc
> include $(top_srcdir)/platform/@with_platform@/Makefile.inc
> LIB = $(top_builddir)/lib
> -LDADD = $(LIB)/libodp.la
> +LDADD = $(LIB)/libodp.la $(LIB)/libodphelper.la
should libodphelper.la be located in $(LIB)/, really?. shouldn't it be
somewhere under helper instead? after make install, they will be together,
yes, but at this time... This is a question, Mike... I probably just need a
explanation...
> AM_CFLAGS += \
> -I$(srcdir) \
> -I$(top_srcdir)/example \
> diff --git a/helper/Makefile.am b/helper/Makefile.am
> index 02af5b3..93f57a4 100644
> --- a/helper/Makefile.am
> +++ b/helper/Makefile.am
> @@ -1 +1,29 @@
> -SUBDIRS = test
> +pkgconfigdir = $(libdir)/pkgconfig
> +pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper.pc
> +
> +LIB = $(top_builddir)/lib
> +AM_CFLAGS = -I$(top_srcdir)/helper/include
> +AM_CFLAGS += -I$(top_srcdir)/platform/@with_platform@/include
> +AM_CFLAGS += -I$(top_srcdir)/platform/linux-generic/include
>
Maybe a simple question, but why do you need to include both the
@with_platform@ AND linux-generic?
+AM_CFLAGS += -I$(top_srcdir)/include
> +
> +include_HEADERS = \
> + $(top_srcdir)/helper/include/odp/helper/ring.h \
> + $(top_srcdir)/helper/include/odp/helper/linux.h \
> + $(top_srcdir)/helper/include/odp/helper/chksum.h\
> + $(top_srcdir)/helper/include/odp/helper/eth.h\
> + $(top_srcdir)/helper/include/odp/helper/icmp.h\
> + $(top_srcdir)/helper/include/odp/helper/ip.h\
> + $(top_srcdir)/helper/include/odp/helper/ipsec.h\
> + $(top_srcdir)/helper/include/odp/helper/tcp.h\
> + $(top_srcdir)/helper/include/odp/helper/udp.h
>
Is there any reason I don't understand for locating these headers file in "
$(top_srcdir)/helper/include/odp/helper" rather than
"$(top_srcdir)/helper/include/"? .I am aware this is not new to this patch,
but I'd like to understand...
+
> +noinst_HEADERS = \
> + $(top_srcdir)/helper/odph_debug.h \
> + $(top_srcdir)/helper/odph_pause.h
> +
> +__LIB__libodphelper_la_SOURCES = \
> + linux.c \
> + ring.c
> +
> +lib_LTLIBRARIES = $(LIB)/libodphelper.la
> diff --git a/helper/test/Makefile.am b/helper/test/Makefile.am
> index 9ac82eb..aed97cc 100644
> --- a/helper/test/Makefile.am
> +++ b/helper/test/Makefile.am
> @@ -24,4 +24,6 @@ bin_PROGRAMS = $(EXECUTABLES) $(COMPILE_ONLY)
>
> dist_odp_chksum_SOURCES = odp_chksum.c
> dist_odp_thread_SOURCES = odp_thread.c
> +odp_thread_LDADD = $(LIB)/libodphelper.la $(LIB)/libodp.la
> dist_odp_process_SOURCES = odp_process.c
> +odp_process_LDADD = $(LIB)/libodphelper.la $(LIB)/libodp.la
> diff --git a/pkgconfig/libodphelper.pc.in b/pkgconfig/libodphelper.pc.in
> new file mode 100644
> index 0000000..2993d71
> --- /dev/null
> +++ b/pkgconfig/libodphelper.pc.in
> @@ -0,0 +1,11 @@
> +prefix=@prefix@
> +exec_prefix=@exec_prefix@
> +libdir=@libdir@
> +includedir=@includedir@
> +
> +Name: libodphelper
> +Description: Helper for the ODP packet processing engine
> +Version: @VERSION@
> +Libs: -L${libdir} -lodphelper
> +Libs.private:
> +Cflags: -I${includedir}
> diff --git a/platform/linux-generic/Makefile.am
> b/platform/linux-generic/Makefile.am
> index 4f2063f..8cf03b2 100644
> --- a/platform/linux-generic/Makefile.am
> +++ b/platform/linux-generic/Makefile.am
> @@ -127,20 +127,6 @@ noinst_HEADERS = \
>
> ${top_srcdir}/platform/linux-generic/include/odp_timer_internal.h \
> ${top_srcdir}/platform/linux-generic/Makefile.inc
>
> -subdirheadersdir = $(includedir)/odp/helper
> -subdirheaders_HEADERS = \
> - $(top_srcdir)/helper/include/odp/helper/chksum.h \
> - $(top_srcdir)/helper/include/odp/helper/eth.h \
> - $(top_srcdir)/helper/include/odp/helper/icmp.h \
> - $(top_srcdir)/helper/include/odp/helper/ip.h \
> - $(top_srcdir)/helper/include/odp/helper/ipsec.h \
> - $(top_srcdir)/helper/include/odp/helper/linux.h \
> - $(top_srcdir)/helper/include/odp/helper/ring.h \
> - $(top_srcdir)/helper/include/odp/helper/tcp.h \
> - $(top_srcdir)/helper/include/odp/helper/udp.h \
> - $(top_srcdir)/helper/odph_debug.h \
> - $(top_srcdir)/helper/odph_pause.h
> -
> __LIB__libodp_la_SOURCES = \
> odp_barrier.c \
> odp_buffer.c \
> @@ -151,14 +137,12 @@ __LIB__libodp_la_SOURCES = \
> odp_event.c \
> odp_init.c \
> odp_impl.c \
> - ../../helper/linux.c \
>
Oh, that feels good :-)
> odp_packet.c \
> odp_packet_flags.c \
> odp_packet_io.c \
> odp_packet_socket.c \
> odp_pool.c \
> odp_queue.c \
> - ../../helper/ring.c \
>
Oh, that feels good :-)
Thanks! - christophe.
> odp_rwlock.c \
> odp_schedule.c \
> odp_shared_memory.c \
> diff --git a/test/Makefile.inc b/test/Makefile.inc
> index 1eb6ed5..2fa61e4 100644
> --- a/test/Makefile.inc
> +++ b/test/Makefile.inc
> @@ -5,7 +5,7 @@ LIB = $(top_builddir)/lib
> #in the following line, the libs using the symbols should come before
> #the libs containing them! The includer is given a chance to add things
> #before libodp by setting PRE_LDADD before the inclusion.
> -LDADD = $(PRE_LDADD) $(LIB)/libodp.la
> +LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp.la
>
> INCFLAGS = -I$(srcdir) \
> -I$(top_srcdir)/test \
> diff --git a/test/validation/Makefile.inc b/test/validation/Makefile.inc
> index 3cdc6a7..31729b8 100644
> --- a/test/validation/Makefile.inc
> +++ b/test/validation/Makefile.inc
> @@ -4,4 +4,4 @@ AM_CFLAGS += -I$(top_srcdir)/test/validation/common
> AM_LDFLAGS += -static
>
> LIBCUNIT_COMMON = $(top_builddir)/test/validation/common/libcunit_common.a
> -LIBODP = $(LIB)/libodp.la
> +LIBODP = $(LIB)/libodphelper.la $(LIB)/libodp.la
> --
> 2.1.4
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
