2014-01-21 20:02:48

by Josh Boyer

[permalink] [raw]
Subject: Weird plugin paths in perf and perf.so binaries with 3.14 merge window

Hi All,

I went to build Linux v3.13-737-g7fe67a1 in Fedora this morning and it
resulted in RPM complaining that the perf and perf.so binaries had
strings in them that matched the RPM_BUILD_ROOT string. That fails
the RPM build.

Looking over the logs, I see that the traceevent plugins are getting a
rather weird -DPLUGIN_DIR define passed to them. E.g.:

gcc -c -g -Wall -I. -I
/home/jwboyer/kernel/kernel-3.13.fc21/linux-3.14.0-0.rc0.git1.1.fc21.x86_64/tools/lib/traceevent/../../include
'-DPLUGIN_DIR="/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64//usr//usr/lib64/traceevent/plugins"'
-D_GNU_SOURCE -std=gnu99 -fPIC
/home/jwboyer/kernel/kernel-3.13.fc21/linux-3.14.0-0.rc0.git1.1.fc21.x86_64/tools/lib/traceevent/parse-filter.c
-o parse-filter.o

We're building perf like so:

make -s -j8 -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1
HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_LIBNUMA=1 NO_STRLCPY=1 NO_BIONIC=1
prefix=/usr DESTDIR=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64
all

and installing it via:

make -s -j8 -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1
HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_LIBNUMA=1 NO_STRLCPY=1 NO_BIONIC=1
prefix=/usr DESTDIR=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64
install-bin

make -s -j8 -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1
HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_LIBNUMA=1 NO_STRLCPY=1 NO_BIONIC=1
prefix=/usr DESTDIR=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64
install-python_ext

This has worked for all the builds up until this point.

Somehow the perf and perf.so binaries are getting the string being
passed via -DPLUGIN_DIR into them, likely through the libtraceevent.a
link. I'm pretty sure that (1) the string being passed is totally
broken and should be "/usr/lib64/traceevent/plugins" and (2) that I
haven't come close to deciphering how to fix this.

So, could you please look this over and see where the define is
getting messed up?

josh


2014-01-22 00:58:46

by Josh Boyer

[permalink] [raw]
Subject: Re: Weird plugin paths in perf and perf.so binaries with 3.14 merge window

On Tue, Jan 21, 2014 at 3:02 PM, Josh Boyer <[email protected]> wrote:
> Hi All,
>
> I went to build Linux v3.13-737-g7fe67a1 in Fedora this morning and it
> resulted in RPM complaining that the perf and perf.so binaries had
> strings in them that matched the RPM_BUILD_ROOT string. That fails
> the RPM build.
>
> Looking over the logs, I see that the traceevent plugins are getting a
> rather weird -DPLUGIN_DIR define passed to them. E.g.:
>
> gcc -c -g -Wall -I. -I
> /home/jwboyer/kernel/kernel-3.13.fc21/linux-3.14.0-0.rc0.git1.1.fc21.x86_64/tools/lib/traceevent/../../include
> '-DPLUGIN_DIR="/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64//usr//usr/lib64/traceevent/plugins"'
> -D_GNU_SOURCE -std=gnu99 -fPIC
> /home/jwboyer/kernel/kernel-3.13.fc21/linux-3.14.0-0.rc0.git1.1.fc21.x86_64/tools/lib/traceevent/parse-filter.c
> -o parse-filter.o
>
> We're building perf like so:
>
> make -s -j8 -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1
> HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_LIBNUMA=1 NO_STRLCPY=1 NO_BIONIC=1
> prefix=/usr DESTDIR=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64
> all
>
> and installing it via:
>
> make -s -j8 -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1
> HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_LIBNUMA=1 NO_STRLCPY=1 NO_BIONIC=1
> prefix=/usr DESTDIR=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64
> install-bin
>
> make -s -j8 -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1
> HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_LIBNUMA=1 NO_STRLCPY=1 NO_BIONIC=1
> prefix=/usr DESTDIR=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64
> install-python_ext
>
> This has worked for all the builds up until this point.
>
> Somehow the perf and perf.so binaries are getting the string being
> passed via -DPLUGIN_DIR into them, likely through the libtraceevent.a
> link. I'm pretty sure that (1) the string being passed is totally
> broken and should be "/usr/lib64/traceevent/plugins" and (2) that I
> haven't come close to deciphering how to fix this.
>
> So, could you please look this over and see where the define is
> getting messed up?

With the patch below (also attached because I'm sure gmail is going to
mess this up), I get a much more sane PLUGIN_DIR value:

gcc -c -g -Wall -I. -I
/home/jwboyer/kernel/kernel-3.13.fc21/linux-3.14.0-0.rc0.git1.1.fc21.x86_64/tools/lib/traceevent/../../include
'-DPLUGIN_DIR="/usr/lib64/traceevent/plugins"' -D_GNU_SOURCE -fPIC
-o plugin_scsi.o
/home/jwboyer/kernel/kernel-3.13.fc21/linux-3.14.0-0.rc0.git1.1.fc21.x86_64/tools/lib/traceevent/plugin_scsi.c

I'm not sure why DESTDIR is included in the -DPLUGIN_DIR definition in
tools/lib/traceevent/Makefile, nor why $(prefix) is in plugindir_SQ in
tools/perf/config/Makefile.

josh

diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index 56d52a3..005c9cc 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -63,7 +63,7 @@ endif
endif

ifeq ($(set_plugin_dir),1)
-PLUGIN_DIR = -DPLUGIN_DIR="$(DESTDIR)/$(plugin_dir)"
+PLUGIN_DIR = -DPLUGIN_DIR="$(plugin_dir)"
PLUGIN_DIR_SQ = '$(subst ','\'',$(PLUGIN_DIR))'
endif

diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index d604e50..c48d449 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -600,5 +600,5 @@ perfexec_instdir_SQ = $(subst ','\'',$(perfexec_instdir))
# Otherwise we install plugins into the global $(libdir).
ifdef DESTDIR
plugindir=$(libdir)/traceevent/plugins
-plugindir_SQ= $(subst ','\'',$(prefix)/$(plugindir))
+plugindir_SQ= $(subst ','\'',$(plugindir))
endif


Attachments:
plugin-dir-perf.patch (855.00 B)

2014-01-22 11:36:48

by Jiri Olsa

[permalink] [raw]
Subject: Re: Weird plugin paths in perf and perf.so binaries with 3.14 merge window

On Tue, Jan 21, 2014 at 03:02:43PM -0500, Josh Boyer wrote:
> Hi All,
>
> I went to build Linux v3.13-737-g7fe67a1 in Fedora this morning and it
> resulted in RPM complaining that the perf and perf.so binaries had
> strings in them that matched the RPM_BUILD_ROOT string. That fails
> the RPM build.
>
> Looking over the logs, I see that the traceevent plugins are getting a
> rather weird -DPLUGIN_DIR define passed to them. E.g.:
>
> gcc -c -g -Wall -I. -I
> /home/jwboyer/kernel/kernel-3.13.fc21/linux-3.14.0-0.rc0.git1.1.fc21.x86_64/tools/lib/traceevent/../../include
> '-DPLUGIN_DIR="/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64//usr//usr/lib64/traceevent/plugins"'
> -D_GNU_SOURCE -std=gnu99 -fPIC
> /home/jwboyer/kernel/kernel-3.13.fc21/linux-3.14.0-0.rc0.git1.1.fc21.x86_64/tools/lib/traceevent/parse-filter.c
> -o parse-filter.o
>
> We're building perf like so:
>
> make -s -j8 -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1
> HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_LIBNUMA=1 NO_STRLCPY=1 NO_BIONIC=1
> prefix=/usr DESTDIR=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64
> all
>
> and installing it via:
>
> make -s -j8 -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1
> HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_LIBNUMA=1 NO_STRLCPY=1 NO_BIONIC=1
> prefix=/usr DESTDIR=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64
> install-bin
>
> make -s -j8 -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1
> HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_LIBNUMA=1 NO_STRLCPY=1 NO_BIONIC=1
> prefix=/usr DESTDIR=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64
> install-python_ext
>
> This has worked for all the builds up until this point.
>
> Somehow the perf and perf.so binaries are getting the string being
> passed via -DPLUGIN_DIR into them, likely through the libtraceevent.a
> link. I'm pretty sure that (1) the string being passed is totally
> broken and should be "/usr/lib64/traceevent/plugins" and (2) that I
> haven't come close to deciphering how to fix this.
>
> So, could you please look this over and see where the define is
> getting messed up?

