All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code"
@ 2016-06-27 22:34 Yann E. MORIN
  2016-08-27 20:23 ` Thomas Petazzoni
  0 siblings, 1 reply; 12+ messages in thread
From: Yann E. MORIN @ 2016-06-27 22:34 UTC (permalink / raw)
  To: buildroot

This reverts commit 73da2ff6f718f2889e3c5024d899f8d58f502863.

The reason for adding support for a local location was to be able to do
development on the Linux kernel source tree on a local direrctory rather
than have to clone it for every build.

We already have a mechanism for that, it's called override-srcdir. It's
been available since September 2011, more than a year before this patch
was committed.

Otherwise, we're going to be adding support for local sources in other
packages. First was U-Boot as submitted by Adam. But what next? We can't
have such support for all packages, especially since override-srcdir
does the job.

Besides, using a local source tree makes the build non-reproducible, so
we don't really want to have this in a .config (or defconfig).

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Rafal Fabich <rafal.fabich@gmail.com>
Cc: Peter Korsgaard <peter@korsgaard.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Adam Duskett <aduskett@gmail.com>

---
Rafal, if your use-case was just to use a local tree for development
purposes, please see to using the override-srcdir mechanism instead.
If you had another use-case that requires using a local source tree,
could you explain it, please? Thanks!
---
 linux/Config.in | 13 -------------
 linux/linux.mk  |  5 +----
 2 files changed, 1 insertion(+), 17 deletions(-)

diff --git a/linux/Config.in b/linux/Config.in
index a4ff8bc..a06fed2 100644
--- a/linux/Config.in
+++ b/linux/Config.in
@@ -69,12 +69,6 @@ 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
@@ -102,12 +96,6 @@ 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.6.3" if BR2_LINUX_KERNEL_LATEST_VERSION
@@ -116,7 +104,6 @@ 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 fb844ef..19a3a6d 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -16,14 +16,11 @@ define LINUX_HELP_CMDS
 endef
 
 # Compute LINUX_SOURCE and LINUX_SITE from the configuration
-ifeq ($(BR2_LINUX_KERNEL_CUSTOM_TARBALL),y)
+ifeq ($(LINUX_VERSION),custom)
 LINUX_TARBALL = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_TARBALL_LOCATION))
 LINUX_SITE = $(patsubst %/,%,$(dir $(LINUX_TARBALL)))
 LINUX_SOURCE = $(notdir $(LINUX_TARBALL))
 BR_NO_CHECK_HASH_FOR += $(LINUX_SOURCE)
-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.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code"
  2016-06-27 22:34 [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code" Yann E. MORIN
@ 2016-08-27 20:23 ` Thomas Petazzoni
  2016-08-27 22:54   ` Yann E. MORIN
  2016-08-28 20:37   ` Peter Korsgaard
  0 siblings, 2 replies; 12+ messages in thread
From: Thomas Petazzoni @ 2016-08-27 20:23 UTC (permalink / raw)
  To: buildroot

Peter, Yann,

Peter: do you have an opinion on this patch. If not, I think I'm going
to apply it, so please raise your opinion if you disagree.

Yann: a few questions below.

On Tue, 28 Jun 2016 00:34:04 +0200, Yann E. MORIN wrote:
> -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.

What about Config.in.legacy handling?

>  # Compute LINUX_SOURCE and LINUX_SITE from the configuration
> -ifeq ($(BR2_LINUX_KERNEL_CUSTOM_TARBALL),y)
> +ifeq ($(LINUX_VERSION),custom)

Why are you doing this change?

Thanks,

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code"
  2016-08-27 20:23 ` Thomas Petazzoni
@ 2016-08-27 22:54   ` Yann E. MORIN
  2016-08-28  7:23     ` Thomas Petazzoni
  2016-08-28 20:37   ` Peter Korsgaard
  1 sibling, 1 reply; 12+ messages in thread
From: Yann E. MORIN @ 2016-08-27 22:54 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2016-08-27 22:23 +0200, Thomas Petazzoni spake thusly:
> Peter, Yann,
> 
> Peter: do you have an opinion on this patch. If not, I think I'm going
> to apply it, so please raise your opinion if you disagree.
> 
> Yann: a few questions below.
> 
> On Tue, 28 Jun 2016 00:34:04 +0200, Yann E. MORIN wrote:
> > -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.
> 
> What about Config.in.legacy handling?

