[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Nicolas Carrier]

From: Rafal Fabich <rafal.fabich@gmail.com>

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.

Signed-off-by: Rafal Fabich <rafal.fabich@gmail.com>
Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>

# Conflicts:
#	linux/Config.in

Signed-off-by: Nicolas Carrier <nicolas.carrier@orolia.com>
---
 linux/Config.in | 13 +++++++++++++
 linux/linux.mk  |  3 +++
 2 files changed, 16 insertions(+)

diff --git a/linux/Config.in b/linux/Config.in
index ecb12d0b16..e6c5c561b9 100644
--- a/linux/Config.in
+++ b/linux/Config.in
@@ -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
diff --git a/linux/linux.mk b/linux/linux.mk
index 476ff16329..c47c792926 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -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

[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Matthew Weber]

Nicolas,

On Wed, Nov 7, 2018 at 7:41 AM Nicolas Carrier
<nicolas.carrier@orolia.com> wrote:
>
> From: Rafal Fabich <rafal.fabich@gmail.com>
>
> 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.

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.

> * 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.
>
> Signed-off-by: Rafal Fabich <rafal.fabich@gmail.com>
> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
>
> # Conflicts:
> #       linux/Config.in
>
> Signed-off-by: Nicolas Carrier <nicolas.carrier@orolia.com>
> ---
>  linux/Config.in | 13 +++++++++++++
>  linux/linux.mk  |  3 +++
>  2 files changed, 16 insertions(+)
>
> diff --git a/linux/Config.in b/linux/Config.in
> index ecb12d0b16..e6c5c561b9 100644
> --- a/linux/Config.in
> +++ b/linux/Config.in
> @@ -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
> diff --git a/linux/linux.mk b/linux/linux.mk
> index 476ff16329..c47c792926 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -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
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot



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

[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Nicolas Carrier]

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 <matthew.weber@rockwellcollins.com>
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,

On Wed, Nov 7, 2018 at 7:41 AM Nicolas Carrier
<nicolas.carrier@orolia.com> wrote:
>
> From: Rafal Fabich <rafal.fabich@gmail.com>
>
> 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.

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.

> * 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.
>
> Signed-off-by: Rafal Fabich <rafal.fabich@gmail.com>
> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
>
> # Conflicts:
> #       linux/Config.in
>
> Signed-off-by: Nicolas Carrier <nicolas.carrier@orolia.com>
> ---
>  linux/Config.in | 13 +++++++++++++
>  linux/linux.mk  |  3 +++
>  2 files changed, 16 insertions(+)
>
> diff --git a/linux/Config.in b/linux/Config.in
> index ecb12d0b16..e6c5c561b9 100644
> --- a/linux/Config.in
> +++ b/linux/Config.in
> @@ -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
> diff --git a/linux/linux.mk b/linux/linux.mk
> index 476ff16329..c47c792926 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -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
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
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>

[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Thomas Petazzoni]

Hello Nicolas,

On Wed, 7 Nov 2018 14:40:39 +0000, Nicolas Carrier wrote:

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

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.

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

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

[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Nicolas Carrier]

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 <thomas.petazzoni@bootlin.com>
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,

On Wed, 7 Nov 2018 14:40:39 +0000, Nicolas Carrier wrote:

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

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.

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

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>

[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Yann E. MORIN]

Nicolas, Thomas, All,

On 2018-11-07 17:44 +0100, Thomas Petazzoni spake thusly:
> On Wed, 7 Nov 2018 14:40:39 +0000, Nicolas Carrier wrote:
> > 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.
> 
> 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 ?

Exactly what I was about to write.

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

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.

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

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

[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Arnout Vandecappelle]

 Hi Nicolas,

On 07/11/18 11:06, Nicolas Carrier wrote:
> From: Rafal Fabich <rafal.fabich@gmail.com>
> 
> 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.
> 
> Signed-off-by: Rafal Fabich <rafal.fabich@gmail.com>
> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

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

> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
> 
> # Conflicts:
> #	linux/Config.in

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


 Regards,
 Arnout

> 
> Signed-off-by: Nicolas Carrier <nicolas.carrier@orolia.com>

[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Arnout Vandecappelle]

On 07/11/18 19:52, Yann E. MORIN wrote:
> Nicolas, Thomas, All,
> 
> On 2018-11-07 17:44 +0100, Thomas Petazzoni spake thusly:
[snip]
>> Alternatively, maybe we should decide that linux and u-boot are special

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

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

 +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

[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Yann E. MORIN]

Arnout, All,

On 2018-11-07 22:26 +0100, Arnout Vandecappelle spake thusly:
[--SNIP--]
>  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.

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?

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

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

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

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

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

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.

>  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

-- 
.-----------------.--------------------.------------------.--------------------.
|  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/1] Added local directory as source of kernel code

[Nicolas Carrier]

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 <yann.morin.1998@gmail.com> on behalf of Yann E. MORIN <yann.morin.1998@free.fr>
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--]
>  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.

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?

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

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

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

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

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

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.

>  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

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

[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Thomas Petazzoni]

Hello Yann,

On Wed, 7 Nov 2018 19:52:26 +0100, Yann E. MORIN wrote:

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

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

[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Thomas Petazzoni]

Hello,

On Wed, 7 Nov 2018 22:26:50 +0100, Arnout Vandecappelle wrote:

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

This is pretty nice, I like this idea.

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

[Buildroot] [PATCH 1/1] Added local directory as source of kernel code

[Nicolas Carrier]

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.

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

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.

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

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