[Buildroot] [PATCH] linux: don't add to toolchain dependency

[Buildroot] [PATCH] linux: don't add to toolchain dependency

[Gustavo Zacarias]

In 63abfb72 the option was added to use the same linux headers
source/version as the kernel.
If the kernel isn't removed from toolchain dependencies then it gets
added at two points resulting in a circular dependency, example:

$ make qemu_aarch64_virt_defconfig && make source
...
mpfr-3.1.3.tar.xz: OK (sha256:
6835a08bd992c8257641791e9a6a2b35b02336c8de26d0a8577953747e514a16)
WARNING: no hash file for linux-4.4.1.tar.xz
kmod-22.tar.xz: OK (sha256:
ba3b1ddea33228b473189fcb05b809024a3b86e9a7cf37d420cae06beb749f82)
pkgconf-0.9.12.tar.bz2: OK (sha256:
7ec8b516e655e247f4ba976837cee808134785819ab8f538f652fe919cc6c09f)
make: Circular toolchain-all-source <- toolchain-buildroot-all-source
dependency dropped.
busybox-1.24.1.tar.bz2: OK (md5: be98a40cadf84ce2d6b05fa41a275c6a)
...

Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 linux/linux.mk | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/linux/linux.mk b/linux/linux.mk
index 7e20255..dc1c79e 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -8,6 +8,9 @@ LINUX_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION))
 LINUX_LICENSE = GPLv2
 LINUX_LICENSE_FILES = COPYING
 
+# Used when BR2_KERNEL_HEADERS_AS_KERNEL=y
+LINUX_ADD_TOOLCHAIN_DEPENDENCY = NO
+
 # Compute LINUX_SOURCE and LINUX_SITE from the configuration
 ifeq ($(BR2_LINUX_KERNEL_CUSTOM_TARBALL),y)
 LINUX_TARBALL = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION))
-- 
2.4.10

[Buildroot] [PATCH] linux: don't add to toolchain dependency

[Thomas Petazzoni]

Gustavo,

Cc'ing Yann.

On Mon, 22 Feb 2016 08:40:44 -0300, Gustavo Zacarias wrote:

> diff --git a/linux/linux.mk b/linux/linux.mk
> index 7e20255..dc1c79e 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -8,6 +8,9 @@ LINUX_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION))
>  LINUX_LICENSE = GPLv2
>  LINUX_LICENSE_FILES = COPYING
>  
> +# Used when BR2_KERNEL_HEADERS_AS_KERNEL=y
> +LINUX_ADD_TOOLCHAIN_DEPENDENCY = NO

This is annoying, because we *do* need the toolchain dependency to
build the Linux kernel. I think we would rather need to do:

LINUX_HEADERS_PATCH_DEPENDENCIES = linux-patch

Though I'm not sure how great it is to have a non-package listed in the
<pkg>_DEPENDENCIES variable. Surely it might break graph-depends and
other similar tooling.

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

[Buildroot] [PATCH] linux: don't add to toolchain dependency

[Arnout Vandecappelle]

On 02/22/16 13:19, Thomas Petazzoni wrote:
> Gustavo,
> 
> Cc'ing Yann.
> 
> On Mon, 22 Feb 2016 08:40:44 -0300, Gustavo Zacarias wrote:
> 
>> diff --git a/linux/linux.mk b/linux/linux.mk
>> index 7e20255..dc1c79e 100644
>> --- a/linux/linux.mk
>> +++ b/linux/linux.mk
>> @@ -8,6 +8,9 @@ LINUX_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION))
>>  LINUX_LICENSE = GPLv2
>>  LINUX_LICENSE_FILES = COPYING
>>  
>> +# Used when BR2_KERNEL_HEADERS_AS_KERNEL=y
>> +LINUX_ADD_TOOLCHAIN_DEPENDENCY = NO
> 
> This is annoying, because we *do* need the toolchain dependency to
> build the Linux kernel. I think we would rather need to do:
> 
> LINUX_HEADERS_PATCH_DEPENDENCIES = linux-patch

 Well, _PATCH_DEPENDENCIES already does that:

# Order-only dependency
$$($(2)_TARGET_PATCH):  | $$(patsubst %,%-patch,$$($(2)_FINAL_PATCH_DEPENDENCIES))


 The problem identified by Gustavo doesn't happen in a normal build, it's only
when you do make source. That's because for source we use:

$(1)-all-source:        $(1)-source
$(1)-all-source:        $$(foreach
p,$$($(2)_FINAL_ALL_DEPENDENCIES),$$(p)-all-source)


 I have to think a bit more about possible solutions. But the good thing is that
