Re: [dpdk-dev] [PATCH v4 00/31] remove make support in DPDK

[Thomas Monjalon <thomas@monjalon.net>]

15/09/2020 18:39, Chautru, Nicolas:
> Hi, 
> 
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Tuesday, September 15, 2020 9:11 AM
> > To: David Marchand <david.marchand@redhat.com>; Chautru, Nicolas
> > <nicolas.chautru@intel.com>; Power, Ciara <ciara.power@intel.com>;
> > Richardson, Bruce <bruce.richardson@intel.com>
> > Cc: dev <dev@dpdk.org>
> > Subject: Re: [PATCH v4 00/31] remove make support in DPDK
> > 
> > 15/09/2020 17:57, Richardson, Bruce:
> > >
> > > > -----Original Message-----
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > Sent: Tuesday, September 15, 2020 4:44 PM
> > > > To: David Marchand <david.marchand@redhat.com>; Chautru, Nicolas
> > > > <nicolas.chautru@intel.com>; Power, Ciara <ciara.power@intel.com>
> > > > Cc: Richardson, Bruce <bruce.richardson@intel.com>; dev
> > > > <dev@dpdk.org>
> > > > Subject: Re: [PATCH v4 00/31] remove make support in DPDK
> > > >
> > > > 15/09/2020 17:08, Power, Ciara:
> > > > > Hi David,
> > > > >
> > > > >
> > > > > >-----Original Message-----
> > > > > >From: David Marchand <david.marchand@redhat.com>
> > > > > >Sent: Tuesday 15 September 2020 15:03
> > > > > >To: Thomas Monjalon <thomas@monjalon.net>; Chautru, Nicolas
> > > > > ><nicolas.chautru@intel.com>
> > > > > >Cc: Richardson, Bruce <bruce.richardson@intel.com>; dev
> > > > > ><dev@dpdk.org>; Power, Ciara <ciara.power@intel.com>
> > > > > >Subject: Re: [PATCH v4 00/31] remove make support in DPDK
> > > > > >
> > > > > >On Tue, Sep 8, 2020 at 11:05 AM Thomas Monjalon
> > > > > ><thomas@monjalon.net>
> > > > > >wrote:
> > > > > >>
> > > > > >> 08/09/2020 10:16, Bruce Richardson:
> > > > > >> > On Tue, Sep 08, 2020 at 12:06:40AM +0200, Thomas Monjalon
> > wrote:
> > > > > >> > > The first 13 commits of this series are pushed to the main
> > > > > >> > > branch, making the make-based build an old story.
> > > > > >> > >
> > > > > >> > > The last 18 commits (devtools, app and doc) are pending for
> > > > > >> > > more reviews, especially because the doc rework is still in
> > > > progress.
> > > > > >> > > This is a big change which requires to be split but must be
> > > > > >> > > completed soon.
> > > > > >> > > *All*, please make the documentation update your highest
> > > > priority.
> > > > > >> > > I will be reluctant to merge some driver changes if the
> > > > > >> > > related doc is not updated.
> > > > > >> > >
> > > > > >> > Thanks for the apply of the code patches, I think it will
> > > > > >> > make all our subsequent patch submissions a bit cleaner.
> > > > > >> >
> > > > > >> > In terms of docs, I believe the Linux and FreeBSD GSG guides
> > > > > >> > to be ready for merge, or if not 100% they are at least good
> > > > > >> > enough IMHO to be applied now and have some minor tweaks in
> > > > > >> > subsequent patches later. While I'm sure the Linux GSG could
> > > > > >> > do with more major rework, that should be separate from the
> > > > > >> > doc updates in this
> > > > patch.
> > > > > >>
> > > > > >> Of course I am not expecting any major rework in these patches,
> > > > > >> but I don't want to apply clean-up half done, which can drop
> > > > > >> some relevant informations or introduce mistakes.
> > > > > >>
> > > > > >> I will review the Linux and FreeBSD user guides first, as you
> > > > suggest.
> > > > > >
> > > > > >I could not find a patch in patchwork for
> > > > > >app/test-bbdev/test-bbdev.py which still references RTE_SDK and
> > > > RTE_TARGET.
> > > > > >
> > > > > >
> > > > > >--
> > > > > >David Marchand
> > > > >
> > > > >
> > > > > Changes to test-bbdev.py were made in patch 15:
> > > > https://patchwork.dpdk.org/patch/76796/ to remove the references to
> > > > RTE_SDK and RTE_TARGET.
> > > >
> > > > I did not merge this patch because it is hardcoding "build" as a
> > > > directory.
> > > > I'm sure we can do better.
> > > >
> > >
> > > We probably can, but is it really worth holding up the patch over something as
> > trivial as that? It is really likely to cause people issues?
> > 
> > I give one more week to Nicolas to better fix this script.
> > If not, I will merge that patch as is.
> > 
> 
> Hi Thomas, Ciara, 
> I had raised that concern a couple of weeks ago but I was personally satisfied with document pointing to `build` as default staging build directory and seeing limited reason in practice for it to be impactful for bbdev users. 
> Also the argument to the script `-p` can be used to bypass default path to any location.  
> See reply here https://patches.dpdk.org/patch/76426/
> The only request I had was to change the default name of the binary (mentioned on same link above) and I don't see that change being made (I may just miss the superseding patch though)

You're right, I remember it was the other reason for not merging.

In any case, I will wait for Ack from Nicolas.