2012-10-20 11:16:30

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] perf: Build install-man target when installing

Ping?

On Fri, Sep 28, 2012 at 07:47:07PM +0200, Borislav Petkov wrote:
> From: Borislav Petkov <[email protected]>
>
> There's a portion in the "perf list" output refering to the exact
> specification of raw hardware events. Since this description is in
> the perf-list manpage, make install-man be built unconditionally when
> installing perf tool and change the reference in the "perf list" output
> accordingly.
>
> Signed-off-by: Borislav Petkov <[email protected]>
> ---
> tools/perf/Makefile | 2 +-
> tools/perf/util/parse-events.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 251dcd7fb5ac..e77741e22cfb 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -1027,7 +1027,7 @@ perfexec_instdir = $(prefix)/$(perfexecdir)
> endif
> perfexec_instdir_SQ = $(subst ','\'',$(perfexec_instdir))
>
> -install: all
> +install: all install-man
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
> $(INSTALL) $(OUTPUT)perf '$(DESTDIR_SQ)$(bindir_SQ)'
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/perl/Perf-Trace-Util/lib/Perf/Trace'
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index bf5d033ee1b4..f84ac3984708 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1092,7 +1092,7 @@ void print_events(const char *event_glob, bool name_only)
> printf(" %-50s [%s]\n",
> "cpu/t1=v1[,t2=v2,t3 ...]/modifier",
> event_type_descriptors[PERF_TYPE_RAW]);
> - printf(" (see 'perf list --help' on how to encode it)\n");
> + printf(" (see 'man perf-list' on how to encode it)\n");
> printf("\n");
>
> printf(" %-50s [%s]\n",
> --
> 1.7.11.rc1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
Regards/Gruss,
Boris.


2012-10-20 15:37:42

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf: Build install-man target when installing

Em Sat, Oct 20, 2012 at 01:16:25PM +0200, Borislav Petkov escreveu:
> Ping?

Merged it to perf/core, thanks for the patch and the ping,

- Arnaldo

2012-10-22 07:25:28

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH] perf: Build install-man target when installing

Hi,

On Sat, 20 Oct 2012 13:16:25 +0200, Borislav Petkov wrote:
> On Fri, Sep 28, 2012 at 07:47:07PM +0200, Borislav Petkov wrote:
>> From: Borislav Petkov <[email protected]>
>>
>> There's a portion in the "perf list" output refering to the exact
>> specification of raw hardware events. Since this description is in
>> the perf-list manpage, make install-man be built unconditionally when
>> installing perf tool and change the reference in the "perf list" output
>> accordingly.
>>
>> Signed-off-by: Borislav Petkov <[email protected]>
>> ---
>> tools/perf/Makefile | 2 +-
>> tools/perf/util/parse-events.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
>> index 251dcd7fb5ac..e77741e22cfb 100644
>> --- a/tools/perf/Makefile
>> +++ b/tools/perf/Makefile
>> @@ -1027,7 +1027,7 @@ perfexec_instdir = $(prefix)/$(perfexecdir)
>> endif
>> perfexec_instdir_SQ = $(subst ','\'',$(perfexec_instdir))
>>
>> -install: all
>> +install: all install-man

It will add additional dependencies of asciidoc, docbook-xsl and/or
something to default perf install command. I don't know it matters
much, but just wanted to say. What if a user want to install the perf
but the required packages for documents are not installed?


>> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
>> $(INSTALL) $(OUTPUT)perf '$(DESTDIR_SQ)$(bindir_SQ)'
>> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/perl/Perf-Trace-Util/lib/Perf/Trace'
>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>> index bf5d033ee1b4..f84ac3984708 100644
>> --- a/tools/perf/util/parse-events.c
>> +++ b/tools/perf/util/parse-events.c
>> @@ -1092,7 +1092,7 @@ void print_events(const char *event_glob, bool name_only)
>> printf(" %-50s [%s]\n",
>> "cpu/t1=v1[,t2=v2,t3 ...]/modifier",
>> event_type_descriptors[PERF_TYPE_RAW]);
>> - printf(" (see 'perf list --help' on how to encode it)\n");
>> + printf(" (see 'man perf-list' on how to encode it)\n");

Is this needed? Shouldn't we fix perf --help handling? Please see my
other post on the issue.

Thanks,
Namhyung


>> printf("\n");
>>
>> printf(" %-50s [%s]\n",
>> --
>> 1.7.11.rc1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/

2012-10-22 10:07:23

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] perf: Build install-man target when installing

