linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@redhat.com>
To: Jiri Olsa <jolsa@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	Corey Ashford <cjashfor@linux.vnet.ibm.com>,
	David Ahern <david.ahern@oracle.com>,
	Ingo Molnar <mingo@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Paul Mackerras <paulus@samba.org>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH 01/15] perf build: Disable default check for libbabeltrace
Date: Fri, 13 Mar 2015 12:36:21 -0300	[thread overview]
Message-ID: <20150313153621.GB3467@redhat.com> (raw)
In-Reply-To: <1426064344-21737-2-git-send-email-jolsa@kernel.org>

Em Wed, Mar 11, 2015 at 09:58:50AM +0100, Jiri Olsa escreveu:
> Remove libbabeltrace check from default features set, because the
> requested version is not released yet in most distributions. We'll
> enable later.
> 
> Calling libbabeltrace check manually via feature_check
> before $(feature-libbabeltrace) is used.

So, before this patch:

[acme@ssdandy perf]$ make
  BUILD:   Doing 'make -j8' parallel build
config/Makefile:425: No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR
config/Makefile:709: No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-dev[el]/libbabeltrace-ctf-dev

Auto-detecting system features:
...                         dwarf: [ on  ]
...                         glibc: [ on  ]
...                          gtk2: [ on  ]
...                      libaudit: [ on  ]
...                        libbfd: [ on  ]
...                        libelf: [ on  ]
...                       libnuma: [ on  ]
...                       libperl: [ on  ]
...                     libpython: [ on  ]
...                      libslang: [ on  ]
...                     libunwind: [ OFF ]
...            libdw-dwarf-unwind: [ on  ]
...                 libbabeltrace: [ OFF ]
...                          zlib: [ on  ]
...     DWARF post unwind library: libdw

It tries to build babeltrace in, does not find the devel files, emits that "No libbabeltrace found"
and then marks it as OFF, ok.

Now, I am ok with the intent of this patch, but... After applying it we get:

[acme@ssdandy linux]$ cd -
/home/acme/git/linux/tools/perf
[acme@ssdandy perf]$ 
[acme@ssdandy perf]$ make
  BUILD:   Doing 'make -j8' parallel build
config/Makefile:423: No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR
config/Makefile:713: No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-dev[el]/libbabeltrace-ctf-dev

Auto-detecting system features:
...                         dwarf: [ on  ]
...                         glibc: [ on  ]
...                          gtk2: [ on  ]
...                      libaudit: [ on  ]
...                        libbfd: [ on  ]
...                        libelf: [ on  ]
...                       libnuma: [ on  ]
...                       libperl: [ on  ]
...                     libpython: [ on  ]
...                      libslang: [ on  ]
...                     libunwind: [ OFF ]
...            libdw-dwarf-unwind: [ on  ]
...                          zlib: [ on  ]
...     DWARF post unwind library: libdw

  GEN      common-cmds.h

It continues trying to find babeltrace, does not find it, emits the warning and
just doesn't show the OFF message :-\

Can you explain _why_ this is needed?  I.e. is it to speed up feature checking?
In what way, etc. For casual readers the intent of this patch may be difficult
to grasp, no?

What am I missing?

- Arnaldo
 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Acked-by: Ingo Molnar <mingo@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: David Ahern <david.ahern@oracle.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> ---
