[Buildroot] [PATCH 0/2] linux: fix using extensions (branch yem/kernel-ext)

[Buildroot] [PATCH 0/2] linux: fix using extensions (branch yem/kernel-ext)

[Yann E. MORIN]

Hello All!

This small series is an attempt at fixing the use of extensions, which
somehow broke unnoticeably since we switched to using the kconfig-package
infrastructure for the linux package.

Regards,
Yann E. MORIN.


The following changes since commit 6407a0070ff6ba6626bbbf2e0271c38c88c52a83:

  busybox: enable arp applet (2015-03-12 23:52:12 +0100)

are available in the git repository at:

  git://git.busybox.net/~ymorin/git/buildroot yem/kernel-ext

for you to fetch changes up to 7a6b64928c074474e7577f762550fe60916ed052:

  linux: fix use of extensions (2015-03-13 19:55:21 +0100)

----------------------------------------------------------------
Yann E. MORIN (2):
      linux: add note about why it's safe to include other .mk files
      linux: fix use of extensions

 linux/linux-ext-fbtft.mk   |  6 ++----
 linux/linux-ext-rtai.mk    |  6 ++----
 linux/linux-ext-xenomai.mk |  6 ++----
 linux/linux.mk             | 15 +++++++++++++++
 4 files changed, 21 insertions(+), 12 deletions(-)

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

[Buildroot] [PATCH 1/2] linux: add note about why it's safe to include other .mk files

[Yann E. MORIN]

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
---
 linux/linux.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/linux/linux.mk b/linux/linux.mk
index 5afcd4e..b1aca41 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -323,6 +323,14 @@ define LINUX_INSTALL_TARGET_CMDS
 	$(LINUX_INSTALL_HOST_TOOLS)
 endef
 
+# 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-ext-*.mk))
 
 $(eval $(kconfig-package))
-- 
1.9.1

[Buildroot] [PATCH 2/2] linux: fix use of extensions

[Yann E. MORIN]

Currently, using externsaions is flawed and does not work in a very
reproducible way, starting from a _clean_ tree:

    make menuconfig
        -> enable a kernel and any externsion (fbtft is nice, since
           it works for recent kernels)

    make linux-menuconfig

Observe how it is not patching the kernel tree with the extensions prior
to running the linux configuratin UI?

This is because dependencies are only acted on at configure time, which
is a step further after the kconfig stage. This probably was not an
issue before we switched to the kconfig infra for the kernel, but that
use-case was completely missed at the time (blame me!).

Fix that:

  - first, the dependency on extensions is moved to a dependency of the
    patch step;

  - then, to avoid circular dependencies (e.g. linux->rtai->linux), do
    not add extensions to LINUX_DEPENDENCIES, instead, add them to a
    special variable, from which we derive both the list of dependencies
    and the list of post-patch hooks.

This makes it slightly easier to write linux extensions: no need to
delve in the .stamp_patched internals for each extension, just a
function to (conditionally) define and a variable to assign.

(Note: this should go into a section of the manual...)

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
---
 linux/linux-ext-fbtft.mk   | 6 ++----
 linux/linux-ext-rtai.mk    | 6 ++----
 linux/linux-ext-xenomai.mk | 6 ++----
 linux/linux.mk             | 7 +++++++
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/linux/linux-ext-fbtft.mk b/linux/linux-ext-fbtft.mk
index 36f4fd2..f89b830 100644
--- a/linux/linux-ext-fbtft.mk
+++ b/linux/linux-ext-fbtft.mk
@@ -5,8 +5,8 @@
 ################################################################################
 
 ifeq ($(BR2_LINUX_KERNEL_EXT_FBTFT),y)
-# Add dependency to fbtft package (download helper for the fbtft source)
-LINUX_DEPENDENCIES += fbtft
+
+LINUX_EXTENSIONS += fbtft
 
 # for linux >= 3.15 install to drivers/video/fbdev/fbtft
 # for linux < 3.15 install to drivers/video/fbtft
@@ -23,6 +23,4 @@ define FBTFT_PREPARE_KERNEL
 	echo 'obj-y += fbtft/' >> $${dest}/Makefile
 endef
 
-LINUX_PRE_PATCH_HOOKS += FBTFT_PREPARE_KERNEL
-
 endif #BR2_LINUX_KERNEL_EXT_FBTFT
diff --git a/linux/linux-ext-rtai.mk b/linux/linux-ext-rtai.mk
index bf998d5..2c272b0 100644
--- a/linux/linux-ext-rtai.mk
+++ b/linux/linux-ext-rtai.mk
@@ -5,8 +5,8 @@
 ################################################################################
 
 ifeq ($(BR2_LINUX_KERNEL_EXT_RTAI),y)
-# Add dependency to RTAI (user-space) which provide kernel patches
-LINUX_DEPENDENCIES += rtai-patch
+
+LINUX_EXTENSIONS += rtai
 
 RTAI_PATCH = $(call qstrip,$(BR2_LINUX_KERNEL_EXT_RTAI_PATCH))
 
@@ -42,6 +42,4 @@ define RTAI_PREPARE_KERNEL
 endef
 endif
 
-LINUX_PRE_PATCH_HOOKS += RTAI_PREPARE_KERNEL
-
 endif #BR2_LINUX_EXT_RTAI
diff --git a/linux/linux-ext-xenomai.mk b/linux/linux-ext-xenomai.mk
index 84d2c17..51c5bd9 100644
--- a/linux/linux-ext-xenomai.mk
+++ b/linux/linux-ext-xenomai.mk
@@ -5,8 +5,8 @@
 ################################################################################
 
 ifeq ($(BR2_LINUX_KERNEL_EXT_XENOMAI),y)
-# Add dependency to xenomai (user-space) which provide ksrc part
-LINUX_DEPENDENCIES += xenomai
+
+LINUX_EXTENSIONS += xenomai
 
 # Adeos patch version
 XENOMAI_ADEOS_PATCH = $(call qstrip,$(BR2_LINUX_KERNEL_EXT_XENOMAI_ADEOS_PATCH))
@@ -25,6 +25,4 @@ define XENOMAI_PREPARE_KERNEL
 		--verbose
 endef
 
-LINUX_PRE_PATCH_HOOKS += XENOMAI_PREPARE_KERNEL
-
 endif #BR2_LINUX_EXT_XENOMAI
diff --git a/linux/linux.mk b/linux/linux.mk
index b1aca41..eb02619 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -333,8 +333,15 @@ endef
 # included here *must* be in the same directory!
 include $(sort $(wildcard linux/linux-ext-*.mk))
 
+# Include extension-provided post-patch hooks
+LINUX_PRE_PATCH_HOOKS = $(call UPPERCASE,$(patsubst %,%_PREPARE_KERNEL,$(LINUX_EXTENSIONS)))
+
 $(eval $(kconfig-package))
 
+# We need to extract and patch all extensions before we can patch the # kernel.
+# Needs to be _after_ we call kconfig-package, because we need LINUX_DIR
+$(LINUX_DIR)/.stamp_patched: | $(patsubst %,%-patch,$(LINUX_EXTENSIONS))
+
 # Support for rebuilding the kernel after the cpio archive has
 # been generated in $(BINARIES_DIR)/rootfs.cpio.
 $(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_target_installed $(LINUX_DIR)/.stamp_images_installed $(BINARIES_DIR)/rootfs.cpio
-- 
1.9.1

[Buildroot] [PATCH 2/2] linux: fix use of extensions

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Fri, 13 Mar 2015 19:57:29 +0100, Yann E. MORIN wrote:
> Currently, using externsaions is flawed and does not work in a very

extensions

> reproducible way, starting from a _clean_ tree:
> 
>     make menuconfig
>         -> enable a kernel and any externsion (fbtft is nice, since
>            it works for recent kernels)
> 
>     make linux-menuconfig
> 
> Observe how it is not patching the kernel tree with the extensions prior
> to running the linux configuratin UI?

configuration

> This is because dependencies are only acted on at configure time, which
> is a step further after the kconfig stage. This probably was not an
> issue before we switched to the kconfig infra for the kernel, but that
> use-case was completely missed at the time (blame me!).

Hum, I am not sure to see why the switch to the kconfig-package
infrastructure would have modified this behavior. So I'd like to
understand how it used to work, if it ever worked (but I believe it
did, no?).

> Fix that:
> 
>   - first, the dependency on extensions is moved to a dependency of the
>     patch step;

This is a bit problematic because then the dependency is unknown to the
package infrastructure. Which means that things like 'make
graph-depends' will no longer display this dependency.

To solve this, we would have to add a <pkg>_PATCH_DEPENDENCIES variable
in pkg-generic, or something like that.

But again, I'd like to understand why we didn't need that until now.

> 
>   - then, to avoid circular dependencies (e.g. linux->rtai->linux), do
>     not add extensions to LINUX_DEPENDENCIES, instead, add them to a
>     special variable, from which we derive both the list of dependencies
>     and the list of post-patch hooks.
> 
> This makes it slightly easier to write linux extensions: no need to
> delve in the .stamp_patched internals for each extension, just a
> function to (conditionally) define and a variable to assign.

I'm not sure to understand how the linux extensions had to delve into
the .stamp_patched internals. They were just registering a
POST_PATCH hook, no?

> (Note: this should go into a section of the manual...)

Yes, indeed, once we settle on a solution :)

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

