linux/tools: move to their own package (branch yem/linux-tools)

PATCH New Yann E. MORIN 3 participants 11 messages Sep 6, 2016 – Sep 22, 2016

lore

Share & tools

lore

Yann E. MORIN

author

2016-09-06 14:29 UTC #0 lore raw

v3 cover linux/tools: move to their own package (branch yem/linux-tools)

Hello All!

This series moves the Linux tools (cpupower, perf and selftests) our of the linux package and into their own package.

As extensively explained in the commit log from the first patch, some configurations are unbuildable, because they cause circular dependencies in our Makefiles.

We fix that by moving the Linux tools to their own package.

Changes v2 -> v3:
- don't symlink the Linux sources; just build in there
- typoes

Changes v1 -> v2:
- properly handle legacy

Regards,
Yann E. MORIN.

The following changes since commit e03cb2eb2edf73b79156e8f54c80d04dc0846e89

libgtk3: fix typo (2016-09-05 23:36:04 +0200)

are available in the git repository at:

git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to bdadcf3de3e5617f0492d88329476963a06fad44

docs/manual: update the linux tools section (2016-09-06 15:22:54 +0200)

----------------------------------------------------------------
Yann E. MORIN (2):
linux/tools: make it a real, separate package docs/manual: update the linux tools section

 Config.in.legacy                                   | 24 +++++++++++
 .../adding-packages-linux-kernel-spec-infra.txt    | 40 +++++++++---------
 linux/Config.in                                    |  2 +-
 linux/linux.mk                                     | 24 +----------
 .../linux-tools/Config.in                          | 14 +++++--
 .../linux-tools}/linux-tool-cpupower.mk            |  8 ++--
 {linux => package/linux-tools}/linux-tool-perf.mk  | 18 ++++----
 .../linux-tools}/linux-tool-selftests.mk           | 14 +++----
 package/linux-tools/linux-tools.mk                 | 48 ++++++++++++++++++++++
 9 files changed, 126 insertions(+), 66 deletions(-)
 rename linux/Config.tools.in => package/linux-tools/Config.in (83%)
 rename {linux => package/linux-tools}/linux-tool-cpupower.mk (79%)
 rename {linux => package/linux-tools}/linux-tool-perf.mk (84%)
 rename {linux => package/linux-tools}/linux-tool-selftests.mk (65%)
 create mode 100644 package/linux-tools/linux-tools.mk

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.  |
'------------------------------^-------^------------------^--------------------'

Yann E. MORIN

author

2016-09-06 14:29 UTC #1 lore raw

PATCH v3 1/2 linux/tools: make it a real, separate package

The kernel source tree also contains the sources for various userland tools, of which cpupower, perf or selftests.

Currently, we have support for building those tools as part of the kernel build procedure. This looked the correct thing to do so far, because, well, they *are* part of the kernel source tree and some really have to be the same version as the kernel that will run.

However, this is causing quite a non-trivial-to-break circular dependency in some configurations. For example, this defconfig fails to build (similar to the one reported by Paul):

    BR2_arm=y
    BR2_cortex_a7=y
    BR2_ARM_FPU_NEON_VFPV4=y
    BR2_TOOLCHAIN_EXTERNAL=y
    BR2_INIT_SYSTEMD=y
    BR2_LINUX_KERNEL=y
    BR2_LINUX_KERNEL_CUSTOM_GIT=y
    BR2_LINUX_KERNEL_CUSTOM_REPO_URL="https://github.com/raspberrypi/linux.git"
    BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION="26f3b72a9c049be10e6af196252283e1f6ab9d1f"
    BR2_LINUX_KERNEL_DEFCONFIG="bcm2709"
    BR2_PACKAGE_LINUX_TOOLS_CPUPOWER=y
    BR2_PACKAGE_CRYPTODEV=y
    BR2_PACKAGE_OPENSSL=y
    BR2_PACKAGE_LIBCURL=y

This causes a circular dependency, as explained by Thomas:

- When libcurl is enabled, systemd depends on it

- When OpenSSL is enabled, obviously, will use it for SSL support

- When cryptodev-linux is enabled, OpenSSL will depend on it to use crypto accelerators supported in the kernel via cryptodev-linux.

- cryptodev-linux being a kernel module, it depends on linux

- linux by itself (the kernel) does not depend on pciutils, but the linux tool "cpupower" (managed in linux-tool-cpupower) depends on pciutils

- pciutils depends on udev when available

- udev is provided by systemd.

