[dpdk-dev] config: make libarchive optional

  The commit 66819e6 has introduced a dependency on libarchive to be able
to use some tar resources in the unit tests.
It is now an optional dependency because some systems do not have it
installed.

If CONFIG_RTE_APP_TEST_RESOURCE_TAR is disabled, the PCI test will not
be run. When a "configure" script will be integrated, the libarchive
availability could be checked to automatically enable the option.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 app/test/Makefile                 | 11 +++++++----
 app/test/resource.c               |  8 ++++++--
 app/test/test_mp_secondary.c      |  4 ++++
 app/test/test_resource.c          |  5 +++++
 config/common_base                |  1 +
 doc/guides/linux_gsg/sys_reqs.rst |  3 +++
 scripts/test-build.sh             |  4 ++++
 7 files changed, 30 insertions(+), 6 deletions(-)
  

Hello Thomas,

On Tue, 14 Jun 2016 10:59:49 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> The commit 66819e6 has introduced a dependency on libarchive to be able
> to use some tar resources in the unit tests.
> It is now an optional dependency because some systems do not have it
> installed.

I am surprised how big deal is this. So, let's live with this fact.
Thank you, Thomas, for proposing a solution.

> 
> If CONFIG_RTE_APP_TEST_RESOURCE_TAR is disabled, the PCI test will not
> be run. When a "configure" script will be integrated, the libarchive
> availability could be checked to automatically enable the option.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>  app/test/Makefile                 | 11 +++++++----
>  app/test/resource.c               |  8 ++++++--
>  app/test/test_mp_secondary.c      |  4 ++++
>  app/test/test_resource.c          |  5 +++++
>  config/common_base                |  1 +
>  doc/guides/linux_gsg/sys_reqs.rst |  3 +++
>  scripts/test-build.sh             |  4 ++++
>  7 files changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test/Makefile b/app/test/Makefile
> index 7e4d484..5ca5c0b 100644
> --- a/app/test/Makefile
> +++ b/app/test/Makefile
> @@ -71,9 +71,6 @@ SRCS-y += test.c
>  SRCS-y += resource.c
>  SRCS-y += test_resource.c
>  $(eval $(call linked_resource,test_resource_c,resource.c))
> -$(eval $(call linked_tar_resource,test_resource_tar,test_resource.c))
> -SRCS-y += test_pci.c
> -$(eval $(call linked_tar_resource,test_pci_sysfs,test_pci_sysfs))
>  SRCS-y += test_prefetch.c
>  SRCS-y += test_byteorder.c
>  SRCS-y += test_per_lcore.c
> @@ -88,6 +85,13 @@ SRCS-y += test_ring.c
>  SRCS-y += test_ring_perf.c
>  SRCS-y += test_pmd_perf.c
>  
> +ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
> +$(eval $(call linked_tar_resource,test_resource_tar,test_resource.c))
> +SRCS-y += test_pci.c
> +$(eval $(call linked_tar_resource,test_pci_sysfs,test_pci_sysfs))
> +LDLIBS += -larchive
> +endif
> +

I don't like this very much. I think, the linked_tar_resource can be
disabled at the place of its definition. What about:

ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
define linked_tar_resource
...
endef
else
linked_tar_resource =
endif

...

SRCS-$(CONFIG_RTE_APP_TEST_RESOURCE_TAR) += test_pci.c

...

ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
LDLIBS += -larchive
endif

>  ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y)
>  SRCS-y += test_table.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += test_table_pipeline.c
> @@ -194,7 +198,6 @@ CFLAGS += $(WERROR_FLAGS)
>  CFLAGS += -D_GNU_SOURCE
>  
>  LDLIBS += -lm
> -LDLIBS += -larchive
>  
>  # Disable VTA for memcpy test
>  ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
> diff --git a/app/test/resource.c b/app/test/resource.c
> index 8c42eea..0e2b62c 100644
> --- a/app/test/resource.c
> +++ b/app/test/resource.c
> @@ -33,8 +33,6 @@
>  
>  #include <stdio.h>
>  #include <string.h>
> -#include <archive.h>
> -#include <archive_entry.h>
>  #include <errno.h>
>  #include <sys/queue.h>
>  
> @@ -97,6 +95,10 @@ int resource_fwrite_file(const struct resource *r, const char *fname)
>  	return ret;
>  }
>  
> +#ifdef RTE_APP_TEST_RESOURCE_TAR
> +#include <archive.h>
> +#include <archive_entry.h>
> +
>  static int do_copy(struct archive *r, struct archive *w)
>  {
>  	const void *buf;
> @@ -295,6 +297,8 @@ fail:
>  	return -1;
>  }
>  
> +#endif /* RTE_APP_TEST_RESOURCE_TAR */
> +