hi,
just get to this.. checking

jirka

2014-01-22 13:02:28

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH] perf tests: Add install prefix tests

On Tue, Jan 21, 2014 at 07:58:41PM -0500, Josh Boyer wrote:
> On Tue, Jan 21, 2014 at 3:02 PM, Josh Boyer <[email protected]> wrote:
> > Hi All,
> >
> > I went to build Linux v3.13-737-g7fe67a1 in Fedora this morning and it
> > resulted in RPM complaining that the perf and perf.so binaries had
>

SNIP

> I'm not sure why DESTDIR is included in the -DPLUGIN_DIR definition in
> tools/lib/traceevent/Makefile, nor why $(prefix) is in plugindir_SQ in
> tools/perf/config/Makefile.
>
> josh
>
> diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
> index 56d52a3..005c9cc 100644
> --- a/tools/lib/traceevent/Makefile
> +++ b/tools/lib/traceevent/Makefile
> @@ -63,7 +63,7 @@ endif
> endif
>
> ifeq ($(set_plugin_dir),1)
> -PLUGIN_DIR = -DPLUGIN_DIR="$(DESTDIR)/$(plugin_dir)"
> +PLUGIN_DIR = -DPLUGIN_DIR="$(plugin_dir)"
> PLUGIN_DIR_SQ = '$(subst ','\'',$(PLUGIN_DIR))'

hi,
this one needs to stay, otherwise perf have wrong path
to load plugins from..


> endif
>
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index d604e50..c48d449 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -600,5 +600,5 @@ perfexec_instdir_SQ = $(subst ','\'',$(perfexec_instdir))
> # Otherwise we install plugins into the global $(libdir).
> ifdef DESTDIR
> plugindir=$(libdir)/traceevent/plugins
> -plugindir_SQ= $(subst ','\'',$(prefix)/$(plugindir))
> +plugindir_SQ= $(subst ','\'',$(plugindir))

this one is proper fix, $(libdir) is defined as:

libdir = $(prefix)/$(lib)

please let me know if you want me to send the patch or
if you do that by yourself..

attached is updated perf make test with new prefix
tests.. we never test this ;-)

thanks for the fix,
jirka


---
Adding automated tests for:
$ make install prefix=/usr DESTDIR=...
$ make_install_bin prefix=/usr DESTDIR=...

Plus some generalization fixies for install tests.

Signed-off-by: Jiri Olsa <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: David Ahern <[email protected]>
---
tools/perf/tests/make | 37 +++++++++++++++++++++++++++++--------
1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/tools/perf/tests/make b/tools/perf/tests/make
index 00544b8..faf3a1f 100644
--- a/tools/perf/tests/make
+++ b/tools/perf/tests/make
@@ -44,6 +44,8 @@ make_install_man := install-man
make_install_html := install-html
make_install_info := install-info
make_install_pdf := install-pdf
+make_install_prefix := install prefix=/usr
+make_install_bin_prefix := install prefix=/usr