And indeed, during the build, we can see that make warns (it's only reported as a *warning*, not as an actual error):

    [...]
    make[1]: Circular /home/ymorin/dev/buildroot/O/build/openssl-1.0.2h/.stamp_configured
    <- cryptodev-linux dependency dropped.
    >>> openssl 1.0.2h Downloading
    [...]

So the build fails later on, when openssl is actually built:

eng_cryptodev.c:57:31: fatal error: crypto/cryptodev.h: No such file or directory compilation terminated.
<builtin>: recipe for target 'eng_cryptodev.o' failed

Furthermore, graph-depends also detects the circular dependency, but treats it as a hard-error:

Recursion detected for : cryptodev-linux which is a dependency of: openssl
which is a dependency of: libcurl
which is a dependency of: systemd
which is a dependency of: udev
which is a dependency of: pciutils
which is a dependency of: linux
which is a dependency of: cryptodev-linux Makefile:738: recipe for target 'graph-depends' failed

Of course, there is no way to break the loop without losing functionality in either one of the involved packages *and* keep our infrastructure and packages as-is.

The only solution is to break the loop at the linux-tools level, by moving them away into their own package, so that the linux package will no longer have the opportunity to depend on another package via a dependency of one the tools.

All three linux tools are thus moved away to their own package.

The package infrastructure only knows of three types of packages: those in package/ , in boot/ , in toolchain/ and the one in linux/ . So we create that new linux-tools package in package/ so that we don't have to fiddle with yet another special case in the infra. Still, we want its configure options to appear in the kernel's sub-menu.

So, we make it a prompt-less package, with only the tools visible as options of that package, but without the usual dependency on their master symbol; they only depend on the Linux kernel.

Furthermore, because the kernel is such a huge pile of code, we would not be very happy to extract it a second time just for the sake of a few tools. We can't extract only the tools/ sub-directory from the kernel source either, because some tools have hard-coded path to includes from the kernel (arch and stuff).

Instead, we just use the linux source tree as our own build tree, and ensure the linux tree is extracted and patched before linux-tools is configured and built.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Paul Ashford <[email protected]>

---
Chamges v2 -> v3:
- don;t use a symlink; directly build in the linux source tree
- typoes

Changes v1 -> v2:
- properly handle legacy

---
Note that the tools were ported over as-is to their new package:
post-build, post-install-target and post-install-staging hooks were used previously, and we still use that with no actual _BUILD_CMDS. We could very well have defined build and install commands for each tool and shoehorned that into the _BUILD_CMDS, but just porting the hooks over was the easiest way forward. A later patch can provide more cleanup if needed.

---
 Config.in.legacy                                   | 24 +++++++++++
 linux/Config.in                                    |  2 +-
 linux/linux.mk                                     | 24 +----------
 .../linux-tools/Config.in                          | 14 +++++--
 .../linux-tools}/linux-tool-cpupower.mk            |  8 ++--
 {linux => package/linux-tools}/linux-tool-perf.mk  | 18 ++++----
 .../linux-tools}/linux-tool-selftests.mk           | 14 +++----
 package/linux-tools/linux-tools.mk                 | 48 ++++++++++++++++++++++
 8 files changed, 105 insertions(+), 47 deletions(-)
 rename linux/Config.tools.in => package/linux-tools/Config.in (83%)
 rename {linux => package/linux-tools}/linux-tool-cpupower.mk (79%)
 rename {linux => package/linux-tools}/linux-tool-perf.mk (84%)
 rename {linux => package/linux-tools}/linux-tool-selftests.mk (65%)
 create mode 100644 package/linux-tools/linux-tools.mk

Config.in.legacy +24 -0

index 59e3f84..0190af2 100644
@@ -145,6 +145,30 @@ endif
 ###############################################################################
 comment "Legacy options removed in 2016.11"
 
+config BR2_LINUX_KERNEL_TOOL_CPUPOWER
+	bool "linux-tool cpupower"
+	depends on BR2_LINUX_KERNEL
+	select BR2_LEGACY
+	select BR2_PACKAGE_LINUX_TOOLS_CPUPOWER
+	help
+	  Linux tool cpupower option was renamed.
+
+config BR2_LINUX_KERNEL_TOOL_PERF
+	bool "linux-tool perf"
+	depends on BR2_LINUX_KERNEL
+	select BR2_LEGACY
+	select BR2_PACKAGE_LINUX_TOOLS_PERF
+	help
+	  Linux tool perf option was renamed.
+
+config BR2_LINUX_KERNEL_TOOL_SELFTESTS
+	bool "linux-tool selftests"
+	depends on BR2_LINUX_KERNEL
+	select BR2_LEGACY
+	select BR2_PACKAGE_LINUX_TOOLS_SELFTESTS
+	help
+	  Linux tool selftests option was renamed.
+
 config BR2_LINUX_KERNEL_CUSTOM_LOCAL
 	bool "Linux kernel local directory option removed"
 	help

linux/Config.in +1 -1

index 43d63c0..8e1d921 100644
@@ -400,7 +400,7 @@ config BR2_LINUX_KERNEL_INSTALL_TARGET
 source "linux/Config.ext.in"
 
 # Linux tools
-source "linux/Config.tools.in"
+source "package/linux-tools/Config.in"
 
 endif # BR2_LINUX_KERNEL

linux/linux.mk +1 ~~-23~~

index 6e41a92..aa1632b 100644
@@ -396,7 +396,7 @@ define LINUX_INSTALL_TARGET_CMDS
 	$(LINUX_INSTALL_HOST_TOOLS)
 endef
 
-# Include all our extensions and tools definitions.
+# Include all our extensions.
 #
 # Note: our package infrastructure uses the full-path of the last-scanned
 # Makefile to determine what package we're currently defining, using the

@@ -407,7 +407,6 @@ endef
 # the current Makefile, we are OK. But this is a hard requirement: files
 # included here *must* be in the same directory!
 include $(sort $(wildcard linux/linux-ext-*.mk))
-include $(sort $(wildcard linux/linux-tool-*.mk))
 
 LINUX_PATCH_DEPENDENCIES += $(foreach ext,$(LINUX_EXTENSIONS),\
 	$(if $(BR2_LINUX_KERNEL_EXT_$(call UPPERCASE,$(ext))),$(ext)))

@@ -416,27 +415,6 @@ LINUX_PRE_PATCH_HOOKS += $(foreach ext,$(LINUX_EXTENSIONS),\
 	$(if $(BR2_LINUX_KERNEL_EXT_$(call UPPERCASE,$(ext))),\
 		$(call UPPERCASE,$(ext))_PREPARE_KERNEL))
 
-# Install Linux kernel tools in the staging directory since some tools
-# may install shared libraries and headers (e.g. cpupower). The kernel
-# image is NOT installed in the staging directory.
-LINUX_INSTALL_STAGING = YES
-
-LINUX_DEPENDENCIES += $(foreach tool,$(LINUX_TOOLS),\
-	$(if $(BR2_LINUX_KERNEL_TOOL_$(call UPPERCASE,$(tool))),\
-		$($(call UPPERCASE,$(tool))_DEPENDENCIES)))
-
-LINUX_POST_BUILD_HOOKS += $(foreach tool,$(LINUX_TOOLS),\
-	$(if $(BR2_LINUX_KERNEL_TOOL_$(call UPPERCASE,$(tool))),\
-		$(call UPPERCASE,$(tool))_BUILD_CMDS))
-
-LINUX_POST_INSTALL_STAGING_HOOKS += $(foreach tool,$(LINUX_TOOLS),\
-	$(if $(BR2_LINUX_KERNEL_TOOL_$(call UPPERCASE,$(tool))),\
-		$(call UPPERCASE,$(tool))_INSTALL_STAGING_CMDS))
-
-LINUX_POST_INSTALL_TARGET_HOOKS += $(foreach tool,$(LINUX_TOOLS),\
-	$(if $(BR2_LINUX_KERNEL_TOOL_$(call UPPERCASE,$(tool))),\
-		$(call UPPERCASE,$(tool))_INSTALL_TARGET_CMDS))
-
 # Checks to give errors that the user can understand
 ifeq ($(BR_BUILDING),y)

package/linux-tools/Config.in renamed from linux/Config.tools.in +11 -3

similarity index 83%
rename from linux/Config.tools.in
rename to package/linux-tools/Config.in
index 5ada98d..61c196f 100644
@@ -1,9 +1,15 @@
 menu "Linux Kernel Tools"
 
-config BR2_LINUX_KERNEL_TOOL_CPUPOWER
+# No prompt, this is sourced by linux/Config.in as this
+# is no real package and really belongs to the kernel.
+config BR2_PACKAGE_LINUX_TOOLS
+	bool
+
+config BR2_PACKAGE_LINUX_TOOLS_CPUPOWER
 	bool "cpupower"
 	depends on !BR2_bfin # pciutils
 	depends on BR2_USE_WCHAR || !BR2_NEEDS_GETTEXT # gettext
+	select BR2_PACKAGE_LINUX_TOOLS
 	select BR2_PACKAGE_PCIUTILS
 	select BR2_PACKAGE_GETTEXT if BR2_NEEDS_GETTEXT
 	help

@@ -14,8 +20,9 @@ comment "cpupower needs a toolchain w/ wchar"
 	depends on !BR2_bfin
 	depends on !BR2_USE_WCHAR && BR2_NEEDS_GETTEXT
 
-config BR2_LINUX_KERNEL_TOOL_PERF
+config BR2_PACKAGE_LINUX_TOOLS_PERF
 	bool "perf"
+	select BR2_PACKAGE_LINUX_TOOLS
 	help
 	  perf (sometimes "Perf Events" or perf tools, originally
 	  "Performance Counters for Linux") - is a performance

@@ -32,10 +39,11 @@ config BR2_LINUX_KERNEL_TOOL_PERF
 
 	  https://perf.wiki.kernel.org/
 
-config BR2_LINUX_KERNEL_TOOL_SELFTESTS
+config BR2_PACKAGE_LINUX_TOOLS_SELFTESTS
 	bool"selftests"
 	depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS # bash
 	depends on BR2_USE_MMU  # bash
+	select BR2_PACKAGE_LINUX_TOOLS
 	select BR2_PACKAGE_BASH # runtime
 	select BR2_PACKAGE_POPT
 	select BR2_PACKAGE_LIBCAP_NG

package/linux-tools/linux-tool-cpupower.mk renamed from linux/linux-tool-cpupower.mk +4 -4

similarity index 79%
rename from linux/linux-tool-cpupower.mk
rename to package/linux-tools/linux-tool-cpupower.mk
index 095a5ef..9958cff 100644
@@ -15,26 +15,26 @@ CPUPOWER_MAKE_OPTS = CROSS=$(TARGET_CROSS) \
 	DEBUG=false
 
 define CPUPOWER_BUILD_CMDS
-	$(Q)if test ! -f $(@D)/tools/power/cpupower/Makefile ; then \
+	$(Q)if test ! -f $(LINUX_DIR)/tools/power/cpupower/Makefile ; then \
 		echo "Your kernel version is too old and does not have the cpupower tool." ; \
 		echo "At least kernel 3.4 must be used." ; \
 		exit 1 ; \
 	fi
 
-	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/tools \
+	$(TARGET_MAKE_ENV) $(MAKE) -C $(LINUX_DIR)/tools \
 		$(CPUPOWER_MAKE_OPTS) \
 		cpupower
 endef
 
 define CPUPOWER_INSTALL_STAGING_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/tools \
+	$(TARGET_MAKE_ENV) $(MAKE) -C $(LINUX_DIR)/tools \
 		$(CPUPOWER_MAKE_OPTS) \
 		DESTDIR=$(STAGING_DIR) \
 		cpupower_install
 endef
 
 define CPUPOWER_INSTALL_TARGET_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/tools \
+	$(TARGET_MAKE_ENV) $(MAKE) -C $(LINUX_DIR)/tools \
 		$(CPUPOWER_MAKE_OPTS) \
 		DESTDIR=$(TARGET_DIR) \
 		cpupower_install

package/linux-tools/linux-tool-perf.mk renamed from linux/linux-tool-perf.mk +9 -9

similarity index 84%
rename from linux/linux-tool-perf.mk
rename to package/linux-tools/linux-tool-perf.mk
index 8143474..16f3a58 100644
@@ -95,25 +95,25 @@ endif
 # We name it 'GNUmakefile' so that GNU make will use it instead of
 # the existing 'Makefile'.
 define PERF_DISABLE_DOCUMENTATION
-	if [ -f $(@D)/tools/perf/Documentation/Makefile ]; then \
-		printf "%%:\n\t@:\n" >$(@D)/tools/perf/Documentation/GNUmakefile; \
+	if [ -f $(LINUX_DIR)/tools/perf/Documentation/Makefile ]; then \
+		printf "%%:\n\t@:\n" >$(LINUX_DIR)/tools/perf/Documentation/GNUmakefile; \
 	fi
 endef
 LINUX_POST_PATCH_HOOKS += PERF_DISABLE_DOCUMENTATION
 
 # O must be redefined here to overwrite the one used by Buildroot for
-# out of tree build. We build perf in $(@D)/tools/perf/ and not just
-# $(@D) so that it isn't built in the root directory of the kernel
+# out of tree build. We build perf in $(LINUX_DIR)/tools/perf/ and not just
+# $(LINUX_DIR) so that it isn't built in the root directory of the kernel
 # sources.
 define PERF_BUILD_CMDS
-	$(Q)if test ! -f $(@D)/tools/perf/Makefile ; then \
+	$(Q)if test ! -f $(LINUX_DIR)/tools/perf/Makefile ; then \
 		echo "Your kernel version is too old and does not have the perf tool." ; \
 		echo "At least kernel 2.6.31 must be used." ; \
 		exit 1 ; \
 	fi
 	$(Q)if test "$(BR2_PACKAGE_ELFUTILS)" = "" ; then \
-		if ! grep -q NO_LIBELF $(@D)/tools/perf/Makefile* ; then \
-			if ! test -r $(@D)/tools/perf/config/Makefile ; then \
+		if ! grep -q NO_LIBELF $(LINUX_DIR)/tools/perf/Makefile* ; then \
+			if ! test -r $(LINUX_DIR)/tools/perf/config/Makefile ; then \
 				echo "The perf tool in your kernel cannot be built without libelf." ; \
 				echo "Either upgrade your kernel to >= 3.7, or enable the elfutils package." ; \
 				exit 1 ; \

@@ -121,14 +121,14 @@ define PERF_BUILD_CMDS
 		fi \
 	fi
 	$(TARGET_MAKE_ENV) $(MAKE1) $(PERF_MAKE_FLAGS) \
-		-C $(@D)/tools/perf O=$(@D)/tools/perf/
+		-C $(LINUX_DIR)/tools/perf O=$(LINUX_DIR)/tools/perf/
 endef
 
 # After installation, we remove the Perl and Python scripts from the
 # target.
 define PERF_INSTALL_TARGET_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE1) $(PERF_MAKE_FLAGS) \
-		-C $(@D)/tools/perf O=$(@D)/tools/perf/ install
+		-C $(LINUX_DIR)/tools/perf O=$(LINUX_DIR)/tools/perf/ install
 	$(RM) -rf $(TARGET_DIR)/usr/libexec/perf-core/scripts/
 	$(RM) -rf $(TARGET_DIR)/usr/libexec/perf-core/tests/
 endef

package/linux-tools/linux-tool-selftests.mk renamed from linux/linux-tool-selftests.mk +7 -7

similarity index 65%
rename from linux/linux-tool-selftests.mk
rename to package/linux-tools/linux-tool-selftests.mk
index 3cbfed2..c4e5bf0 100644
@@ -23,8 +23,8 @@ SELFTESTS_MAKE_FLAGS = \
 	ARCH=$(SELFTESTS_ARCH)
 
 # O must be redefined here to overwrite the one used by Buildroot for
-# out of tree build. We build the selftests in $(@D)/tools/selftests and
-# not just $(@D) so that it isn't built in the root directory of the kernel
+# out of tree build. We build the selftests in $(LINUX_DIR)/tools/selftests and
+# not just $(LINUX_DIR) so that it isn't built in the root directory of the kernel
 # sources.
 #
 # The headers_install step here is important as some kernel selftests use a

@@ -33,14 +33,14 @@ SELFTESTS_MAKE_FLAGS = \
 # The headers_install target will install the kernel headers locally inside
 # the Linux build dir
 define SELFTESTS_BUILD_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE1) -C $(@D) $(SELFTESTS_MAKE_FLAGS) \
+	$(TARGET_MAKE_ENV) $(MAKE1) -C $(LINUX_DIR) $(SELFTESTS_MAKE_FLAGS) \
 		headers_install
-	$(TARGET_MAKE_ENV) $(MAKE1) -C $(@D)/tools/testing/selftests \
-		$(SELFTESTS_MAKE_FLAGS) O=$(@D)/tools/testing/selftests
+	$(TARGET_MAKE_ENV) $(MAKE1) -C $(LINUX_DIR)/tools/testing/selftests \
+		$(SELFTESTS_MAKE_FLAGS) O=$(LINUX_DIR)/tools/testing/selftests
 endef
 
 define SELFTESTS_INSTALL_TARGET_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE1) -C $(@D)/tools/testing/selftests \
-		$(SELFTESTS_MAKE_FLAGS) O=$(@D)/tools/testing/selftests \
+	$(TARGET_MAKE_ENV) $(MAKE1) -C $(LINUX_DIR)/tools/testing/selftests \
+		$(SELFTESTS_MAKE_FLAGS) O=$(LINUX_DIR)/tools/testing/selftests \
 		INSTALL_PATH=$(TARGET_DIR)/usr/lib/kselftests install
 endef

package/linux-tools/linux-tools.mk new file +48 -0

48 lines not on this page · raw message

Signature · 1 line

2.7.4

Thomas Petazzoni

2016-09-22 10:51 UTC #2 lore raw

PATCH v3 1/2 linux/tools: make it a real, separate package

Hello,

I've applied, after doing a few changes, see below.

package/linux-tools/linux-tools.mk line 11

Quoted 13 lines · Yann E. MORIN

The kernel source tree also contains the sources for various userland tools, of which cpupower, perf or selftests.

package/linux-tools/linux-tools.mk new file +11 -0

