2021-03-15 16:56:41

by Denys Zagorui

[permalink] [raw]
Subject: [PATCH v2] perf: build reproducibility improvements

This patch helps to make perf build more reproducible

It seems there is some need to have an ability to invoke
perf from build directory without installation
(84cfac7f05e1: perf tools: Set and pass DOCDIR to builtin-report.c)
DOCDIR contains an absolute path to kernel source directory.
In such case this path can be determined in runtime by using
/proc/self/exe link. In case of building perf with O=
Documentation/tips.txt can be copied to output directory.

There is also python binding test where PYTHONPATH is used to store
absolute path to python/perf.so library. This path can be
also determined in runtime.

bison stores full paths in generated files. This can be
remapped by using --file-prefix-map option that is available
starting from version 3.7.1.

Signed-off-by: Denys Zagorui <[email protected]>
---
tools/lib/subcmd/exec-cmd.c | 21 +++++++++++++++++++++
tools/lib/subcmd/exec-cmd.h | 1 +
tools/perf/Build | 1 -
tools/perf/Makefile.perf | 3 +++
tools/perf/builtin-report.c | 21 ++++++++++++++++++---
tools/perf/tests/Build | 2 +-
tools/perf/tests/python-use.c | 14 +++++++++++++-
tools/perf/util/Build | 10 +++++++---
8 files changed, 64 insertions(+), 9 deletions(-)

diff --git a/tools/lib/subcmd/exec-cmd.c b/tools/lib/subcmd/exec-cmd.c
index 33e94fb83986..3116246ec629 100644
--- a/tools/lib/subcmd/exec-cmd.c
+++ b/tools/lib/subcmd/exec-cmd.c
@@ -208,3 +208,24 @@ int execl_cmd(const char *cmd,...)
argv[argc] = NULL;
return execv_cmd(argv);
}
+
+/* The caller is responsible to free the returned buffer */
+char *get_exec_abs_path(void)
+{
+ int ret;
+ int i;
+ char *buf;
+
+ buf = malloc(PATH_MAX);
+ ret = readlink("/proc/self/exe", buf, PATH_MAX - 1);
+ if (ret <= 0) {
+ free(buf);
+ return NULL;
+ }
+
+ for (i = ret - 1; buf[i] != '/'; i--);
+
+ buf[i + 1] = 0;
+
+ return buf;
+}
diff --git a/tools/lib/subcmd/exec-cmd.h b/tools/lib/subcmd/exec-cmd.h
index aba591b8d254..9dc98248dadf 100644
--- a/tools/lib/subcmd/exec-cmd.h
+++ b/tools/lib/subcmd/exec-cmd.h
@@ -13,5 +13,6 @@ extern int execl_cmd(const char *cmd, ...);
/* get_argv_exec_path and system_path return malloc'd string, caller must free it */
extern char *get_argv_exec_path(void);
extern char *system_path(const char *path);
+extern char *get_exec_abs_path(void);

#endif /* __SUBCMD_EXEC_CMD_H */
diff --git a/tools/perf/Build b/tools/perf/Build
index db61dbe2b543..56d0189f1029 100644
--- a/tools/perf/Build
+++ b/tools/perf/Build
@@ -45,7 +45,6 @@ CFLAGS_perf.o += -DPERF_HTML_PATH="BUILD_STR($(htmldir_SQ))" \
-DPREFIX="BUILD_STR($(prefix_SQ))"
CFLAGS_builtin-trace.o += -DSTRACE_GROUPS_DIR="BUILD_STR($(STRACE_GROUPS_DIR_SQ))"
CFLAGS_builtin-report.o += -DTIPDIR="BUILD_STR($(tipdir_SQ))"
-CFLAGS_builtin-report.o += -DDOCDIR="BUILD_STR($(srcdir_SQ)/Documentation)"

