Re: [PATCH 1/3] perf tests: Add testing for Makefile.perf
Hi everyone,
On Tue, Jun 23, 2015 at 01:09:12PM +0200, Jiri Olsa wrote:
> On Tue, Jun 23, 2015 at 09:44:23AM +0200, Ingo Molnar wrote:
> > * Jiri Olsa <jolsa@redhat.com> wrote:
> > > On Mon, Jun 22, 2015 at 04:48:54PM +0200, Ingo Molnar wrote:
> > > > (off list, Arnaldo Cc:-ed)
> > > > * Jiri Olsa <jolsa@kernel.org> wrote:
> > > > > Currently we test only builds through top level Makefile,
> > > > > but seems like there's a bunch of users using Makefile.perf
> > > > > directly.
> SNIP
> > > > We should fix/enhance the top level Makefile to meet legitimate complaints, but
> > > > otherwise I think it would be a mistake to have a dual build interface in essence
> > > > ...
> > > looks like it's already in the debian linux-tools package make:
> > > http://marc.info/?l=linux-kernel&m=143465795114146&w=2
> > > I haven't asked, but I guess he's involved in debian packaging
Just as a clarification, I'm not the maintainer of the "linux-tools"
package. The Debian Kernel Team is. The package tracker names three
uploaders all of whom I'm adding to Cc:
https://tracker.debian.org/pkg/linux-tools
That it's me who reported this is just a coincidence. I'm currently
doing kernel development and to build and test my stuff I use the
original Debian kernel packaging framework. Since I dev on bleeding
edge rc releases I have to update the Debian packages to new kernel
versions earlier than most others, that's why it's me who found and
reported that issue. I've also submitted a bug with a patch to adapt
the "linux-tools" package to 4.1 to hopefully make the maintainers'
lives easier:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=789269
> > So I guess the solution is to pass through 'prefix' parameters, because that is
> > why they are using Makefile.perf directly?
> not really, if they used Makefile the issue Lukas fixed would not be visible,
> let's ask him directly..
>
> Lukas,
> what's the reason for using Makefile.perf instead of the top level Makefile?
I don't know as I'm not a maintainer of the package. :-)
> > We should steer packagers towards the top Makefile as well, unless we want an ever
> > growing schizm of the build UI ...
> agreed
Yes that's reasonable, but:
It's just not a very good idea to use the same variable name for two
different things, the behaviour of the Makefile becomes difficult to
predict, the code of the Makefile becomes difficult to maintain and
generally trouble is invited.
Specifying "prefix=/foo" as a command line parameter to a Makefile is
a convention which I believe was popularized by GNU autotools.
The default prefix is normally "/usr", a typical usage is to specify
"/usr/local" instead.
The issue here is that tools/build/Makefile.build uses a temporary
variable to calculate a prefix that gets added to Makefile targets,
and that temporary variable is unfortunately *also* named "prefix".
A parameter specified on the command line normally overrides variables
defined in a Makefile, so because the two variables share the same name,
the temporary variable gets assigned the value of the command line
parameter.
So I guess what I'm trying to say is, a temporary variable just
shouldn't use the same name as a command line parameter, in particular
if that command line parameter has become a popular convention.
Notwithstanding the goal to steer packagers to a single Makefile,
I believe the name of that temporary variable in Makefile.build
should be changed, as is done by the submitted patch.
Thanks,
Lukas
Reply to: