DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: David Marchand <david.marchand@redhat.com>,
	"Jiang, Cheng1" <cheng1.jiang@intel.com>,
	Maxime Coquelin <maxime.coquelin@redhat.com>,
	"Xia, Chenbo" <chenbo.xia@intel.com>, dev <dev@dpdk.org>,
	"Fu, Patrick" <patrick.fu@intel.com>,
	"Yang, YvonneX" <yvonnex.yang@intel.com>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>,
	"Hu, Jiayu" <jiayu.hu@intel.com>
Subject: Re: [dpdk-dev] [PATCH v10 0/4] add async data path in vhost sample
Date: Tue, 10 Nov 2020 15:40:43 +0100
Message-ID: <2341281.IIjsMd4WEc@thomas> (raw)
In-Reply-To: <20201110143442.GI1641@bricha3-MOBL.ger.corp.intel.com>

10/11/2020 15:34, Bruce Richardson:
> On Tue, Nov 10, 2020 at 02:37:02PM +0100, Thomas Monjalon wrote:
> > 10/11/2020 12:19, Bruce Richardson:
> > > On Tue, Nov 10, 2020 at 09:17:36AM +0100, David Marchand wrote:
> > > > On Tue, Nov 10, 2020 at 4:02 AM Jiang, Cheng1 <cheng1.jiang@intel.com> wrote:
> > > > > > - This series breaks external compilation, as the external Makefile was not
> > > > > > updated.
> > > > > >
> > > > >
> > > > > I'm not sure I understand what you mean by external Makefile,
> > > > > because as far as I know, makefile has been deprecated.
> > > > 
> > > > make support is dropped for dpdk compilation itself.
> > > > 
> > > > For the examples, the users will use make to compile them, as this is
> > > > the only way provided to users *out of* dpdk.
> > > > But the examples are compiled too via meson when compiling dpdk itself
> > > > if you pass -Dexamples= options.
> > > > 
> > > > 
> > > > Bruce,
> > > > 
> > > > I want to avoid more misses like this one.
> > > > 
> > > > If we want to keep the examples compilation in meson, we need a
> > > > consistent framework to compile in both cases.
> > > > Right now, we don't export meson for examples, and it makes no sense
> > > > in their current form.
> > > > It seems simpler to me to only maintain make support, and meson can
> > > > still call make for each example.
> > > > 
> > > > Another solution is to rely only on test-meson-builds.sh, but then it
> > > > ends up on the maintainer shoulders, so I prefer the solution above.
> > > > 
> > > > Other ideas?
> > > > 
> > > 
> > > Hi David,
> > > 
> > > I've been thinking a bit about this since I got your email, but inspiration
> > > for new ideas has yet to strike me.
> > > 
> > > While I can see the downside of having both make and meson build files for
> > > the examples, I'm loath to see one of them dropped. Here is my current
> > > thinking on it:
> > > 
> > > * We obviously need to keep the basic makefiles so as to make it easy for
> > >   end-users to compile up and use the examples separate from DPDK itself.
> > >   The makefiles serve as good examples as to how to pull the DPDK info from
> > >   pkg-config.
> > 
> > The external compilation is part of the example, yes.
> > 
> > > * On the other hand, being able to build the examples as part of a regular
> > >   meson build is a big advantage over having to build them separately, and
> > >   allows errors with examples to be caught quicker.
> > 
> > In the past we had a Makefile which builds all examples.
> > If you want, it can even been called at the end of meson DPDK compilation.
> > 
> 
> Yes, we can do that, but my concern is not so much about having them built
> as part of build tests, but rather having them part of a build workflow for
> developers during development.  Thinking here in particular of those coding
> with IDEs such as eclipse or VScode, which I know a lot of people use -
> including myself from time to time, depending on what I am working on.
> 
> > >   It's also useful for those of us working on specific components
> > >   with associated sample apps to have just that one app built constantly
> > >   as we work.
> > 
> > I don't understand this point:
> > 	ninja -C build && make -C examples/myexample
> > 
> 
> That works fine for building on the commandline, but does not work well for
> building as part of "build-on-save" inside an IDE, which is the biggest
> reason I want to keep support for building examples using meson. For doing
> development with a feature and associated sample app, being able to
> configure your build folder to rebuild a particular sample app as part of a
> main infrastructure rebuild is really useful - and works really quickly
> too, since incremental builds of C file changes happen really fast with
> meson.
> 
> > > * Therefore, while it looks like the more logical part to drop is indeed
> > >   the meson support for the examples,
> > 
> > Yes, building the examples from the inside build system is strange,
> > and hide issues.
> > 
> 
> Sorry, while it may hide issues with the makefile because people weren't
> really aware that they were sticking around, it definitely helps pick up
> issues with C code changes as you are developing.
> 
> > >   we may struggle to implement clean
> > >   building of the examples from meson using make, at least until meson 0.54
> > >   becomes our standard. Before that version, we don't have a
> > >   "meson-uninstalled" folder with build-time package-config file to use as
> > >   source of build flags for each example.
> > 
> > We don't have to use meson at all.
> > 
> 
> No, we don't, so long as we don't miss out on the benefits we currently get
> from having it integrated.
> 
> > > * One final idea I had and investigated in the past was whether we could at
> > >   build or install time auto-generate the Makefile for each example from
> > >   the meson.build file. Unfortunately, nothing came of that idea the first
> > >   time I tried it, but it might still be worth looking at. Even if it works
> > >   for 80-90% of cases, it means that we have a much smaller subset of
> > >   examples where we need to test independently the make and meson builds.
> > 
> > Hand-crafted Makefile is enough. They may be improved.
> > If we feel it is too hard, we can use another build system
> > in examples, like cmake.
> > 
> > > So overall my assessment is that it needs a bit of investigation and
> > > prototyping to see what we can come up with.
> > 
> > I think testing external build + removing build from internal meson
> > would be a good start to ensure quality of examples maintenance.
> > 
> If we remove the meson build of the examples, I think we need equivalent
> functionality provided some other way. Calling make for examples from
> within meson may be good enough, so long as it's not too slow.
> 
> > > On a semi-related note, it's perhaps a bigger problem that we cannot rely
> > > on test-meson-builds and CI infrastructure to prevent issues like this.
> > 
> > We can. We just have to add all examples in test-meson-builds.sh.
> > 
> 
> Yep, definite +1 here.
> Once that is done, I'd then like to wait and see what future issues crop up
> before we start ripping out other bits. This is the first release where we
> have removed the make build system, so there are lots of teething issues
> all over the place, of which this is but one example!
> 
> > > Surely this is what CI is there for - to help reduce the workload for
> > > maintainers. The fact that we are considering removing the meson build of
> > > examples because we cannot rely on CI is surely more worthy of a solution
> > > than trying to find a way to build examples with make from within meson?
> > 
> > No the concern is to have all contributors work on the same
> > single build path.
> > 
> Building all examples from test-meson-build should probably be sufficient
> here, I think.
> 
> > > Perhaps we need to see about stricter measures from CI failure, e.g.
> > > anything failing travis build automatically gets marked as changes
> > > requested in patchwork, and the author gets an email informing them of
> > > such?
> > 
> > When there is a failure, authors receive an email,
> > and maintainers can see a red flag. I think it's OK.
> > 
> > The only issue was that this build path was not tested.
> > I think David is going to fix it by compiling all possible examples
> > with external make build from test-meson-builds.sh.
> > 
> 
> Seems like we are all aligned on the first step anyway. Let's see beyond
> that what needs to be done in 21.02.