perf-y += util/
perf-y += arch/
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index f6e609673de2..c9e3aae6c47f 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -627,6 +627,9 @@ $(OUTPUT)python/perf.so: $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS) $(LIBTRACEEVENT_D
--quiet build_ext; \
cp $(PYTHON_EXTBUILD_LIB)perf*.so $(OUTPUT)python/

+_dummy := $(shell [ -d '$(OUTPUT)Documentation' ] || (mkdir -p '$(OUTPUT)Documentation' && \
+ cp '$(srcdir)/Documentation/tips.txt' '$(OUTPUT)Documentation/'))
+
please_set_SHELL_PATH_to_a_more_modern_shell:
$(Q)$$(:)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2a845d6cac09..d9441055357e 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -610,12 +610,27 @@ static int report__browse_hists(struct report *rep)
struct perf_session *session = rep->session;
struct evlist *evlist = session->evlist;
const char *help = perf_tip(system_path(TIPDIR));
+ char *exec_path;
+ char *docdir;

if (help == NULL) {
/* fallback for people who don't install perf ;-) */
- help = perf_tip(DOCDIR);
- if (help == NULL)
- help = "Cannot load tips.txt file, please install perf!";
+ exec_path = get_exec_abs_path();
+ if (exec_path == NULL || asprintf(&docdir, "%sDocumentation", exec_path) < 0) {
+ docdir = NULL;
+ help = "Not enough memory or some other internal error occurred!";
+ }
+
+ if (docdir != NULL) {
+ help = perf_tip(docdir);
+ if (help == NULL)
+ help = "Cannot load tips.txt file, please install perf!";
+ }
+
+ if (exec_path)
+ free(exec_path);
+ if (docdir)
+ free(docdir);
}

switch (use_browser) {
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 650aec19d490..a20098dcdbc4 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -98,5 +98,5 @@ perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
endif

CFLAGS_attr.o += -DBINDIR="BUILD_STR($(bindir_SQ))" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
-CFLAGS_python-use.o += -DPYTHONPATH="BUILD_STR($(OUTPUT)python)" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
+CFLAGS_python-use.o += -DPYTHON="BUILD_STR($(PYTHON_WORD))"
CFLAGS_dwarf-unwind.o += -fno-optimize-sibling-calls
diff --git a/tools/perf/tests/python-use.c b/tools/perf/tests/python-use.c
index 98c6d474aa6f..8b4865c90d5d 100644
--- a/tools/perf/tests/python-use.c
+++ b/tools/perf/tests/python-use.c
@@ -8,16 +8,28 @@
#include <linux/compiler.h>
#include "tests.h"
#include "util/debug.h"
+#include <subcmd/exec-cmd.h>

int test__python_use(struct test *test __maybe_unused, int subtest __maybe_unused)
{
char *cmd;
int ret;
+ char *exec_path = NULL;
+ char *pythonpath;
+
+ exec_path = get_exec_abs_path();
+ if (exec_path == NULL)
+ return -1;
+
+ if (asprintf(&pythonpath, "%spython", exec_path) < 0)
+ return -1;

if (asprintf(&cmd, "echo \"import sys ; sys.path.append('%s'); import perf\" | %s %s",
- PYTHONPATH, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0)
+ pythonpath, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0)
return -1;

+ free(exec_path);
+ free(pythonpath);
pr_debug("python usage test: \"%s\"\n", cmd);
ret = system(cmd) ? -1 : 0;
free(cmd);
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index e3e12f9d4733..11ad0fdaf71e 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -197,6 +197,10 @@ CFLAGS_llvm-utils.o += -DPERF_INCLUDE_DIR="BUILD_STR($(perf_include_dir_SQ))"
# avoid compiler warnings in 32-bit mode
CFLAGS_genelf_debug.o += -Wno-packed

+ifeq ($(shell expr $(shell $(BISON) --version | grep bison | sed -e 's/.\+ \([0-9]\+\).\([0-9]\+\).\([0-9]\+\)/\1\2\3/g') \>\= 371), 1)
+ BISON_FILE_PREFIX_MAP := --file-prefix-map=$(OUTPUT)=
+endif
+
$(OUTPUT)util/parse-events-flex.c $(OUTPUT)util/parse-events-flex.h: util/parse-events.l $(OUTPUT)util/parse-events-bison.c
$(call rule_mkdir)
$(Q)$(call echo-cmd,flex)$(FLEX) -o $(OUTPUT)util/parse-events-flex.c \
@@ -204,7 +208,7 @@ $(OUTPUT)util/parse-events-flex.c $(OUTPUT)util/parse-events-flex.h: util/parse-

$(OUTPUT)util/parse-events-bison.c $(OUTPUT)util/parse-events-bison.h: util/parse-events.y
$(call rule_mkdir)
- $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) \
+ $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
-o $(OUTPUT)util/parse-events-bison.c -p parse_events_

$(OUTPUT)util/expr-flex.c $(OUTPUT)util/expr-flex.h: util/expr.l $(OUTPUT)util/expr-bison.c
@@ -214,7 +218,7 @@ $(OUTPUT)util/expr-flex.c $(OUTPUT)util/expr-flex.h: util/expr.l $(OUTPUT)util/e

$(OUTPUT)util/expr-bison.c $(OUTPUT)util/expr-bison.h: util/expr.y
$(call rule_mkdir)
- $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) \
+ $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
-o $(OUTPUT)util/expr-bison.c -p expr_

$(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-flex.h: util/pmu.l $(OUTPUT)util/pmu-bison.c
@@ -224,7 +228,7 @@ $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-flex.h: util/pmu.l $(OUTPUT)util/pmu-

$(OUTPUT)util/pmu-bison.c $(OUTPUT)util/pmu-bison.h: util/pmu.y
$(call rule_mkdir)
- $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) \
+ $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
-o $(OUTPUT)util/pmu-bison.c -p perf_pmu_

FLEX_GE_26 := $(shell expr $(shell $(FLEX) --version | sed -e 's/flex \([0-9]\+\).\([0-9]\+\)/\1\2/g') \>\= 26)
--
2.26.2.Cisco


2021-03-15 17:35:16

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH v2] perf: build reproducibility improvements