+################################################################################
+#
+# linux-tools
+#
+################################################################################
+
+# Vampirising sources from the kernel tree, so no source nor site specified.
+# Instead, we directly build in the sources of the linux package. We can do
+# that, because we're not building in the same location and the same files.
+#
+# So, all tools refer to $(LINUX_DIR) instead of #(@D).

Typo: $(@D) instead of #(@D)

package/linux-tools/linux-tools.mk line 30

Quoted 11 lines

package/linux-tools/linux-tools.mk new file +11 -0

+# Include all our tools definitions.
+#
+# Note: our package infrastructure uses the full-path of the last-scanned
+# Makefile to determine what package we're currently defining, using the
+# last directory component in the path. As such, including other Makefile,
+# like below, before we call one of the *-package macro is usally not
+# working.
+# However, since the files we include here are in the same directory as
+# the current Makefile, we are OK. But this is a hard requirement: files
+# included here *must* be in the same directory!
+include $(sort $(wildcard linux/linux-tools/linux-ext-*.mk))

This include path is wrong, so I've changed it to:

include $(sort $(wildcard package/linux-tools/linux-tool-*.mk))

and in fact, I fixed it in the original commit, and then realized I messed up, so I had to fix it again in a follow-up commit.

Thanks for doing this work. Glad to see this issues fixed!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Yann E. MORIN

author

2016-09-22 16:33 UTC #3 lore raw

PATCH v3 1/2 linux/tools: make it a real, separate package

Thomas, All,

On 2016-09-22 12:51 +0200, Thomas Petazzoni spake thusly:

Quoted 15 lines · Yann E. MORIN

package/linux-tools/linux-tools.mk line 30

Quoted 11 lines

package/linux-tools/linux-tools.mk new file +11 -0

+# Include all our tools definitions.
+#
+# Note: our package infrastructure uses the full-path of the last-scanned
+# Makefile to determine what package we're currently defining, using the
+# last directory component in the path. As such, including other Makefile,
+# like below, before we call one of the *-package macro is usally not
+# working.
+# However, since the files we include here are in the same directory as
+# the current Makefile, we are OK. But this is a hard requirement: files
+# included here *must* be in the same directory!
+include $(sort $(wildcard linux/linux-tools/linux-ext-*.mk))

This include path is wrong, so I've changed it to:

Quoted 4 lines

include $(sort $(wildcard package/linux-tools/linux-tool-*.mk))
and in fact, I fixed it in the original commit, and then realized I messed up, so I had to fix it again in a follow-up commit.

And I've just sent a patch to completely remove that include directive
altogether: tools would register twice, wihch is not nice (built twice, installed twice...)

I did not catch this during my tests, because that problem was hidden:
- the $(wildcard) would return nothing,
- $(sort) would happily have nothing to sort and would return nothing,
- include would be happy to have nothing to include,
- but each individual .mk files would already be included from top-level Makefile, in the correct order,
- so I did not see the path was wrong and did not see the tools were registered twice.

Sorry for the mess... :-/

Regards,
Yann E. MORIN.

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.  |
'------------------------------^-------^------------------^--------------------'

Peter Korsgaard

2016-09-22 14:58 UTC #4 lore raw

PATCH v3 1/2 linux/tools: make it a real, separate package

In reply to Yann E. MORIN

Quoted 40 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Yann" == Yann E MORIN <[email protected]> writes:
The kernel source tree also contains the sources for various userland tools, of which cpupower, perf or selftests.
Currently, we have support for building those tools as part of the kernel build procedure. This looked the correct thing to do so far, because, well, they *are* part of the kernel source tree and some really have to be the same version as the kernel that will run.
However, this is causing quite a non-trivial-to-break circular dependency in some configurations. For example, this defconfig fails to build (similar to the one reported by Paul):
    BR2_arm=y
    BR2_cortex_a7=y
    BR2_ARM_FPU_NEON_VFPV4=y
    BR2_TOOLCHAIN_EXTERNAL=y
    BR2_INIT_SYSTEMD=y
    BR2_LINUX_KERNEL=y
    BR2_LINUX_KERNEL_CUSTOM_GIT=y
    BR2_LINUX_KERNEL_CUSTOM_REPO_URL="https://github.com/raspberrypi/linux.git"
    BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION="26f3b72a9c049be10e6af196252283e1f6ab9d1f"
    BR2_LINUX_KERNEL_DEFCONFIG="bcm2709"
    BR2_PACKAGE_LINUX_TOOLS_CPUPOWER=y
    BR2_PACKAGE_CRYPTODEV=y
    BR2_PACKAGE_OPENSSL=y
    BR2_PACKAGE_LIBCURL=y
This causes a circular dependency, as explained by Thomas:
- When libcurl is enabled, systemd depends on it
- When OpenSSL is enabled, obviously, will use it for SSL support
- When cryptodev-linux is enabled, OpenSSL will depend on it to use crypto accelerators supported in the kernel via cryptodev-linux.
- cryptodev-linux being a kernel module, it depends on linux

Quoted 7 lines

- linux by itself (the kernel) does not depend on pciutils, but the linux tool "cpupower" (managed in linux-tool-cpupower) depends on pciutils
- pciutils depends on udev when available
- udev is provided by systemd.

In this case there isn't actually a real circular dependency, as the only dependency between openssl and cryptodev-linux is the cryptodev.h header, right?

So another solution could have been to split the cryptodev-linux package in two, like we do for mesa3d and mesa3d-headers. The cryptodev-linux package would build the kernel module (and depend on linux), whereas the cryptodev-linux-headers package would only install cryptodev.h (and not have a compile time dependency on linux).

But ok, there could be other such dependency circles.

Signature · 1 line

Bye, Peter Korsgaard

Thomas Petazzoni

2016-09-22 17:49 UTC #5 lore raw

PATCH v3 1/2 linux/tools: make it a real, separate package

Hello,

Quoted 6 lines · Peter Korsgaard

In this case there isn't actually a real circular dependency, as the only dependency between openssl and cryptodev-linux is the cryptodev.h header, right?
So another solution could have been to split the cryptodev-linux package in two, like we do for mesa3d and mesa3d-headers.

Yes, possibly this could have been done. However, mesa3d/mesa3d-headers is not about a split really. mesa3d-headers is only here to provide the OpenGL headers when they are not provided by the selected OpenGL implementation. So you never use mesa3d+mesa3d-headers, for example.

Quoted 3 lines

The cryptodev-linux package would build the kernel module (and depend on linux), whereas the cryptodev-linux-headers package would only install cryptodev.h (and not have a compile time dependency on linux).

Correct. But there was really no reason for those "Linux tools" to be built as part of the linux package, so I think Yann's approach is just fine.

BTW, Yann sent his initial series on July 8th, so 2.5 months ago, so you should have proposed alternate solutions by then :-) Why do you wait for me to apply the patches to start the discussion ? :-)

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Peter Korsgaard

2016-09-22 18:01 UTC #6 lore raw

PATCH v3 1/2 linux/tools: make it a real, separate package

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas Petazzoni <[email protected]> writes:

Hi,

Quoted 3 lines

Correct. But there was really no reason for those "Linux tools" to be built as part of the linux package, so I think Yann's approach is just fine.

Agreed.

Quoted 3 lines

BTW, Yann sent his initial series on July 8th, so 2.5 months ago, so you should have proposed alternate solutions by then :-) Why do you wait for me to apply the patches to start the discussion ? :-)

Sorry - To my defense I have been away on holidays for 6 weeks since July 1st and still haven't fully gotten to the bottom of my mailbox.

Signature · 2 lines

Venlig hilsen,
Peter Korsgaard

Thomas Petazzoni

2016-09-22 18:14 UTC #7 lore raw

PATCH v3 1/2 linux/tools: make it a real, separate package

Hello,

Quoted 6 lines · Peter Korsgaard

Quoted 3 lines
BTW, Yann sent his initial series on July 8th, so 2.5 months ago, so you should have proposed alternate solutions by then :-) Why do you wait for me to apply the patches to start the discussion ? :-)
Sorry - To my defense I have been away on holidays for 6 weeks since July 1st and still haven't fully gotten to the bottom of my mailbox.

No problem, I was definitely not trying to blame. Just saying that this patch series has been around for a while. In this case, if nobody commented or proposed a better solution, and the proposed solution looks good enough to me, then I commit. Keeping things forever in patchwork is not very good to encourage contributions.

In the worst case, if there are comments and suggestions when applying the patches, it's never too late to rework what has been merged.

Cheers!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Peter Korsgaard

2016-09-22 18:17 UTC #8 lore raw

PATCH v3 1/2 linux/tools: make it a real, separate package

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas Petazzoni <[email protected]> writes:

Hi,

Quoted 5 lines

No problem, I was definitely not trying to blame. Just saying that this patch series has been around for a while. In this case, if nobody commented or proposed a better solution, and the proposed solution looks good enough to me, then I commit. Keeping things forever in patchwork is not very good to encourage contributions.

And that is how it should be, thanks for taking care of the backlog!

Quoted 2 lines

In the worst case, if there are comments and suggestions when applying the patches, it's never too late to rework what has been merged.

Indeed!

Signature · 2 lines

Venlig hilsen,
Peter Korsgaard

Yann E. MORIN

author

2016-09-06 14:29 UTC #9 lore raw

PATCH v3 2/2 docs/manual: update the linux tools section

In reply to Yann E. MORIN

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Romain Naour <[email protected]>

---
 .../adding-packages-linux-kernel-spec-infra.txt    | 40 ++++++++++++----------
 1 file changed, 21 insertions(+), 19 deletions(-)

docs/manual/adding-packages-linux-kernel-spec-infra.txt +21 ~~-19~~

index d394ae6..a7003cc 100644
@@ -12,21 +12,18 @@ hooks for building Linux kernel tools or/and building Linux kernel extensions.
 
 Buildroot offers a helper infrastructure to build some userspace tools
 for the target available within the Linux kernel sources. Since their
-source code is part of the kernel source code, it is not very
-practical to use separate packages for them as they often need to be
-built with the same kernel version as the kernel being used on the
-target. The small Linux kernel tools infrastructure is a simplified
-packaging mechanism based on the generic package infrastructure to
-help building those tools.
+source code is part of the kernel source code, a special package,
++linux-tools+, exists and re-uses the sources of the Linux kernel that
+runs on the target.
 
 Let's look at an example of a Linux tool. For a new Linux tool named
 +foo+, create a new menu entry in the existing
-+linux/Config.tools.in+.  This file will contain the option
++package/linux-tools/Config.in+.  This file will contain the option
 descriptions related to each kernel tool that will be used and
 displayed in the configuration tool. It would basically look like:
 
 ------------------------------
-01: config BR2_LINUX_KERNEL_TOOL_FOO
+01: config BR2_PACKAGE_LINUX_TOOLS_FOO
 02: 	bool "foo"
 03: 	help
 04: 	  This is a comment that explains what foo kernel tool is.

@@ -34,11 +31,16 @@ displayed in the configuration tool. It would basically look like:
 06: 	  http://foosoftware.org/foo/
 ------------------------------
 
-The name of the option starts with the prefix +BR2_LINUX_KERNEL_TOOL_+,
+The name of the option starts with the prefix +BR2_PACKAGE_LINUX_TOOLS_+,
 followed by the uppercase name of the tool (like is done for packages).
 
-Then for each linux tool, add a new +.mk+ file named +linux/linux-tool-foo.mk+.
-It would basically look like:
+.Note
+Unlike other packages, the +linux-tools+ package options appear in the
++linux+ kernel menu, under the `Linux Kernel Tools` sub-menu, not under
+the `Target packages` main menu.
+
+Then for each linux tool, add a new +.mk+ file named
++package/linux-tools/linux-tool-foo.mk+. It would basically look like:
 
 ------------------------------
 01: ################################################################################

@@ -52,19 +54,19 @@ It would basically look like:
 09: FOO_DEPENDENCIES = libbbb
 10:
 11: define FOO_BUILD_CMDS
-12:	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/tools foo
+12: 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/linux/tools foo
 13: endef
 14:
 15: define FOO_INSTALL_STAGING_CMDS
-16:	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/tools \
-17:		DESTDIR=$(STAGING_DIR) \
-18:		foo_install
+16: 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/linux/tools \
+17: 		DESTDIR=$(STAGING_DIR) \
+18: 		foo_install
 19: endef
 20:
 21: define FOO_INSTALL_TARGET_CMDS
-22:	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/tools \
-23:		DESTDIR=$(@D) \
-24:		foo_install
+22: 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/linux/tools \
+23: 		DESTDIR=$(TARGET_DIR) \
+24: 		foo_install
 25: endef
 --------------------------------

@@ -84,7 +86,7 @@ used only when the +foo+ tool is selected. The only supported commands are
 .Note
 One *must not* call +$(eval $(generic-package))+ or any other
 package infrastructure! Linux tools are not packages by themselves,
-they are part of the +linux+ package.
+they are part of the +linux-tools+ package.
 
 [[linux-kernel-ext]]
 ==== linux-kernel-extensions

Signature · 1 line

2.7.4

Thomas Petazzoni

2016-09-22 10:56 UTC #10 lore raw

PATCH v3 2/2 docs/manual: update the linux tools section

Hello,

Quoted 4 lines · Yann E. MORIN

------------------------------
-01: config BR2_LINUX_KERNEL_TOOL_FOO
+01: config BR2_PACKAGE_LINUX_TOOLS_FOO
02: bool "foo"

This example lacks the "select BR2_PACKAGE_LINUX_TOOLS" which I believe is now very important to mention.

Quoted 1 line

+16: $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/linux/tools \

Why $(@D)/linux/tools ? This doesn't seem right. I've changed it to $(LINUX_DIR)/tools, which is what packages really use.

Committed with those issues fixed.

Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com