From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 0744B1B53B; Thu, 7 Feb 2019 16:03:33 +0100 (CET) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Feb 2019 07:03:32 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,344,1544515200"; d="scan'208";a="114426574" Received: from bricha3-mobl.ger.corp.intel.com ([10.237.221.54]) by orsmga006.jf.intel.com with SMTP; 07 Feb 2019 07:03:29 -0800 Received: by (sSMTP sendmail emulation); Thu, 07 Feb 2019 15:03:28 +0000 Date: Thu, 7 Feb 2019 15:03:28 +0000 From: Bruce Richardson To: Neil Horman Cc: dev@dpdk.org, stable@dpdk.org, David Marchand , Anatoly Burakov Message-ID: <20190207150328.GA121112@bricha3-MOBL.ger.corp.intel.com> References: <20190110111104.56464-1-bruce.richardson@intel.com> <20190206110130.55135-1-bruce.richardson@intel.com> <20190206122254.GA16887@hmswarspite.think-freely.org> <20190206141744.GA236864@bricha3-MOBL.ger.corp.intel.com> <20190207143426.GA23613@hmswarspite.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190207143426.GA23613@hmswarspite.think-freely.org> User-Agent: Mutt/1.11.2 (2019-01-07) Subject: Re: [dpdk-dev] [PATCH v3] compat: merge compat library into EAL X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Feb 2019 15:03:34 -0000 On Thu, Feb 07, 2019 at 09:34:26AM -0500, Neil Horman wrote: > On Wed, Feb 06, 2019 at 02:17:45PM +0000, Bruce Richardson wrote: > > On Wed, Feb 06, 2019 at 07:22:54AM -0500, Neil Horman wrote: > > > On Wed, Feb 06, 2019 at 11:01:30AM +0000, Bruce Richardson wrote: > > > > Since compat library is only a single header, we can easily move it into > > > > the EAL common headers instead of tracking it separately. The downside of > > > > this is that it becomes a little more difficult to have any libs that are > > > > built before EAL depend on it. Thankfully, this is not a major problem as > > > > the only library which uses rte_compat.h and is built before EAL (kvargs) > > > > already has the path to the compat.h header file explicitly called out as > > > > an include path. > > > > > > > > However, to ensure that we don't hit problems later with this, we can add > > > > EAL common headers folder to the global include list in the meson build > > > > which means that all common headers can be safely used by all libraries, no > > > > matter what their build order. > > > > > > > This assumes that the compat lib will always just be a header though, no? Will > > > this work in the event that someone wants to add some compatibility code that > > > requires its own C compilation unit? > > > > > > > No, it probably won't work, you'll hit an issue with any libraries that > > don't depend on EAL and need that functionality. The question is whether > > this is likely to be an issue in the future for us. I'd say the possiblity > > is fairly remote, but I'm open to input on it. > > > Im afraid I don't have any more visibility on that than anyone else. The fact > that it hasn't been needed yet is likely a good sign, but I am concerned at the > notion that this change enjoins us from having that flexibility. > Yes. However, in general is it not the case that compatibility code belongs in the actual library wanting to provide the compatibility? That is what has been done up till now. If we do need compatibility code placed more centrally, I think EAL is as good a place for it as any - the only library which doesn't depend on EAL now is kvargs, so our risk area is pretty low, I think. Also, if we do need a compat libraries with .c files in it, there is no reason we can't undo this change. It would be no more user visible than adding a .c file to the existing structure, given that in both cases an extra .so file will appear in the build output. > > > > As a side-effect, this patch also fixes an issue with building on BSD using > > > > meson, due to compat lib no longer needing to be listed as a dependency. > > > > > > > Can you elaborate here a bit please? listing a lib as a dependency seems like a > > > fundamental function of a build system, was there a bug with meson in this > > > capacity? > > > > > > > It was a bug in DPDK. There was already a dependency on the compat > > library from libeal from linux, but not from BSD, so when a further > > dependency was added globally, the BSD build broke, but the linux one > > didn't. > > > Do you have a link to the breakage details? I'd like to look at it to see if > there is a way around this without enjoining us from adding compat C files in > the future. > Don't have a link handy, but the basics, as I understand them are as below: Commit a8499f65a1d1 ("log: add missing experimental tag"), added the experimental tag to a log function. To get this tag definition rte_compat.h header was included in the rte_log.h file in the "common" EAL folder. In build tests on linux, this was no problem since linuxapp/eal/meson.build had the line: eal_inc += include_directories('include', '../../../librte_compat') However, the BSD eal/meson.build file did not have this line, giving a compiler error. /Bruce PS: Now that I also look at the files more closely, with this move to put compat.h into EAL, we can probably remove the special case processing in lib/meson.build for libraries with an empty source array.