# all the NO_* variable combined
make_minimal := NO_LIBPERL=1 NO_LIBPYTHON=1 NO_NEWT=1 NO_GTK2=1
@@ -75,6 +77,8 @@ run += make_perf_o
run += make_util_map_o
run += make_install
run += make_install_bin
+run += make_install_prefix
+run += make_install_bin_prefix
# FIXME 'install-*' commented out till they're fixed
# run += make_install_doc
# run += make_install_man
@@ -117,16 +121,17 @@ test_make_perf_o := test -f $(PERF)/perf.o
test_make_util_map_o := test -f $(PERF)/util/map.o

define test_dest_files
- for file in $(1); do \
- if [ ! -x $$TMP_DEST/$$file ]; then \
- echo " failed to find: $$file"; \
- fi \
+ for file in $(1); do \
+ if [ ! -x $$TMP_DEST/$$file ]; then \
+ echo " failed to find: $$TMP_DEST/$$file"; \
+ fi \
done
endef

-installed_files_bin := bin/perf
-installed_files_bin += etc/bash_completion.d/perf
-installed_files_bin += libexec/perf-core/perf-archive
+installed_files_bin_no_prefix := etc/bash_completion.d/perf
+
+installed_files_bin_prefix += bin/perf
+installed_files_bin_prefix += libexec/perf-core/perf-archive

installed_files_plugins := $(lib)/traceevent/plugins/plugin_cfg80211.so
installed_files_plugins += $(lib)/traceevent/plugins/plugin_scsi.so
@@ -139,7 +144,11 @@ installed_files_plugins += $(lib)/traceevent/plugins/plugin_kmem.so
installed_files_plugins += $(lib)/traceevent/plugins/plugin_hrtimer.so
installed_files_plugins += $(lib)/traceevent/plugins/plugin_jbd2.so

-installed_files_all := $(installed_files_bin)
+installed_files_bin := $(installed_files_bin_no_prefix)
+installed_files_bin += $(nstalled_files_bin_prefix)
+
+installed_files_all := $(installed_files_bin_no_prefix)
+installed_files_all += $(installed_files_bin)
installed_files_all += $(installed_files_plugins)

test_make_install := $(call test_dest_files,$(installed_files_all))
@@ -147,6 +156,18 @@ test_make_install_O := $(call test_dest_files,$(installed_files_all))
test_make_install_bin := $(call test_dest_files,$(installed_files_bin))
test_make_install_bin_O := $(call test_dest_files,$(installed_files_bin))

+installed_files_prefix_bin := $(addprefix usr/,$(installed_files_bin_prefix))
+installed_files_prefix_bin += $(installed_files_bin_no_prefix)
+
+installed_files_prefix_all := $(addprefix usr/,$(installed_files_plugins))
+installed_files_prefix_all += $(addprefix usr/,$(installed_files_bin_prefix))
+installed_files_prefix_all += $(installed_files_bin_no_prefix)
+
+test_make_install_prefix := $(call test_dest_files,$(installed_files_prefix_all))
+test_make_install_prefix_O := $(call test_dest_files,$(installed_files_prefix_all))
+test_make_install_bin_prefix := $(call test_dest_files,$(installed_files_prefix_bin))
+test_make_install_bin_prefix_O := $(call test_dest_files,$(installed_files_prefix_bin))
+
# FIXME nothing gets installed
test_make_install_man := test -f $$TMP_DEST/share/man/man1/perf.1
test_make_install_man_O := $(test_make_install_man)
--
1.8.3.1

2014-01-22 14:03:27

by Josh Boyer

[permalink] [raw]
Subject: Re: [PATCH] perf tests: Add install prefix tests