On Mon, Oct 22, 2012 at 04:25:24PM +0900, Namhyung Kim wrote:
> It will add additional dependencies of asciidoc, docbook-xsl and/or
> something to default perf install command. I don't know it matters
> much, but just wanted to say. What if a user want to install the perf
> but the required packages for documents are not installed?

Ok, this is a valid point, more or less.

What we could do is leave the reference to "man perf-list" in perf list
output and *not* build install-man unconditionally. This way, people
who want to see the manpage, will need to install required packages. On
distros the manpage is already present so all is taken care of.

This way, we avoid the text duplication of adding this to perf --help.

--
Regards/Gruss,
Boris.

2012-10-22 14:03:52

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf: Build install-man target when installing

Em Mon, Oct 22, 2012 at 12:07:19PM +0200, Borislav Petkov escreveu:
> On Mon, Oct 22, 2012 at 04:25:24PM +0900, Namhyung Kim wrote:
> > It will add additional dependencies of asciidoc, docbook-xsl and/or
> > something to default perf install command. I don't know it matters
> > much, but just wanted to say. What if a user want to install the perf
> > but the required packages for documents are not installed?

> Ok, this is a valid point, more or less.

Yes, its a valid point, to address it I think we should just do as we
with other features, like, say, when python-devel is not installed warn
the user that python support won't be included, i.e. if asciidoc, etc
are not installed, tell the user that documentation will not be
installed, install package A, B and C to have documentation installed.

- Arnaldo

2012-10-22 23:13:08

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] perf: Build install-man target when installing

On Mon, Oct 22, 2012 at 07:03:39AM -0700, Arnaldo Carvalho de Melo wrote:
> Em Mon, Oct 22, 2012 at 12:07:19PM +0200, Borislav Petkov escreveu:
> > On Mon, Oct 22, 2012 at 04:25:24PM +0900, Namhyung Kim wrote:
> > > It will add additional dependencies of asciidoc, docbook-xsl and/or
> > > something to default perf install command. I don't know it matters
> > > much, but just wanted to say. What if a user want to install the perf
> > > but the required packages for documents are not installed?
>
> > Ok, this is a valid point, more or less.
>
> Yes, its a valid point, to address it I think we should just do as we
> with other features, like, say, when python-devel is not installed warn
> the user that python support won't be included, i.e. if asciidoc, etc
> are not installed, tell the user that documentation will not be
> installed, install package A, B and C to have documentation installed.

Ok, how's that: we check the presence of executables on the system and
error out if not in the path. I found some helpful make functions in
config/utilities.mak and decided to reuse them because I'm lazy :)

--
diff --git a/tools/perf/Documentation/Makefile b/tools/perf/Documentation/Makefile
index 9f2e44f2b17a..a2bfd6e3cce4 100644
--- a/tools/perf/Documentation/Makefile
+++ b/tools/perf/Documentation/Makefile
@@ -1,3 +1,5 @@
+include ../config/utilities.mak
+
OUTPUT := ./
ifeq ("$(origin O)", "command line")
ifneq ($(O),)
@@ -64,6 +66,7 @@ MAKEINFO=makeinfo
INSTALL_INFO=install-info
DOCBOOK2X_TEXI=docbook2x-texi
DBLATEX=dblatex
+XMLTO=xmlto
ifndef PERL_PATH
PERL_PATH = /usr/bin/perl
endif
@@ -170,7 +173,11 @@ pdf: $(OUTPUT)user-manual.pdf

install: install-man

