From: "Gaëtan Rivet" <gaetan.rivet@6wind.com>
To: "Wiles, Keith" <keith.wiles@intel.com>
Cc: "Richardson, Bruce" <bruce.richardson@intel.com>,
Thomas Monjalon <thomas@monjalon.net>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [RFC PATCH] mk: symlink every headers first
Date: Wed, 21 Jun 2017 17:14:51 +0200 [thread overview]
Message-ID: <20170621151451.GG2344@bidouze.vm.6wind.com> (raw)
In-Reply-To: <2D791B20-6CAC-482D-811B-4B4829242710@intel.com>
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@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@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.
Cheers,
--
Gaëtan Rivet
6WIND
next prev parent reply other threads:[~2017-06-21 15:15 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-20 21:21 Thomas Monjalon
2017-06-21 10:27 ` Bruce Richardson
2017-06-21 10:41 ` Thomas Monjalon
2017-06-21 12:52 ` Richardson, Bruce
2017-06-21 14:27 ` Wiles, Keith
2017-06-21 15:14 ` Gaëtan Rivet [this message]
2017-06-21 15:53 ` Wiles, Keith
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170621151451.GG2344@bidouze.vm.6wind.com \
--to=gaetan.rivet@6wind.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=keith.wiles@intel.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).