From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH] config: make libarchive optional
Date: Tue, 14 Jun 2016 11:50:44 +0200
Message-ID: <1887327.ZH1KvsIjBr@xps13>
References: <575FBF1A.5080005@intel.com>
 <1465894789-20733-1-git-send-email-thomas.monjalon@6wind.com>
 <20160614113343.2d6fd116@pcviktorin.fit.vutbr.cz>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: David Hunt <david.hunt@intel.com>, dev@dpdk.org
To: Jan Viktorin <viktorin@rehivetech.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f47.google.com (mail-wm0-f47.google.com [74.125.82.47])
 by dpdk.org (Postfix) with ESMTP id D60C56CC1
 for <dev@dpdk.org>; Tue, 14 Jun 2016 11:50:46 +0200 (CEST)
Received: by mail-wm0-f47.google.com with SMTP id k204so114757536wmk.0
 for <dev@dpdk.org>; Tue, 14 Jun 2016 02:50:46 -0700 (PDT)
In-Reply-To: <20160614113343.2d6fd116@pcviktorin.fit.vutbr.cz>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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.