On Mon, Mar 15, 2021 at 09:46:35AM -0700, Denys Zagorui wrote:
> This patch helps to make perf build more reproducible
>
> It seems there is some need to have an ability to invoke
> perf from build directory without installation
> (84cfac7f05e1: perf tools: Set and pass DOCDIR to builtin-report.c)
> DOCDIR contains an absolute path to kernel source directory.
> In such case this path can be determined in runtime by using
> /proc/self/exe link. In case of building perf with O=
> Documentation/tips.txt can be copied to output directory.
>
> There is also python binding test where PYTHONPATH is used to store
> absolute path to python/perf.so library. This path can be
> also determined in runtime.
>
> bison stores full paths in generated files. This can be
> remapped by using --file-prefix-map option that is available
> starting from version 3.7.1.
>
> Signed-off-by: Denys Zagorui <[email protected]>
> ---
> tools/lib/subcmd/exec-cmd.c | 21 +++++++++++++++++++++
> tools/lib/subcmd/exec-cmd.h | 1 +
> tools/perf/Build | 1 -
> tools/perf/Makefile.perf | 3 +++
> tools/perf/builtin-report.c | 21 ++++++++++++++++++---
> tools/perf/tests/Build | 2 +-
> tools/perf/tests/python-use.c | 14 +++++++++++++-
> tools/perf/util/Build | 10 +++++++---
> 8 files changed, 64 insertions(+), 9 deletions(-)
>
> diff --git a/tools/lib/subcmd/exec-cmd.c b/tools/lib/subcmd/exec-cmd.c
> index 33e94fb83986..3116246ec629 100644
> --- a/tools/lib/subcmd/exec-cmd.c
> +++ b/tools/lib/subcmd/exec-cmd.c
> @@ -208,3 +208,24 @@ int execl_cmd(const char *cmd,...)
> argv[argc] = NULL;
> return execv_cmd(argv);
> }
> +
> +/* The caller is responsible to free the returned buffer */
> +char *get_exec_abs_path(void)
> +{

we have perf_exe for the whole perf binary, so let's call this perf_exe_path

> + int ret;
> + int i;
> + char *buf;
> +
> + buf = malloc(PATH_MAX);
> + ret = readlink("/proc/self/exe", buf, PATH_MAX - 1);
> + if (ret <= 0) {
> + free(buf);
> + return NULL;
> + }

you could call perf_exe for this ^^^

> +
> + for (i = ret - 1; buf[i] != '/'; i--);
> +
> + buf[i + 1] = 0;
> +
> + return buf;
> +}
> diff --git a/tools/lib/subcmd/exec-cmd.h b/tools/lib/subcmd/exec-cmd.h
> index aba591b8d254..9dc98248dadf 100644
> --- a/tools/lib/subcmd/exec-cmd.h
> +++ b/tools/lib/subcmd/exec-cmd.h
> @@ -13,5 +13,6 @@ extern int execl_cmd(const char *cmd, ...);
> /* get_argv_exec_path and system_path return malloc'd string, caller must free it */
> extern char *get_argv_exec_path(void);
> extern char *system_path(const char *path);
> +extern char *get_exec_abs_path(void);
>
> #endif /* __SUBCMD_EXEC_CMD_H */
> diff --git a/tools/perf/Build b/tools/perf/Build
> index db61dbe2b543..56d0189f1029 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -45,7 +45,6 @@ CFLAGS_perf.o += -DPERF_HTML_PATH="BUILD_STR($(htmldir_SQ))" \
> -DPREFIX="BUILD_STR($(prefix_SQ))"
> CFLAGS_builtin-trace.o += -DSTRACE_GROUPS_DIR="BUILD_STR($(STRACE_GROUPS_DIR_SQ))"
> CFLAGS_builtin-report.o += -DTIPDIR="BUILD_STR($(tipdir_SQ))"
> -CFLAGS_builtin-report.o += -DDOCDIR="BUILD_STR($(srcdir_SQ)/Documentation)"
>
> perf-y += util/
> perf-y += arch/
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index f6e609673de2..c9e3aae6c47f 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -627,6 +627,9 @@ $(OUTPUT)python/perf.so: $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS) $(LIBTRACEEVENT_D
> --quiet build_ext; \
> cp $(PYTHON_EXTBUILD_LIB)perf*.so $(OUTPUT)python/
>
> +_dummy := $(shell [ -d '$(OUTPUT)Documentation' ] || (mkdir -p '$(OUTPUT)Documentation' && \
> + cp '$(srcdir)/Documentation/tips.txt' '$(OUTPUT)Documentation/'))
> +

why do we need this? also should be in Documentation make

> please_set_SHELL_PATH_to_a_more_modern_shell:
> $(Q)$$(:)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 2a845d6cac09..d9441055357e 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -610,12 +610,27 @@ static int report__browse_hists(struct report *rep)
> struct perf_session *session = rep->session;
> struct evlist *evlist = session->evlist;
> const char *help = perf_tip(system_path(TIPDIR));
> + char *exec_path;
> + char *docdir;
>
> if (help == NULL) {
> /* fallback for people who don't install perf ;-) */
> - help = perf_tip(DOCDIR);
> - if (help == NULL)
> - help = "Cannot load tips.txt file, please install perf!";
> + exec_path = get_exec_abs_path();
> + if (exec_path == NULL || asprintf(&docdir, "%sDocumentation", exec_path) < 0) {
> + docdir = NULL;
> + help = "Not enough memory or some other internal error occurred!";
> + }
> +
> + if (docdir != NULL) {
> + help = perf_tip(docdir);
> + if (help == NULL)
> + help = "Cannot load tips.txt file, please install perf!";
> + }
> +
> + if (exec_path)
> + free(exec_path);
> + if (docdir)
> + free(docdir);
> }
>
> switch (use_browser) {
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 650aec19d490..a20098dcdbc4 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -98,5 +98,5 @@ perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
> endif
>
> CFLAGS_attr.o += -DBINDIR="BUILD_STR($(bindir_SQ))" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
> -CFLAGS_python-use.o += -DPYTHONPATH="BUILD_STR($(OUTPUT)python)" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
> +CFLAGS_python-use.o += -DPYTHON="BUILD_STR($(PYTHON_WORD))"
> CFLAGS_dwarf-unwind.o += -fno-optimize-sibling-calls
> diff --git a/tools/perf/tests/python-use.c b/tools/perf/tests/python-use.c
> index 98c6d474aa6f..8b4865c90d5d 100644
> --- a/tools/perf/tests/python-use.c
> +++ b/tools/perf/tests/python-use.c
> @@ -8,16 +8,28 @@
> #include <linux/compiler.h>
> #include "tests.h"
> #include "util/debug.h"
> +#include <subcmd/exec-cmd.h>
>
> int test__python_use(struct test *test __maybe_unused, int subtest __maybe_unused)
> {
> char *cmd;
> int ret;
> + char *exec_path = NULL;
> + char *pythonpath;
> +
> + exec_path = get_exec_abs_path();
> + if (exec_path == NULL)
> + return -1;
> +
> + if (asprintf(&pythonpath, "%spython", exec_path) < 0)

are we sure there's always trailing '/' in exec_path ?

> + return -1;
>
> if (asprintf(&cmd, "echo \"import sys ; sys.path.append('%s'); import perf\" | %s %s",
> - PYTHONPATH, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0)
> + pythonpath, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0)
> return -1;
>
> + free(exec_path);
> + free(pythonpath);
> pr_debug("python usage test: \"%s\"\n", cmd);
> ret = system(cmd) ? -1 : 0;
> free(cmd);
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index e3e12f9d4733..11ad0fdaf71e 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -197,6 +197,10 @@ CFLAGS_llvm-utils.o += -DPERF_INCLUDE_DIR="BUILD_STR($(perf_include_dir_SQ))"
> # avoid compiler warnings in 32-bit mode
> CFLAGS_genelf_debug.o += -Wno-packed
>
> +ifeq ($(shell expr $(shell $(BISON) --version | grep bison | sed -e 's/.\+ \([0-9]\+\).\([0-9]\+\).\([0-9]\+\)/\1\2\3/g') \>\= 371), 1)
> + BISON_FILE_PREFIX_MAP := --file-prefix-map=$(OUTPUT)=
> +endif

seems too small for feature detection, but please add it to
Makefile.config and make new variable under .config-detected

thanks,
jirka

> +
> $(OUTPUT)util/parse-events-flex.c $(OUTPUT)util/parse-events-flex.h: util/parse-events.l $(OUTPUT)util/parse-events-bison.c
> $(call rule_mkdir)
> $(Q)$(call echo-cmd,flex)$(FLEX) -o $(OUTPUT)util/parse-events-flex.c \
> @@ -204,7 +208,7 @@ $(OUTPUT)util/parse-events-flex.c $(OUTPUT)util/parse-events-flex.h: util/parse-
>
> $(OUTPUT)util/parse-events-bison.c $(OUTPUT)util/parse-events-bison.h: util/parse-events.y
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) \
> + $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
> -o $(OUTPUT)util/parse-events-bison.c -p parse_events_
>
> $(OUTPUT)util/expr-flex.c $(OUTPUT)util/expr-flex.h: util/expr.l $(OUTPUT)util/expr-bison.c
> @@ -214,7 +218,7 @@ $(OUTPUT)util/expr-flex.c $(OUTPUT)util/expr-flex.h: util/expr.l $(OUTPUT)util/e
>
> $(OUTPUT)util/expr-bison.c $(OUTPUT)util/expr-bison.h: util/expr.y
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) \
> + $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
> -o $(OUTPUT)util/expr-bison.c -p expr_
>
> $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-flex.h: util/pmu.l $(OUTPUT)util/pmu-bison.c
> @@ -224,7 +228,7 @@ $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-flex.h: util/pmu.l $(OUTPUT)util/pmu-
>
> $(OUTPUT)util/pmu-bison.c $(OUTPUT)util/pmu-bison.h: util/pmu.y
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) \
> + $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
> -o $(OUTPUT)util/pmu-bison.c -p perf_pmu_
>
> FLEX_GE_26 := $(shell expr $(shell $(FLEX) --version | sed -e 's/flex \([0-9]\+\).\([0-9]\+\)/\1\2/g') \>\= 26)
> --
> 2.26.2.Cisco
>

