2014-01-22 15:02:05

by Josh Boyer

Subject: [PATCH] perf tools: Fix traceevent plugin path definitions

The plugindir_SQ definition contains $(prefix) which is not needed
as the $(libdir) definition already contains prefix in it. This
leads to the path including an extra prefix in it, e.g. /usr/usr/lib64.

The -DPLUGIN_DIR defintion includes DESTDIR. This is incorrect, as it
sets the plugin search path to include the value of DESTDIR. DESTDIR
is a mechanism to install in a non-standard location such as a chroot
or an RPM build root. In the RPM case, this leads to the search path
being incorrect after the resulting RPM is installed (or in some cases
an RPM build failure).

Remove both of these unnecessary inclusions.

Signed-off-by: Josh Boyer <[email protected]>
---
tools/lib/traceevent/Makefile | 2 +-
tools/perf/config/Makefile | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

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

2014-01-22 15:34:56

by Jiri Olsa

Subject: Re: [PATCH] perf tools: Fix traceevent plugin path definitions

On Wed, Jan 22, 2014 at 10:01:48AM -0500, Josh Boyer wrote:
> The plugindir_SQ definition contains $(prefix) which is not needed
> as the $(libdir) definition already contains prefix in it. This
> leads to the path including an extra prefix in it, e.g. /usr/usr/lib64.
>
> The -DPLUGIN_DIR defintion includes DESTDIR. This is incorrect, as it
> sets the plugin search path to include the value of DESTDIR. DESTDIR
> is a mechanism to install in a non-standard location such as a chroot
> or an RPM build root. In the RPM case, this leads to the search path
> being incorrect after the resulting RPM is installed (or in some cases
> an RPM build failure).
>
> Remove both of these unnecessary inclusions.
>
> Signed-off-by: Josh Boyer <[email protected]>

Acked-by: Jiri Olsa <[email protected]>

thanks,
jirka

2014-01-25 14:24:29

by tip-bot for Vasyl Gomonovych

Subject: [tip:perf/urgent] perf tools: Fix traceevent plugin path definitions

Commit-ID: b935a58dbff457c27fd63e1e1bb29db20b2ee6a8
Gitweb: http://git.kernel.org/tip/b935a58dbff457c27fd63e1e1bb29db20b2ee6a8
Author: Josh Boyer <[email protected]>
AuthorDate: Wed, 22 Jan 2014 10:01:48 -0500
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 23 Jan 2014 15:48:12 -0300

perf tools: Fix traceevent plugin path definitions

The plugindir_SQ definition contains $(prefix) which is not needed as
the $(libdir) definition already contains prefix in it. This leads to
the path including an extra prefix in it, e.g. /usr/usr/lib64.

The -DPLUGIN_DIR defintion includes DESTDIR. This is incorrect, as it
sets the plugin search path to include the value of DESTDIR. DESTDIR is
a mechanism to install in a non-standard location such as a chroot or an
RPM build root. In the RPM case, this leads to the search path being
incorrect after the resulting RPM is installed (or in some cases an RPM
build failure).

Remove both of these unnecessary inclusions.

Signed-off-by: Josh Boyer <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/Makefile | 2 +-
tools/perf/config/Makefile | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

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

2014-01-27 05:24:04

by Namhyung Kim

Subject: Re: [PATCH] perf tools: Fix traceevent plugin path definitions

On Wed, 22 Jan 2014 10:01:48 -0500, Josh Boyer wrote:
> The plugindir_SQ definition contains $(prefix) which is not needed
> as the $(libdir) definition already contains prefix in it. This
> leads to the path including an extra prefix in it, e.g. /usr/usr/lib64.
>
> The -DPLUGIN_DIR defintion includes DESTDIR. This is incorrect, as it
> sets the plugin search path to include the value of DESTDIR. DESTDIR
> is a mechanism to install in a non-standard location such as a chroot
> or an RPM build root. In the RPM case, this leads to the search path
> being incorrect after the resulting RPM is installed (or in some cases
> an RPM build failure).
>
> Remove both of these unnecessary inclusions.

Oops, I replied before reading this and previous discussion.. :-/

Anyway it looks good to me too.

Thanks,
Namhyung