This looks OK.

>  void resource_register(struct resource *r)
>  {
>  	TAILQ_INSERT_TAIL(&resource_list, r, next);
> diff --git a/app/test/test_mp_secondary.c b/app/test/test_mp_secondary.c
> index 4dfe418..f66b68f 100644
> --- a/app/test/test_mp_secondary.c
> +++ b/app/test/test_mp_secondary.c
> @@ -245,6 +245,7 @@ run_object_creation_tests(void)
>  	printf("# Checked rte_lpm_create() OK\n");
>  #endif
>  
> +#ifdef RTE_APP_TEST_RESOURCE_TAR
>  	/* Run a test_pci call */
>  	if (test_pci() != 0) {
>  		printf("PCI scan failed in secondary\n");
> @@ -252,6 +253,7 @@ run_object_creation_tests(void)
>  			return -1;
>  	} else
>  		printf("PCI scan succeeded in secondary\n");
> +#endif

Is it right to call a test from another test? I think this is
wrong... A user should first test the PCI and then the mp_seconday...
Or?

>  
>  	return 0;
>  }
> @@ -266,9 +268,11 @@ test_mp_secondary(void)
>  {
>  	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
>  		if (!test_pci_run) {
> +#ifdef RTE_APP_TEST_RESOURCE_TAR
>  			printf("=== Running pre-requisite test of test_pci\n");
>  			test_pci();
>  			printf("=== Requisite test done\n");
> +#endif

Similar here.

>  		}
>  		return run_secondary_instances();
>  	}
> diff --git a/app/test/test_resource.c b/app/test/test_resource.c
> index 1e85040..39a6468 100644
> --- a/app/test/test_resource.c
> +++ b/app/test/test_resource.c
> @@ -85,6 +85,7 @@ static int test_resource_c(void)
>  	return 0;
>  }
>  
> +#ifdef RTE_APP_TEST_RESOURCE_TAR
>  REGISTER_LINKED_RESOURCE(test_resource_tar);
>  
>  static int test_resource_tar(void)
> @@ -111,6 +112,8 @@ static int test_resource_tar(void)
>  	return 0;
>  }
>  
> +#endif /* RTE_APP_TEST_RESOURCE_TAR */
> +

This looks OK.

