[dpdk-dev,03/11] mk: fix build dependency of drivers on pmdinfogen

Message ID 1467905790-10597-4-git-send-email-thomas.monjalon@6wind.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Thomas Monjalon July 7, 2016, 3:36 p.m. UTC
  When compiling the drivers, some code is generated with pmdinfogen.
A fresh parallel build can fail if a driver is compiled before pmdinfogen:
	build/buildtools/dpdk-pmdinfogen: Permission denied

There was a dependency declared in drivers/Makefile but it cannot work
because this file is based on mk/rte.subdir.mk which do not handle
dependencies.

It is fixed by declaring the whole buildtools as (order only) prerequisite
of drivers.

Fixes: cb6696d22023 ("drivers: update registration macro usage")

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 drivers/Makefile   | 2 --
 mk/rte.sdkbuild.mk | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)
  

Comments

Neil Horman July 7, 2016, 3:56 p.m. UTC | #1
On Thu, Jul 07, 2016 at 05:36:22PM +0200, Thomas Monjalon wrote:
> When compiling the drivers, some code is generated with pmdinfogen.
> A fresh parallel build can fail if a driver is compiled before pmdinfogen:
> 	build/buildtools/dpdk-pmdinfogen: Permission denied
> 
> There was a dependency declared in drivers/Makefile but it cannot work
> because this file is based on mk/rte.subdir.mk which do not handle
> dependencies.
> 
> It is fixed by declaring the whole buildtools as (order only) prerequisite
> of drivers.
> 
> Fixes: cb6696d22023 ("drivers: update registration macro usage")
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>  drivers/Makefile   | 2 --
>  mk/rte.sdkbuild.mk | 1 +
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 75a3168..81c03a8 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -34,6 +34,4 @@ include $(RTE_SDK)/mk/rte.vars.mk
>  DIRS-y += net
>  DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
>  
> -DEPDIRS-y += buildtools/pmdinfo
> -
>  include $(RTE_SDK)/mk/rte.subdir.mk
> diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
> index fb68af2..354f006 100644
> --- a/mk/rte.sdkbuild.mk
> +++ b/mk/rte.sdkbuild.mk
> @@ -49,6 +49,7 @@ $(1): $(sort $(LOCAL_DEPDIRS-$(1)))
>  endef
>  
>  $(foreach d,$(ROOTDIRS-y),$(eval $(call depdirs_rule,$(d))))
> +drivers: | buildtools
>  
I'm not sure i understand the reasoning here, DEPDIRS is meant to declare
prerequisites to a directory (and its children) getting built, right?  A 
parallel make should block any drivers getting built prior to pmdinfogen getting
built.  What am I missing?

Neil

>  #
>  # build and clean targets
> -- 
> 2.7.0
> 
>
  
Thomas Monjalon July 7, 2016, 4:21 p.m. UTC | #2
2016-07-07 11:56, Neil Horman:
> On Thu, Jul 07, 2016 at 05:36:22PM +0200, Thomas Monjalon wrote:
> > When compiling the drivers, some code is generated with pmdinfogen.
> > A fresh parallel build can fail if a driver is compiled before pmdinfogen:
> > 	build/buildtools/dpdk-pmdinfogen: Permission denied
> > 
> > There was a dependency declared in drivers/Makefile but it cannot work
> > because this file is based on mk/rte.subdir.mk which do not handle
> > dependencies.
> > 
> > It is fixed by declaring the whole buildtools as (order only) prerequisite
> > of drivers.
[...]
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -34,6 +34,4 @@ include $(RTE_SDK)/mk/rte.vars.mk
> >  DIRS-y += net
> >  DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
> >  
> > -DEPDIRS-y += buildtools/pmdinfo
> > -
> >  include $(RTE_SDK)/mk/rte.subdir.mk
> > diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
> > index fb68af2..354f006 100644
> > --- a/mk/rte.sdkbuild.mk
> > +++ b/mk/rte.sdkbuild.mk
> > @@ -49,6 +49,7 @@ $(1): $(sort $(LOCAL_DEPDIRS-$(1)))
> >  endef
> >  
> >  $(foreach d,$(ROOTDIRS-y),$(eval $(call depdirs_rule,$(d))))
> > +drivers: | buildtools
> >  
> I'm not sure i understand the reasoning here, DEPDIRS is meant to declare
> prerequisites to a directory (and its children) getting built, right?  A 
> parallel make should block any drivers getting built prior to pmdinfogen getting
> built.  What am I missing?

DEPDIRS-y is not parsed at all in the context of rte.subdir.mk.
That's why this line was ignored by the build system.
  
Neil Horman July 7, 2016, 5:44 p.m. UTC | #3
On Thu, Jul 07, 2016 at 06:21:49PM +0200, Thomas Monjalon wrote:
> 2016-07-07 11:56, Neil Horman:
> > On Thu, Jul 07, 2016 at 05:36:22PM +0200, Thomas Monjalon wrote:
> > > When compiling the drivers, some code is generated with pmdinfogen.
> > > A fresh parallel build can fail if a driver is compiled before pmdinfogen:
> > > 	build/buildtools/dpdk-pmdinfogen: Permission denied
> > > 
> > > There was a dependency declared in drivers/Makefile but it cannot work
> > > because this file is based on mk/rte.subdir.mk which do not handle
> > > dependencies.
> > > 
> > > It is fixed by declaring the whole buildtools as (order only) prerequisite
> > > of drivers.
> [...]
> > > --- a/drivers/Makefile
> > > +++ b/drivers/Makefile
> > > @@ -34,6 +34,4 @@ include $(RTE_SDK)/mk/rte.vars.mk
> > >  DIRS-y += net
> > >  DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
> > >  
> > > -DEPDIRS-y += buildtools/pmdinfo
> > > -
> > >  include $(RTE_SDK)/mk/rte.subdir.mk
> > > diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
> > > index fb68af2..354f006 100644
> > > --- a/mk/rte.sdkbuild.mk
> > > +++ b/mk/rte.sdkbuild.mk
> > > @@ -49,6 +49,7 @@ $(1): $(sort $(LOCAL_DEPDIRS-$(1)))
> > >  endef
> > >  
> > >  $(foreach d,$(ROOTDIRS-y),$(eval $(call depdirs_rule,$(d))))
> > > +drivers: | buildtools
> > >  
> > I'm not sure i understand the reasoning here, DEPDIRS is meant to declare
> > prerequisites to a directory (and its children) getting built, right?  A 
> > parallel make should block any drivers getting built prior to pmdinfogen getting
> > built.  What am I missing?
> 
> DEPDIRS-y is not parsed at all in the context of rte.subdir.mk.
> That's why this line was ignored by the build system.
> 
Ah, ok then (though it seems like that might be a useful variable to query,
rather than dead reconing it inside the sdk build file

Acked-by: Neil Horman <nhorman@tuxdriver.com>
  

Patch

diff --git a/drivers/Makefile b/drivers/Makefile
index 75a3168..81c03a8 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -34,6 +34,4 @@  include $(RTE_SDK)/mk/rte.vars.mk
 DIRS-y += net
 DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
 
-DEPDIRS-y += buildtools/pmdinfo
-
 include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
index fb68af2..354f006 100644
--- a/mk/rte.sdkbuild.mk
+++ b/mk/rte.sdkbuild.mk
@@ -49,6 +49,7 @@  $(1): $(sort $(LOCAL_DEPDIRS-$(1)))
 endef
 
 $(foreach d,$(ROOTDIRS-y),$(eval $(call depdirs_rule,$(d))))
+drivers: | buildtools
 
 #
 # build and clean targets