[Buildroot] [PATCH 1/2] linux: add note about why it's safe to include other .mk files

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Fri, 13 Mar 2015 19:57:28 +0100, Yann E. MORIN wrote:
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>

Applied, thanks!

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

[Buildroot] [PATCH 2/2] linux: fix use of extensions

[Yann E. MORIN]

Thomas, All,

On 2015-03-13 21:47 +0100, Thomas Petazzoni spake thusly:
> On Fri, 13 Mar 2015 19:57:29 +0100, Yann E. MORIN wrote:
> > Currently, using externsaions is flawed and does not work in a very
> extensions
[--SNIP--]
> > to running the linux configuratin UI?
> configuration

Damn, I even re-read myself before sending. Seems it was not enough...
:-/

> > This is because dependencies are only acted on at configure time, which
> > is a step further after the kconfig stage. This probably was not an
> > issue before we switched to the kconfig infra for the kernel, but that
> > use-case was completely missed at the time (blame me!).
> 
> Hum, I am not sure to see why the switch to the kconfig-package
> infrastructure would have modified this behavior. So I'd like to
> understand how it used to work, if it ever worked (but I believe it
> did, no?).

Fair enough, I'll double-check that it did / did not work back then.

> > Fix that:
> > 
> >   - first, the dependency on extensions is moved to a dependency of the
> >     patch step;
> 
> This is a bit problematic because then the dependency is unknown to the
> package infrastructure. Which means that things like 'make
> graph-depends' will no longer display this dependency.

Right. Note however that this was already the case for the RTAI
externsion, because it was declaring:

    LINUX_DEPENDENCIES += rtai-patch

so that was already missed (or at least mis-interpreted) by graph-depends
anyway.

> To solve this, we would have to add a <pkg>_PATCH_DEPENDENCIES variable
> in pkg-generic, or something like that.
> 
> But again, I'd like to understand why we didn't need that until now.

As said above, I'll double-check on that.

> >   - then, to avoid circular dependencies (e.g. linux->rtai->linux), do
> >     not add extensions to LINUX_DEPENDENCIES, instead, add them to a
> >     special variable, from which we derive both the list of dependencies
> >     and the list of post-patch hooks.
> > 
> > This makes it slightly easier to write linux extensions: no need to
> > delve in the .stamp_patched internals for each extension, just a
> > function to (conditionally) define and a variable to assign.
> 
> I'm not sure to understand how the linux extensions had to delve into
> the .stamp_patched internals. They were just registering a
> POST_PATCH hook, no?

No, they _did not_ have to so far.

What I meant is that the switch to a dependency of the patch step
required that they would now all have had to write something like:

    $(LINUX_DIR)/.stamp_patched: | EXT-patch

(where EXT is the name of the extension.)

To avoid such an arcane code that would have to be replicated (and
potentially tracked down in case we change something in dependency
handling), I found it would be better to have it all handled in a single
location.

Anyway, I'll look at your suggestion of introducing FOO_PATCH_DEPENDENCIES.
However, linux is the sole package that requires such handling, and I
wonder if it is worth introducing for just a single package (note that
I rehash your own argument, hehe! ;-) )

> > (Note: this should go into a section of the manual...)
> 
> Yes, indeed, once we settle on a solution :)

Eh, yes! :-)

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH 2/2] linux: fix use of extensions

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Fri, 13 Mar 2015 23:10:13 +0100, Yann E. MORIN wrote:

> > Hum, I am not sure to see why the switch to the kconfig-package
> > infrastructure would have modified this behavior. So I'd like to
> > understand how it used to work, if it ever worked (but I believe it
> > did, no?).
> 
> Fair enough, I'll double-check that it did / did not work back then.

Great.

> > This is a bit problematic because then the dependency is unknown to the
> > package infrastructure. Which means that things like 'make
> > graph-depends' will no longer display this dependency.
> 
> Right. Note however that this was already the case for the RTAI
> externsion, because it was declaring:
> 
>     LINUX_DEPENDENCIES += rtai-patch
> 
> so that was already missed (or at least mis-interpreted) by graph-depends
> anyway.

Indeed, correct. And that's not nice :/


> > I'm not sure to understand how the linux extensions had to delve into
> > the .stamp_patched internals. They were just registering a
> > POST_PATCH hook, no?
> 
> No, they _did not_ have to so far.
> 
> What I meant is that the switch to a dependency of the patch step
> required that they would now all have had to write something like:
> 
>     $(LINUX_DIR)/.stamp_patched: | EXT-patch
> 
> (where EXT is the name of the extension.)
> 
> To avoid such an arcane code that would have to be replicated (and
> potentially tracked down in case we change something in dependency
> handling), I found it would be better to have it all handled in a single
> location.

Ok, understood.

> 
> Anyway, I'll look at your suggestion of introducing FOO_PATCH_DEPENDENCIES.
> However, linux is the sole package that requires such handling, and I
> wonder if it is worth introducing for just a single package (note that
> I rehash your own argument, hehe! ;-) )

Point taken :-)

But my argument about putting things in the infra only if at least a
significant number of packages need it is only valid if there's another
way of doing it that works. For example, if you have three packages
that do --disable-foobar, then it's not a strong argument to put it in
the infra because it can perfectly be done in a per-package fashion.

However, things such making sure that the infra is aware of weird
dependencies is not something you can fix at the per-package level, you
need some support from the infrastructure.

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

[Buildroot] [PATCH 2/2] linux: fix use of extensions

[Yann E. MORIN]

Thomas, All,

On 2015-03-13 21:47 +0100, Thomas Petazzoni spake thusly:
> On Fri, 13 Mar 2015 19:57:29 +0100, Yann E. MORIN wrote:
[--SNIP--]
> > This is because dependencies are only acted on at configure time, which
> > is a step further after the kconfig stage. This probably was not an
> > issue before we switched to the kconfig infra for the kernel, but that
> > use-case was completely missed at the time (blame me!).
> 
> Hum, I am not sure to see why the switch to the kconfig-package
> infrastructure would have modified this behavior. So I'd like to
> understand how it used to work, if it ever worked (but I believe it
> did, no?).

OK, so I think I know what happened when we migrated linux over to the
kconfig-package infra, and what we loose then.

Back before we used the kconfig-package infra, linux-menuconfig (as well
as the other configurators) was written as custom rules, that were
basically something like:

    linux-menuconfig: linux-configure
        $(MAKE) -C $(LINUX_DIR) menuconfig

And there we had our dependency on a previously configured linux tree,
and *that* was kicking-in our dependency on extensions.

Fast forward post 2015.02, and we now have linux use the kconfig-package infra,
which completely decorrelates the kconfig-part of the configuration,
from the actual package-part of the configuration, as it introduces an
intermediate 'kconfig-fixup' step:

    linux-configure -------> kconfig-fixup --> .config --> $(LINUX_CONFIG_FILE)
                        /
    linux-menuconfig --'

So, linux-menuconfig now no longer depends on the linux tree to be
configured. Hence the extensions breaking...

And this, I hope, concludes the nalaysis and is enough to convince you
this is a new problem, caused by the kconfig-package infra. Hehe! :-)

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH 2/2] linux: fix use of extensions

[Yann E. MORIN]

Thomas, All,

On 2015-03-13 21:47 +0100, Thomas Petazzoni spake thusly:
> On Fri, 13 Mar 2015 19:57:29 +0100, Yann E. MORIN wrote:
[--SNIP--]
> > Fix that:
> > 
> >   - first, the dependency on extensions is moved to a dependency of the
> >     patch step;
> 
> This is a bit problematic because then the dependency is unknown to the
> package infrastructure. Which means that things like 'make
> graph-depends' will no longer display this dependency.
> 
> To solve this, we would have to add a <pkg>_PATCH_DEPENDENCIES variable
> in pkg-generic, or something like that.

OK, so I tried to implement your proposal, but I stumbled against
various issues:

 1- what should be done with a patch-dependency: must it be handled like
    a normal dependency, i.e. be fully built and installed? Or should it
    just be extracted and patched itself?

    We're trying to introduce this new type of dependency to solve the
    linux extension case. So, we do not really require the extension to
    be built in the first place; we only need their files so we can
    vampirise them, so simply extracting and patching is all we currently
    need. The RTAI extension so far goes as far as explicitly adding a
    dependency on "rtai-patch" instead of just "rtai", as rtai depends
    on linux. So, to use this new feature for linux extensions, we
    really want *not* to build the extension.

    However, introducing this new feature in the pkg-generic infra means
    it is available to other packages as well, and we can not predict
    what packages could use it for. Would there be packages that
    want/need a built patch-dependency?

    There's a kind of discrepancy in there, as normal dependencies are
    dependencies to be built, while patch-dependencies are dependencies
    that only need to be, well, patched...  (I find it odd.)

 2- extensions are really not supported for _OVERRIDE_SRCDIR, since they
    are handled as pre-patch hooks, and _OVERRIDE_SRCDIR does not do
    patches.

    Is that something we want to live by, or do we want to support
    extensions even in the _OVERRIDE_SRCDIR?

    Note that this is not a new issue: it has always been the case so
    far, even before the migration to kconfig-package.

[Though this be madness, yet there is method in't.]

Regards,
Yann E. MORIN.

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