>  static int test_resource(void)
>  {
>  	if (test_resource_dpdk())
> @@ -119,8 +122,10 @@ static int test_resource(void)
>  	if (test_resource_c())
>  		return -1;
>  
> +#ifdef RTE_APP_TEST_RESOURCE_TAR
>  	if (test_resource_tar())
>  		return -1;
> +#endif /* RTE_APP_TEST_RESOURCE_TAR */

This looks OK.

>  
>  	return 0;
>  }
> diff --git a/config/common_base b/config/common_base
> index 47c26f6..b9ba405 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -546,6 +546,7 @@ CONFIG_RTE_INSECURE_FUNCTION_WARNING=n
>  # Compile the test application
>  #
>  CONFIG_RTE_APP_TEST=y
> +CONFIG_RTE_APP_TEST_RESOURCE_TAR=n
>  
>  #
>  # Compile the PMD test application
> diff --git a/doc/guides/linux_gsg/sys_reqs.rst b/doc/guides/linux_gsg/sys_reqs.rst
> index 959709e..b321544 100644
> --- a/doc/guides/linux_gsg/sys_reqs.rst
> +++ b/doc/guides/linux_gsg/sys_reqs.rst
> @@ -101,6 +101,9 @@ Compilation of the DPDK
>  *   libpcap headers and libraries (libpcap-devel) to compile and use the libpcap-based poll-mode driver.
>      This driver is disabled by default and can be enabled by setting ``CONFIG_RTE_LIBRTE_PMD_PCAP=y`` in the build time config file.
>  
> +*   libarchive headers and library are needed for some unit tests using tar to get their resources.
> +
> +
>  Running DPDK Applications
>  -------------------------
>  
> diff --git a/scripts/test-build.sh b/scripts/test-build.sh
> index 48539c1..9a11f94 100755
> --- a/scripts/test-build.sh
> +++ b/scripts/test-build.sh
> @@ -35,6 +35,7 @@ default_path=$PATH
>  # Load config options:
>  # - AESNI_MULTI_BUFFER_LIB_PATH
>  # - DPDK_BUILD_TEST_CONFIGS (defconfig1+option1+option2 defconfig2)
> +# - DPDK_DEP_ARCHIVE
>  # - DPDK_DEP_CFLAGS
>  # - DPDK_DEP_LDFLAGS
>  # - DPDK_DEP_MOFED (y/[n])
> @@ -111,6 +112,7 @@ reset_env ()
>  {
>  	export PATH=$default_path
>  	unset CROSS
> +	unset DPDK_DEP_ARCHIVE
>  	unset DPDK_DEP_CFLAGS
>  	unset DPDK_DEP_LDFLAGS
>  	unset DPDK_DEP_MOFED
> @@ -149,6 +151,8 @@ config () # <directory> <target> <options>
>  		sed -ri         's,(PCI_CONFIG=)n,\1y,' $1/.config
>  		sed -ri    's,(LIBRTE_IEEE1588=)n,\1y,' $1/.config
>  		sed -ri             's,(BYPASS=)n,\1y,' $1/.config
> +		test "$DPDK_DEP_ARCHIVE" != y || \
> +		sed -ri       's,(RESOURCE_TAR=)n,\1y,' $1/.config
>  		test "$DPDK_DEP_MOFED" != y || \
>  		sed -ri           's,(MLX._PMD=)n,\1y,' $1/.config
>  		test "$DPDK_DEP_SZE" != y || \

This look OK.

Regards
Jan
  

2016-06-14 11:33, Jan Viktorin:
> Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> > +ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
> > +$(eval $(call linked_tar_resource,test_resource_tar,test_resource.c))
> > +SRCS-y += test_pci.c
> > +$(eval $(call linked_tar_resource,test_pci_sysfs,test_pci_sysfs))
> > +LDLIBS += -larchive
> > +endif
> 
> I don't like this very much. I think, the linked_tar_resource can be
> disabled at the place of its definition. What about:
> 
> ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
> define linked_tar_resource
> ...
> endef
> else
> linked_tar_resource =
> endif
> 
> ...
> 
> SRCS-$(CONFIG_RTE_APP_TEST_RESOURCE_TAR) += test_pci.c
> 
> ...
> 
> ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
> LDLIBS += -larchive
> endif

Yes, that's better.

> > --- a/app/test/test_mp_secondary.c
> > +++ b/app/test/test_mp_secondary.c
> > @@ -245,6 +245,7 @@ run_object_creation_tests(void)
> >  	printf("# Checked rte_lpm_create() OK\n");
> >  #endif
> >  
> > +#ifdef RTE_APP_TEST_RESOURCE_TAR
> >  	/* Run a test_pci call */
> >  	if (test_pci() != 0) {
> >  		printf("PCI scan failed in secondary\n");
> > @@ -252,6 +253,7 @@ run_object_creation_tests(void)
> >  			return -1;
> >  	} else
> >  		printf("PCI scan succeeded in secondary\n");
> > +#endif
> 
> Is it right to call a test from another test? I think this is
> wrong... A user should first test the PCI and then the mp_seconday...
> Or?

:)
This is out of scope for this patch.
Yes this is a part of app/test/ which could be improved a lot.
We don't even have a maintainer for the autotest architecture.