From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 9746695D1 for ; Tue, 7 Jun 2016 15:03:49 +0200 (CEST) Received: from [216.85.170.150] (helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1bAGfY-00060e-Gs; Tue, 07 Jun 2016 09:03:46 -0400 Date: Tue, 7 Jun 2016 09:03:37 -0400 From: Neil Horman To: Thomas Monjalon Cc: dev@dpdk.org, Bruce Richardson , Stephen Hemminger , Panu Matilainen Message-ID: <20160607130337.GA13025@neilslaptop.think-freely.org> References: <1463431287-4551-1-git-send-email-nhorman@tuxdriver.com> <2333946.Qkm8bLFkOf@xps13> <20160607120454.GA3903@hmsreliant.think-freely.org> <9045763.zD3817Zzj4@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9045763.zD3817Zzj4@xps13> User-Agent: Mutt/1.6.1 (2016-04-27) X-Spam-Score: -1.0 (-) X-Spam-Status: No Subject: Re: [dpdk-dev] [PATCHv6 1/7] pmdinfogen: Add buildtools and pmdinfogen utility X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Jun 2016 13:03:49 -0000 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. 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 :) > > > > +#if __x86_64__ || __aarch64__ > > > > > > Better to use CONFIG_RTE_ARCH_64. > > > > > I'm not sure why, given that every supported compiler defines the arches I use, > > but sure, fine. > > Because it will work for every 64-bit arch in DPDK. > Ok, fair enough. > > > > --- /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? Neil