Re: [dpdk-dev] [PATCHv6 1/7] pmdinfogen: Add buildtools and pmdinfogen utility

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2016-06-07 09:03, Neil Horman:
> On Tue, Jun 07, 2016 at 02:53:36PM +0200, Thomas Monjalon wrote:
> > 2016-06-07 08:04, Neil Horman:
> > > On Tue, Jun 07, 2016 at 11:57:42AM +0200, Thomas Monjalon wrote:
> > > > 2016-05-31 09:57, Neil Horman:
> > > > > +++ b/buildtools/Makefile
> > > > > @@ -0,0 +1,36 @@
> > > > > +#   BSD LICENSE
> > > > > +#
> > > > > +#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> > > > > +#   All rights reserved.
> > > > 
> > > > I really think it is a strange copyright for a new empty file.
> > > > 
> > > Its not empty, It lists the subdirectories to build.  And given that the DPDK is
> > > licensed under multiple licenses (BSD/GPL/LGPL), it introduces confusion to not
> > > call out the license in a specific file, file size is really irrelevant to that.
> > 
> > Neil, please take a drink :)
> > I'm not talking about license but about copyright.
> > Don't you think it's strange to put "2010-2014 Intel" copyright on top of
> > the few lines you wrote?
> >  
> Ah, yes, I copied the file, so the copyright years are wrong, so that should be
> fixed.

Not only the years, the copyright holder should be you or your company.

> That said, you asked if it was strange to put a copyright on an empty file, and
> the answer is no, because its not empty, and it nees a copyright for clarity :)

Of course, yes.

> > > > > --- /dev/null
> > > > > +++ b/mk/rte.buildtools.mk
> > > > 
> > > > I'm sorry I really do not agree it is a good practice to create a new
> > > > makefile type just for a new directory.
> > > > My opinion is that you should use and improve rte.hostapp.mk to make
> > > > it usable for possible other host apps.
> > > > 
> > > I am so exhausted by this argument.
> > > 
> > > They are the same file Thomas.  I'm not sure how you don't see that.  I've
> > > explained to you that they are, with the exception of whitespace noise,
> > > identical.  buildtools is a better nomenclature because it more closely
> > > describes what is being built at the moment.  The only reason we still have
> > > hostapp is because you didn't remove it when you removed the applications that,
> > > in your own words from the commit log, are "useless".  The argument that we
> > > should keep the build file, and its naming convention on the off chance that
> > > someone might use it in the future really doesn't hold water with me, at least
> > > not to the point that, when we have something that duplicates its function we
> > > should do anything other than take the path of least resistance to make it work.
> > > I'm not sure how you expected anyone to know there is a makefile in place in the
> > > DPDK to build local application, when there are currently no applications in
> > > place, but asking people to use it after the fact is really just the height of
> > > busywork.  If it was already building other utilities, I'd feel differently, but
> > > given that its just sitting there, a vestigual file, makes this all just silly.
> > > 
> > > But clearly, this isn't going to be done until I do what you want, regardless of
> > > what either of us think of it, So I'll make the change.
> > 
> > You can keep it as is if you find someone else to say that having a makefile
> > template named and specific to only the buildtools usage is fine.
> > And no, it is not identical to rte.hostapp.mk.
> > But I was probably not clear enough:
> > I do not like rte.hostapp.mk. I just like its explicit name.
> > I see the same issue in rte.hostapp.mk and rte.buildtools.mk: they should be
> > build in the app/ subdir like any other app.
> > 
> > So my suggestion is to replace rte.hostapp.mk with your implementation in
> > a separate patch with the build path changed to app/ instead of hostapp/ or
> > buildtools/.
> > 
> Soo, I'm confused now.  You don't want rte.buildtools.mk, and you don't really
> want rte.hostapp.mk, you want a different makefile, that just builds to the /app
> subdirectory?

The apps and examples use rte.app.mk to build a DPDK app.
Here you make a standard app, without DPDK dependency, to run on the host.
So you cannot use rte.app.mk. I think rte.hostapp.mk is not a so bad name
(I have no better one).
About the build directory, the app/ one looks OK, no need to put a reference
to buildtools which is just the user of this makefile.
Except these considerations, the content of your makefile is probably good.