Bummer...

> >  # Compute LINUX_SOURCE and LINUX_SITE from the configuration
> > -ifeq ($(BR2_LINUX_KERNEL_CUSTOM_TARBALL),y)
> > +ifeq ($(LINUX_VERSION),custom)
> 
> Why are you doing this change?

Before, both local tarball and local directory could set LINUX_VERSION
to "custom". Now only the local tarball can. So I switched.

In restrospect, this change is not nice: all other cases deal with the
option from the choice, so we should indeed keep it for local tarball
too. Besides, LINUX_VERSION is derived from the local tarball option, so
better to use the "mother" option than the derived one.

I'll fix that and take care of legacy, then respin.

Thanks! :-)

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code"
  2016-08-27 22:54   ` Yann E. MORIN
@ 2016-08-28  7:23     ` Thomas Petazzoni
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Petazzoni @ 2016-08-28  7:23 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 28 Aug 2016 00:54:29 +0200, Yann E. MORIN wrote:

> > >  # Compute LINUX_SOURCE and LINUX_SITE from the configuration
> > > -ifeq ($(BR2_LINUX_KERNEL_CUSTOM_TARBALL),y)
> > > +ifeq ($(LINUX_VERSION),custom)  
> > 
> > Why are you doing this change?  
> 
> Before, both local tarball and local directory could set LINUX_VERSION
> to "custom". Now only the local tarball can. So I switched.
> 
> In restrospect, this change is not nice: all other cases deal with the
> option from the choice, so we should indeed keep it for local tarball
> too. Besides, LINUX_VERSION is derived from the local tarball option, so
> better to use the "mother" option than the derived one.

Exactly my thinking.

> I'll fix that and take care of legacy, then respin.

Thanks!

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code"
  2016-08-27 20:23 ` Thomas Petazzoni
  2016-08-27 22:54   ` Yann E. MORIN
@ 2016-08-28 20:37   ` Peter Korsgaard
  2016-08-28 21:48     ` Yann E. MORIN
  1 sibling, 1 reply; 12+ messages in thread
From: Peter Korsgaard @ 2016-08-28 20:37 UTC (permalink / raw)
  To: buildroot

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

 > Peter, Yann,
 > Peter: do you have an opinion on this patch. If not, I think I'm going
 > to apply it, so please raise your opinion if you disagree.

There probably are users of the option somewhere, so removing it will be
a bit annoying for them - But changing to override srcdir should be
fairly simple so I'm fine with it (for next).

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code"
  2016-08-28 20:37   ` Peter Korsgaard
@ 2016-08-28 21:48     ` Yann E. MORIN
  2016-08-29  7:16       ` Thomas Petazzoni
  0 siblings, 1 reply; 12+ messages in thread
From: Yann E. MORIN @ 2016-08-28 21:48 UTC (permalink / raw)
  To: buildroot

Peter, All,

On 2016-08-28 22:37 +0200, Peter Korsgaard spake thusly:
> >>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
> 
>  > Peter, Yann,
>  > Peter: do you have an opinion on this patch. If not, I think I'm going
>  > to apply it, so please raise your opinion if you disagree.
> 
> There probably are users of the option somewhere, so removing it will be
> a bit annoying for them

There are *always* users of any option we deprecate or remove.

All we can do is make it easy for them to upgrade. Override-srcdir has
been available for ages now (5 years in September), so it's not as if
it were something new or complex to use.

> - But changing to override srcdir should be
> fairly simple so I'm fine with it (for next).

Yes, obviously that's for next now.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code"
  2016-08-28 21:48     ` Yann E. MORIN
@ 2016-08-29  7:16       ` Thomas Petazzoni
  2016-12-12  3:06         ` James Knight
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Petazzoni @ 2016-08-29  7:16 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 28 Aug 2016 23:48:05 +0200, Yann E. MORIN wrote:

> There are *always* users of any option we deprecate or remove.
> 
> All we can do is make it easy for them to upgrade. Override-srcdir has
> been available for ages now (5 years in September), so it's not as if
> it were something new or complex to use.

The only major difference being that "override source dir" is meant to
be a "local" override, i.e something that developers do on their own
machine, for their own development. While the existing
BR2_LINUX_KERNEL_CUSTOM_LOCAL is really about having it as part of the
Buildroot configuration.

But nothing prevents people from having a version-controlled local.mk,
which makes it not-local anymore.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code"
  2016-08-29  7:16       ` Thomas Petazzoni
@ 2016-12-12  3:06         ` James Knight
  2016-12-12 16:27           ` Thomas Petazzoni
  2016-12-12 17:35           ` Yann E. MORIN
  0 siblings, 2 replies; 12+ messages in thread
From: James Knight @ 2016-12-12  3:06 UTC (permalink / raw)
  To: buildroot

All,

On Mon, Aug 29, 2016 at 3:16 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> The only major difference being that "override source dir" is meant to
> be a "local" override, i.e something that developers do on their own
> machine, for their own development. While the existing
> BR2_LINUX_KERNEL_CUSTOM_LOCAL is really about having it as part of the
> Buildroot configuration.
>
> But nothing prevents people from having a version-controlled local.mk,
> which makes it not-local anymore.

This.

(sorry for the delay. was trying trying to upgrade from a
2015.11.x-based series to a 2016.11.x-based series and just found out
that BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH is gone; original comments
here [1], if your email is past retention)

For my build environment, I prepare a series of sources before
invoking a Buildroot make. This includes preparing BR2_EXTERNAL
modules, cache content (dl) and my target kernel sources. The
initialization phase allows me to adjust source locations for a
specific build, for example, targeting a local Git repository for
kernel sources instead of a specific tarball. I was able to easily
achieve this by defining a custom local path in my Buildroot
configuration as follows:

    BR2_LINUX_KERNEL_CUSTOM_LOCAL=y
    BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH="$(MYCUSTOM_LINUX_DIR)"

With the revert of this patch, it seems the best "correct" approach is
to use the override-srcdir mechanism. As Thomas mentions though,
modifying my `local.mk` file to be under version control makes it no
longer local anymore. While I would love to have the feature back, if
this is a rare/odd usage, I don't mind modifying my repository to
support custom kernel local paths (rather than customizing a local.mk
file). Thoughts?

I don't believe BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH hurts
reproducibility in all cases. If a build environment ensures the
kernel custom path's content is managed properly for a build invoke,
would there be a reproducibility issue (unless I'm overlooking
something)?

---

On a related note, if the intent is to still leave this reverted,
BR2_LINUX_KERNEL_CUSTOM_LOCAL is still being reference in the
`linux-header` package [2] and `linux/Config.in` [3] (just dead code).
I can whip up a patch if no one wants to take a stab at it?

[1]: https://patchwork.ozlabs.org/patch/641254/
[2]: https://git.buildroot.net/buildroot/tree/package/linux-headers/linux-headers.mk?id=0d7383a0a7f5c69cb0e4a4eb0d32d2536cd7e0e8#n20
[3]: https://git.buildroot.net/buildroot/tree/linux/Config.in#n113

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code"
  2016-12-12  3:06         ` James Knight
@ 2016-12-12 16:27           ` Thomas Petazzoni
  2016-12-12 17:35           ` Yann E. MORIN
  1 sibling, 0 replies; 12+ messages in thread
From: Thomas Petazzoni @ 2016-12-12 16:27 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 11 Dec 2016 22:06:44 -0500, James Knight wrote:

> > But nothing prevents people from having a version-controlled local.mk,
> > which makes it not-local anymore.  
> 
> This.
> 
> (sorry for the delay. was trying trying to upgrade from a
> 2015.11.x-based series to a 2016.11.x-based series and just found out
> that BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH is gone; original comments
> here [1], if your email is past retention)
> 
> For my build environment, I prepare a series of sources before
> invoking a Buildroot make. This includes preparing BR2_EXTERNAL
> modules, cache content (dl) and my target kernel sources. The
> initialization phase allows me to adjust source locations for a
> specific build, for example, targeting a local Git repository for
> kernel sources instead of a specific tarball. I was able to easily
> achieve this by defining a custom local path in my Buildroot
> configuration as follows:
> 
>     BR2_LINUX_KERNEL_CUSTOM_LOCAL=y
>     BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH="$(MYCUSTOM_LINUX_DIR)"
> 
> With the revert of this patch, it seems the best "correct" approach is
> to use the override-srcdir mechanism. As Thomas mentions though,
> modifying my `local.mk` file to be under version control makes it no
> longer local anymore. While I would love to have the feature back, if
> this is a rare/odd usage, I don't mind modifying my repository to
> support custom kernel local paths (rather than customizing a local.mk
> file). Thoughts?
> 
> I don't believe BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH hurts
> reproducibility in all cases. If a build environment ensures the
> kernel custom path's content is managed properly for a build invoke,
> would there be a reproducibility issue (unless I'm overlooking
> something)?

I don't really feel very strongly about this.

However, what was odd with BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH is that
we were providing this for the Linux kernel package, but not for any
other package. I think the reason we removed it is because someone
wanted to add the same feature to U-Boot or some other package, and we
said we didn't want to add this possibility to each and every package
in Buildroot.

I am however surprised that you need something like this for a build
that will be done by others. Why is it that you need to "prepare your
kernel sources" before starting the build? Why aren't you referencing a
given commit in some Git repository?

In the worst case, adding local.mk (which you can rename, it's
configurable in BR2_PACKAGE_OVERRIDE_FILE) to your version control
system will do the trick. But I'm still interested in understanding
your more global use case for this functionality.

> On a related note, if the intent is to still leave this reverted,
> BR2_LINUX_KERNEL_CUSTOM_LOCAL is still being reference in the
> `linux-header` package [2] and `linux/Config.in` [3] (just dead code).
> I can whip up a patch if no one wants to take a stab at it?
> 
> [1]: https://patchwork.ozlabs.org/patch/641254/
> [2]: https://git.buildroot.net/buildroot/tree/package/linux-headers/linux-headers.mk?id=0d7383a0a7f5c69cb0e4a4eb0d32d2536cd7e0e8#n20
> [3]: https://git.buildroot.net/buildroot/tree/linux/Config.in#n113

Please submit a patch :-)

Thanks!

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code"
  2016-12-12  3:06         ` James Knight
  2016-12-12 16:27           ` Thomas Petazzoni
@ 2016-12-12 17:35           ` Yann E. MORIN
  2016-12-12 17:52             ` Thomas Petazzoni
  1 sibling, 1 reply; 12+ messages in thread
From: Yann E. MORIN @ 2016-12-12 17:35 UTC (permalink / raw)
  To: buildroot

James, All,

On 2016-12-11 22:06 -0500, James Knight spake thusly:
> On Mon, Aug 29, 2016 at 3:16 AM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
> > The only major difference being that "override source dir" is meant to
> > be a "local" override, i.e something that developers do on their own
> > machine, for their own development. While the existing
> > BR2_LINUX_KERNEL_CUSTOM_LOCAL is really about having it as part of the
> > Buildroot configuration.
> >
> > But nothing prevents people from having a version-controlled local.mk,
> > which makes it not-local anymore.
> 
> This.
> 
> (sorry for the delay. was trying trying to upgrade from a
> 2015.11.x-based series to a 2016.11.x-based series and just found out
> that BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH is gone; original comments
> here [1], if your email is past retention)
> 
> For my build environment, I prepare a series of sources before
> invoking a Buildroot make. This includes preparing BR2_EXTERNAL
> modules, cache content (dl) and my target kernel sources. The
> initialization phase allows me to adjust source locations for a
> specific build, for example, targeting a local Git repository for
> kernel sources instead of a specific tarball. I was able to easily
> achieve this by defining a custom local path in my Buildroot
> configuration as follows:
> 
>     BR2_LINUX_KERNEL_CUSTOM_LOCAL=y
>     BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH="$(MYCUSTOM_LINUX_DIR)"

But what I want is to be able to use local sources fior Qt5 that I
prepare for my board; so I'll want to add the same feature for Qt5.
Then Thomas will want to use local sources for barebox; so he'll want to
add the same feature for barebox. Then Peter will need it for whatever,
so he'll want to add it for that as well...

It's an endless hunt, and one we do not want to pursue.

We already have a way to specify a local source tree, it is the override
mechanism.

If your build system already "prepares" the sources for the linux kernel
(I gues, git clone, git checkout stuff, apply patches, whatever), it
means you have the opportunity to also generate a local.mk file with:

    LINUX_OVERRIDE_SRCDIR = $(MYCUSTOM_LINUX_DIR)

And as Thomas pointed out, the file can be renamed.

I personally have a local.mk that looks like:

    ################################################################################
    # <FOO>_OVERRIDE_SRCDIR go in there:
    -include $(dir $(lastword $(MAKEFILE_LIST)))/local-override.mk

    ################################################################################
    # Clean more, please!
    clean: local-clean
    distclean: local-distclean

    local-distclean: local-clean

    local-clean:
        @rm -f br.log
        @rm -f $(O)/br.log

So I have all my build dirs with this one local.mk that includes the
override one if it exists at all, and where I put the OVERRIDE_SRCDIR
variables.

Sorry, but I still believe your use-case does not require
BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH at all, and that you'd probably be
better served with using OVERRIDE_SRCDIR from a generated local.mk.

You can thus generate the linux tree however you want in your
Jenkins/Gitlab-CI/whatever/...

(although, like Thomas, I wonder why you don't simply have said sources
in git directly and refer to a sha1 or tag from your config files...)

Regards,
Yann E. MORIN.

> With the revert of this patch, it seems the best "correct" approach is
> to use the override-srcdir mechanism. As Thomas mentions though,
> modifying my `local.mk` file to be under version control makes it no
> longer local anymore. While I would love to have the feature back, if
> this is a rare/odd usage, I don't mind modifying my repository to
> support custom kernel local paths (rather than customizing a local.mk
> file). Thoughts?
> 
> I don't believe BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH hurts
> reproducibility in all cases. If a build environment ensures the
> kernel custom path's content is managed properly for a build invoke,
> would there be a reproducibility issue (unless I'm overlooking
> something)?
> 
> ---
> 
> On a related note, if the intent is to still leave this reverted,
> BR2_LINUX_KERNEL_CUSTOM_LOCAL is still being reference in the
> `linux-header` package [2] and `linux/Config.in` [3] (just dead code).
> I can whip up a patch if no one wants to take a stab at it?
> 
> [1]: https://patchwork.ozlabs.org/patch/641254/
> [2]: https://git.buildroot.net/buildroot/tree/package/linux-headers/linux-headers.mk?id=0d7383a0a7f5c69cb0e4a4eb0d32d2536cd7e0e8#n20
> [3]: https://git.buildroot.net/buildroot/tree/linux/Config.in#n113

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code"
  2016-12-12 17:35           ` Yann E. MORIN
@ 2016-12-12 17:52             ` Thomas Petazzoni
  2016-12-12 22:31               ` James Knight
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Petazzoni @ 2016-12-12 17:52 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon, 12 Dec 2016 18:35:27 +0100, Yann E. MORIN wrote:

> But what I want is to be able to use local sources fior Qt5 that I
> prepare for my board; so I'll want to add the same feature for Qt5.
> Then Thomas will want to use local sources for barebox; so he'll want to
> add the same feature for barebox. Then Peter will need it for whatever,
> so he'll want to add it for that as well...

One could however say (with good reason) that the Linux kernel, and a
few other HW-related packages (bootloaders) are special.

And indeed, for those packages (Linux, U-Boot, Barebox, etc.), we do
support fetching from custom Git/Mercurial/etc. repositories. But we
would never do that for *all* packages.

So Linux is already special compared to other packages in that respect,
and this argument could be used (and in fact was used) to add a feature
like BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH.

But as I said, I don't really feel very strongly about this.

Best regards,

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code"
  2016-12-12 17:52             ` Thomas Petazzoni
@ 2016-12-12 22:31               ` James Knight
  0 siblings, 0 replies; 12+ messages in thread
From: James Knight @ 2016-12-12 22:31 UTC (permalink / raw)
  To: buildroot

Thomas, Yann;

On Mon, Dec 12, 2016 at 11:27 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> I think the reason we removed it is because someone
> wanted to add the same feature to U-Boot or some other package, and we
> said we didn't want to add this possibility to each and every package
> in Buildroot.
...
On Mon, Dec 12, 2016 at 12:35 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> It's an endless hunt, and one we do not want to pursue.

I completely understand. The only reason why thought it was more
"acceptable" for Linux kernel sources is just as Thomas mentioned:

On Mon, Dec 12, 2016 at 12:52 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> One could however say (with good reason) that the Linux kernel, and a
> few other HW-related packages (bootloaders) are special.

That being said, it doesn't mean I couldn't just make respective
modifications into `local.mk`. My initial reasoning for wanting to
avoid the override file was that I didn't want to require/share the
use of the `local.mk` file. I didn't want to impose changes in my
Buildroot repository or my external Buildroot module (BR2_EXTERNAL)
that could affect others using the same modules but for other purposes
(ie. their own platform work). I did, however, overlook the
BR2_PACKAGE_OVERRIDE_FILE configuration option. I should be able to
use this, point to a custom sub-folder for my platform variant and
make appropriate changes for pulling in kernel sources. I'll just hope
any future modifications to my dependent Buildroot external modules do
not force the use of their own BR2_PACKAGE_OVERRIDE_FILE (which I
assume they should never do). Also, developers needing to make their
own system-testing modifications can just modify the file specified in
BR2_PACKAGE_OVERRIDE_FILE (instead of local.mk) when dealing with
custom tests for the platform variants using this kernel-fetch
modification (although, I could probably just add a line to chain an
existing local.mk file as well).

On Mon, Dec 12, 2016 at 11:27 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Why is it that you need to "prepare your
> kernel sources" before starting the build? Why aren't you referencing a
> given commit in some Git repository?
> ...
> But I'm still interested in understanding
> your more global use case for this functionality.

Now knowing of alternative methods available, preference. For my
environment, I drive all my builds from a `releng` module. To produce
a complete build of all my current platform variants, I require a
Buildroot repository, external Buildroot repositories (well, only one
(1) at this time), kernel repository and cache repository (dl
packages). The driver of releng can specify where to acquire any
repository from a series of servers, just custom locations (if sources
have been prepared using other means; ie. a developer environment,
Jenkins, etc.) or mixed. By defining all my variants to have something
like this:

    BR2_LINUX_KERNEL_CUSTOM_LOCAL=y
    BR2_LINUX_KERNEL_CUSTOM_LOCAL_PATH="$(MYCUSTOM_LINUX_DIR)"
    BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y
    BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="$(BR2_EXTERNAL)/board/proj_x86_MYCUSTOM/configurations/linux-${MYCUSTOM_LINUX_VER_SERIES}-x86.config"

I'm able to target variant kernel versions/branches using the releng
module without modifying my variant Buildroot configurations
(although, this is probably something Buildroot (by design) may be
trying to avoid). What I like about this approach is that I can also
perform the fetch stage independent to the build stage, which has
helped in a series of minor use cases (configuration management,
quick-prep for pending tags, etc.); however, even though I may like
this type of approach, it may not be ideal for Buildroot (for
maintenance, etc.). I'll try to focus on getting
BR2_PACKAGE_OVERRIDE_FILE (and respective changes) working.

---

I assume when I eventually add LINUX_OVERRIDE_SRCDIR (and possibly
LINUX_HEADERS_OVERRIDE_SRCDIR), it doesn't matter which kernel
(header) choice configuration is selected?

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-12-12 22:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-27 22:34 [Buildroot] [PATCH RFC] Revert "Added local directory as source of kernel code" Yann E. MORIN
2016-08-27 20:23 ` Thomas Petazzoni
2016-08-27 22:54   ` Yann E. MORIN
2016-08-28  7:23     ` Thomas Petazzoni
2016-08-28 20:37   ` Peter Korsgaard
2016-08-28 21:48     ` Yann E. MORIN
2016-08-29  7:16       ` Thomas Petazzoni
2016-12-12  3:06         ` James Knight
2016-12-12 16:27           ` Thomas Petazzoni
2016-12-12 17:35           ` Yann E. MORIN
2016-12-12 17:52             ` Thomas Petazzoni
2016-12-12 22:31               ` James Knight

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.