LKML Archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified
       [not found] <201506181332.t5IDWuUq027242@int-mx13.intmail.prod.int.phx2.redhat.com>
@ 2015-06-18 19:29 ` Jiri Olsa
  2015-06-18 19:59   ` Lukas Wunner
  2015-06-18 21:52 ` Jiri Olsa
  1 sibling, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2015-06-18 19:29 UTC (permalink / raw
  To: Lukas Wunner; +Cc: Jiri Olsa, Arnaldo Carvalho de Melo, linux-kernel

On Thu, Jun 18, 2015 at 01:00:32PM +0200, Lukas Wunner wrote:
> Invoking Makefile.perf with prefix= breaks the build since Makefile.perf
> hands that variable down to Makefile.build where it overrides
>     prefix       := $(subst ./,,$(OUTPUT)$(dir)/)
> 
> leading to errors like this:
>     No rule to make target '/usrabspath.o', needed by '/usrlibperf-in.o'

hum, what specific make command is failing?

jirka

> 
> Fixes: c819e2cf2eb6f65d3208d195d7a0edef6108d5
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  tools/build/Makefile.build | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/build/Makefile.build b/tools/build/Makefile.build
> index 10df572..98cfc38 100644
> --- a/tools/build/Makefile.build
> +++ b/tools/build/Makefile.build
> @@ -94,12 +94,12 @@ obj-y        := $(patsubst %/, %/$(obj)-in.o, $(obj-y))
>  subdir-obj-y := $(filter %/$(obj)-in.o, $(obj-y))
>  
>  # '$(OUTPUT)/dir' prefix to all objects
> -prefix       := $(subst ./,,$(OUTPUT)$(dir)/)
> -obj-y        := $(addprefix $(prefix),$(obj-y))
> -subdir-obj-y := $(addprefix $(prefix),$(subdir-obj-y))
> +objprefix    := $(subst ./,,$(OUTPUT)$(dir)/)
> +obj-y        := $(addprefix $(objprefix),$(obj-y))
> +subdir-obj-y := $(addprefix $(objprefix),$(subdir-obj-y))
>  
>  # Final '$(obj)-in.o' object
> -in-target := $(prefix)$(obj)-in.o
> +in-target := $(objprefix)$(obj)-in.o
>  
>  PHONY += $(subdir-y)
>  
> -- 
> 1.8.5.2 (Apple Git-48)
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified
  2015-06-18 19:29 ` [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified Jiri Olsa
@ 2015-06-18 19:59   ` Lukas Wunner
  2015-06-18 20:26     ` David Ahern
  0 siblings, 1 reply; 10+ messages in thread
From: Lukas Wunner @ 2015-06-18 19:59 UTC (permalink / raw
  To: Jiri Olsa; +Cc: Arnaldo Carvalho de Melo, linux-kernel

Hi Jirka,

On Thu, Jun 18, 2015 at 09:29:32PM +0200, Jiri Olsa wrote:
> On Thu, Jun 18, 2015 at 01:00:32PM +0200, Lukas Wunner wrote:
> > Invoking Makefile.perf with prefix= breaks the build since Makefile.perf
> > hands that variable down to Makefile.build where it overrides
> >     prefix       := $(subst ./,,$(OUTPUT)$(dir)/)
> > 
> > leading to errors like this:
> >     No rule to make target '/usrabspath.o', needed by '/usrlibperf-in.o'
> hum, what specific make command is failing?

Makefile.perf may be invoked with a "prefix" parameter:

	@echo '  HINT: use "prefix" or "DESTDIR" to install to a particular'
	@echo '        path like "make prefix=/usr/local install install-doc"'

Source: https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/tools/perf/Makefile.perf#n414


E.g. the Debian "linux-tools" package makes use of that feature:

	MAKE_PERF := $(MAKE) prefix=/usr V=1 ARCH=$(KERNEL_ARCH_PERF) EXTRA_WARNINGS=-Wno-error

Source: http://anonscm.debian.org/viewvc/kernel/dists/trunk/linux-tools/debian/build/tools/perf/Makefile?view=markup (line 29)


The "prefix" parameter is handed down from Makefile.perf to Makefile.build
because it's invoked with $(MAKE), so the command line parameters are
inherited to Makefile.build:

	$(Q)$(MAKE) $(build)=perf

Source: https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/tools/perf/Makefile.perf#n282


That "prefix" feature is broken in all 4.1 release candidates because a
variable definition specified on the make command line is an "overriding
variable", so this definition in line 97 of tools/build/Makefile.build
has no effect:

	prefix       := $(subst ./,,$(OUTPUT)$(dir)/)

Source: https://www.gnu.org/software/make/manual/html_node/Overriding.html


So $(prefix) contains the wrong value, yet is subsequently used in
Makefile.build, causing the build to break.


Best regards,

Lukas

> 
> jirka
> 
> > 
> > Fixes: c819e2cf2eb6f65d3208d195d7a0edef6108d5
> > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > ---
> >  tools/build/Makefile.build | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/build/Makefile.build b/tools/build/Makefile.build
> > index 10df572..98cfc38 100644
> > --- a/tools/build/Makefile.build
> > +++ b/tools/build/Makefile.build
> > @@ -94,12 +94,12 @@ obj-y        := $(patsubst %/, %/$(obj)-in.o, $(obj-y))
> >  subdir-obj-y := $(filter %/$(obj)-in.o, $(obj-y))
> >  
> >  # '$(OUTPUT)/dir' prefix to all objects
> > -prefix       := $(subst ./,,$(OUTPUT)$(dir)/)
> > -obj-y        := $(addprefix $(prefix),$(obj-y))
> > -subdir-obj-y := $(addprefix $(prefix),$(subdir-obj-y))
> > +objprefix    := $(subst ./,,$(OUTPUT)$(dir)/)
> > +obj-y        := $(addprefix $(objprefix),$(obj-y))
> > +subdir-obj-y := $(addprefix $(objprefix),$(subdir-obj-y))
> >  
> >  # Final '$(obj)-in.o' object
> > -in-target := $(prefix)$(obj)-in.o
> > +in-target := $(objprefix)$(obj)-in.o
> >  
> >  PHONY += $(subdir-y)
> >  
> > -- 
> > 1.8.5.2 (Apple Git-48)
> > 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified
  2015-06-18 19:59   ` Lukas Wunner
@ 2015-06-18 20:26     ` David Ahern
  2015-06-18 20:39       ` Lukas Wunner
  0 siblings, 1 reply; 10+ messages in thread
From: David Ahern @ 2015-06-18 20:26 UTC (permalink / raw
  To: Lukas Wunner, Jiri Olsa; +Cc: Arnaldo Carvalho de Melo, linux-kernel

On 6/18/15 1:59 PM, Lukas Wunner wrote:
> E.g. the Debian "linux-tools" package makes use of that feature:
>
> 	MAKE_PERF := $(MAKE) prefix=/usr V=1 ARCH=$(KERNEL_ARCH_PERF) EXTRA_WARNINGS=-Wno-error
>
> Source: http://anonscm.debian.org/viewvc/kernel/dists/trunk/linux-tools/debian/build/tools/perf/Makefile?view=markup (line 29)
>
>
> The "prefix" parameter is handed down from Makefile.perf to Makefile.build
> because it's invoked with $(MAKE), so the command line parameters are
> inherited to Makefile.build:
>
> 	$(Q)$(MAKE) $(build)=perf
>
> Source: https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/tools/perf/Makefile.perf#n282
>
>
> That "prefix" feature is broken in all 4.1 release candidates because a

It worked for me last week with OL6.

I created a standalone perf rpm with 4.1-rc6; it builds just fine with 
_prefix (rpm variable) set to /usr:

%build
# prepare directories
rm -rf $RPM_BUILD_ROOT

cd perf-%{kversion}

%global perf_make \
   make -s -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1 
HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_STRLCPY=1 NO_BIONIC=1 prefix=%{_prefix}

%{perf_make} DESTDIR=$RPM_BUILD_ROOT all

###
### install
###

%install
[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT
cd perf-%{kversion}
%{perf_make} DESTDIR=$RPM_BUILD_ROOT lib=%{_lib} install man




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified
  2015-06-18 20:26     ` David Ahern
@ 2015-06-18 20:39       ` Lukas Wunner
  2015-06-18 21:51         ` Jiri Olsa
                           ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Lukas Wunner @ 2015-06-18 20:39 UTC (permalink / raw
  To: David Ahern; +Cc: Jiri Olsa, Arnaldo Carvalho de Melo, linux-kernel

Hi David,

On Thu, Jun 18, 2015 at 02:26:25PM -0600, David Ahern wrote:
> It worked for me last week with OL6.
> I created a standalone perf rpm with 4.1-rc6; it builds just fine with
> _prefix (rpm variable) set to /usr:
[...]
> %global perf_make \
>   make -s -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1 HAVE_CPLUS_DEMANGLE=1
> NO_GTK2=1 NO_STRLCPY=1 NO_BIONIC=1 prefix=%{_prefix}
> %{perf_make} DESTDIR=$RPM_BUILD_ROOT all

You're not invoking tools/perf/Makefile.perf but tools/perf/Makefile
and I would say this in line 18 avoids that prefix= is passed down
to Makefile.perf:

	# We don't want to pass along options like -j:
	unexport MAKEFLAGS

Sources:
https://www.gnu.org/software/make/manual/html_node/Options_002fRecursion.html
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/tools/perf/Makefile#n18

So the prefix parameter should have no effect at all in your case,
no matter to what you set it.

Best regards,

Lukas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified
  2015-06-18 20:39       ` Lukas Wunner
@ 2015-06-18 21:51         ` Jiri Olsa
  2015-06-18 22:07         ` David Ahern
  2015-06-18 22:17         ` Jiri Olsa
  2 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2015-06-18 21:51 UTC (permalink / raw
  To: Lukas Wunner; +Cc: David Ahern, Arnaldo Carvalho de Melo, linux-kernel

On Thu, Jun 18, 2015 at 10:39:35PM +0200, Lukas Wunner wrote:
> Hi David,
> 
> On Thu, Jun 18, 2015 at 02:26:25PM -0600, David Ahern wrote:
> > It worked for me last week with OL6.
> > I created a standalone perf rpm with 4.1-rc6; it builds just fine with
> > _prefix (rpm variable) set to /usr:
> [...]
> > %global perf_make \
> >   make -s -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1 HAVE_CPLUS_DEMANGLE=1
> > NO_GTK2=1 NO_STRLCPY=1 NO_BIONIC=1 prefix=%{_prefix}
> > %{perf_make} DESTDIR=$RPM_BUILD_ROOT all
> 
> You're not invoking tools/perf/Makefile.perf but tools/perf/Makefile
> and I would say this in line 18 avoids that prefix= is passed down
> to Makefile.perf:
> 
> 	# We don't want to pass along options like -j:
> 	unexport MAKEFLAGS
> 
> Sources:
> https://www.gnu.org/software/make/manual/html_node/Options_002fRecursion.html
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/tools/perf/Makefile#n18
> 
> So the prefix parameter should have no effect at all in your case,
> no matter to what you set it.

ouch, right:

"make automatically passes down variable values that were defined on the command line, by putting them in the MAKEFLAGS variable. See the next section. "

haven't realized Makefile.perf is used as primary makefile :-\

I think the patch is ok, I will try to come up with some tests
for this Makefile.perf usage.

thanks,
jirka

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified
       [not found] <201506181332.t5IDWuUq027242@int-mx13.intmail.prod.int.phx2.redhat.com>
  2015-06-18 19:29 ` [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified Jiri Olsa
@ 2015-06-18 21:52 ` Jiri Olsa
  1 sibling, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2015-06-18 21:52 UTC (permalink / raw
  To: Lukas Wunner; +Cc: Jiri Olsa, Arnaldo Carvalho de Melo, linux-kernel

On Thu, Jun 18, 2015 at 01:00:32PM +0200, Lukas Wunner wrote:
> Invoking Makefile.perf with prefix= breaks the build since Makefile.perf
> hands that variable down to Makefile.build where it overrides
>     prefix       := $(subst ./,,$(OUTPUT)$(dir)/)
> 
> leading to errors like this:
>     No rule to make target '/usrabspath.o', needed by '/usrlibperf-in.o'
> 
> Fixes: c819e2cf2eb6f65d3208d195d7a0edef6108d5
> Signed-off-by: Lukas Wunner <lukas@wunner.de>

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified
  2015-06-18 20:39       ` Lukas Wunner
  2015-06-18 21:51         ` Jiri Olsa
@ 2015-06-18 22:07         ` David Ahern
  2015-06-18 22:17         ` Jiri Olsa
  2 siblings, 0 replies; 10+ messages in thread
From: David Ahern @ 2015-06-18 22:07 UTC (permalink / raw
  To: Lukas Wunner; +Cc: Jiri Olsa, Arnaldo Carvalho de Melo, linux-kernel

On 6/18/15 2:39 PM, Lukas Wunner wrote:
> So the prefix parameter should have no effect at all in your case,
> no matter to what you set it.

It does:

rpmbuild -bb --define="_prefix /tmp/junk" SPECS/perf.spec

...
+ make -s -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1 
HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_STRLCPY=1 NO_BIONIC=1 
prefix=/tmp/junk DESTDIR=/root/rpmbuild/BUILDROOT/perf-4.1.rc6-1.x86_64 all

...

Note that prefix=/tmp/junk when tools/perf/Makefile gets invoked.

[root@f21-vbox rpmbuild]# rpm -qvlp 
/root/rpmbuild/RPMS/x86_64/perf-4.1.rc6-1.x86_64.rpm | grep bin/perf
-rwxr-xr-x    2 root    root                  2382176 Jun 18 13:10 
/tmp/junk/bin/perf



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified
  2015-06-18 20:39       ` Lukas Wunner
  2015-06-18 21:51         ` Jiri Olsa
  2015-06-18 22:07         ` David Ahern
@ 2015-06-18 22:17         ` Jiri Olsa
  2015-06-19 11:54           ` Lukas Wunner
  2 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2015-06-18 22:17 UTC (permalink / raw
  To: Lukas Wunner; +Cc: David Ahern, Arnaldo Carvalho de Melo, linux-kernel

On Thu, Jun 18, 2015 at 10:39:35PM +0200, Lukas Wunner wrote:
> Hi David,
> 
> On Thu, Jun 18, 2015 at 02:26:25PM -0600, David Ahern wrote:
> > It worked for me last week with OL6.
> > I created a standalone perf rpm with 4.1-rc6; it builds just fine with
> > _prefix (rpm variable) set to /usr:
> [...]
> > %global perf_make \
> >   make -s -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1 HAVE_CPLUS_DEMANGLE=1
> > NO_GTK2=1 NO_STRLCPY=1 NO_BIONIC=1 prefix=%{_prefix}
> > %{perf_make} DESTDIR=$RPM_BUILD_ROOT all
> 
> You're not invoking tools/perf/Makefile.perf but tools/perf/Makefile
> and I would say this in line 18 avoids that prefix= is passed down
> to Makefile.perf:
> 
> 	# We don't want to pass along options like -j:
> 	unexport MAKEFLAGS
> 
> Sources:
> https://www.gnu.org/software/make/manual/html_node/Options_002fRecursion.html
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/tools/perf/Makefile#n18
> 
> So the prefix parameter should have no effect at all in your case,
> no matter to what you set it.

it had effect.. prefix is passed to build framework throught .config-detected
file as prefix_SQ variable, which is then referenced in perf Build makefiles

the issue is that having 'unexport MAKEFLAGS' in perf's Makefile causes
'prefix' variable to change from 'command line' origin to environment

so the tools/build/Makefile.build can set prefix if called from perf's
'Makefile', but will not override it if called from perf's Makefile.perf,
because prefix will have 'command line' origin and we would need to use
'override'

jirka

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified
  2015-06-18 22:17         ` Jiri Olsa
@ 2015-06-19 11:54           ` Lukas Wunner
  2015-06-19 14:19             ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 10+ messages in thread
From: Lukas Wunner @ 2015-06-19 11:54 UTC (permalink / raw
  To: Jiri Olsa; +Cc: David Ahern, Arnaldo Carvalho de Melo, linux-kernel

Hi Jiri, Hi David,

On Fri, Jun 19, 2015 at 12:17:06AM +0200, Jiri Olsa wrote:
> it had effect.. prefix is passed to build framework throught .config-detected
> file as prefix_SQ variable, which is then referenced in perf Build makefiles

You're right, thanks for the clarification.

The underlying issue is basically that the same variable name is used for
two different things, to calculate the paths of the build targets in
Makefile.build and to pass the install directory around between Makefiles.
And which one "wins" depends on invocation. If we just rename the variable
in Makefile.build, the issue is gone. That's what the patch does.

Thanks again,

Lukas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified
  2015-06-19 11:54           ` Lukas Wunner
@ 2015-06-19 14:19             ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-19 14:19 UTC (permalink / raw
  To: Lukas Wunner; +Cc: Jiri Olsa, David Ahern, linux-kernel

Em Fri, Jun 19, 2015 at 01:54:17PM +0200, Lukas Wunner escreveu:
> Hi Jiri, Hi David,
> 
> On Fri, Jun 19, 2015 at 12:17:06AM +0200, Jiri Olsa wrote:
> > it had effect.. prefix is passed to build framework throught .config-detected
> > file as prefix_SQ variable, which is then referenced in perf Build makefiles
> 
> You're right, thanks for the clarification.

Yes, this is all very involved, lots of details, that would be great to
have in the changeset log, so that we could read it when scratching our
heads trying to understand all this :-\

Could someone, please, send a v2 with these explanations?

- Arnaldo
 
> The underlying issue is basically that the same variable name is used for
> two different things, to calculate the paths of the build targets in
> Makefile.build and to pass the install directory around between Makefiles.
> And which one "wins" depends on invocation. If we just rename the variable
> in Makefile.build, the issue is gone. That's what the patch does.
> 
> Thanks again,
> 
> Lukas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-06-19 14:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <201506181332.t5IDWuUq027242@int-mx13.intmail.prod.int.phx2.redhat.com>
2015-06-18 19:29 ` [PATCH regression 4.0 -> 4.1] tools perf: Fix build breakage if prefix= is specified Jiri Olsa
2015-06-18 19:59   ` Lukas Wunner
2015-06-18 20:26     ` David Ahern
2015-06-18 20:39       ` Lukas Wunner
2015-06-18 21:51         ` Jiri Olsa
2015-06-18 22:07         ` David Ahern
2015-06-18 22:17         ` Jiri Olsa
2015-06-19 11:54           ` Lukas Wunner
2015-06-19 14:19             ` Arnaldo Carvalho de Melo
2015-06-18 21:52 ` Jiri Olsa

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 read-only IMAP folder(s) and NNTP newsgroup(s).