2021-03-17 11:02:58

by Denys Zagorui

[permalink] [raw]
Subject: Re: [PATCH v2] perf: build reproducibility improvements

> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index f6e609673de2..c9e3aae6c47f 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -627,6 +627,9 @@ $(OUTPUT)python/perf.so: $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS) $(LIBTRACEEVENT_D
> > --quiet build_ext; \
> > cp $(PYTHON_EXTBUILD_LIB)perf*.so $(OUTPUT)python/
> >
> > +_dummy := $(shell [ -d '$(OUTPUT)Documentation' ] || (mkdir -p '$(OUTPUT)Documentation' && \
> > + cp '$(srcdir)/Documentation/tips.txt' '$(OUTPUT)Documentation/'))
> > +
>
> why do we need this? also should be in Documentation make

I added this for case when we build perf with O= . Further in runtime perf will try to find tips.txt file
in Documentation directory. Documentation has to be in the same location as perf executable. If we
build perf without O= option this copying is not needed as Documentation is already in the same
location as perf.

> > diff --git a/tools/perf/tests/python-use.c b/tools/perf/tests/python-use.c
> > index 98c6d474aa6f..8b4865c90d5d 100644
> > --- a/tools/perf/tests/python-use.c
> > +++ b/tools/perf/tests/python-use.c
> > @@ -8,16 +8,28 @@
> > #include <linux/compiler.h>
> > #include "tests.h"
> > #include "util/debug.h"
> > +#include <subcmd/exec-cmd.h>
> >
> > int test__python_use(struct test *test __maybe_unused, int subtest __maybe_unused)
> > {
> > char *cmd;
> > int ret;
> > + char *exec_path = NULL;
> > + char *pythonpath;
> > +
> > + exec_path = get_exec_abs_path();
> > + if (exec_path == NULL)
> > + return -1;
> > +
> > + if (asprintf(&pythonpath, "%spython", exec_path) < 0)
>
> are we sure there's always trailing '/' in exec_path ?

Yes, it should be. /proc/self/exe is a absolute symlink to running executable

________________________________________
From: Jiri Olsa <[email protected]>
Sent: 15 March 2021 19:32
To: Denys Zagorui -X (dzagorui - GLOBALLOGIC INC at Cisco)
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v2] perf: build reproducibility improvements

On Mon, Mar 15, 2021 at 09:46:35AM -0700, Denys Zagorui wrote:
> This patch helps to make perf build more reproducible
>
> It seems there is some need to have an ability to invoke
> perf from build directory without installation
> (84cfac7f05e1: perf tools: Set and pass DOCDIR to builtin-report.c)
> DOCDIR contains an absolute path to kernel source directory.
> In such case this path can be determined in runtime by using
> /proc/self/exe link. In case of building perf with O=
> Documentation/tips.txt can be copied to output directory.
>
> There is also python binding test where PYTHONPATH is used to store
> absolute path to python/perf.so library. This path can be
> also determined in runtime.
>
> bison stores full paths in generated files. This can be
> remapped by using --file-prefix-map option that is available
> starting from version 3.7.1.
>
> Signed-off-by: Denys Zagorui <[email protected]>
> ---
> tools/lib/subcmd/exec-cmd.c | 21 +++++++++++++++++++++
> tools/lib/subcmd/exec-cmd.h | 1 +
> tools/perf/Build | 1 -
> tools/perf/Makefile.perf | 3 +++
> tools/perf/builtin-report.c | 21 ++++++++++++++++++---
> tools/perf/tests/Build | 2 +-
> tools/perf/tests/python-use.c | 14 +++++++++++++-
> tools/perf/util/Build | 10 +++++++---
> 8 files changed, 64 insertions(+), 9 deletions(-)
>
> diff --git a/tools/lib/subcmd/exec-cmd.c b/tools/lib/subcmd/exec-cmd.c
> index 33e94fb83986..3116246ec629 100644
> --- a/tools/lib/subcmd/exec-cmd.c
> +++ b/tools/lib/subcmd/exec-cmd.c
> @@ -208,3 +208,24 @@ int execl_cmd(const char *cmd,...)
> argv[argc] = NULL;
> return execv_cmd(argv);
> }
> +
> +/* The caller is responsible to free the returned buffer */
> +char *get_exec_abs_path(void)
> +{

we have perf_exe for the whole perf binary, so let's call this perf_exe_path

> + int ret;
> + int i;
> + char *buf;
> +
> + buf = malloc(PATH_MAX);
> + ret = readlink("/proc/self/exe", buf, PATH_MAX - 1);
> + if (ret <= 0) {
> + free(buf);
> + return NULL;
> + }

you could call perf_exe for this ^^^

> +
> + for (i = ret - 1; buf[i] != '/'; i--);
> +
> + buf[i + 1] = 0;
> +
> + return buf;
> +}
> diff --git a/tools/lib/subcmd/exec-cmd.h b/tools/lib/subcmd/exec-cmd.h
> index aba591b8d254..9dc98248dadf 100644
> --- a/tools/lib/subcmd/exec-cmd.h
> +++ b/tools/lib/subcmd/exec-cmd.h
> @@ -13,5 +13,6 @@ extern int execl_cmd(const char *cmd, ...);
> /* get_argv_exec_path and system_path return malloc'd string, caller must free it */
> extern char *get_argv_exec_path(void);
> extern char *system_path(const char *path);
> +extern char *get_exec_abs_path(void);
>
> #endif /* __SUBCMD_EXEC_CMD_H */
> diff --git a/tools/perf/Build b/tools/perf/Build
> index db61dbe2b543..56d0189f1029 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -45,7 +45,6 @@ CFLAGS_perf.o += -DPERF_HTML_PATH="BUILD_STR($(htmldir_SQ))" \
> -DPREFIX="BUILD_STR($(prefix_SQ))"
> CFLAGS_builtin-trace.o += -DSTRACE_GROUPS_DIR="BUILD_STR($(STRACE_GROUPS_DIR_SQ))"
> CFLAGS_builtin-report.o += -DTIPDIR="BUILD_STR($(tipdir_SQ))"
> -CFLAGS_builtin-report.o += -DDOCDIR="BUILD_STR($(srcdir_SQ)/Documentation)"
>
> perf-y += util/
> perf-y += arch/
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index f6e609673de2..c9e3aae6c47f 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -627,6 +627,9 @@ $(OUTPUT)python/perf.so: $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS) $(LIBTRACEEVENT_D
> --quiet build_ext; \
> cp $(PYTHON_EXTBUILD_LIB)perf*.so $(OUTPUT)python/
>
> +_dummy := $(shell [ -d '$(OUTPUT)Documentation' ] || (mkdir -p '$(OUTPUT)Documentation' && \
> + cp '$(srcdir)/Documentation/tips.txt' '$(OUTPUT)Documentation/'))
> +

why do we need this? also should be in Documentation make

> please_set_SHELL_PATH_to_a_more_modern_shell:
> $(Q)$$(:)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 2a845d6cac09..d9441055357e 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -610,12 +610,27 @@ static int report__browse_hists(struct report *rep)
> struct perf_session *session = rep->session;
> struct evlist *evlist = session->evlist;
> const char *help = perf_tip(system_path(TIPDIR));
> + char *exec_path;
> + char *docdir;
>
> if (help == NULL) {
> /* fallback for people who don't install perf ;-) */
> - help = perf_tip(DOCDIR);
> - if (help == NULL)
> - help = "Cannot load tips.txt file, please install perf!";
> + exec_path = get_exec_abs_path();
> + if (exec_path == NULL || asprintf(&docdir, "%sDocumentation", exec_path) < 0) {
> + docdir = NULL;
> + help = "Not enough memory or some other internal error occurred!";
> + }
> +
> + if (docdir != NULL) {
> + help = perf_tip(docdir);
> + if (help == NULL)
> + help = "Cannot load tips.txt file, please install perf!";
> + }
> +
> + if (exec_path)
> + free(exec_path);
> + if (docdir)
> + free(docdir);
> }
>
> switch (use_browser) {
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 650aec19d490..a20098dcdbc4 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -98,5 +98,5 @@ perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
> endif
>
> CFLAGS_attr.o += -DBINDIR="BUILD_STR($(bindir_SQ))" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
> -CFLAGS_python-use.o += -DPYTHONPATH="BUILD_STR($(OUTPUT)python)" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
> +CFLAGS_python-use.o += -DPYTHON="BUILD_STR($(PYTHON_WORD))"
> CFLAGS_dwarf-unwind.o += -fno-optimize-sibling-calls
> diff --git a/tools/perf/tests/python-use.c b/tools/perf/tests/python-use.c
> index 98c6d474aa6f..8b4865c90d5d 100644
> --- a/tools/perf/tests/python-use.c
> +++ b/tools/perf/tests/python-use.c
> @@ -8,16 +8,28 @@
> #include <linux/compiler.h>
> #include "tests.h"
> #include "util/debug.h"
> +#include <subcmd/exec-cmd.h>
>
> int test__python_use(struct test *test __maybe_unused, int subtest __maybe_unused)
> {
> char *cmd;
> int ret;
> + char *exec_path = NULL;
> + char *pythonpath;
> +
> + exec_path = get_exec_abs_path();
> + if (exec_path == NULL)
> + return -1;
> +
> + if (asprintf(&pythonpath, "%spython", exec_path) < 0)

are we sure there's always trailing '/' in exec_path ?

> + return -1;
>
> if (asprintf(&cmd, "echo \"import sys ; sys.path.append('%s'); import perf\" | %s %s",
> - PYTHONPATH, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0)
> + pythonpath, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0)
> return -1;
>
> + free(exec_path);
> + free(pythonpath);
> pr_debug("python usage test: \"%s\"\n", cmd);
> ret = system(cmd) ? -1 : 0;
> free(cmd);
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index e3e12f9d4733..11ad0fdaf71e 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -197,6 +197,10 @@ CFLAGS_llvm-utils.o += -DPERF_INCLUDE_DIR="BUILD_STR($(perf_include_dir_SQ))"
> # avoid compiler warnings in 32-bit mode
> CFLAGS_genelf_debug.o += -Wno-packed
>
> +ifeq ($(shell expr $(shell $(BISON) --version | grep bison | sed -e 's/.\+ \([0-9]\+\).\([0-9]\+\).\([0-9]\+\)/\1\2\3/g') \>\= 371), 1)
> + BISON_FILE_PREFIX_MAP := --file-prefix-map=$(OUTPUT)=
> +endif

seems too small for feature detection, but please add it to
Makefile.config and make new variable under .config-detected

thanks,
jirka

> +
> $(OUTPUT)util/parse-events-flex.c $(OUTPUT)util/parse-events-flex.h: util/parse-events.l $(OUTPUT)util/parse-events-bison.c
> $(call rule_mkdir)
> $(Q)$(call echo-cmd,flex)$(FLEX) -o $(OUTPUT)util/parse-events-flex.c \
> @@ -204,7 +208,7 @@ $(OUTPUT)util/parse-events-flex.c $(OUTPUT)util/parse-events-flex.h: util/parse-
>
> $(OUTPUT)util/parse-events-bison.c $(OUTPUT)util/parse-events-bison.h: util/parse-events.y
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) \
> + $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
> -o $(OUTPUT)util/parse-events-bison.c -p parse_events_
>
> $(OUTPUT)util/expr-flex.c $(OUTPUT)util/expr-flex.h: util/expr.l $(OUTPUT)util/expr-bison.c
> @@ -214,7 +218,7 @@ $(OUTPUT)util/expr-flex.c $(OUTPUT)util/expr-flex.h: util/expr.l $(OUTPUT)util/e
>
> $(OUTPUT)util/expr-bison.c $(OUTPUT)util/expr-bison.h: util/expr.y
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) \
> + $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
> -o $(OUTPUT)util/expr-bison.c -p expr_
>
> $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-flex.h: util/pmu.l $(OUTPUT)util/pmu-bison.c
> @@ -224,7 +228,7 @@ $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-flex.h: util/pmu.l $(OUTPUT)util/pmu-
>
> $(OUTPUT)util/pmu-bison.c $(OUTPUT)util/pmu-bison.h: util/pmu.y
> $(call rule_mkdir)
> - $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) \
> + $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
> -o $(OUTPUT)util/pmu-bison.c -p perf_pmu_
>
> FLEX_GE_26 := $(shell expr $(shell $(FLEX) --version | sed -e 's/flex \([0-9]\+\).\([0-9]\+\)/\1\2/g') \>\= 26)
> --
> 2.26.2.Cisco
>