DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Richardson, Bruce" <bruce.richardson@intel.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] reserve 'make install' for future use
Date: Mon, 30 Nov 2015 11:41:32 +0000	[thread overview]
Message-ID: <59AF69C657FD0841A61C55336867B5B03598A978@IRSMSX103.ger.corp.intel.com> (raw)
In-Reply-To: <2760426.SYAZ0tSqOg@xps13>



> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, November 30, 2015 11:27 AM
> To: Richardson, Bruce <bruce.richardson@intel.com>
> Cc: Panu Matilainen <pmatilai@redhat.com>; dev@dpdk.org;
> olivier.matz@6wind.com
> Subject: Re: [dpdk-dev] [PATCH] reserve 'make install' for future use
> 
> 2015-11-30 11:08, Richardson, Bruce:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > Why is it a step in the right direction?
> > >
> > > We just need to install the files in a different hierarchy and adapt
> > > the makefiles to be able to compile an application while keeping the
> > > RTE_SDK variable to specify the root directory (previously built
> > > thanks to DESTDIR).
> > > As the hierarchy could be tuned, we need more variables, e.g.:
> > > 	DPDK_INC_DIR (default = RTE_SDK/include/dpdk)
> > > 	DPDK_LIB_DIR (default = RTE_SDK/lib)
> > >
> > > While doing it, we can have a specific handling of T= to keep
> > > compatibility with the current (old) syntax.
> > >
> > > What have I missed?
> > >
> >
> >
> > I'm not sure our existing "make install" is suitable for use for this,
> without having it heavily overloaded. The existing T= behavior has support
> for wildcards and compiling multiple instances at the same time -
> something that won't work with a scheme to actually install DPDK
> throughout the filesystem hierarchy. Having it sometimes behave as now,
> and sometimes behave as a standard make install is a bad idea IMHO, as it
> confuses things. Having lots of extra environment variables is also not a
> great idea, to my mind.
> 
> Yes I agree.
> I forgot to mention it, but in my idea, we can drop the support for
> multiple targets. So the T= compatibility would be only a shortcut to do
> "make config" and name the build directory based on the template name.
> 
> About the environment variables:
> An application requires CFLAGS and LDFLAGS (at least). The standard way to
> provide them is pkgconfig (not implemented yet).
> For applications using the DPDK makefiles, the only input is RTE_SDK.
> When allowing more tuning in paths, we need more variables when using the
> DPDK makefiles to build an application.
> 
> > My opinion is that we should rename our existing "make install" to
> something more suitable - my patch suggestion was "make sdk" but it could
> be "make target" or something else if people prefer. Once that is done, we
> can then look to implement a proper "make install" command that works in a
> standard way, perhaps alongside a configure script of some description.
> 
> I think we don't need to rename or move some code.
> Just drop and replace some of them.
> 
> The configure script is a great idea but it is a totally different idea.
> I do not think that installation and configuration should be related.
> Please let's consider "make install" first.
> 
> > For an easy enough solution, I would look to apply this patch to create
> "make sdk" and also http://dpdk.org/dev/patchwork/patch/8076/ to have a
> "make install" command that works in the build dir. That way:
> > * you can have existing behavior using "make sdk T=<target>"
> > * you can have standard(ish) configure/make/make install behavior using:
> > 	make config T=<target>
> > 	cd build
> > 	make
> > 	make install
> >   and the "make config" step can subsequently be wrapped in a configure
> script to eliminate the need to know what the best target to use is, etc.
> 
> As Panu commented, I do not think it is a good idea to have different
> behaviours inside and outside of the build directory.
> I would even say that this embedded makefile is only confusing and should
> be dropped.
> We need to have *one* right building methods, not to bring more confusion.

I disagree. I don't think we can have *one* right building method, because to
do so means completely throwing away our existing methods of building DPDK
and using sample applications. That general method, using RTE_SDK and RTE_TARGET
needs to be supported for some time for those projects already familiar with it
and using it.
As well as this, we also need a sane way of building DPDK inside the "build" 
directory, and having a "make install" target that installs the libraries
and headers inside /usr/local (or whatever was specified as $prefix).

With regards to different behavior, since different targets are provided, I
don't see it as a problem. In the root directory, "make config" and "make sdk"
are provided for backward compatibility. Inside the build directory you have
your standard "make" and "make install" commands. Since the command set is
very limited, it's easy enough to print a suitable error when the wrong
command is used in the wrong place. 

Yes, I would like the ideal state where we have one set of build commands that
are run from just one location. However, I don't think we can get to that objective
without going through a transition phase where we support both old and new options.

/Bruce

  reply	other threads:[~2015-11-30 11:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-06 10:24 Bruce Richardson
2015-11-06 10:24 ` [dpdk-dev] [PATCH] mk: rename 'make install' to 'make sdk' Bruce Richardson
2015-11-06 12:57 ` [dpdk-dev] [PATCH] reserve 'make install' for future use Bruce Richardson
2015-11-06 13:04   ` Thomas Monjalon
2015-11-24 16:54     ` Bruce Richardson
2015-11-25  8:48       ` Panu Matilainen
2015-11-27 17:33         ` Thomas Monjalon
2015-11-30 11:08           ` Richardson, Bruce
2015-11-30 11:27             ` Thomas Monjalon
2015-11-30 11:41               ` Richardson, Bruce [this message]
2015-11-30 11:49                 ` Bruce Richardson
2015-11-30 14:26                   ` Thomas Monjalon
2015-12-04 15:57                     ` Thomas Monjalon
2015-12-04 16:21                       ` Richardson, Bruce
2015-11-30 12:26           ` Panu Matilainen
2015-11-30 14:19             ` Thomas Monjalon

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=59AF69C657FD0841A61C55336867B5B03598A978@IRSMSX103.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=thomas.monjalon@6wind.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
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).