[dpdk-dev] [RFC PATCH] mk: symlink every headers first

[Wiles, Keith]

> On Jun 21, 2017, at 10:14 AM, Gaëtan Rivet <gaetan.rivet at 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.richardson at 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 <thomas at 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