Re: [dpdk-dev] [PATCH] reserve 'make install' for future use

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Nov 30, 2015 at 11:41:32AM +0000, Richardson, Bruce wrote:
> 
> 
> > -----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. 

By way of follow-up to my own email, I'd also state that I would indeed prefer
not to have different targets in different places, and that ideally you would
do configure/make/make-install from the root directory. The reason I suggested
having "make install" work inside the build directory is because of our
existing use of "make install" for something different in the root directory.
This is also the reason I sent out this patch. By renaming the "make install"
command in 2.2, we give ourselves the option in future releases of adding in
a new "make install" command that behaves as we want, without having to worry
about conflict with a legacy make install.

That is why I feel this one patch should go in - it opens up more options for
us in future releases. It's not an end in itself. :-)

/Bruce

> 
> 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
>