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 <[email protected]>
> ---
> 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)
>
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 <[email protected]>
> > ---
> > 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)
> >
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
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
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
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
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
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
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