On Wed, Jan 22, 2014 at 8:02 AM, Jiri Olsa <[email protected]> wrote:
> On Tue, Jan 21, 2014 at 07:58:41PM -0500, Josh Boyer wrote:
>> On Tue, Jan 21, 2014 at 3:02 PM, Josh Boyer <[email protected]> wrote:
>> > Hi All,
>> >
>> > I went to build Linux v3.13-737-g7fe67a1 in Fedora this morning and it
>> > resulted in RPM complaining that the perf and perf.so binaries had
>>
>
> SNIP
>
>> I'm not sure why DESTDIR is included in the -DPLUGIN_DIR definition in
>> tools/lib/traceevent/Makefile, nor why $(prefix) is in plugindir_SQ in
>> tools/perf/config/Makefile.
>>
>> josh
>>
>> diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
>> index 56d52a3..005c9cc 100644
>> --- a/tools/lib/traceevent/Makefile
>> +++ b/tools/lib/traceevent/Makefile
>> @@ -63,7 +63,7 @@ endif
>> endif
>>
>> ifeq ($(set_plugin_dir),1)
>> -PLUGIN_DIR = -DPLUGIN_DIR="$(DESTDIR)/$(plugin_dir)"
>> +PLUGIN_DIR = -DPLUGIN_DIR="$(plugin_dir)"
>> PLUGIN_DIR_SQ = '$(subst ','\'',$(PLUGIN_DIR))'
>
> hi,
> this one needs to stay, otherwise perf have wrong path
> to load plugins from..

Hm. Are you sure? I believe I tested with just the $(prefix) removed
below and it still failed the RPM build. If DESTDIR is included in
what is passed as DPLUGIN_DIR, it will include the buildroot path in
the binaries, but when the RPM is installed it does not install to the
buildroot path. E.g. you will have:

-DPLUGIN_DIR="/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64//usr/lib64/traceevent/plugins"

but the plugins will be installed to /usr/lib64/traceevent/plugins by
RPM. I'll try again with just the libdir fix, but I'm fairly sure
what I've said above is correct. At least in terms of how RPMs are
built.

josh

2014-01-22 14:45:12

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf tests: Add install prefix tests

On Wed, Jan 22, 2014 at 09:03:21AM -0500, Josh Boyer wrote:
> On Wed, Jan 22, 2014 at 8:02 AM, Jiri Olsa <[email protected]> wrote:
> > On Tue, Jan 21, 2014 at 07:58:41PM -0500, Josh Boyer wrote:
> >> On Tue, Jan 21, 2014 at 3:02 PM, Josh Boyer <[email protected]> wrote:
> >> > Hi All,
> >> >
> >> > I went to build Linux v3.13-737-g7fe67a1 in Fedora this morning and it
> >> > resulted in RPM complaining that the perf and perf.so binaries had
> >>
> >
> > SNIP
> >
> >> I'm not sure why DESTDIR is included in the -DPLUGIN_DIR definition in
> >> tools/lib/traceevent/Makefile, nor why $(prefix) is in plugindir_SQ in
> >> tools/perf/config/Makefile.
> >>
> >> josh
> >>
> >> diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
> >> index 56d52a3..005c9cc 100644
> >> --- a/tools/lib/traceevent/Makefile
> >> +++ b/tools/lib/traceevent/Makefile
> >> @@ -63,7 +63,7 @@ endif
> >> endif
> >>
> >> ifeq ($(set_plugin_dir),1)
> >> -PLUGIN_DIR = -DPLUGIN_DIR="$(DESTDIR)/$(plugin_dir)"
> >> +PLUGIN_DIR = -DPLUGIN_DIR="$(plugin_dir)"
> >> PLUGIN_DIR_SQ = '$(subst ','\'',$(PLUGIN_DIR))'
> >
> > hi,
> > this one needs to stay, otherwise perf have wrong path
> > to load plugins from..
>
> Hm. Are you sure? I believe I tested with just the $(prefix) removed
> below and it still failed the RPM build. If DESTDIR is included in
> what is passed as DPLUGIN_DIR, it will include the buildroot path in
> the binaries, but when the RPM is installed it does not install to the
> buildroot path. E.g. you will have:
>
> -DPLUGIN_DIR="/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64//usr/lib64/traceevent/plugins"
>
> but the plugins will be installed to /usr/lib64/traceevent/plugins by
> RPM. I'll try again with just the libdir fix, but I'm fairly sure
> what I've said above is correct. At least in terms of how RPMs are
> built.

