[1/2,v3] 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" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Paul Ashford <paul.ashford@zurria.co.uk>

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

Hello,

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

On Tue,  6 Sep 2016 16:29:14 +0200, Yann E. MORIN wrote:
> The kernel source tree also contains the sources for various userland
> tools, of which cpupower, perf or selftests.
> +################################################################################
> +#
> +# 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)

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

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> 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

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

Thomas, All,

On 2016-09-22 12:51 +0200, Thomas Petazzoni spake thusly:
> On Tue,  6 Sep 2016 16:29:14 +0200, Yann E. MORIN wrote:
> > +# 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.

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.

Hello,

On Thu, 22 Sep 2016 16:58:17 +0200, Peter Korsgaard wrote:

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

> 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

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

Hi,

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

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

Hello,

On Thu, 22 Sep 2016 20:01:19 +0200, Peter Korsgaard wrote:

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

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

Hi,

 > 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!

 > 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!