On Tue, Jun 21, 2016 at 01:25:52PM +0300, Panu Matilainen wrote: > On 06/21/2016 01:01 PM, Dumitrescu, Cristian wrote: > > > > > >>-----Original Message----- > >>From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Panu Matilainen > >>Sent: Tuesday, June 21, 2016 9:12 AM > >>To: dev at dpdk.org > >>Cc: christian.ehrhardt at canonical.com; thomas.monjalon at 6wind.com > >>Subject: [dpdk-dev] [PATCH 1/3] mk: fix librte_pipeline dependency list > >>truncation > >> > >>In other libraries, dependency list is always appended to, but > >>in commit 6cbf4f75e059 it with an assignment. This causes the > >>librte_eal dependency added in commit 6cbf4f75e059 to get discarded, > >>resulting in missing dependency on librte_eal. > >> > >>Fixes: b3688bee81a8 ("pipeline: new packet framework logic") > >>Fixes: 6cbf4f75e059 ("mk: fix missing internal dependencies") > >> > >>Signed-off-by: Panu Matilainen <pmatilai at redhat.com> > >>--- > >> lib/librte_pipeline/Makefile | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >>diff --git a/lib/librte_pipeline/Makefile b/lib/librte_pipeline/Makefile > >>index 95387aa..a8f3128 100644 > >>--- a/lib/librte_pipeline/Makefile > >>+++ b/lib/librte_pipeline/Makefile > >>@@ -53,7 +53,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_PIPELINE)-include += > >>rte_pipeline.h > >> > >> # this lib depends upon: > >> DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_eal > >>-DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) := lib/librte_table > >>+DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_table > >> DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_port > >> > >> include $(RTE_SDK)/mk/rte.lib.mk > >>-- > >>2.5.5 > > > > > >In release 16.4, EAL was missing from the dependency list, now it is added. > >The librte_pipeline uses rte_malloc, therefore it depends on librte_eal > >being present. > > > >In the Makefile of the other Packet Framework libraries (librte_port, > >librte_table), it looks like the first dependency in the list is EAL, which > >is listed with the assignment operator, followed by others that are listed > >with the append operator: > > DEPDIRS-$(CONFIG_RTE_LIBRTE_XYZ) := lib/librte_eal > > DEPDIRS-$(CONFIG_RTE_LIBRTE_XYZ) += lib/librte_some other lib > > > >Therefore, at least for cosmetic reasons, we should probably do the same in > >librte_pipeline, which requires changing both the librte_eal and the > >librte_table lines as below: > > DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) := lib/librte_eal > > DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_table > > DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_port > > Ah, didn't notice those because the assignment is first of the dependencies. > > > > >However, some other libraries e.g. librte_lpm simply add the EAL dependency > >using the append operator: > > DEPDIRS-$(CONFIG_RTE_LIBRTE_LPM) += lib/librte_eal > > > >To be honest, I need to refresh my knowledge on make, I don't remember right > >now when we should use the assignment and when the append. Do we need to use > >the assign for first dependency (EAL) and append for others or should we use > >append everywhere? > > At least in automake, you need to assign before you can append. But in gmake > this apparently is not the case, quoting from > https://www.gnu.org/software/make/manual/html_node/Appending.html: > > "When the variable in question has not been defined before, ?+=? acts just > like normal ?=?: it defines a recursively-expanded variable. However, when > there is a previous definition, exactly what ?+=? does depends on what > flavor of variable you defined originally." > > So there's no need to use := anywhere for the dependencies, in fact its > probably best avoided to avoid issues like this. Of course after the third > patch in this "series" is applied, mistakes like these can no longer go > unnoticed. > Will the build be any slower with everything defaulting to recursively expanded variables rather than the simply-expanded variables defined by the initial ":="?
/Bruce
