linux-tool-perf: permit TUI build

PATCH Superseded John Keeping 3 participants 5 messages Apr 2, 2018 – Apr 28, 2018

lore

Share & tools

lore

John Keeping

author

2018-04-02 12:51 UTC #0 lore raw

PATCH 1/1 linux-tool-perf: permit TUI build

Since Linux 3.10, perf's NO_NEWT configuration option simply sets NO_SLANG=1 and there is no dependency on libnewt.

We already handle NO_SLANG correctly based on whether or not BR2_PACKAGE_SLANG is selected, so all we accomplish by setting NO_NEWT=1 is disabling perf's TUI when all of the dependencies are available.

Linux 3.10 is almost 5 years old now, so there doesn't seem to be any point in jumping through additional hoops to make perf from older kernel versions build when the newt package is not available.

Signed-off-by John Keeping <[email protected]>

---
 package/linux-tools/linux-tool-perf.mk.in | 1 -
 1 file changed, 1 deletion(-)

package/linux-tools/linux-tool-perf.mk.in +0 -1

index 69492ba8da..bb9d1655e6 100644
@@ -22,7 +22,6 @@ PERF_MAKE_FLAGS = \
 	prefix=/usr \
 	WERROR=0 \
 	NO_LIBAUDIT=1 \
-	NO_NEWT=1 \
 	NO_GTK2=1 \
 	NO_LIBPERL=1 \
 	NO_LIBPYTHON=1 \

Signature · 1 line

2.16.3

Yann E. MORIN

2018-04-02 13:07 UTC #1 lore raw

PATCH 1/1 linux-tool-perf: permit TUI build

John, All,

On 2018-04-02 13:51 +0100, John Keeping spake thusly:

Quoted 10 lines

Since Linux 3.10, perf's NO_NEWT configuration option simply sets NO_SLANG=1 and there is no dependency on libnewt.
We already handle NO_SLANG correctly based on whether or not BR2_PACKAGE_SLANG is selected, so all we accomplish by setting NO_NEWT=1 is disabling perf's TUI when all of the dependencies are available.
Linux 3.10 is almost 5 years old now, so there doesn't seem to be any point in jumping through additional hoops to make perf from older kernel versions build when the newt package is not available.

But we still support kernels back to 3.0...

I would instead suggest you add an option in
package/linux-tools/Config.in:

config BR2_PACKAGE_LINUX_TOOLS_PERF_TUI

And do a check in package/linux-tools/linux-tool-perf.mk.in whether it is valid o not, if needed.

Regards,
Yann E. MORIN.

Quoted 20 lines

Signed-off-by John Keeping <[email protected]>

---
 package/linux-tools/linux-tool-perf.mk.in | 1 -
 1 file changed, 1 deletion(-)

package/linux-tools/linux-tool-perf.mk.in +0 -1

index 69492ba8da..bb9d1655e6 100644
@@ -22,7 +22,6 @@ PERF_MAKE_FLAGS = \
 	prefix=/usr \
 	WERROR=0 \
 	NO_LIBAUDIT=1 \
-	NO_NEWT=1 \
 	NO_GTK2=1 \
 	NO_LIBPERL=1 \
 	NO_LIBPYTHON=1 \

Signature · 1 line

2.16.3

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

John Keeping

author

2018-04-02 14:02 UTC #2 lore raw

PATCH v2 1/1 linux-tool-perf: permit TUI build

Since Linux 3.10, perf's NO_NEWT configuration option simply sets NO_SLANG=1 and there is no dependency on libnewt.

To simplify all of this, introduce a new config option to enable the perf TUI which depends on slang and add a check to prevent building the TUI on versions which are too old. The check for NO_SLANG is equivalent to checking if NO_NEWT is required as NO_SLANG was added in the same commit that removed the libnewt dependency and deprecated NO_NEWT (6692c262df4f, "perf tools: Remove dependency on libnewt", 2013-03-28).

Signed-off-by John Keeping <[email protected]>

---

Quoted 14 lines · Yann E. MORIN

On 2018-04-02 13:51 +0100, John Keeping spake thusly:
Quoted 10 lines
Since Linux 3.10, perf's NO_NEWT configuration option simply sets NO_SLANG=1 and there is no dependency on libnewt.
We already handle NO_SLANG correctly based on whether or not BR2_PACKAGE_SLANG is selected, so all we accomplish by setting NO_NEWT=1 is disabling perf's TUI when all of the dependencies are available.
Linux 3.10 is almost 5 years old now, so there doesn't seem to be any point in jumping through additional hoops to make perf from older kernel versions build when the newt package is not available.
But we still support kernels back to 3.0...

Quoted 7 lines

I would instead suggest you add an option in
package/linux-tools/Config.in:
config BR2_PACKAGE_LINUX_TOOLS_PERF_TUI
And do a check in package/linux-tools/linux-tool-perf.mk.in whether it is valid o not, if needed.

Something like this? I can't see a neater way of checking the version dependency and this matches the existing handling of the elfutils dependency.

 package/linux-tools/Config.in             | 12 ++++++++++++
 package/linux-tools/linux-tool-perf.mk.in | 12 +++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

package/linux-tools/Config.in +12 -0

index e3ccd850f9..1d31ce84f1 100644
@@ -54,6 +54,18 @@ config BR2_PACKAGE_LINUX_TOOLS_PERF
 
 	  https://perf.wiki.kernel.org/
 
+if BR2_PACKAGE_LINUX_TOOLS_PERF
+
+config BR2_PACKAGE_LINUX_TOOLS_PERF_TUI
+	bool "enable perf TUI"
+	select BR2_PACKAGE_SLANG
+	help
+	  Enable the TUI interface for perf which requires a TTY and
+	  enables zooming into DSOs and threads as well as other
+	  features.
+
+endif
+
 config BR2_PACKAGE_LINUX_TOOLS_SELFTESTS
 	bool"selftests"
 	depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS # bash

package/linux-tools/linux-tool-perf.mk.in +9 -3

index 69492ba8da..80e00c3c56 100644
@@ -22,7 +22,6 @@ PERF_MAKE_FLAGS = \
 	prefix=/usr \
 	WERROR=0 \
 	NO_LIBAUDIT=1 \
-	NO_NEWT=1 \
 	NO_GTK2=1 \
 	NO_LIBPERL=1 \
 	NO_LIBPYTHON=1 \

@@ -59,10 +58,10 @@ ifeq ($(BR2_arc),y)
 PERF_MAKE_FLAGS += NO_BACKTRACE=1
 endif
 
-ifeq ($(BR2_PACKAGE_SLANG),y)
+ifeq ($(BR2_PACKAGE_LINUX_TOOLS_PERF_TUI),y)
 PERF_DEPENDENCIES += slang
 else
-PERF_MAKE_FLAGS += NO_SLANG=1
+PERF_MAKE_FLAGS += NO_NEWT=1 NO_SLANG=1
 endif
 
 ifeq ($(BR2_PACKAGE_LIBUNWIND),y)

@@ -128,6 +127,13 @@ define PERF_BUILD_CMDS
 			fi \
 		fi \
 	fi
+	$(Q)if test "$(BR2_PACKAGE_LINUX_TOOLS_PERF_TUI)" = "y" ; then \
+		if ! grep -q NO_SLANG $(LINUX_DIR)/tools/perf/Makefile* ; then \
+			echo "The perf tool in your kernel cannot be build with the TUI." ; \
+			echo "Either upgrade your kernel to >= 3.10, or disable the TUI." ; \
+			exit 1 ; \
+		fi \
+	fi
 	$(TARGET_MAKE_ENV) $(MAKE1) $(PERF_MAKE_FLAGS) \
 		-C $(LINUX_DIR)/tools/perf O=$(LINUX_DIR)/tools/perf/
 endef

Signature · 1 line

2.16.3

Yann E. MORIN

2018-04-02 14:18 UTC #3 lore raw

PATCH v2 1/1 linux-tool-perf: permit TUI build

John, All,

On 2018-04-02 15:02 +0100, John Keeping spake thusly:

Quoted 35 lines

Since Linux 3.10, perf's NO_NEWT configuration option simply sets NO_SLANG=1 and there is no dependency on libnewt.
We already handle NO_SLANG correctly based on whether or not BR2_PACKAGE_SLANG is selected, so all we accomplish by setting NO_NEWT=1 is disabling perf's TUI when all of the dependencies are available.
To simplify all of this, introduce a new config option to enable the perf TUI which depends on slang and add a check to prevent building the TUI on versions which are too old. The check for NO_SLANG is equivalent to checking if NO_NEWT is required as NO_SLANG was added in the same commit that removed the libnewt dependency and deprecated NO_NEWT (6692c262df4f, "perf tools: Remove dependency on libnewt", 2013-03-28).
Signed-off-by John Keeping <[email protected]>
---
Quoted 17 lines · Yann E. MORIN
On 2018-04-02 13:51 +0100, John Keeping spake thusly:
Quoted 10 lines
Since Linux 3.10, perf's NO_NEWT configuration option simply sets NO_SLANG=1 and there is no dependency on libnewt.
We already handle NO_SLANG correctly based on whether or not BR2_PACKAGE_SLANG is selected, so all we accomplish by setting NO_NEWT=1 is disabling perf's TUI when all of the dependencies are available.
Linux 3.10 is almost 5 years old now, so there doesn't seem to be any point in jumping through additional hoops to make perf from older kernel versions build when the newt package is not available.
But we still support kernels back to 3.0...
I would instead suggest you add an option in
package/linux-tools/Config.in:

Quoted 8 lines

Quoted 4 lines
config BR2_PACKAGE_LINUX_TOOLS_PERF_TUI
And do a check in package/linux-tools/linux-tool-perf.mk.in whether it is valid o not, if needed.
Something like this? I can't see a neater way of checking the version dependency and this matches the existing handling of the elfutils dependency.

Yes, that is exactly what I had in mind. :-)

Quoted 9 lines

 package/linux-tools/Config.in             | 12 ++++++++++++
 package/linux-tools/linux-tool-perf.mk.in | 12 +++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

package/linux-tools/Config.in +0 -0

index e3ccd850f9..1d31ce84f1 100644

package/linux-tools/Config.in config BR2_PACKAGE_LINUX_TOOLS_PERF · line 61

Quoted 8 lines

package/linux-tools/Config.in +5 -0

@@ -54,6 +54,18 @@ config BR2_PACKAGE_LINUX_TOOLS_PERF
 
 	  https://perf.wiki.kernel.org/
 
+if BR2_PACKAGE_LINUX_TOOLS_PERF
+
+config BR2_PACKAGE_LINUX_TOOLS_PERF_TUI
+	bool "enable perf TUI"
+	select BR2_PACKAGE_SLANG

You forgot to propagate the dependency from slang:

depends on BR2_USE_MMU # slang

Otherwise, looks fine.

Regards,
Yann E. MORIN.

Quoted 52 lines

package/linux-tools/Config.in +7 -0

+	help
+	  Enable the TUI interface for perf which requires a TTY and
+	  enables zooming into DSOs and threads as well as other
+	  features.
+
+endif
+
 config BR2_PACKAGE_LINUX_TOOLS_SELFTESTS
 	bool"selftests"
 	depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS # bash

package/linux-tools/linux-tool-perf.mk.in +9 -3

index 69492ba8da..80e00c3c56 100644
@@ -22,7 +22,6 @@ PERF_MAKE_FLAGS = \
 	prefix=/usr \
 	WERROR=0 \
 	NO_LIBAUDIT=1 \
-	NO_NEWT=1 \
 	NO_GTK2=1 \
 	NO_LIBPERL=1 \
 	NO_LIBPYTHON=1 \

@@ -59,10 +58,10 @@ ifeq ($(BR2_arc),y)
 PERF_MAKE_FLAGS += NO_BACKTRACE=1
 endif
 
-ifeq ($(BR2_PACKAGE_SLANG),y)
+ifeq ($(BR2_PACKAGE_LINUX_TOOLS_PERF_TUI),y)
 PERF_DEPENDENCIES += slang
 else
-PERF_MAKE_FLAGS += NO_SLANG=1
+PERF_MAKE_FLAGS += NO_NEWT=1 NO_SLANG=1
 endif
 
 ifeq ($(BR2_PACKAGE_LIBUNWIND),y)

@@ -128,6 +127,13 @@ define PERF_BUILD_CMDS
 			fi \
 		fi \
 	fi
+	$(Q)if test "$(BR2_PACKAGE_LINUX_TOOLS_PERF_TUI)" = "y" ; then \
+		if ! grep -q NO_SLANG $(LINUX_DIR)/tools/perf/Makefile* ; then \
+			echo "The perf tool in your kernel cannot be build with the TUI." ; \
+			echo "Either upgrade your kernel to >= 3.10, or disable the TUI." ; \
+			exit 1 ; \
+		fi \
+	fi
 	$(TARGET_MAKE_ENV) $(MAKE1) $(PERF_MAKE_FLAGS) \
 		-C $(LINUX_DIR)/tools/perf O=$(LINUX_DIR)/tools/perf/
 endef

Signature · 1 line

2.16.3

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Thomas Petazzoni

Applied

2018-04-28 13:20 UTC #4 lore raw

PATCH v2 1/1 linux-tool-perf: permit TUI build

In reply to John Keeping

Hello John,

Quoted 14 lines · John Keeping

Since Linux 3.10, perf's NO_NEWT configuration option simply sets NO_SLANG=1 and there is no dependency on libnewt.
We already handle NO_SLANG correctly based on whether or not BR2_PACKAGE_SLANG is selected, so all we accomplish by setting NO_NEWT=1 is disabling perf's TUI when all of the dependencies are available.
To simplify all of this, introduce a new config option to enable the perf TUI which depends on slang and add a check to prevent building the TUI on versions which are too old. The check for NO_SLANG is equivalent to checking if NO_NEWT is required as NO_SLANG was added in the same commit that removed the libnewt dependency and deprecated NO_NEWT (6692c262df4f, "perf tools: Remove dependency on libnewt", 2013-03-28).

Quoted 3 lines

Signed-off-by John Keeping <[email protected]>
---
On Mon, 2 Apr 2018 15:07:16 +0200, Yann E. MORIN wrote:

Applied to master after adding the BR2_USE_MMU, as pointed out by Yann, and after tweaking a bit the commit title. Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com