[PATCH 1/3] tools/build: Fix feature detection output due to eval expansion

[PATCH 1/3] tools/build: Fix feature detection output due to eval expansion

[roberto.sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

As the first eval expansion is used only to generate Makefile statements,
messages should not be displayed at this stage, as for example conditional
expressions are not evaluated.

It can be seen for example in the output of feature detection for bpftool,
where the number of detected features does not change, despite turning on
the verbose mode (VF = 1) and there are additional features to display.

Fix this issue by escaping the $ before $(info) statements, to ensure that
messages are printed only when the function containing them is actually
executed, and not when it is expanded.

In addition, move the $(info) statement out of feature_print_status, due to
the fact that is called both inside and outside an eval context, and place
it to the caller so that the $ can be escaped when necessary. For symmetry,
move the $(info) statement also out of feature_print_text, and place it to
the caller.

Force the TMP variable evaluation in verbose mode, to display the features
in FEATURE_TESTS that are not in FEATURE_DISPLAY.

Reorder perf feature detection messages (first non-verbose, then verbose
ones) by moving the call to feature_display_entries earlier, before the VF
environment variable check.

Also, remove the newline from that function, as perf might display
additional messages. Move the newline to perf Makefile, and display another
one if displaying the detection result is not deferred as in the case of
bpftool.

Fixes: 0afc5cad387db ("perf build: Separate feature make support into config/Makefile.feature")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/build/Makefile.feature | 19 ++++++++-----------
 tools/perf/Makefile.config   | 15 ++++++++-------
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index fc6ce0b2535a..9d3afbc37e15 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -177,7 +177,7 @@ endif
 #
 # Print the result of the feature test:
 #
-feature_print_status = $(eval $(feature_print_status_code)) $(info $(MSG))
+feature_print_status = $(eval $(feature_print_status_code))
 
 define feature_print_status_code
   ifeq ($(feature-$(1)), 1)
@@ -187,7 +187,7 @@ define feature_print_status_code
   endif
 endef
 
-feature_print_text = $(eval $(feature_print_text_code)) $(info $(MSG))
+feature_print_text = $(eval $(feature_print_text_code))
 define feature_print_text_code
     MSG = $(shell printf '...%30s: %s' $(1) $(2))
 endef
@@ -247,21 +247,18 @@ 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),))
-    ifneq ($(feature_verbose),1)
-      $(info )
-    endif
+    $$(info )
+    $$(info Auto-detecting system features:)
+    $(foreach feat,$(FEATURE_DISPLAY),$(call feature_print_status,$(feat),) $$(info $(MSG)))
   endif
 
   ifeq ($(feature_verbose),1)
-    TMP := $(filter-out $(FEATURE_DISPLAY),$(FEATURE_TESTS))
-    $(foreach feat,$(TMP),$(call feature_print_status,$(feat),))
-    $(info )
+    $(eval TMP := $(filter-out $(FEATURE_DISPLAY),$(FEATURE_TESTS)))
+    $(foreach feat,$(TMP),$(call feature_print_status,$(feat),) $$(info $(MSG)))
   endif
 endef
 
 ifeq ($(FEATURE_DISPLAY_DEFERRED),)
   $(call feature_display_entries)
+  $(info )
 endif
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 0661a1cf9855..f4de6e16fbe2 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -1304,11 +1304,15 @@ define print_var_code
     MSG = $(shell printf '...%30s: %s' $(1) $($(1)))
 endef
 
+ifeq ($(feature_display),1)
+  $(call feature_display_entries)
+endif
+
 ifeq ($(VF),1)
   # Display EXTRA features which are detected manualy
   # from here with feature_check call and thus cannot
   # be partof global state output.
-  $(foreach feat,$(FEATURE_TESTS_EXTRA),$(call feature_print_status,$(feat),))
+  $(foreach feat,$(FEATURE_TESTS_EXTRA),$(call feature_print_status,$(feat),) $(info $(MSG)))
   $(call print_var,prefix)
   $(call print_var,bindir)
   $(call print_var,libdir)
@@ -1318,11 +1322,12 @@ ifeq ($(VF),1)
   $(call print_var,JDIR)
 
   ifeq ($(dwarf-post-unwind),1)
-    $(call feature_print_text,"DWARF post unwind library", $(dwarf-post-unwind-text))
+    $(call feature_print_text,"DWARF post unwind library", $(dwarf-post-unwind-text)) $(info $(MSG))
   endif
-  $(info )
 endif
 
+$(info )
+
 $(call detected_var,bindir_SQ)
 $(call detected_var,PYTHON_WORD)
 ifneq ($(OUTPUT),)
@@ -1352,7 +1357,3 @@ endif
 # tests.
 $(shell rm -f $(FEATURE_DUMP_FILENAME))
 $(foreach feat,$(FEATURE_TESTS),$(shell echo "$(call feature_assign,$(feat))" >> $(FEATURE_DUMP_FILENAME)))
-
-ifeq ($(feature_display),1)
-  $(call feature_display_entries)
-endif
-- 
2.25.1

[PATCH 2/3] tools/build: Increment room for feature name in feature detection output

[roberto.sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Since now there are features with a long name, increase the room for them,
so that fields are correctly aligned.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/build/Makefile.feature | 6 +++---
 tools/perf/Makefile.config   | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 9d3afbc37e15..6c809941ff01 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -181,15 +181,15 @@ feature_print_status = $(eval $(feature_print_status_code))
 
 define feature_print_status_code
   ifeq ($(feature-$(1)), 1)
-    MSG = $(shell printf '...%30s: [ \033[32mon\033[m  ]' $(1))
+    MSG = $(shell printf '...%40s: [ \033[32mon\033[m  ]' $(1))
   else
-    MSG = $(shell printf '...%30s: [ \033[31mOFF\033[m ]' $(1))
+    MSG = $(shell printf '...%40s: [ \033[31mOFF\033[m ]' $(1))
   endif
 endef
 
 feature_print_text = $(eval $(feature_print_text_code))
 define feature_print_text_code
-    MSG = $(shell printf '...%30s: %s' $(1) $(2))
+    MSG = $(shell printf '...%40s: %s' $(1) $(2))
 endef
 
 #
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index f4de6e16fbe2..c41a090c0652 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -1301,7 +1301,7 @@ endif
 
 print_var = $(eval $(print_var_code)) $(info $(MSG))
 define print_var_code
-    MSG = $(shell printf '...%30s: %s' $(1) $($(1)))
+    MSG = $(shell printf '...%40s: %s' $(1) $($(1)))
 endef
 
 ifeq ($(feature_display),1)
-- 
2.25.1

[PATCH 3/3] tools/build: Display logical OR of a feature flavors

[roberto.sassu]

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.

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

Re: [PATCH 3/3] tools/build: Display logical OR of a feature flavors

[Arnaldo Carvalho de Melo]

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

Re: [PATCH 3/3] tools/build: Display logical OR of a feature flavors

[Roberto Sassu]

On Thu, 2022-08-18 at 10:09 -0300, Arnaldo Carvalho de Melo wrote:
> 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!

Thanks for testing and for adapting/pushing the other patches!

Roberto

Re: [PATCH 3/3] tools/build: Display logical OR of a feature flavors

[Quentin Monnet]

On 18/08/2022 14:25, Roberto Sassu wrote:
> On Thu, 2022-08-18 at 10:09 -0300, Arnaldo Carvalho de Melo wrote:
>> 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!
> 
> Thanks for testing and for adapting/pushing the other patches!
> 
> Roberto
> 

Tested locally for bpftool and I also observe "libbfd: [ on  ]" only.
This looks much better, thank you Roberto for following up on this!

Quentin

Re: [PATCH 3/3] tools/build: Display logical OR of a feature flavors

[Arnaldo Carvalho de Melo]

Em Thu, Aug 18, 2022 at 05:40:04PM +0100, Quentin Monnet escreveu:
> On 18/08/2022 14:25, Roberto Sassu wrote:
> > On Thu, 2022-08-18 at 10:09 -0300, Arnaldo Carvalho de Melo wrote:
> >> 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!
> > 
> > Thanks for testing and for adapting/pushing the other patches!
> > 
> > Roberto
> > 
> 
> Tested locally for bpftool and I also observe "libbfd: [ on  ]" only.
> This looks much better, thank you Roberto for following up on this!

So I'll add your Tested-by: to this one as well, maybe to all the
patches in this series?

- Arnaldo

Re: [PATCH 3/3] tools/build: Display logical OR of a feature flavors

[Quentin Monnet]

On Thu, 18 Aug 2022 at 19:14, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Thu, Aug 18, 2022 at 05:40:04PM +0100, Quentin Monnet escreveu:
> > On 18/08/2022 14:25, Roberto Sassu wrote:
> > > On Thu, 2022-08-18 at 10:09 -0300, Arnaldo Carvalho de Melo wrote:
> > >> 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!
> > >
> > > Thanks for testing and for adapting/pushing the other patches!
> > >
> > > Roberto
> > >
> >
> > Tested locally for bpftool and I also observe "libbfd: [ on  ]" only.
> > This looks much better, thank you Roberto for following up on this!
>
> So I'll add your Tested-by: to this one as well, maybe to all the
> patches in this series?

Sorry, I haven't tested the first two patches other than by applying
them, so just for the third one preferably. Thanks!

Re: [PATCH 3/3] tools/build: Display logical OR of a feature flavors

[Jiri Olsa]

On Thu, Aug 18, 2022 at 02:09:57PM +0200, roberto.sassu@huaweicloud.com wrote:
> 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.
> 
> 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

nice, I checked and could not find any other 'flavours' instance
like libbfd, but it might happen in future

for the whole patchset:

Tested-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka


> +
>  # 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
> 

Re: [PATCH 3/3] tools/build: Display logical OR of a feature flavors

[Roberto Sassu]

On Mon, 2022-08-22 at 12:58 +0200, Jiri Olsa wrote:
> On Thu, Aug 18, 2022 at 02:09:57PM +0200, 

[...]

> 
> > In verbose mode, of if no group is defined for a feature, show the
> > feature
> > detection result as before.

Thanks Jiri.

'or' instead of 'of', if the patch can be edited.

Roberto

> > 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
> 
> nice, I checked and could not find any other 'flavours' instance
> like libbfd, but it might happen in future
> 
> for the whole patchset:
> 
> Tested-by: Jiri Olsa <jolsa@kernel.org>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> 
> thanks,
> jirka
> 
> 
> > +
> >  # 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
> > 

Re: [PATCH 3/3] tools/build: Display logical OR of a feature flavors

[Roberto Sassu]

On Mon, 2022-08-22 at 13:24 +0200, Roberto Sassu wrote:
> On Mon, 2022-08-22 at 12:58 +0200, Jiri Olsa wrote:
> > On Thu, Aug 18, 2022 at 02:09:57PM +0200, 
> 
> [...]
> 
> > > In verbose mode, of if no group is defined for a feature, show
> > > the
> > > feature
> > > detection result as before.
> 
> Thanks Jiri.
> 
> 'or' instead of 'of', if the patch can be edited.

Hi Arnaldo

will you take these patches?

Thanks

Roberto

Re: [PATCH 3/3] tools/build: Display logical OR of a feature flavors

[Arnaldo Carvalho de Melo]

Em Tue, Sep 27, 2022 at 09:14:34AM +0200, Roberto Sassu escreveu:
> On Mon, 2022-08-22 at 13:24 +0200, Roberto Sassu wrote:
> > On Mon, 2022-08-22 at 12:58 +0200, Jiri Olsa wrote:
> > > On Thu, Aug 18, 2022 at 02:09:57PM +0200, 
> > 
> > [...]
> > 
> > > > In verbose mode, of if no group is defined for a feature, show
> > > > the
> > > > feature
> > > > detection result as before.
> > 
> > Thanks Jiri.
> > 
> > 'or' instead of 'of', if the patch can be edited.
> 
> Hi Arnaldo
> 
> will you take these patches?

The tools/build one I have in my perf/core branch, for v6.1.

⬢[acme@toolbox perf]$ git log --oneline --author roberto.sassu@huawei.com tools/{build,perf,lib}
924b0da1154fa814 tools build: Display logical OR of a feature flavors
1903f4ac2f3a6d33 tools build: Increment room for feature name in feature detection output
48ab65e0fec644b4 tools build: Fix feature detection output due to eval expansion
5b245985a6de5ac1 tools build: Switch to new openssl API for test-libcrypto
dd6775f986144a9e perf build: Remove FEATURE_CHECK_LDFLAGS-disassembler-{four-args,init-styled} setting
629b98e2b1c6efcf tools, build: Retry detection of bfd-related features
⬢[acme@toolbox perf]$

Quentin, may I pick the ones that touch bpftool?

- Arnaldo

Re: [PATCH 3/3] tools/build: Display logical OR of a feature flavors

[Roberto Sassu]

On Tue, 2022-09-27 at 13:21 +0100, Arnaldo Carvalho de Melo wrote:
> Em Tue, Sep 27, 2022 at 09:14:34AM +0200, Roberto Sassu escreveu:
> > On Mon, 2022-08-22 at 13:24 +0200, Roberto Sassu wrote:
> > > On Mon, 2022-08-22 at 12:58 +0200, Jiri Olsa wrote:
> > > > On Thu, Aug 18, 2022 at 02:09:57PM +0200, 
> > > 
> > > [...]
> > > 
> > > > > In verbose mode, of if no group is defined for a feature,
> > > > > show
> > > > > the
> > > > > feature
> > > > > detection result as before.
> > > 
> > > Thanks Jiri.
> > > 
> > > 'or' instead of 'of', if the patch can be edited.
> > 
> > Hi Arnaldo
> > 
> > will you take these patches?
> 
> The tools/build one I have in my perf/core branch, for v6.1.
> 
> ⬢[acme@toolbox perf]$ git log --oneline --author 
> roberto.sassu@huawei.com tools/{build,perf,lib}
> 924b0da1154fa814 tools build: Display logical OR of a feature flavors
> 1903f4ac2f3a6d33 tools build: Increment room for feature name in
> feature detection output
> 48ab65e0fec644b4 tools build: Fix feature detection output due to
> eval expansion
> 5b245985a6de5ac1 tools build: Switch to new openssl API for test-
> libcrypto
> dd6775f986144a9e perf build: Remove FEATURE_CHECK_LDFLAGS-
> disassembler-{four-args,init-styled} setting
> 629b98e2b1c6efcf tools, build: Retry detection of bfd-related
> features
> ⬢[acme@toolbox perf]$

Oh, thanks. I had a quick look today at the web interface. I didn't see
them.

Roberto

Re: [PATCH 3/3] tools/build: Display logical OR of a feature flavors

[Quentin Monnet]

Tue Sep 27 2022 13:21:58 GMT+0100 ~ Arnaldo Carvalho de Melo
<acme@kernel.org>
> Em Tue, Sep 27, 2022 at 09:14:34AM +0200, Roberto Sassu escreveu:
>> On Mon, 2022-08-22 at 13:24 +0200, Roberto Sassu wrote:
>>> On Mon, 2022-08-22 at 12:58 +0200, Jiri Olsa wrote:
>>>> On Thu, Aug 18, 2022 at 02:09:57PM +0200, 
>>>
>>> [...]
>>>
>>>>> In verbose mode, of if no group is defined for a feature, show
>>>>> the
>>>>> feature
>>>>> detection result as before.
>>>
>>> Thanks Jiri.
>>>
>>> 'or' instead of 'of', if the patch can be edited.
>>
>> Hi Arnaldo
>>
>> will you take these patches?
> 
> The tools/build one I have in my perf/core branch, for v6.1.
> 
> ⬢[acme@toolbox perf]$ git log --oneline --author roberto.sassu@huawei.com tools/{build,perf,lib}
> 924b0da1154fa814 tools build: Display logical OR of a feature flavors
> 1903f4ac2f3a6d33 tools build: Increment room for feature name in feature detection output
> 48ab65e0fec644b4 tools build: Fix feature detection output due to eval expansion
> 5b245985a6de5ac1 tools build: Switch to new openssl API for test-libcrypto
> dd6775f986144a9e perf build: Remove FEATURE_CHECK_LDFLAGS-disassembler-{four-args,init-styled} setting
> 629b98e2b1c6efcf tools, build: Retry detection of bfd-related features
> ⬢[acme@toolbox perf]$
> 
> Quentin, may I pick the ones that touch bpftool?

Would be fine by me, although I'm not the one merging bpftool patches to
bpf-next (Alexei, Andrii or Daniel do it).

But for the current patchset I think there's nothing touching bpftool
directly, you already have all three patches in your tree as far as I
can tell

Quentin