OK



      reply	other threads:[~2020-11-10 14:40 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-10  6:43 [dpdk-dev] [PATCH v1 " Cheng Jiang
2020-09-10  6:43 ` [dpdk-dev] [PATCH v1 1/4] example/vhost: add async vhost driver args parsing function Cheng Jiang
2020-09-23  8:25   ` Maxime Coquelin
2020-09-28  6:09     ` Jiang, Cheng1
2020-09-10  6:43 ` [dpdk-dev] [PATCH v1 2/4] example/vhost: add support for vhost async data path Cheng Jiang
2020-09-10  6:43 ` [dpdk-dev] [PATCH v1 3/4] doc: update vhost sample doc " Cheng Jiang
2020-09-10  6:43 ` [dpdk-dev] [PATCH v1 4/4] doc: update release notes for vhost sample Cheng Jiang
2020-09-29  6:42 ` [dpdk-dev] [PATCH v2 0/4] add async data path in " Cheng Jiang
2020-09-29  6:42   ` [dpdk-dev] [PATCH v2 1/4] example/vhost: add async vhost args parsing function Cheng Jiang
2020-09-29  6:42   ` [dpdk-dev] [PATCH v2 2/4] example/vhost: add support for vhost async data path Cheng Jiang
2020-09-29  6:42   ` [dpdk-dev] [PATCH v2 3/4] doc: update vhost sample doc " Cheng Jiang
2020-09-29  6:42   ` [dpdk-dev] [PATCH v2 4/4] doc: update release notes for vhost sample Cheng Jiang
2020-09-30  3:08   ` [dpdk-dev] [PATCH v3 0/4] add async data path in " Cheng Jiang
2020-09-30  3:08     ` [dpdk-dev] [PATCH v3 1/4] example/vhost: add async vhost args parsing function Cheng Jiang
2020-09-30  3:08     ` [dpdk-dev] [PATCH v3 2/4] example/vhost: add support for vhost async data path Cheng Jiang
2020-09-30  3:08     ` [dpdk-dev] [PATCH v3 3/4] doc: update vhost sample doc " Cheng Jiang
2020-09-30  3:08     ` [dpdk-dev] [PATCH v3 4/4] doc: update release notes for vhost sample Cheng Jiang
2020-10-12  4:54     ` [dpdk-dev] [PATCH v4 0/4] add async data path in " Cheng Jiang
2020-10-12  4:54       ` [dpdk-dev] [PATCH v4 1/4] example/vhost: add async vhost args parsing function Cheng Jiang
2020-10-14  9:23         ` Maxime Coquelin
2020-10-15  5:11           ` Jiang, Cheng1
2020-10-12  4:54       ` [dpdk-dev] [PATCH v4 2/4] example/vhost: add support for vhost async data path Cheng Jiang
2020-10-12  4:54       ` [dpdk-dev] [PATCH v4 3/4] doc: update vhost sample doc " Cheng Jiang
2020-10-12  4:54       ` [dpdk-dev] [PATCH v4 4/4] doc: update release notes for vhost sample Cheng Jiang
2020-10-15  4:54       ` [dpdk-dev] [PATCH v5 0/4] add async data path in " Cheng Jiang
2020-10-15  4:54         ` [dpdk-dev] [PATCH v5 1/4] example/vhost: add async vhost args parsing function Cheng Jiang
2020-10-15 15:13           ` Maxime Coquelin
2020-10-15  4:54         ` [dpdk-dev] [PATCH v5 2/4] example/vhost: add support for vhost async data path Cheng Jiang
2020-10-15  4:54         ` [dpdk-dev] [PATCH v5 3/4] doc: update vhost sample doc " Cheng Jiang
2020-10-15  4:54         ` [dpdk-dev] [PATCH v5 4/4] doc: update release notes for vhost sample Cheng Jiang
2020-10-16  4:29         ` [dpdk-dev] [PATCH v6 0/4] add async data path in " Cheng Jiang
2020-10-16  4:29           ` [dpdk-dev] [PATCH v6 1/4] example/vhost: add async vhost args parsing function Cheng Jiang
2020-10-16  4:29           ` [dpdk-dev] [PATCH v6 2/4] example/vhost: add support for vhost async data path Cheng Jiang
2020-10-16  4:29           ` [dpdk-dev] [PATCH v6 3/4] doc: update vhost sample doc " Cheng Jiang
2020-10-16  4:29           ` [dpdk-dev] [PATCH v6 4/4] doc: update release notes for vhost sample Cheng Jiang
2020-10-19  5:49             ` Jiang, Cheng1
2020-10-20 11:20           ` [dpdk-dev] [PATCH v7 0/4] add async data path in " Cheng Jiang
2020-10-20 11:20             ` [dpdk-dev] [PATCH v7 1/4] example/vhost: add async vhost args parsing function Cheng Jiang
2020-10-20 11:20             ` [dpdk-dev] [PATCH v7 2/4] example/vhost: add support for vhost async data path Cheng Jiang
2020-10-20 11:20             ` [dpdk-dev] [PATCH v7 3/4] doc: update vhost sample doc " Cheng Jiang
2020-10-20 11:20             ` [dpdk-dev] [PATCH v7 4/4] doc: update release notes for vhost sample Cheng Jiang
2020-10-21  6:50             ` [dpdk-dev] [PATCH v8 0/4] add async data path in " Cheng Jiang
2020-10-21  6:50               ` [dpdk-dev] [PATCH v8 1/4] example/vhost: add async vhost args parsing function Cheng Jiang
2020-10-21  6:50               ` [dpdk-dev] [PATCH v8 2/4] example/vhost: add support for vhost async data path Cheng Jiang
2020-10-21  6:50               ` [dpdk-dev] [PATCH v8 3/4] doc: update vhost sample doc " Cheng Jiang
2020-10-21  6:50               ` [dpdk-dev] [PATCH v8 4/4] doc: update release notes for vhost sample Cheng Jiang
2020-10-22  6:46               ` [dpdk-dev] [PATCH v9 0/4] add async data path in " Cheng Jiang
2020-10-22  6:46                 ` [dpdk-dev] [PATCH v9 1/4] example/vhost: add async vhost args parsing function Cheng Jiang
2020-10-22  6:46                 ` [dpdk-dev] [PATCH v9 2/4] example/vhost: add support for vhost async data path Cheng Jiang
2020-10-22  6:46                 ` [dpdk-dev] [PATCH v9 3/4] doc: update vhost sample doc " Cheng Jiang
2020-10-22  6:46                 ` [dpdk-dev] [PATCH v9 4/4] doc: update release notes for vhost sample Cheng Jiang
2020-10-22  9:10                 ` [dpdk-dev] [PATCH v9 0/4] add async data path in " Maxime Coquelin
2020-10-22  9:14                   ` Jiang, Cheng1
2020-10-22  8:59 ` [dpdk-dev] [PATCH v10 " Cheng Jiang
2020-10-22  8:59   ` [dpdk-dev] [PATCH v10 1/4] example/vhost: add async vhost args parsing function Cheng Jiang
2020-10-23 11:08     ` Maxime Coquelin
2020-10-22  8:59   ` [dpdk-dev] [PATCH v10 2/4] example/vhost: add support for vhost async data path Cheng Jiang
2020-10-23 11:12     ` Maxime Coquelin
2020-10-22  8:59   ` [dpdk-dev] [PATCH v10 3/4] doc: update vhost sample doc " Cheng Jiang
2020-10-23 11:15     ` Maxime Coquelin
2020-10-22  8:59   ` [dpdk-dev] [PATCH v10 4/4] doc: update release notes for vhost sample Cheng Jiang
2020-10-23 11:15     ` Maxime Coquelin
2020-10-23 11:23   ` [dpdk-dev] [PATCH v10 0/4] add async data path in " Maxime Coquelin
2020-10-23 13:20     ` Ferruh Yigit
2020-11-09 12:40     ` David Marchand
2020-11-10  3:02       ` Jiang, Cheng1
2020-11-10  8:17         ` David Marchand
2020-11-10 11:19           ` Bruce Richardson
2020-11-10 13:37             ` Thomas Monjalon
2020-11-10 14:34               ` Bruce Richardson
2020-11-10 14:40                 ` Thomas Monjalon [this message]

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=2341281.IIjsMd4WEc@thomas \
    --to=thomas@monjalon.net \
    --cc=bruce.richardson@intel.com \
    --cc=chenbo.xia@intel.com \
    --cc=cheng1.jiang@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jiayu.hu@intel.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=patrick.fu@intel.com \
    --cc=yvonnex.yang@intel.com \
    /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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git