* [Buildroot] [PATCH] linux: don't add to toolchain dependency @ 2016-02-22 11:40 Gustavo Zacarias 2016-02-22 12:19 ` Thomas Petazzoni 0 siblings, 1 reply; 6+ messages in thread From: Gustavo Zacarias @ 2016-02-22 11:40 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] linux: don't add to toolchain dependency 2016-02-22 11:40 [Buildroot] [PATCH] linux: don't add to toolchain dependency Gustavo Zacarias @ 2016-02-22 12:19 ` Thomas Petazzoni 2016-02-23 23:55 ` Arnout Vandecappelle 0 siblings, 1 reply; 6+ messages in thread From: Thomas Petazzoni @ 2016-02-22 12:19 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] linux: don't add to toolchain dependency 2016-02-22 12:19 ` Thomas Petazzoni @ 2016-02-23 23:55 ` Arnout Vandecappelle 2016-02-24 13:02 ` Gustavo Zacarias 0 siblings, 1 reply; 6+ messages in thread From: Arnout Vandecappelle @ 2016-02-23 23:55 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] linux: don't add to toolchain dependency 2016-02-23 23:55 ` Arnout Vandecappelle @ 2016-02-24 13:02 ` Gustavo Zacarias 2016-02-24 14:04 ` Thomas Petazzoni 0 siblings, 1 reply; 6+ messages in thread From: Gustavo Zacarias @ 2016-02-24 13:02 UTC (permalink / raw) To: buildroot 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] linux: don't add to toolchain dependency 2016-02-24 13:02 ` Gustavo Zacarias @ 2016-02-24 14:04 ` Thomas Petazzoni 2016-03-01 8:47 ` Peter Korsgaard 0 siblings, 1 reply; 6+ messages in thread From: Thomas Petazzoni @ 2016-02-24 14:04 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] linux: don't add to toolchain dependency 2016-02-24 14:04 ` Thomas Petazzoni @ 2016-03-01 8:47 ` Peter Korsgaard 0 siblings, 0 replies; 6+ messages in thread From: Peter Korsgaard @ 2016-03-01 8:47 UTC (permalink / raw) To: buildroot >>>>> "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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-01 8:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-22 11:40 [Buildroot] [PATCH] linux: don't add to toolchain dependency Gustavo Zacarias 2016-02-22 12:19 ` Thomas Petazzoni 2016-02-23 23:55 ` Arnout Vandecappelle 2016-02-24 13:02 ` Gustavo Zacarias 2016-02-24 14:04 ` Thomas Petazzoni 2016-03-01 8:47 ` Peter Korsgaard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox