Added local directory as source of kernel code

From: Rafal Fabich <[email protected]>

This patch is a resubmit of commit 73da2ff6f718f2889e3c5024d899f8d58f502863, which was reverted by commit e782cd5b1bc231dda527d5d0a04e6a338669b92c in 2016.

This is the original commit message:
Add the option to use a local directory as the source for building the Linux kernel, which can be useful during kernel development.

As for the arguments used in the commit message when the patch was reverted:
* Using _OVERRIDE_SRCDIR is not a good solution since it forces some content in local.mk, which should stay a mechanism local to the developer's workspace.
* Without this patch, the linux kernel has no equivalent to the FOO_SITE_METHOD = local variable which is available for almost all the other packages.
* When using packages with _SITE_METHOD = local, an external tool is used to retrieve the packages' sources, like a sub-repository mechanism or google repo. This tool has the responsibility of the build reproductibility in regard of these packages.
It is the same for a linux kernel using the local method.

# Conflicts:
# linux/Config.in

---
 linux/Config.in | 13 +++++++++++++
 linux/linux.mk  |  3 +++
 2 files changed, 16 insertions(+)

index ecb12d0b16..e6c5c561b9 100644
@@ -91,6 +91,12 @@ config BR2_LINUX_KERNEL_CUSTOM_SVN
 	  This option allows Buildroot to get the Linux kernel source
 	  code from a Subversion repository.
 
+config BR2_LINUX_KERNEL_CUSTOM_LOCAL
+	bool "Local directory"
+	help
+	  This option allows Buildroot to get the Linux kernel source
+	  code from a local directory.
+
 endchoice
 
 config BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE

@@ -118,6 +124,12 @@ config BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION
 
 endif
 
+config BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH
+	string "Path to the local directory"
+	depends on BR2_LINUX_KERNEL_CUSTOM_LOCAL
+	help
+	  Path to the local directory with the Linux kernel source code.
+
 config BR2_LINUX_KERNEL_VERSION
 	string
 	default "4.18.14" if BR2_LINUX_KERNEL_LATEST_VERSION

@@ -127,6 +139,7 @@ config BR2_LINUX_KERNEL_VERSION
 	default "custom" if BR2_LINUX_KERNEL_CUSTOM_TARBALL
 	default BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION \
 		if BR2_LINUX_KERNEL_CUSTOM_GIT || BR2_LINUX_KERNEL_CUSTOM_HG || BR2_LINUX_KERNEL_CUSTOM_SVN
+	default "custom" if BR2_LINUX_KERNEL_CUSTOM_LOCAL
 
 #
 # Patch selection

index 476ff16329..c47c792926 100644
@@ -20,6 +20,9 @@ ifeq ($(BR2_LINUX_KERNEL_CUSTOM_TARBALL),y)
 LINUX_TARBALL = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION))
 LINUX_SITE = $(patsubst %/,%,$(dir $(LINUX_TARBALL)))
 LINUX_SOURCE = $(notdir $(LINUX_TARBALL))
+else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_LOCAL),y)
+LINUX_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH))
+LINUX_SITE_METHOD = local
 else ifeq ($(BR2_LINUX_KERNEL_CUSTOM_GIT),y)
 LINUX_SITE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_REPO_URL))
 LINUX_SITE_METHOD = git

2.11.0

Nicolas,

OVERRIDE_SRCDIR seems to work fine for the use cases where I've had a similar need. Yann's revert commit description makes sense and follows the same conclusion of why it was removed.

Please share your use case if you could or why it wouldn't work.

Matthew L Weber / Pr Software Engineer
Airborne Information Systems / RC Linux Secure Platforms
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
www.rockwellcollins.com

Note: Any Export License Required Information and License Restricted
Third Party Intellectual Property (TPIP) content must be encrypted and
sent to matthew.weber at corp.rockwellcollins.com.

Hello,

I have a workspace managed by google repo, which fetches the sources of the external git repositories for which we have created .mk buildroot Makefiles.

Because the sources are cloned by repo, I don't want them to be fetched by buildroot, hence I used "_SITE_METHOD = local" for all the packages.

For linux and u-boot, I can't do that because they dont support the local method and I think there is no reason to make these two packages an exception.

As for modifying the local.mk file to put some LINUX_OVERRIDE_SRCDIR value inside, I want to avoid it because it would force me to version the local.mk file for things to work out-of-the-box. Versionning this file is problematic because developers needing to modify it temporarily may commit it by error.

Is there another solution to achieve what I want ? I found none so far but I may have missed something.

________________________________
From: Matthew Weber <[email protected]>
Sent: Wednesday, November 7, 2018 2:57:28 PM
To: Nicolas Carrier
Cc: buildroot; Peter Korsgaard; rafal.fabich at gmail.com
Subject: Re: [Buildroot] [PATCH 1/1] Added local directory as source of kernel code

Nicolas,

OVERRIDE_SRCDIR seems to work fine for the use cases where I've had a similar need. Yann's revert commit description makes sense and follows the same conclusion of why it was removed.

Please share your use case if you could or why it wouldn't work.

buildroot Info Page - BusyBox<http://lists.busybox.net/mailman/listinfo/buildroot>
lists.busybox.net
Buildroot is a set of Makefiles and patches that makes it easy generate a cross-compilation toolchain and root filesystem for your target Linux system.

--
Matthew L Weber / Pr Software Engineer
Airborne Information Systems / RC Linux Secure Platforms MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA www.rockwellcollins.com<http://www.rockwellcollins.com>

Note: Any Export License Required Information and License Restricted
Third Party Intellectual Property (TPIP) content must be encrypted and
sent to matthew.weber at corp.rockwellcollins.com.
ATTENTION: This email came from an external source.
Do not open attachments or click on links from unknown senders or unexpected emails. -------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20181107/d0a6e3a8/attachment.html>

Hello Nicolas,

Well your patch is actually making u-boot and linux be exceptions. What if someone wants to build busybox from a local source tree ? And qt5 ? And openssl ? And then this other package ?

As you can see, this approach does not really scale: we would have to add Config.in options to all packages to say "please build using the source code from folder XYZ instead of the normal upstream source code". That's why we have chosen to support this use case using OVERRIDE_SRCDIR.

Here is what you could do:

- Set BR2_PACKAGE_OVERRIDE_FILE to point to whatever file you like instead of local.mk, say package-overrides.mk.

- In this file, you set LINUX_OVERRIDE_SRCDIR as appropriate, but you also do

-include local.mk

And then you version control package-overrides.mk. Then your developer can continue locally to tinker with local.mk to have their own custom OVERRIDE_SRCDIR, and still have LINUX_OVERRIDE_SRCDIR used.

Alternatively, maybe we should decide that linux and u-boot are special packages, and just like for those packages we support fetching from custom tarballs and custom Git repositories, we should support fetching from custom local directories (this is what your patch implements, but until now, we have resisted going into this direction).

Best regards,

Thomas

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

I think I understand better the issue.

The thing is there are 2 sort of packages, the ones managed internally by buildroot and the external ones. Linux and u-boot are special, at least for us, because they are internal buildroot packages for which we want to manage the sources like we do for external ones by using the "local" method.

I think the solution you propose will work and I'm going to use it if the patch is refused.

But still I find it a little bit hackish compared to the possibility to use a local source tree, which is an alternative which makes sense, when we already have the possibility to use, a custom svn, git, tarball...

If you end up deciding to integrate the patch, I have the u-boot patch, originally written by Adam Duskett, rebased on top of master and ready for submission ^^

________________________________
From: Thomas Petazzoni <[email protected]>
Sent: Wednesday, November 7, 2018 5:44:11 PM
To: Nicolas Carrier
Cc: Matthew Weber; Peter Korsgaard; rafal.fabich at gmail.com; buildroot
Subject: Re: [Buildroot] [PATCH 1/1] Added local directory as source of kernel code

Hello Nicolas,

Well your patch is actually making u-boot and linux be exceptions. What if someone wants to build busybox from a local source tree ? And qt5 ? And openssl ? And then this other package ?

As you can see, this approach does not really scale: we would have to add Config.in options to all packages to say "please build using the source code from folder XYZ instead of the normal upstream source code". That's why we have chosen to support this use case using OVERRIDE_SRCDIR.

Here is what you could do:

- Set BR2_PACKAGE_OVERRIDE_FILE to point to whatever file you like instead of local.mk, say package-overrides.mk.

