linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: roberto.sassu@huaweicloud.com
Cc: peterz@infradead.org, mingo@redhat.com, mark.rutland@arm.com,
	alexander.shishkin@linux.intel.com, jolsa@kernel.org,
	namhyung@kernel.org, quentin@isovalent.com,
	linux-perf-users@vger.kernel.org, bpf@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Roberto Sassu <roberto.sassu@huawei.com>
Subject: Re: [PATCH 3/3] tools/build: Display logical OR of a feature flavors
Date: Thu, 18 Aug 2022 10:09:37 -0300	[thread overview]
Message-ID: <Yv46EW6KbUe9zjur@kernel.org> (raw)
In-Reply-To: <20220818120957.319995-3-roberto.sassu@huaweicloud.com>

Em Thu, Aug 18, 2022 at 02:09:57PM +0200, roberto.sassu@huaweicloud.com escreveu:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Sometimes, features are simply different flavors of another feature, to
> properly detect the exact dependencies needed by different Linux
> distributions.
> 
> For example, libbfd has three flavors: libbfd if the distro does not
> require any additional dependency; libbfd-liberty if it requires libiberty;
> libbfd-liberty-z if it requires libiberty and libz.
> 
> It might not be clear to the user whether a feature has been successfully
> detected or not, given that some of its flavors will be set to OFF, others
> to ON.
> 
> Instead, display only the feature main flavor if not in verbose mode
> (VF != 1), and set it to ON if at least one of its flavors has been
> successfully detected (logical OR), OFF otherwise. Omit the other flavors.
> 
> Accomplish that by declaring a FEATURE_GROUP_MEMBERS-<feature main flavor>
> variable, with the list of the other flavors as variable value. For now, do
> it just for libbfd.
> 
> In verbose mode, of if no group is defined for a feature, show the feature
> detection result as before.

Looks cool, tested and added this to the commit log message here in my
local branch, that will go public after further tests for the other
csets in it:

Committer testing:

Collecting the output from:

  $ make -C tools/bpf/bpftool/ clean
  $ make -C tools/bpf/bpftool/ |& grep "Auto-detecting system features" -A10

  $ diff -u before after
  --- before    2022-08-18 10:06:40.422086966 -0300
  +++ after     2022-08-18 10:07:59.202138282 -0300
  @@ -1,6 +1,4 @@
   Auto-detecting system features:
   ...                                  libbfd: [ on  ]
  -...                          libbfd-liberty: [ on  ]
  -...                        libbfd-liberty-z: [ on  ]
   ...                                  libcap: [ on  ]
   ...                         clang-bpf-co-re: [ on  ]
  $

Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Thanks for working on this!

- Arnaldo
 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  tools/build/Makefile.feature | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 6c809941ff01..57619f240b56 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -137,6 +137,12 @@ FEATURE_DISPLAY ?=              \
>           libaio			\
>           libzstd
>  
> +#
> +# Declare group members of a feature to display the logical OR of the detection
> +# result instead of each member result.
> +#
> +FEATURE_GROUP_MEMBERS-libbfd = libbfd-liberty libbfd-liberty-z
> +
>  # Set FEATURE_CHECK_(C|LD)FLAGS-all for all FEATURE_TESTS features.
>  # If in the future we need per-feature checks/flags for features not
>  # mentioned in this list we need to refactor this ;-).
> @@ -179,8 +185,17 @@ endif
>  #
>  feature_print_status = $(eval $(feature_print_status_code))
>  
> +feature_group = $(eval $(feature_gen_group)) $(GROUP)
> +
> +define feature_gen_group
> +  GROUP := $(1)
> +  ifneq ($(feature_verbose),1)
> +    GROUP += $(FEATURE_GROUP_MEMBERS-$(1))
> +  endif
> +endef
> +
>  define feature_print_status_code
> -  ifeq ($(feature-$(1)), 1)
> +  ifneq (,$(filter 1,$(foreach feat,$(call feature_group,$(feat)),$(feature-$(feat)))))
>      MSG = $(shell printf '...%40s: [ \033[32mon\033[m  ]' $(1))
>    else
>      MSG = $(shell printf '...%40s: [ \033[31mOFF\033[m ]' $(1))
> @@ -244,12 +259,20 @@ ifeq ($(VF),1)
>    feature_verbose := 1
>  endif
>  
> +ifneq ($(feature_verbose),1)
> +  #
> +  # Determine the features to omit from the displayed message, as only the
> +  # logical OR of the detection result will be shown.
> +  #
> +  FEATURE_OMIT := $(foreach feat,$(FEATURE_DISPLAY),$(FEATURE_GROUP_MEMBERS-$(feat)))
> +endif
> +
>  feature_display_entries = $(eval $(feature_display_entries_code))
>  define feature_display_entries_code
>    ifeq ($(feature_display),1)
>      $$(info )
>      $$(info Auto-detecting system features:)
> -    $(foreach feat,$(FEATURE_DISPLAY),$(call feature_print_status,$(feat),) $$(info $(MSG)))
> +    $(foreach feat,$(filter-out $(FEATURE_OMIT),$(FEATURE_DISPLAY)),$(call feature_print_status,$(feat),) $$(info $(MSG)))
>    endif
>  
>    ifeq ($(feature_verbose),1)
> -- 
> 2.25.1

-- 

- Arnaldo

  reply	other threads:[~2022-08-18 13:09 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-18 12:09 [PATCH 1/3] tools/build: Fix feature detection output due to eval expansion roberto.sassu
2022-08-18 12:09 ` [PATCH 2/3] tools/build: Increment room for feature name in feature detection output roberto.sassu
2022-08-18 12:09 ` [PATCH 3/3] tools/build: Display logical OR of a feature flavors roberto.sassu
2022-08-18 13:09   ` Arnaldo Carvalho de Melo [this message]
2022-08-18 13:25     ` Roberto Sassu
2022-08-18 16:40       ` Quentin Monnet
2022-08-18 18:14         ` Arnaldo Carvalho de Melo
2022-08-18 22:15           ` Quentin Monnet
2022-08-22 10:58   ` Jiri Olsa
2022-08-22 11:24     ` Roberto Sassu
2022-09-27  7:14       ` Roberto Sassu
2022-09-27 12:21         ` Arnaldo Carvalho de Melo
2022-09-27 12:25           ` Roberto Sassu
2022-09-27 13:24           ` Quentin Monnet

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=Yv46EW6KbUe9zjur@kernel.org \
    --to=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bpf@vger.kernel.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=quentin@isovalent.com \
    --cc=roberto.sassu@huawei.com \
    --cc=roberto.sassu@huaweicloud.com \
    /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).