it's less important: a normal build will still work without warning, and make
source will also still work and have all the packages, it just has a warning
that is not so nice. So if it doesn't get fixed for 2016.02 it's no disaster.


 Regards,
 Arnout

> 
> Though I'm not sure how great it is to have a non-package listed in the
> <pkg>_DEPENDENCIES variable. Surely it might break graph-depends and
> other similar tooling.
> 
> Thomas
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] linux: don't add to toolchain dependency

[Gustavo Zacarias]

On 23/02/16 20:55, Arnout Vandecappelle wrote:

>   Well, _PATCH_DEPENDENCIES already does that:
>
> # Order-only dependency
> $$($(2)_TARGET_PATCH):  | $$(patsubst %,%-patch,$$($(2)_FINAL_PATCH_DEPENDENCIES))
>
>
>   The problem identified by Gustavo doesn't happen in a normal build, it's only
> when you do make source. That's because for source we use:
>
> $(1)-all-source:        $(1)-source
> $(1)-all-source:        $$(foreach
> p,$$($(2)_FINAL_ALL_DEPENDENCIES),$$(p)-all-source)
>
>
>   I have to think a bit more about possible solutions. But the good thing is that
> it's less important: a normal build will still work without warning, and make
> source will also still work and have all the packages, it just has a warning
> that is not so nice. So if it doesn't get fixed for 2016.02 it's no disaster.

It's not only -source, any dependency-using routines are affected, 
legal-info, graph-depends as well as long as you build a kernel.
In fact graph-depends is broken:

$ make graph-depends
Getting targets
Getting dependencies for ['toolchain-buildroot', 'toolchain', 'busybox', 
'glibc', 'initscripts', 'linux-headers', 'skeleton', 'linux', 
'host-fakeroot', 'host-makedevs', 'rootfs-cpio', 'rootfs-initramfs']
Getting dependencies for ['host-kmod', 'host-gcc-final', 
'host-gcc-initial', 'host-gawk']
Getting dependencies for ['host-gmp', 'host-binutils', 'host-pkgconf', 
'host-mpfr', 'host-mpc']
Getting dependencies for ['host-m4']

Recursion detected for  : toolchain
which is a dependency of: linux
which is a dependency of: linux-headers
which is a dependency of: glibc
which is a dependency of: host-gcc-final
which is a dependency of: toolchain-buildroot
which is a dependency of: toolchain
Makefile:721: recipe for target 'graph-depends' failed
make: *** [graph-depends] Error 1

So it's not as innocent as you think.
Regards.

[Buildroot] [PATCH] linux: don't add to toolchain dependency

[Thomas Petazzoni]

Hello,

On Wed, 24 Feb 2016 10:02:22 -0300, Gustavo Zacarias wrote:

> It's not only -source, any dependency-using routines are affected, 
> legal-info, graph-depends as well as long as you build a kernel.
> In fact graph-depends is broken:
> 
> $ make graph-depends
> Getting targets
> Getting dependencies for ['toolchain-buildroot', 'toolchain', 'busybox', 
> 'glibc', 'initscripts', 'linux-headers', 'skeleton', 'linux', 
> 'host-fakeroot', 'host-makedevs', 'rootfs-cpio', 'rootfs-initramfs']
> Getting dependencies for ['host-kmod', 'host-gcc-final', 
> 'host-gcc-initial', 'host-gawk']
> Getting dependencies for ['host-gmp', 'host-binutils', 'host-pkgconf', 
> 'host-mpfr', 'host-mpc']
> Getting dependencies for ['host-m4']
> 
> Recursion detected for  : toolchain
> which is a dependency of: linux
> which is a dependency of: linux-headers
> which is a dependency of: glibc
> which is a dependency of: host-gcc-final
> which is a dependency of: toolchain-buildroot
> which is a dependency of: toolchain
> Makefile:721: recipe for target 'graph-depends' failed
> make: *** [graph-depends] Error 1
> 
> So it's not as innocent as you think.

I agree. In the end, I believe the mistake is to have linux-headers
depends on linux.

I continue to believe that it's much easier to duplicate in
linux-headers the 10-20 lines of linux.mk logic that infer the
_SOURCE/_SITE/_VERSION from the BR2_LINUX_KERNEL_* variables.

Yes, it means we will extract the kernel sources twice, but when you're
building an internal toolchain, this is really negligible.

And this way, we break the dependency from linux-headers on linux, and
the problem is solved.

Something along the lines of (quickly tested) of the following patch.
Yes, we duplicate logic, but we don't change this logic every day, so
maybe it's acceptable.