- In this file, you set LINUX_OVERRIDE_SRCDIR as appropriate, but you also do

-include local.mk

And then you version control package-overrides.mk. Then your developer can continue locally to tinker with local.mk to have their own custom OVERRIDE_SRCDIR, and still have LINUX_OVERRIDE_SRCDIR used.

Alternatively, maybe we should decide that linux and u-boot are special packages, and just like for those packages we support fetching from custom tarballs and custom Git repositories, we should support fetching from custom local directories (this is what your patch implements, but until now, we have resisted going into this direction).

Best regards,

Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Bootlin ? Embedded Linux and kernel engineering<https://bootlin.com/>
bootlin.com
The Embedded Linux Conference Europe edition 2018 took place last week in Edinburgh, Scotland, and no less than 9 engineers from Bootlin attended the conference.

ATTENTION: This email came from an external source.
Do not open attachments or click on links from unknown senders or unexpected emails. -------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20181107/8bf0a47e/attachment.html>

Nicolas, Thomas, All,

On 2018-11-07 17:44 +0100, Thomas Petazzoni spake thusly:

Exactly what I was about to write.

Exactly what I was going to suggest, e.g.:

    BR2_PACKAGE_OVERRIDE_FILE="$(TOPDIR)/package-overrides.mk"

which would contain something like:

    LINUX_OVERRIDE_SRCDIR="$(TOPDIR)/../sources/linux"
    -include local.mk

Obviously, you want this file to always be used, so you set it in your defconfig files.

Alternatively, you can try to even be smart and snarky, and add a file in your Buildroot tree, name 'GNUmakefile'. GNU make will favour using that one if it is present, so you could do something like:

    $ cat GNUMakefile
    LINUX_OVERRIDE_SRCDIR="$(TOPDIR)/../sources/linux"
    include $(dir $(lastword $(MAKEFILE_LIST)))Makefile

This way, you do not even have to remember to set it in your defconfig files. But this is really snarky.

I would say we should not go that route, otherwise it would be difficult to resist adding yet more exceptions, for people that have local "forks" for some packages. And then it becomes a nightmare to maintain.

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

[snip]

... and also barebox, at91bootstrap, at91bootstrap3, arm-trusted-firmware,
mxs-bootlets, and I probably forget a few.

+1

So I've marked the patch as Rejected in patchwork.

Nicolas does have a point, though, that using OVERRIDE_SRCDIR in local.mk is a
non-intuitive way for doing this. So I have a few ideas to make it a little more convenient.

1. Add an additional config option to a file that is included just before local.mk. So completely equivalent to Thomas's suggestion, but a little "cleaner" because you'll get both the configured file and the local.mk as overrides.

2. In a BR2_EXTERNAL, a file with a well-known name (e.g. override.mk) is included and can contain overrides. Note that you can't specify overrides in the external.mk because that is included only after the packages, and _OVERRIDE_DIR has to be defined before the package is included.

3. Add an addition config option that points to a directory, where each subdirectory will be treated as a local override for a package with that name. Similar to BR2_GLOBAL_PATCH_DIR.

4. Like 3 but with a well-known name in BR2_EXTERNAL (i.e. everything in $(BR2_EXTERNAL_FOO)/override/ is used as an override dir.)

I actually use the equivalent of option 4 by pointing to an override.mk that
contains