ok, now I'm woken up... you're right, both hunks are needed,
got confused with the DESTDIR real meaning, sry

jirka

2014-01-22 14:46:39

by Josh Boyer

[permalink] [raw]
Subject: Re: [PATCH] perf tests: Add install prefix tests

On Wed, Jan 22, 2014 at 9:44 AM, Jiri Olsa <[email protected]> wrote:
> On Wed, Jan 22, 2014 at 09:03:21AM -0500, Josh Boyer wrote:
>> On Wed, Jan 22, 2014 at 8:02 AM, Jiri Olsa <[email protected]> wrote:
>> > On Tue, Jan 21, 2014 at 07:58:41PM -0500, Josh Boyer wrote:
>> >> On Tue, Jan 21, 2014 at 3:02 PM, Josh Boyer <[email protected]> wrote:
>> >> > Hi All,
>> >> >
>> >> > I went to build Linux v3.13-737-g7fe67a1 in Fedora this morning and it
>> >> > resulted in RPM complaining that the perf and perf.so binaries had
>> >>
>> >
>> > SNIP
>> >
>> >> I'm not sure why DESTDIR is included in the -DPLUGIN_DIR definition in
>> >> tools/lib/traceevent/Makefile, nor why $(prefix) is in plugindir_SQ in
>> >> tools/perf/config/Makefile.
>> >>
>> >> josh
>> >>
>> >> diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
>> >> index 56d52a3..005c9cc 100644
>> >> --- a/tools/lib/traceevent/Makefile
>> >> +++ b/tools/lib/traceevent/Makefile
>> >> @@ -63,7 +63,7 @@ endif
>> >> endif
>> >>
>> >> ifeq ($(set_plugin_dir),1)
>> >> -PLUGIN_DIR = -DPLUGIN_DIR="$(DESTDIR)/$(plugin_dir)"
>> >> +PLUGIN_DIR = -DPLUGIN_DIR="$(plugin_dir)"
>> >> PLUGIN_DIR_SQ = '$(subst ','\'',$(PLUGIN_DIR))'
>> >
>> > hi,
>> > this one needs to stay, otherwise perf have wrong path
>> > to load plugins from..
>>
>> Hm. Are you sure? I believe I tested with just the $(prefix) removed
>> below and it still failed the RPM build. If DESTDIR is included in
>> what is passed as DPLUGIN_DIR, it will include the buildroot path in
>> the binaries, but when the RPM is installed it does not install to the
>> buildroot path. E.g. you will have:
>>
>> -DPLUGIN_DIR="/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64//usr/lib64/traceevent/plugins"
>>
>> but the plugins will be installed to /usr/lib64/traceevent/plugins by
>> RPM. I'll try again with just the libdir fix, but I'm fairly sure
>> what I've said above is correct. At least in terms of how RPMs are
>> built.
>
> ok, now I'm woken up... you're right, both hunks are needed,
> got confused with the DESTDIR real meaning, sry

OK, good. FWIW, I did just retest with the DESTDIR hunk removed and
it does fail as I said above. I'll come up with an actual patch with
a S-o-B and send it out shortly.

Thanks!

josh

2014-01-27 05:14:59

by Namhyung Kim

[permalink] [raw]
Subject: Re: Weird plugin paths in perf and perf.so binaries with 3.14 merge window

Hi Jiri and Josh,