-install-man: man
+check-man-tools:
+ $(call check-executable-or-error-out,$(ASCIIDOC))
+ $(call check-executable-or-error-out,$(XMLTO))
+
+install-man: check-man-tools man
$(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
# $(INSTALL) -d -m 755 $(DESTDIR)$(man5dir)
# $(INSTALL) -d -m 755 $(DESTDIR)$(man7dir)
@@ -246,7 +253,7 @@ $(MAN_HTML): $(OUTPUT)%.html : %.txt

$(OUTPUT)%.1 $(OUTPUT)%.5 $(OUTPUT)%.7 : $(OUTPUT)%.xml
$(QUIET_XMLTO)$(RM) $@ && \
- xmlto -o $(OUTPUT) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
+ $(XMLTO) -o $(OUTPUT) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<

$(OUTPUT)%.xml : %.txt
$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
index 8046182a19eb..f8b5d713ff15 100644
--- a/tools/perf/config/utilities.mak
+++ b/tools/perf/config/utilities.mak
@@ -168,6 +168,8 @@ _is-executable-sh = $(call shell-sq,test -f $(1) -a -x $(1) && echo y)
get-executable = $(if $(1),$(if $(is-absolute),$(_ge-abspath),$(lookup)))
_ge-abspath = $(if $(is-executable),$(1))

+check-executable-or-error-out = $(if $(call get-executable,$(1)),,$(error $(1) not found, please install it on your distro))
+
# get-supplied-or-default-executable
#
# Usage: absolute-executable-path-or-empty = $(call get-executable-or-default,variable,default)

--
Regards/Gruss,
Boris.

2012-10-23 13:40:37

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf: Build install-man target when installing

Em Tue, Oct 23, 2012 at 01:13:01AM +0200, Borislav Petkov escreveu:
> On Mon, Oct 22, 2012 at 07:03:39AM -0700, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Oct 22, 2012 at 12:07:19PM +0200, Borislav Petkov escreveu:
> > > On Mon, Oct 22, 2012 at 04:25:24PM +0900, Namhyung Kim wrote:
> > > > It will add additional dependencies of asciidoc, docbook-xsl and/or
> > > > something to default perf install command. I don't know it matters
> > > > much, but just wanted to say. What if a user want to install the perf
> > > > but the required packages for documents are not installed?
> >
> > > Ok, this is a valid point, more or less.
> >
> > Yes, its a valid point, to address it I think we should just do as we
> > with other features, like, say, when python-devel is not installed warn
> > the user that python support won't be included, i.e. if asciidoc, etc
> > are not installed, tell the user that documentation will not be
> > installed, install package A, B and C to have documentation installed.
>
> Ok, how's that: we check the presence of executables on the system and
> error out if not in the path. I found some helpful make functions in
> config/utilities.mak and decided to reuse them because I'm lazy :)

Better now:

make[1]: Entering directory `/home/git/linux/tools/lib/traceevent'
make[1]: Entering directory `/home/git/linux/tools/perf/Documentation'
make[2]: warning: jobserver unavailable: using -j1. Add `+' to parent
make rule.
make[1]: Leaving directory `/home/git/linux/tools/lib/traceevent'
CC /home/acme/git/build/perf/perf.o
LINK /home/acme/git/build/perf/perf
make[2]: Entering directory `/home/git/linux/tools/perf'
make[2]: `/home/acme/git/build/perf/PERF-VERSION-FILE' is up to date.
make[2]: Leaving directory `/home/git/linux/tools/perf'
Makefile:177: *** asciidoc not found, please install it on your distro.
Stop.
make[1]: Leaving directory `/home/git/linux/tools/perf/Documentation'
make: *** [install-man] Error 2
make: Leaving directory `/home/git/linux/tools/perf'
[acme@sandy linux]$

But it shouldn't stop the build, just warn the user that Documentation
is not going to be installed. It should error like that if 'make
install-man' is called.

Trying to do that now.

- Arnaldo

> --
> diff --git a/tools/perf/Documentation/Makefile b/tools/perf/Documentation/Makefile
> index 9f2e44f2b17a..a2bfd6e3cce4 100644
> --- a/tools/perf/Documentation/Makefile
> +++ b/tools/perf/Documentation/Makefile
> @@ -1,3 +1,5 @@
> +include ../config/utilities.mak
> +
> OUTPUT := ./
> ifeq ("$(origin O)", "command line")
> ifneq ($(O),)
> @@ -64,6 +66,7 @@ MAKEINFO=makeinfo
> INSTALL_INFO=install-info
> DOCBOOK2X_TEXI=docbook2x-texi
> DBLATEX=dblatex
> +XMLTO=xmlto
> ifndef PERL_PATH
> PERL_PATH = /usr/bin/perl
> endif
> @@ -170,7 +173,11 @@ pdf: $(OUTPUT)user-manual.pdf
>
> install: install-man
>
> -install-man: man
> +check-man-tools:
> + $(call check-executable-or-error-out,$(ASCIIDOC))
> + $(call check-executable-or-error-out,$(XMLTO))
> +
> +install-man: check-man-tools man
> $(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
> # $(INSTALL) -d -m 755 $(DESTDIR)$(man5dir)
> # $(INSTALL) -d -m 755 $(DESTDIR)$(man7dir)
> @@ -246,7 +253,7 @@ $(MAN_HTML): $(OUTPUT)%.html : %.txt
>
> $(OUTPUT)%.1 $(OUTPUT)%.5 $(OUTPUT)%.7 : $(OUTPUT)%.xml
> $(QUIET_XMLTO)$(RM) $@ && \
> - xmlto -o $(OUTPUT) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
> + $(XMLTO) -o $(OUTPUT) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
>
> $(OUTPUT)%.xml : %.txt
> $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
> index 8046182a19eb..f8b5d713ff15 100644
> --- a/tools/perf/config/utilities.mak
> +++ b/tools/perf/config/utilities.mak
> @@ -168,6 +168,8 @@ _is-executable-sh = $(call shell-sq,test -f $(1) -a -x $(1) && echo y)
> get-executable = $(if $(1),$(if $(is-absolute),$(_ge-abspath),$(lookup)))
> _ge-abspath = $(if $(is-executable),$(1))
>
> +check-executable-or-error-out = $(if $(call get-executable,$(1)),,$(error $(1) not found, please install it on your distro))
> +
> # get-supplied-or-default-executable
> #
> # Usage: absolute-executable-path-or-empty = $(call get-executable-or-default,variable,default)
>
> --
> Regards/Gruss,
> Boris.

2012-10-24 17:03:30

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf: Build install-man target when installing

Em Tue, Oct 23, 2012 at 06:39:51AM -0700, Arnaldo Carvalho de Melo escreveu:
> > Ok, how's that: we check the presence of executables on the system and
> > error out if not in the path. I found some helpful make functions in
> > config/utilities.mak and decided to reuse them because I'm lazy :)

> Better now:

> Makefile:177: *** asciidoc not found, please install it on your distro.
> Stop.
> make[1]: Leaving directory `/home/git/linux/tools/perf/Documentation'
> make: *** [install-man] Error 2
> make: Leaving directory `/home/git/linux/tools/perf'
> [acme@sandy linux]$

> But it shouldn't stop the build, just warn the user that Documentation
> is not going to be installed. It should error like that if 'make
> install-man' is called.

> Trying to do that now.

[acme@sandy linux]$ make -j8 -C tools/perf/ O=/home/acme/git/build/perf try-install-man
make: Entering directory `/home/git/linux/tools/perf'
make -C Documentation try-install-man
make[1]: Entering directory `/home/git/linux/tools/perf/Documentation'
make[2]: Entering directory `/home/git/linux/tools/perf'
make[2]: `/home/acme/git/build/perf/PERF-VERSION-FILE' is up to date.
make[2]: Leaving directory `/home/git/linux/tools/perf'
Makefile:202: Please install the asciidoc package to have the man pages installed
Makefile:202: Please install the xmlto package to have the man pages installed
make[1]: `try-install-man' is up to date.
make[1]: Leaving directory `/home/git/linux/tools/perf/Documentation'
make: Leaving directory `/home/git/linux/tools/perf'
[acme@sandy linux]$

The 'install-man' target will use your check-executable-or-error-out
stuff, but the main 'install' target will call just 'try-install-man',
that will not stop the build if there are missing tools to generate the
man pages.

Installing just xmlto makes it ask just for asciidoc, install both and it
generates the man pages, please try to check if it works for you so that I can
have your Tested-by.

- Arnaldo


Attachments:
(No filename) (1.92 kB)
has_xmlto.patch (3.51 kB)
Download all attachments

2012-10-24 17:21:32

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf: Build install-man target when installing

Em Wed, Oct 24, 2012 at 10:03:16AM -0700, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Oct 23, 2012 at 06:39:51AM -0700, Arnaldo Carvalho de Melo escreveu:
> > > Ok, how's that: we check the presence of executables on the system and
> > > error out if not in the path. I found some helpful make functions in
> > > config/utilities.mak and decided to reuse them because I'm lazy :)
>
> > Better now:
>
> > Makefile:177: *** asciidoc not found, please install it on your distro.
> > Stop.
> > make[1]: Leaving directory `/home/git/linux/tools/perf/Documentation'
> > make: *** [install-man] Error 2
> > make: Leaving directory `/home/git/linux/tools/perf'
> > [acme@sandy linux]$
>
> > But it shouldn't stop the build, just warn the user that Documentation
> > is not going to be installed. It should error like that if 'make
> > install-man' is called.
>
> > Trying to do that now.

Namhyung, can I have your acked-by for this? I.e. changing the message
to 'man perf-list' and just warning the user that documentation won't be
installed if xmlto & asciidoc are not found?

I'm fine with it this way.

- Arnaldo

> [acme@sandy linux]$ make -j8 -C tools/perf/ O=/home/acme/git/build/perf try-install-man
> make: Entering directory `/home/git/linux/tools/perf'
> make -C Documentation try-install-man
> make[1]: Entering directory `/home/git/linux/tools/perf/Documentation'
> make[2]: Entering directory `/home/git/linux/tools/perf'
> make[2]: `/home/acme/git/build/perf/PERF-VERSION-FILE' is up to date.
> make[2]: Leaving directory `/home/git/linux/tools/perf'
> Makefile:202: Please install the asciidoc package to have the man pages installed
> Makefile:202: Please install the xmlto package to have the man pages installed
> make[1]: `try-install-man' is up to date.
> make[1]: Leaving directory `/home/git/linux/tools/perf/Documentation'
> make: Leaving directory `/home/git/linux/tools/perf'
> [acme@sandy linux]$
>
> The 'install-man' target will use your check-executable-or-error-out
> stuff, but the main 'install' target will call just 'try-install-man',
> that will not stop the build if there are missing tools to generate the
> man pages.
>
> Installing just xmlto makes it ask just for asciidoc, install both and it
> generates the man pages, please try to check if it works for you so that I can
> have your Tested-by.
>
> - Arnaldo

> diff --git a/tools/perf/Documentation/Makefile b/tools/perf/Documentation/Makefile
> index 9f2e44f..8cb65ea 100644
> --- a/tools/perf/Documentation/Makefile
> +++ b/tools/perf/Documentation/Makefile
> @@ -1,3 +1,5 @@
> +include ../config/utilities.mak
> +
> OUTPUT := ./
> ifeq ("$(origin O)", "command line")
> ifneq ($(O),)
> @@ -64,6 +66,7 @@ MAKEINFO=makeinfo
> INSTALL_INFO=install-info
> DOCBOOK2X_TEXI=docbook2x-texi
> DBLATEX=dblatex
> +XMLTO=xmlto
> ifndef PERL_PATH
> PERL_PATH = /usr/bin/perl
> endif
> @@ -71,6 +74,15 @@ endif
> -include ../config.mak.autogen
> -include ../config.mak
>
> +have_asciidoc := $(call get-executable,$(ASCIIDOC))
> +have_xmlto := $(call get-executable,$(XMLTO))
> +ifeq ($(have_asciidoc),)
> + missing_tools = true
> +endif
> +ifeq ($(have_xmlto),)
> + missing_tools = true
> +endif
> +
> #
> # For asciidoc ...
> # -7.1.2, no extra settings are needed.
> @@ -170,7 +182,11 @@ pdf: $(OUTPUT)user-manual.pdf
>
> install: install-man
>
> -install-man: man
> +check-man-tools:
> + $(call check-executable-or-error-out,$(ASCIIDOC))
> + $(call check-executable-or-error-out,$(XMLTO))
> +
> +do-install-man: man
> $(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
> # $(INSTALL) -d -m 755 $(DESTDIR)$(man5dir)
> # $(INSTALL) -d -m 755 $(DESTDIR)$(man7dir)
> @@ -178,6 +194,20 @@ install-man: man
> # $(INSTALL) -m 644 $(DOC_MAN5) $(DESTDIR)$(man5dir)
> # $(INSTALL) -m 644 $(DOC_MAN7) $(DESTDIR)$(man7dir)
>
> +install-man: check-man-tools man
> +
> +try-install-man:
> +ifdef missing_tools
> +ifeq ($(have_asciidoc),)
> + $(warning Please install the asciidoc package to have the man pages installed)
> +endif
> +ifeq ($(have_xmlto),)
> + $(warning Please install the xmlto package to have the man pages installed)
> +endif
> +else
> + $(MAKE) do-install-man
> +endif
> +
> install-info: info
> $(INSTALL) -d -m 755 $(DESTDIR)$(infodir)
> $(INSTALL) -m 644 $(OUTPUT)perf.info $(OUTPUT)perfman.info $(DESTDIR)$(infodir)
> @@ -246,7 +276,7 @@ $(MAN_HTML): $(OUTPUT)%.html : %.txt
>
> $(OUTPUT)%.1 $(OUTPUT)%.5 $(OUTPUT)%.7 : $(OUTPUT)%.xml
> $(QUIET_XMLTO)$(RM) $@ && \
> - xmlto -o $(OUTPUT) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
> + $(XMLTO) -o $(OUTPUT) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
>
> $(OUTPUT)%.xml : %.txt
> $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 404fdb4..b14eeb8 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -1041,7 +1041,7 @@ perfexec_instdir = $(prefix)/$(perfexecdir)
> endif
> perfexec_instdir_SQ = $(subst ','\'',$(perfexec_instdir))
>
> -install: all install-man
> +install: all try-install-man
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
> $(INSTALL) $(OUTPUT)perf '$(DESTDIR_SQ)$(bindir_SQ)'
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/perl/Perf-Trace-Util/lib/Perf/Trace'
> @@ -1067,6 +1067,9 @@ install-doc:
> install-man:
> $(MAKE) -C Documentation install-man
>
> +try-install-man:
> + $(MAKE) -C Documentation try-install-man
> +
> install-html:
> $(MAKE) -C Documentation install-html
>
> diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
> index ea853c2..42e264a 100644
> --- a/tools/perf/config/utilities.mak
> +++ b/tools/perf/config/utilities.mak
> @@ -168,6 +168,8 @@ _is-executable-sh = $(call shell-sq,test -f $(1) -a -x $(1) && echo y)
> get-executable = $(if $(1),$(if $(is-absolute),$(_ge-abspath),$(lookup)))
> _ge-abspath = $(if $(is-executable),$(1))
>
> +check-executable-or-error-out = $(if $(call get-executable,$(1)),,$(error $(1) not found, please install it on your distro))
> +
> # get-supplied-or-default-executable
> #
> # Usage: absolute-executable-path-or-empty = $(call get-executable-or-default,variable,default)

2012-10-24 19:30:51

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] perf: Build install-man target when installing

On Wed, Oct 24, 2012 at 10:03:16AM -0700, Arnaldo Carvalho de Melo wrote:
> The 'install-man' target will use your check-executable-or-error-out
> stuff, but the main 'install' target will call just 'try-install-man',
> that will not stop the build if there are missing tools to generate
> the man pages.
>
> Installing just xmlto makes it ask just for asciidoc, install both and
> it generates the man pages, please try to check if it works for you so
> that I can have your Tested-by.

Ok, then this can be done even simpler, I went and simplified your
version and we don't need that check-executable-or-error-out helper
anymore, I'm including it below.

For it you can have my

Signed-off-by: Borislav Petkov <[email protected]>
Tested-by: Borislav Petkov <[email protected]>

Btw, I keep seeing those errors when building on your perf/core branch:

...
FLEX util/pmu-flex.c
FLEX util/parse-events-flex.c
CC util/pmu-flex.o
CC util/pmu.o
CC util/parse-events.o
gcc: error: python_ext_build/tmp//../../libtraceevent.a: No such file or directory
error: command 'gcc' failed with exit status 1
cp: cannot stat `python_ext_build/lib/perf.so': No such file or directory
make: *** [python/perf.so] Error 1
make: *** Waiting for unfinished jobs....
Makefile:513: No libunwind found, disabling post unwind support. Please install libunwind-dev[el] >= 0.99
Makefile:568: No libaudit.h found, disables 'trace' tool, please install audit-libs-devel or libaudit-dev
Makefile:579: newt not found, disables TUI support. Please install newt-devel or libnewt-dev
SUBDIR ../lib/traceevent/

...

Thanks.

---
diff --git a/tools/perf/Documentation/Makefile b/tools/perf/Documentation/Makefile
index 9f2e44f2b17a..ef6d22e879eb 100644
--- a/tools/perf/Documentation/Makefile
+++ b/tools/perf/Documentation/Makefile
@@ -1,3 +1,5 @@
+include ../config/utilities.mak
+
OUTPUT := ./
ifeq ("$(origin O)", "command line")
ifneq ($(O),)
@@ -64,6 +66,7 @@ MAKEINFO=makeinfo
INSTALL_INFO=install-info
DOCBOOK2X_TEXI=docbook2x-texi
DBLATEX=dblatex
+XMLTO=xmlto
ifndef PERL_PATH
PERL_PATH = /usr/bin/perl
endif
@@ -71,6 +74,16 @@ endif
-include ../config.mak.autogen
-include ../config.mak

+_tmp_tool_path := $(call get-executable,$(ASCIIDOC))
+ifeq ($(_tmp_tool_path),)
+ missing_tools = $(ASCIIDOC)
+endif
+
+_tmp_tool_path := $(call get-executable,$(XMLTO))
+ifeq ($(_tmp_tool_path),)
+ missing_tools += $(XMLTO)
+endif
+
#
# For asciidoc ...
# -7.1.2, no extra settings are needed.
@@ -170,7 +183,12 @@ pdf: $(OUTPUT)user-manual.pdf

install: install-man

-install-man: man
+check-man-tools:
+ifdef missing_tools
+ $(error "You need to install $(missing_tools) for man pages")
+endif
+
+do-install-man: man
$(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
# $(INSTALL) -d -m 755 $(DESTDIR)$(man5dir)
# $(INSTALL) -d -m 755 $(DESTDIR)$(man7dir)
@@ -178,6 +196,15 @@ install-man: man
# $(INSTALL) -m 644 $(DOC_MAN5) $(DESTDIR)$(man5dir)
# $(INSTALL) -m 644 $(DOC_MAN7) $(DESTDIR)$(man7dir)

+install-man: check-man-tools man
+
+try-install-man:
+ifdef missing_tools
+ $(warning Please install $(missing_tools) to have the man pages installed)
+else
+ $(MAKE) do-install-man
+endif
+
install-info: info
$(INSTALL) -d -m 755 $(DESTDIR)$(infodir)
$(INSTALL) -m 644 $(OUTPUT)perf.info $(OUTPUT)perfman.info $(DESTDIR)$(infodir)
@@ -246,7 +273,7 @@ $(MAN_HTML): $(OUTPUT)%.html : %.txt

$(OUTPUT)%.1 $(OUTPUT)%.5 $(OUTPUT)%.7 : $(OUTPUT)%.xml
$(QUIET_XMLTO)$(RM) $@ && \
- xmlto -o $(OUTPUT) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
+ $(XMLTO) -o $(OUTPUT) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<

$(OUTPUT)%.xml : %.txt
$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 76190a7e2934..3252563bfca7 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -1038,7 +1038,7 @@ perfexec_instdir = $(prefix)/$(perfexecdir)
endif
perfexec_instdir_SQ = $(subst ','\'',$(perfexec_instdir))

-install: all install-man
+install: all try-install-man
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
$(INSTALL) $(OUTPUT)perf '$(DESTDIR_SQ)$(bindir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/perl/Perf-Trace-Util/lib/Perf/Trace'
@@ -1064,6 +1064,9 @@ install-doc:
install-man:
$(MAKE) -C Documentation install-man

+try-install-man:
+ $(MAKE) -C Documentation try-install-man
+
install-html:
$(MAKE) -C Documentation install-html


--
Regards/Gruss,
Boris.

2012-10-24 19:41:34

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf: Build install-man target when installing

Em Wed, Oct 24, 2012 at 09:30:46PM +0200, Borislav Petkov escreveu:
> On Wed, Oct 24, 2012 at 10:03:16AM -0700, Arnaldo Carvalho de Melo wrote:
> > The 'install-man' target will use your check-executable-or-error-out
> > stuff, but the main 'install' target will call just 'try-install-man',
> > that will not stop the build if there are missing tools to generate
> > the man pages.
> >
> > Installing just xmlto makes it ask just for asciidoc, install both and
> > it generates the man pages, please try to check if it works for you so
> > that I can have your Tested-by.
>
> Ok, then this can be done even simpler, I went and simplified your
> version and we don't need that check-executable-or-error-out helper
> anymore, I'm including it below.
>
> For it you can have my
>
> Signed-off-by: Borislav Petkov <[email protected]>
> Tested-by: Borislav Petkov <[email protected]>
>
> Btw, I keep seeing those errors when building on your perf/core branch:

That is fixed in the perf/urgent one, I'm reworking my perf/core branch
on top of Ingo's tip/perf/core, that already merged tip/perf/urgent so
has that fix.

Testing your new version now, thanks!

- Arnaldo

> ...
> FLEX util/pmu-flex.c
> FLEX util/parse-events-flex.c
> CC util/pmu-flex.o
> CC util/pmu.o
> CC util/parse-events.o
> gcc: error: python_ext_build/tmp//../../libtraceevent.a: No such file or directory
> error: command 'gcc' failed with exit status 1
> cp: cannot stat `python_ext_build/lib/perf.so': No such file or directory
> make: *** [python/perf.so] Error 1
> make: *** Waiting for unfinished jobs....
> Makefile:513: No libunwind found, disabling post unwind support. Please install libunwind-dev[el] >= 0.99
> Makefile:568: No libaudit.h found, disables 'trace' tool, please install audit-libs-devel or libaudit-dev
> Makefile:579: newt not found, disables TUI support. Please install newt-devel or libnewt-dev
> SUBDIR ../lib/traceevent/
>
> ...
>
> Thanks.
>
> ---
> diff --git a/tools/perf/Documentation/Makefile b/tools/perf/Documentation/Makefile
> index 9f2e44f2b17a..ef6d22e879eb 100644
> --- a/tools/perf/Documentation/Makefile
> +++ b/tools/perf/Documentation/Makefile
> @@ -1,3 +1,5 @@
> +include ../config/utilities.mak
> +
> OUTPUT := ./
> ifeq ("$(origin O)", "command line")
> ifneq ($(O),)
> @@ -64,6 +66,7 @@ MAKEINFO=makeinfo
> INSTALL_INFO=install-info
> DOCBOOK2X_TEXI=docbook2x-texi
> DBLATEX=dblatex
> +XMLTO=xmlto
> ifndef PERL_PATH
> PERL_PATH = /usr/bin/perl
> endif
> @@ -71,6 +74,16 @@ endif
> -include ../config.mak.autogen
> -include ../config.mak
>
> +_tmp_tool_path := $(call get-executable,$(ASCIIDOC))
> +ifeq ($(_tmp_tool_path),)
> + missing_tools = $(ASCIIDOC)
> +endif
> +
> +_tmp_tool_path := $(call get-executable,$(XMLTO))
> +ifeq ($(_tmp_tool_path),)
> + missing_tools += $(XMLTO)
> +endif
> +
> #
> # For asciidoc ...
> # -7.1.2, no extra settings are needed.
> @@ -170,7 +183,12 @@ pdf: $(OUTPUT)user-manual.pdf
>
> install: install-man
>
> -install-man: man
> +check-man-tools:
> +ifdef missing_tools
> + $(error "You need to install $(missing_tools) for man pages")
> +endif
> +
> +do-install-man: man
> $(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
> # $(INSTALL) -d -m 755 $(DESTDIR)$(man5dir)
> # $(INSTALL) -d -m 755 $(DESTDIR)$(man7dir)
> @@ -178,6 +196,15 @@ install-man: man
> # $(INSTALL) -m 644 $(DOC_MAN5) $(DESTDIR)$(man5dir)
> # $(INSTALL) -m 644 $(DOC_MAN7) $(DESTDIR)$(man7dir)
>
> +install-man: check-man-tools man
> +
> +try-install-man:
> +ifdef missing_tools
> + $(warning Please install $(missing_tools) to have the man pages installed)
> +else
> + $(MAKE) do-install-man
> +endif
> +
> install-info: info
> $(INSTALL) -d -m 755 $(DESTDIR)$(infodir)
> $(INSTALL) -m 644 $(OUTPUT)perf.info $(OUTPUT)perfman.info $(DESTDIR)$(infodir)
> @@ -246,7 +273,7 @@ $(MAN_HTML): $(OUTPUT)%.html : %.txt
>
> $(OUTPUT)%.1 $(OUTPUT)%.5 $(OUTPUT)%.7 : $(OUTPUT)%.xml
> $(QUIET_XMLTO)$(RM) $@ && \
> - xmlto -o $(OUTPUT) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
> + $(XMLTO) -o $(OUTPUT) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
>
> $(OUTPUT)%.xml : %.txt
> $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 76190a7e2934..3252563bfca7 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -1038,7 +1038,7 @@ perfexec_instdir = $(prefix)/$(perfexecdir)
> endif
> perfexec_instdir_SQ = $(subst ','\'',$(perfexec_instdir))
>
> -install: all install-man
> +install: all try-install-man
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
> $(INSTALL) $(OUTPUT)perf '$(DESTDIR_SQ)$(bindir_SQ)'
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/perl/Perf-Trace-Util/lib/Perf/Trace'
> @@ -1064,6 +1064,9 @@ install-doc:
> install-man:
> $(MAKE) -C Documentation install-man
>
> +try-install-man:
> + $(MAKE) -C Documentation try-install-man
> +
> install-html:
> $(MAKE) -C Documentation install-html
>
>
> --
> Regards/Gruss,
> Boris.