AUTO_OVERRIDES = $(notdir $(wildcard $(BR2_EXTERNAL_FOO_PATH))/override/*)) do_override = $(1)_OVERRIDE_SRCDIR = $$(BR2_EXTERNAL_FOO_PATH)/src/$(2)

$(foreach override,$(AUTO_OVERRIDES),\
        $(eval $(call do_override,$(call UPPERCASE,$(override)),$(override))))

so it's easy enough to implement that.

Regards,
Arnout

Arnout, All,

On 2018-11-07 22:26 +0100, Arnout Vandecappelle spake thusly:
[--SNIP--]

I would suggest that we do not add another way of doing what can already be done with the current infrastructure, especially if it does *exactly* what is currently doable.

Instead, why not expand the help text, or add some examples to the manual?

That would be doable, but I'm not entirely sold on this either...

That would not work if the packages are not flat in that directory (i.e. if they are grouped by topics in sub-dirs, for example).

Same issue as (3).

I believe the solution that Thomas proposed is the way to handle those kinds of setups. I've been using variants of it for ages, and it works nicely.

The only slight drawback it has, is that it can't easily address packages which source trees are stored in a br2-external tree. But in the case of using a tool like repo, that does not change much, because the br2-external tree *has* to be managed by repo as well, so its location is known (relative to $(TOPDIR) at least), so it's still doable.

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

I just tested the method proposed by Thomas and as expected, it works fine, but what should I choose for the kernel version ?

I selected "Latest version", but then, the configuration shows a version which is not what is used in the end, which I find is really misleading. I persist saying that albeit perfectly functional, this solution is still a hack.

What's more all the other choices are less logical, we can use a custom git/svn/bzr repository through a local source tree, provided the cloning tool is able to fetch it. But accessing a local source tree with the current existing choices isn't possible. That's why I think that the LOCAL method should have been the first proposed choice.

________________________________
From: Yann E. MORIN <[email protected]> on behalf of Yann E. MORIN <[email protected]>
Sent: Wednesday, November 7, 2018 11:32:27 PM
To: Arnout Vandecappelle
Cc: Thomas Petazzoni; Peter Korsgaard; Nicolas Carrier; rafal.fabich at gmail.com; buildroot
Subject: Re: [Buildroot] [PATCH 1/1] Added local directory as source of kernel code

Arnout, All,

On 2018-11-07 22:26 +0100, Arnout Vandecappelle spake thusly:
[--SNIP--]

I would suggest that we do not add another way of doing what can already be done with the current infrastructure, especially if it does *exactly* what is currently doable.

Instead, why not expand the help text, or add some examples to the manual?

That would be doable, but I'm not entirely sold on this either...

That would not work if the packages are not flat in that directory (i.e. if they are grouped by topics in sub-dirs, for example).

Same issue as (3).

I believe the solution that Thomas proposed is the way to handle those kinds of setups. I've been using variants of it for ages, and it works nicely.

The only slight drawback it has, is that it can't easily address packages which source trees are stored in a br2-external tree. But in the case of using a tool like repo, that does not change much, because the br2-external tree *has* to be managed by repo as well, so its location is known (relative to $(TOPDIR) at least), so it's still doable.

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

'------------------------------^-------^------------------^--------------------'
ATTENTION: This email came from an external source.
Do not open attachments or click on links from unknown senders or unexpected emails. -------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20181108/2674d969/attachment.html>

Hello,

This is pretty nice, I like this idea.

Thomas

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Hello Yann,

Well, there are a few packages that we do consider "special", because for them we provide the possibility of specifying a custom tarball, a custom git/hg repository. So we do recognize that some packages are special, and we have added special logic to them.

So our argument "we don't want to go down that route as it would be a nightmare to have that for all packages" already doesn't hold very
well: we have some special logic for custom fetching in packages like linux, uboot or barebox, and we have not accepted to propagate such logic to other less special packages.

So, we have already identified some special packages for which it makes sense to provide more flexibility on how to fetch the source code, and it that sense, providing the option to use a local folder would seem quite logical.

Don't get me wrong: I am not pushing hard to this patch to be merged. I just want to point out that the argument we use to reject the patch is not very solid. And I don't blame you specifically, because I used the exact same argument :-)

Best regards,

Thomas

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Hi Nicolas,

In a situation like this, you can't keep the original Acked/Tested/Reviewed-by
tags, because the context has changed.

Also, there is (normally) no need to keep the conflicts information when you
rebase a patch.

Regards,
Arnout

Sorry, I missed your mail, outlook is soooo awful to follow a mailing list thread :(. I had to configure a mail client to see it.

Ok, I wasn't sure, thank you for the explanation. If there is still a chance that the patch can be applied, I'll repost it without the tags.

Ok, thank you again.

And sadly, on my side, I'll have to keep this patch (and the corresponding one for u-boot), because even if I dislike the idea of maintaining buildroot patches locally, having kernel and u-boot versions differ between what is built and what is set in the configuration is, a more serious issue.

I still have some hope that this patch can be considered for inclusion or that a clean alternative solution is proposed, though :).

Regards,
Nicolas