On Wed, 22 Jan 2014 12:36:30 +0100, Jiri Olsa wrote:
> On Tue, Jan 21, 2014 at 03:02:43PM -0500, Josh Boyer wrote:
>> Hi All,
>>
>> I went to build Linux v3.13-737-g7fe67a1 in Fedora this morning and it
>> resulted in RPM complaining that the perf and perf.so binaries had
>> strings in them that matched the RPM_BUILD_ROOT string. That fails
>> the RPM build.
>>
>> Looking over the logs, I see that the traceevent plugins are getting a
>> rather weird -DPLUGIN_DIR define passed to them. E.g.:
>>
>> gcc -c -g -Wall -I. -I
>> /home/jwboyer/kernel/kernel-3.13.fc21/linux-3.14.0-0.rc0.git1.1.fc21.x86_64/tools/lib/traceevent/../../include
>> '-DPLUGIN_DIR="/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64//usr//usr/lib64/traceevent/plugins"'
>> -D_GNU_SOURCE -std=gnu99 -fPIC
>> /home/jwboyer/kernel/kernel-3.13.fc21/linux-3.14.0-0.rc0.git1.1.fc21.x86_64/tools/lib/traceevent/parse-filter.c
>> -o parse-filter.o
>>
>> We're building perf like so:
>>
>> make -s -j8 -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1
>> HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_LIBNUMA=1 NO_STRLCPY=1 NO_BIONIC=1
>> prefix=/usr DESTDIR=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64
>> all
>>
>> and installing it via:
>>
>> make -s -j8 -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1
>> HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_LIBNUMA=1 NO_STRLCPY=1 NO_BIONIC=1
>> prefix=/usr DESTDIR=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64
>> install-bin
>>
>> make -s -j8 -C tools/perf V=1 WERROR=0 NO_LIBUNWIND=1
>> HAVE_CPLUS_DEMANGLE=1 NO_GTK2=1 NO_LIBNUMA=1 NO_STRLCPY=1 NO_BIONIC=1
>> prefix=/usr DESTDIR=/home/jwboyer/rpmbuild/BUILDROOT/kernel-3.14.0-0.rc0.git1.1.fc21.x86_64
>> install-python_ext
>>
>> This has worked for all the builds up until this point.
>>
>> Somehow the perf and perf.so binaries are getting the string being
>> passed via -DPLUGIN_DIR into them, likely through the libtraceevent.a
>> link. I'm pretty sure that (1) the string being passed is totally
>> broken and should be "/usr/lib64/traceevent/plugins" and (2) that I
>> haven't come close to deciphering how to fix this.
>>
>> So, could you please look this over and see where the define is
>> getting messed up?

I found three things on the issue:

1. perf defines plugin_dir if DESTDIR is set like below:

ifdef DESTDIR
plugindir=$(libdir)/traceevent/plugins
plugindir_SQ= $(subst ','\'',$(prefix)/$(plugindir))
endif

But this is wrong since $plugindir includes $libdir and $libdir
includes $prefix in it. So there'll be two $prefix's in
$plugindir_SQ as you can see above (...//usr//usr/...).


2. And libtraceevent prepends DESTDIR to the plugindir:

ifeq ($(set_plugin_dir),1)
PLUGIN_DIR = -DPLUGIN_DIR="$(DESTDIR)/$(plugin_dir)"
PLUGIN_DIR_SQ = '$(subst ','\'',$(PLUGIN_DIR))'
endif

I don't know if it's desired but as Josh reported it seems not.
Maybe we'd better adding just $prefix part not the $DESTDIR.


3. libtraceevent's default plugin_dir is set like:

ifeq ($(prefix),$(HOME))
override plugin_dir = $(HOME)/.traceevent/plugins
set_plugin_dir := 0
else
override plugin_dir = $(prefix)/lib/traceevent/plugins
endif

But it'd better using lib64/ instead of lib/ on 64 bit machines. We
can reuse same logic (libdir) in the perf Makefile but it's not a big
deal as we can set it on the cmdline so I won't insist strongly. ;-)


Thanks,
Namhyung