diff --git a/package/linux-headers/linux-headers.mk b/package/linux-headers/linux-headers.mk
index 6339280..ad31e55 100644
--- a/package/linux-headers/linux-headers.mk
+++ b/package/linux-headers/linux-headers.mk
@@ -9,14 +9,63 @@
 
 ifeq ($(BR2_KERNEL_HEADERS_AS_KERNEL),y)
 
-LINUX_HEADERS_VERSION = none
-LINUX_HEADERS_SOURCE =
+LINUX_HEADERS_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION))
+
+# Compute LINUX_SOURCE and LINUX_SITE from the configuration
+ifeq ($(BR2_LINUX_KERNEL_CUSTOM_TARBALL),y)
+LINUX_HEADERS_TARBALL = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION))
+LINUX_HEADERS_SITE = $(patsubst %/,%,$(dir $(LINUX_HEADERS_TARBALL)))
+LINUX_HEADERS_SOURCE = $(notdir $(LINUX_HEADERS_TARBALL))
+BR_NO_CHECK_HASH_FOR += $(LINUX_HEADERS_SOURCE)
+else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_LOCAL),y)
+LINUX_HEADERS_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH))
+LINUX_HEADERS_SITE_METHOD = local
+else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_GIT),y)
+LINUX_HEADERS_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL))
+LINUX_HEADERS_SITE_METHOD = git
+else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_HG),y)
+LINUX_HEADERS_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL))
+LINUX_HEADERS_SITE_METHOD = hg
+else
+LINUX_HEADERS_SOURCE = linux-$(LINUX_VERSION).tar.xz
+ifeq ($(BR2_LINUX_KERNEL_CUSTOM_VERSION),y)
+BR_NO_CHECK_HASH_FOR += $(LINUX_HEADERS_SOURCE)
+endif
+# In X.Y.Z, get X and Y. We replace dots and dashes by spaces in order
+# to use the $(word) function. We support versions such as 4.0, 3.1,
+# 2.6.32, 2.6.32-rc1, 3.0-rc6, etc.
+ifeq ($(findstring x2.6.,x$(LINUX_HEADERS_VERSION)),x2.6.)
+LINUX_HEADERS_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v2.6
+else ifeq ($(findstring x3.,x$(LINUX_HEADERS_VERSION)),x3.)
+LINUX_HEADERS_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v3.x
+else ifeq ($(findstring x4.,x$(LINUX_HEADERS_VERSION)),x4.)
+LINUX_HEADERS_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v4.x
+endif
+# release candidates are in testing/ subdir
+ifneq ($(findstring -rc,$(LINUX_HEADERS_VERSION)),)
+LINUX_HEADERS_SITE := $(LINUX_HEADERS_SITE)/testing
+endif # -rc
+endif
 
