> On Jun 21, 2017, at 10:14 AM, Gaëtan Rivet <gaetan.ri...@6wind.com> wrote: > > Hi, > > On Wed, Jun 21, 2017 at 02:27:49PM +0000, Wiles, Keith wrote: >> >>> On Jun 21, 2017, at 5:27 AM, Bruce Richardson <bruce.richard...@intel.com> >>> wrote: >>> >>> On Tue, Jun 20, 2017 at 11:21:39PM +0200, Thomas Monjalon wrote: >>>> If a library or a build tool uses a definition from a driver, >>>> there is a build ordering issue, like seen when moving PCI code >>>> into a bus driver. >>>> >>>> One option is to keep PCI helpers and some common definitions in EAL. >>>> The other option is to symlink every headers at the beginning of >>>> the build so they can be included by any other component. >>>> >>>> This patch shows how to achieve the second option. >>>> >>>> Signed-off-by: Thomas Monjalon <tho...@monjalon.net> >>>> --- >>> >>> My 2c. >>> >>> This may be worth doing, however, two points to consider. >>> >>> 1. If we look to change build system this may not be worth doing unless >>> a compelling need becomes obvious in the short term. In the meantime, >>> for cases where it is needed... >>> 2. libraries can already access the includes from drivers or other >>> places fairly easily, just by adding the relevant "-I" flag to the >>> CFLAGS for that lib. >>> >>> That said, since the work is already done developing this, and if it >>> doesn't hurt in terms of build time, I suppose we might as well merge >>> it in. >>> >>> So tentative ack from me, subject to testing and feedback from others. >> >> +1, I already have to make sure everything is symlinked first in my private >> DPDK work for other reasons. This patch would allow me to remove that >> special code. >> >>> >>> /Bruce >> >> Regards, >> Keith >> > > Personally I do not like this approach. > > A well designed architecture introduces constraints that developers > ought to follow. These constraints, when well defined, foster a cleaner > growth on top of it with better practices. > > This solution is a crutch meant to alleviate other deep-rooted issues that > should be fixed anyway. It makes them disappear right now, only for > them to reappear when people will write libs and drivers with blurred > hierarchies and hard-to-determine dependencies. > > These constraints should be a tool for developers to be critical of their work > and help them see whether they are making a mistake, for example by > putting implementation specific data in generic structures (as seen by > the problems at the root of this RFC: KNI, pmdinfogen dependencies). > > This would have led for example eventdev, cryptodev, ethdev to leave PCI > specific data within their structures. Yes, it works. But it is not > clean, it leads to ABI instability, unsafe design practices. > > This RFC is the easy way out, making it work, at the price of technical > debt. > > I understand that it is a lot of work to properly clean up the > structures and that ressource is scarce. Having a clear architecture and > proper structures however helps external eyes to read, understand, use, > and ultimately contribute. > > This kind of move goes against the previous work that was done to > make devices, drivers and buses generic, which I think was right. > > I do not feel that it is my place to nack, nor that it is constructive > to block this if others are thinking that it is useful; however I hope > to sway others to my position.
I do not think this is a crutch as it allows the headers to be included from a single location, instead of relative includes, which can be the worst method. The real problem is we did not police or restrict usage of structures/includes in the current work or patches. I suggest we start the cleanup of these dependencies and be more strict on what can be included in a feature or file. This way we can have both and it makes it easier for locating headers. We also do not really restrict or create public/private headers. Private headers should never be accessed by another feature/module or exported to the global location. Most if not all of the PMD headers should be private or they need to be split into a public/private set of headers. > > Cheers, > -- > Gaëtan Rivet > 6WIND Regards, Keith
