Skip to contentSkip to conversation

linux: don't add to toolchain dependency

PATCH New Gustavo Zacarias 4 participants 6 messages Feb 22, 2016 – Mar 1, 2016

lore
Share & tools
lore

Gustavo Zacarias

author
#0 lore raw

PATCH 1/1 linux: don't add to toolchain dependency

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 <[email protected]>
---
 linux/linux.mk | 3 +++
 1 file changed, 3 insertions(+)
linux/linux.mk +3 -0
index 7e20255..dc1c79e 100644
@@ -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))
Signature · 1 line
2.4.10
#1 lore raw

PATCH 1/1 linux: don't add to toolchain dependency

Gustavo,

Cc'ing Yann.

linux/linux.mk LINUX_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION)) · line 12

Quoted 10 lines · Gustavo Zacarias
linux/linux.mk +2 -0
index 7e20255..dc1c79e 100644
@@ -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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#2 lore raw

PATCH 1/1 linux: don't add to toolchain dependency

Quoted 14 lines · Thomas Petazzoni

Gustavo,

Cc'ing Yann.

Quoted 8 lines · Gustavo Zacarias
linux/linux.mk +0 -0
index 7e20255..dc1c79e 100644
@@ -8,6 +8,9 @@ LINUX_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION))
 LINUX_LICENSE = GPLv2
 LINUX_LICENSE_FILES = COPYING
Quoted 7 lines

linux/linux.mk LINUX_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION)) · line 12

Quoted 2 lines
linux/linux.mk +2 -0
+# 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

Quoted 7 lines

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

Signature · 6 lines
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
author
#3 lore raw

PATCH 1/1 linux: don't add to toolchain dependency

Quoted 14 lines · Arnout Vandecappelle

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)

Quoted 4 lines

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.

#4 lore raw

PATCH 1/1 linux: don't add to toolchain dependency

Hello,

Quoted 25 lines · Gustavo Zacarias

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

Quoted 1 line

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.

package/linux-headers/linux-headers.mk +59 -12
index 6339280..ad31e55 100644
@@ -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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#5 lore raw

PATCH 1/1 linux: don't add to toolchain dependency

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

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

Hi,

Quoted 12 lines
Quoted 1 line

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.

Quoted 6 lines

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.

package/linux-headers/linux-headers.mk line 14

Quoted 13 lines
package/linux-headers/linux-headers.mk +3 -2
index 6339280..ad31e55 100644
@@ -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 ;)

Quoted 4 lines
package/linux-headers/linux-headers.mk +4 -0
+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))

package/linux-headers/linux-headers.mk line 28

Quoted 10 lines
package/linux-headers/linux-headers.mk +10 -0
+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.

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

Quoted 2 lines
package/linux-headers/linux-headers.mk +2 -0
+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.

Signature · 1 line
Bye, Peter Korsgaard