-LINUX_HEADERS_LICENSE = $(LINUX_LICENSE)
-LINUX_HEADERS_LICENSE_FILES = $(LINUX_LICENSE_FILES)
+LINUX_HEADERS_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
+
+# We rely on the generic package infrastructure to download and apply
+# remote patches (downloaded from ftp, http or https). For local
+# patches, we can't rely on that infrastructure, because there might
+# be directories in the patch list (unlike for other packages).
+LINUX_HEADERS_PATCH = $(filter ftp://% http://% https://%,$(LINUX_HEADERS_PATCHES))
+
+define LINUX_HEADERS_APPLY_LOCAL_PATCHES
+	for p in $(filter-out ftp://% http://% https://%,$(LINUX_HEADERS_PATCHES)) ; do \
+		if test -d $$p ; then \
+			$(APPLY_PATCHES) $(@D) $$p \*.patch || exit 1 ; \
+		else \
+			$(APPLY_PATCHES) $(@D) `dirname $$p` `basename $$p` || exit 1; \
+		fi \
+	done
+endef
 
-LINUX_HEADERS_PATCH_DEPENDENCIES = linux
-LINUX_HEADERS_REAL_DIR = $(LINUX_DIR)
+LINUX_HEADERS_POST_PATCH_HOOKS += LINUX_HEADERS_APPLY_LOCAL_PATCHES
 
 else # ! BR2_KERNEL_HEADERS_AS_KERNEL
 
@@ -30,13 +79,11 @@ LINUX_HEADERS_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/v4.x
 endif
 LINUX_HEADERS_SOURCE = linux-$(LINUX_HEADERS_VERSION).tar.xz
 
+endif # ! BR2_KERNEL_HEADERS_AS_KERNEL
+
 LINUX_HEADERS_LICENSE = GPLv2
 LINUX_HEADERS_LICENSE_FILES = COPYING
 
-LINUX_HEADERS_REAL_DIR = $(@D)
-
-endif # ! BR2_KERNEL_HEADERS_AS_KERNEL
-
 LINUX_HEADERS_INSTALL_STAGING = YES
 
 # linux-headers is part of the toolchain so disable the toolchain dependency
@@ -53,7 +100,7 @@ LINUX_HEADERS_ADD_TOOLCHAIN_DEPENDENCY = NO
 # uClibc building. This way uClibc doesn't modify linux headers on installation
 # of "its" headers
 define LINUX_HEADERS_CONFIGURE_CMDS
-	(cd $(LINUX_HEADERS_REAL_DIR); \
+	(cd $(@D); \
 		$(TARGET_MAKE_ENV) $(MAKE) \
 			ARCH=$(KERNEL_ARCH) \
 			HOSTCC="$(HOSTCC)" \
@@ -64,7 +111,7 @@ define LINUX_HEADERS_CONFIGURE_CMDS
 endef
 
 define LINUX_HEADERS_INSTALL_STAGING_CMDS
-	(cd $(LINUX_HEADERS_REAL_DIR); \
+	(cd $(@D); \
 		$(TARGET_MAKE_ENV) $(MAKE) \
 			ARCH=$(KERNEL_ARCH) \
 			HOSTCC="$(HOSTCC)" \


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

[Buildroot] [PATCH] linux: don't add to toolchain dependency

[Peter Korsgaard]

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

Hi,

>> So it's not as innocent as you think.

 > I agree. In the end, I believe the mistake is to have linux-headers
 > depends on linux.

 > I continue to believe that it's much easier to duplicate in
 > linux-headers the 10-20 lines of linux.mk logic that infer the
 > _SOURCE/_SITE/_VERSION from the BR2_LINUX_KERNEL_* variables.

 > Yes, it means we will extract the kernel sources twice, but when you're
 > building an internal toolchain, this is really negligible.

 > And this way, we break the dependency from linux-headers on linux, and
 > the problem is solved.

 > Something along the lines of (quickly tested) of the following patch.
 > Yes, we duplicate logic, but we don't change this logic every day, so
 > maybe it's acceptable.

Yes, I think this is the safest solution for 2016.02.

 > diff --git a/package/linux-headers/linux-headers.mk b/package/linux-headers/linux-headers.mk
 > index 6339280..ad31e55 100644
 > --- a/package/linux-headers/linux-headers.mk
 > +++ b/package/linux-headers/linux-headers.mk
 > @@ -9,14 +9,63 @@
 
 >  ifeq ($(BR2_KERNEL_HEADERS_AS_KERNEL),y)
 
 > -LINUX_HEADERS_VERSION = none
 > -LINUX_HEADERS_SOURCE =
 > +LINUX_HEADERS_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION))
 > +
 > +# Compute LINUX_SOURCE and LINUX_SITE from the configuration

This should be LINUX_HEADERS_SOURCE/SITE ;)

> +ifeq ($(BR2_LINUX_KERNEL_CUSTOM_TARBALL),y)
 > +LINUX_HEADERS_TARBALL = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION))
 > +LINUX_HEADERS_SITE = $(patsubst %/,%,$(dir $(LINUX_HEADERS_TARBALL)))
 > +LINUX_HEADERS_SOURCE = $(notdir $(LINUX_HEADERS_TARBALL))
 > +BR_NO_CHECK_HASH_FOR += $(LINUX_HEADERS_SOURCE)
 > +else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_LOCAL),y)
 > +LINUX_HEADERS_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH))
 > +LINUX_HEADERS_SITE_METHOD = local
 > +else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_GIT),y)
 > +LINUX_HEADERS_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL))
 > +LINUX_HEADERS_SITE_METHOD = git
 > +else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_HG),y)
 > +LINUX_HEADERS_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL))
 > +LINUX_HEADERS_SITE_METHOD = hg

It's a bit painful to git/hg clone a kernel tree twice, so I've added

LINUX_HEADERS_SOURCE = linux-$(LINUX_HEADERS_VERSION).tar.gz

To both of these so the download infrastructure knows it is the same
data and doesn't try to do it twice.

> +else
 > +LINUX_HEADERS_SOURCE = linux-$(LINUX_VERSION).tar.xz

This should really be LINUX_HEADERS_VERSION to not rely on the internal
linux.mk symbols.

Other than that I have verified that this logic is identical with the
code in linux.mk after s/LINUX_HEADERS_/LINUX_.

Committed, thanks.

-- 
Bye, Peter Korsgaard