>  tools/perf/config/Makefile                  | 11 +++++------
>  tools/perf/config/feature-checks/Makefile   |  2 +-
>  tools/perf/config/feature-checks/test-all.c |  9 ++++++++-
>  3 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index d44c64d64465..45f61253ccce 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -227,7 +227,6 @@ CORE_FEATURE_TESTS =			\
>  	stackprotector-all		\
>  	timerfd				\
>  	libdw-dwarf-unwind		\
> -	libbabeltrace			\
>  	zlib
>  
>  LIB_FEATURE_TESTS =			\
> @@ -243,7 +242,6 @@ LIB_FEATURE_TESTS =			\
>  	libslang			\
>  	libunwind			\
>  	libdw-dwarf-unwind		\
> -	libbabeltrace			\
>  	zlib
>  
>  VF_FEATURE_TESTS =			\
> @@ -705,14 +703,15 @@ else
>  endif
>  
>  ifndef NO_LIBBABELTRACE
> -  ifeq ($(feature-libbabeltrace), 0)
> -    msg := $(warning No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-dev[el]/libbabeltrace-ctf-dev);
> -    NO_LIBBABELTRACE := 1
> -  else
> +  $(call feature_check,libbabeltrace)
> +  ifeq ($(feature-libbabeltrace), 1)
>      CFLAGS += -DHAVE_LIBBABELTRACE_SUPPORT $(LIBBABELTRACE_CFLAGS)
>      LDFLAGS += $(LIBBABELTRACE_LDFLAGS)
>      EXTLIBS += -lbabeltrace-ctf
>      $(call detected,CONFIG_LIBBABELTRACE)
> +  else
> +    msg := $(warning No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-dev[el]/libbabeltrace-ctf-dev);
> +    NO_LIBBABELTRACE := 1
>    endif
>  endif
>  
> diff --git a/tools/perf/config/feature-checks/Makefile b/tools/perf/config/feature-checks/Makefile
> index 8fe067864957..b6e2e312ba54 100644
> --- a/tools/perf/config/feature-checks/Makefile
> +++ b/tools/perf/config/feature-checks/Makefile
> @@ -44,7 +44,7 @@ BUILD = $(CC) $(CFLAGS) -Wall -Werror -o $(OUTPUT)$@ $(patsubst %.bin,%.c,$@) $(
>  ###############################
>  
>  test-all.bin:
> -	$(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -laudit -I/usr/include/slang -lslang $(shell $(PKG_CONFIG) --libs --cflags gtk+-2.0 2>/dev/null) $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz -lbabeltrace
> +	$(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -laudit -I/usr/include/slang -lslang $(shell $(PKG_CONFIG) --libs --cflags gtk+-2.0 2>/dev/null) $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz
>  
>  test-hello.bin:
>  	$(BUILD)
> diff --git a/tools/perf/config/feature-checks/test-all.c b/tools/perf/config/feature-checks/test-all.c
> index 1ffc3da5ca10..06f5c8a67cb2 100644
> --- a/tools/perf/config/feature-checks/test-all.c
> +++ b/tools/perf/config/feature-checks/test-all.c
> @@ -101,9 +101,17 @@
>  # include "test-pthread_attr_setaffinity_np.c"
>  #undef main
>  
> +# if 0
> +/*
> + * Disable libbabeltrace check for test-all, because the requested
> + * library version is not released yet in most distributions. Will
> + * reenable later.
> + */
> +
>  #define main main_test_libbabeltrace
>  # include "test-libbabeltrace.c"
>  #undef main
> +#endif
>  
>  int main(int argc, char *argv[])
>  {
> @@ -130,7 +138,6 @@ int main(int argc, char *argv[])
>  	main_test_sync_compare_and_swap(argc, argv);
>  	main_test_zlib();
>  	main_test_pthread_attr_setaffinity_np();
> -	main_test_libbabeltrace();
>  
>  	return 0;
>  }
> -- 
> 1.9.3

  reply	other threads:[~2015-03-13 16:02 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-11  8:58 [PATCHv2 00/15] tools build: Move features framework into tools/build Jiri Olsa
2015-03-11  8:58 ` [PATCH 01/15] perf build: Disable default check for libbabeltrace Jiri Olsa
2015-03-13 15:36   ` Arnaldo Carvalho de Melo [this message]
2015-03-13 16:20     ` Jiri Olsa
2015-03-13 20:19       ` Arnaldo Carvalho de Melo
2015-03-15 17:32         ` Jiri Olsa
2015-03-11  8:58 ` [PATCH 02/15] perf build: Add dump of features build Jiri Olsa
2015-03-11  8:58 ` [PATCH 03/15] perf build: Fix pthread-attr-setaffinity-np include in test-all Jiri Olsa
2015-03-11  8:58 ` [PATCH 04/15] perf build: Get rid of LIB_INCLUDE variable Jiri Olsa
2015-03-11  8:58 ` [PATCH 05/15] perf build: Rename CORE_FEATURE_TESTS to FEATURE_TESTS Jiri Olsa
2015-03-11  8:58 ` [PATCH 06/15] perf build: Get rid of VF_FEATURE_TESTS Jiri Olsa
2015-03-11  8:58 ` [PATCH 07/15] perf build: Rename display_lib into feature_display Jiri Olsa
2015-03-11  8:58 ` [PATCH 08/15] perf build: Rename display_vf to feature_verbose Jiri Olsa
2015-03-11  8:58 ` [PATCH 09/15] perf build: Rename PERF-FEATURES into FEATURE-DUMP Jiri Olsa
2015-03-11  8:58 ` [PATCH 10/15] perf build: Rename feature_print_var_code to print_var_code Jiri Olsa
2015-03-11  8:59 ` [PATCH 11/15] perf build: Separate feature make support into config/Makefile.feature Jiri Olsa
2015-03-11  8:59 ` [PATCH 12/15] perf build: Make features checks directory configurable Jiri Olsa
2015-03-11  8:59 ` [PATCH 13/15] tools build: Move feature checks code under tools/build Jiri Olsa
2015-03-11  8:59 ` [PATCH 14/15] tools build: Allow to override feature checks setup Jiri Olsa
2015-03-11  8:59 ` [PATCH 15/15] tools build: Fix feature_check name clash Jiri Olsa
2015-03-18 18:50 ` [PATCHv2 00/15] tools build: Move features framework into tools/build Arnaldo Carvalho de Melo
2015-03-18 18:59   ` Jiri Olsa
2015-03-18 19:53     ` Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150313153621.GB3467@redhat.com \
    --to=acme@redhat.com \
    --cc=cjashfor@linux.vnet.ibm.com \
    --cc=david.ahern@oracle.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).