Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [Patch v2 1/1] Fix selection of bootloaders from $(BR2_EXTERNAL)
@ 2014-10-07 13:01 Eric Le Bihan
  2014-10-07 13:48 ` Thomas Petazzoni
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Le Bihan @ 2014-10-07 13:01 UTC (permalink / raw)
  To: buildroot

If a package is based on "generic-package", pkg-generic.mk will compute
the name of the Kconfig variable to use for checking if this package has
been selected by the user.

Unfortunately, this mechanism does not take into account the case where
a bootloader is declared in a $(BR2_EXTERNAL)/boot directory.

So, even if the bootloader has been selected, it will not be added to
$(TARGETS) and will not be built.

This patch fixes this issue, as well as handle toolchains.

Changes v1 -> v2:

  - use multiple patterns in filter (suggested by ThomasDS).
  - handle toolchains.

Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
---
 package/pkg-generic.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index d04fd36..e201803 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -614,9 +614,9 @@ $$($(2)_TARGET_DIRCLEAN):		PKG=$(2)
 # kernel case, the bootloaders case, and the normal packages case.
 ifeq ($(1),linux)
 $(2)_KCONFIG_VAR = BR2_LINUX_KERNEL
-else ifneq ($$(filter boot/%,$(pkgdir)),)
+else ifneq ($$(filter boot/% $(BR2_EXTERNAL)/boot/%,$(pkgdir)),)
 $(2)_KCONFIG_VAR = BR2_TARGET_$(2)
-else ifneq ($$(filter toolchain/%,$(pkgdir)),)
+else ifneq ($$(filter toolchain/% $(BR2_EXTERNAL)/toolchain/%,$(pkgdir)),)
 $(2)_KCONFIG_VAR = BR2_$(2)
 else
 $(2)_KCONFIG_VAR = BR2_PACKAGE_$(2)
--
1.9.1

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

* [Buildroot] [Patch v2 1/1] Fix selection of bootloaders from $(BR2_EXTERNAL)
  2014-10-07 13:01 [Buildroot] [Patch v2 1/1] Fix selection of bootloaders from $(BR2_EXTERNAL) Eric Le Bihan
@ 2014-10-07 13:48 ` Thomas Petazzoni
  2014-10-07 21:29   ` Eric Le Bihan
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2014-10-07 13:48 UTC (permalink / raw)
  To: buildroot

Dear Eric Le Bihan,

On Tue,  7 Oct 2014 15:01:32 +0200, Eric Le Bihan wrote:
> If a package is based on "generic-package", pkg-generic.mk will compute
> the name of the Kconfig variable to use for checking if this package has
> been selected by the user.
> 
> Unfortunately, this mechanism does not take into account the case where
> a bootloader is declared in a $(BR2_EXTERNAL)/boot directory.
> 
> So, even if the bootloader has been selected, it will not be added to
> $(TARGETS) and will not be built.
> 
> This patch fixes this issue, as well as handle toolchains.
> 
> Changes v1 -> v2:
> 
>   - use multiple patterns in filter (suggested by ThomasDS).
>   - handle toolchains.
> 
> Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
> ---
>  package/pkg-generic.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index d04fd36..e201803 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -614,9 +614,9 @@ $$($(2)_TARGET_DIRCLEAN):		PKG=$(2)
>  # kernel case, the bootloaders case, and the normal packages case.
>  ifeq ($(1),linux)
>  $(2)_KCONFIG_VAR = BR2_LINUX_KERNEL
> -else ifneq ($$(filter boot/%,$(pkgdir)),)
> +else ifneq ($$(filter boot/% $(BR2_EXTERNAL)/boot/%,$(pkgdir)),)
>  $(2)_KCONFIG_VAR = BR2_TARGET_$(2)
> -else ifneq ($$(filter toolchain/%,$(pkgdir)),)
> +else ifneq ($$(filter toolchain/% $(BR2_EXTERNAL)/toolchain/%,$(pkgdir)),)
>  $(2)_KCONFIG_VAR = BR2_$(2)
>  else
>  $(2)_KCONFIG_VAR = BR2_PACKAGE_$(2)

Well, this makes the assumption that $(BR2_EXTERNAL) bootloaders should
be in boot/ and $(BR2_EXTERNAL) toolchain stuff should be in toolchain/.

In practice, the boot/ and toolchain/ organization in the main
Buildroot does not necessarily need to be reflected in $(BR2_EXTERNAL):
you can have a bootloader in $(BR2_EXTERNAL)/package/foobar/, just name
its option BR2_PACKAGE_FOOBAR and not BR2_TARGET_FOOBAR.

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 v2 1/1] Fix selection of bootloaders from $(BR2_EXTERNAL)
  2014-10-07 13:48 ` Thomas Petazzoni
@ 2014-10-07 21:29   ` Eric Le Bihan
  2014-10-07 22:29     ` Yann E. MORIN
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Le Bihan @ 2014-10-07 21:29 UTC (permalink / raw)
  To: buildroot

Thomas, All

On Tue, Oct 07, 2014 at 03:48:51PM +0200, Thomas Petazzoni wrote:
> Dear Eric Le Bihan,
>
> On Tue,  7 Oct 2014 15:01:32 +0200, Eric Le Bihan wrote:
> > If a package is based on "generic-package", pkg-generic.mk will compute
> > the name of the Kconfig variable to use for checking if this package has
> > been selected by the user.
> >
> > Unfortunately, this mechanism does not take into account the case where
> > a bootloader is declared in a $(BR2_EXTERNAL)/boot directory.
> >
> > So, even if the bootloader has been selected, it will not be added to
> > $(TARGETS) and will not be built.
> >
> > This patch fixes this issue, as well as handle toolchains.
> >
> > Changes v1 -> v2:
> >
> >   - use multiple patterns in filter (suggested by ThomasDS).
> >   - handle toolchains.
> >
> > Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr>
> > ---
> >  package/pkg-generic.mk | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> > index d04fd36..e201803 100644
> > --- a/package/pkg-generic.mk
> > +++ b/package/pkg-generic.mk
> > @@ -614,9 +614,9 @@ $$($(2)_TARGET_DIRCLEAN):		PKG=$(2)
> >  # kernel case, the bootloaders case, and the normal packages case.
> >  ifeq ($(1),linux)
> >  $(2)_KCONFIG_VAR = BR2_LINUX_KERNEL
> > -else ifneq ($$(filter boot/%,$(pkgdir)),)
> > +else ifneq ($$(filter boot/% $(BR2_EXTERNAL)/boot/%,$(pkgdir)),)
> >  $(2)_KCONFIG_VAR = BR2_TARGET_$(2)
> > -else ifneq ($$(filter toolchain/%,$(pkgdir)),)
> > +else ifneq ($$(filter toolchain/% $(BR2_EXTERNAL)/toolchain/%,$(pkgdir)),)
> >  $(2)_KCONFIG_VAR = BR2_$(2)
> >  else
> >  $(2)_KCONFIG_VAR = BR2_PACKAGE_$(2)
>
> Well, this makes the assumption that $(BR2_EXTERNAL) bootloaders should
> be in boot/ and $(BR2_EXTERNAL) toolchain stuff should be in toolchain/.
>
> In practice, the boot/ and toolchain/ organization in the main
> Buildroot does not necessarily need to be reflected in $(BR2_EXTERNAL):
> you can have a bootloader in $(BR2_EXTERNAL)/package/foobar/, just name
> its option BR2_PACKAGE_FOOBAR and not BR2_TARGET_FOOBAR.

On the contrary, I find it confusing *not* to reflect the main Buildroot
organization in $(BR2_EXTERNAL).

To me, the $(BR2_EXTERNAL)/package directory should contain Makefiles for
programs to be deployed in the rootfs of the target or host tools to build
them. But a bootloader is not to be deployed on the rootfs, but to be built
next to it. So it is more sensible to put the Makefile for a new/proprietary
bootloader in $(BR2_EXTERNAL)/boot.

For $(BR2_EXTERNAL)/toolchain, a user may want to use a proprietary toolchain
that is not available as a tarball and thus not installable via the
traditional means.

Best regards,
ELB

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

* [Buildroot] [Patch v2 1/1] Fix selection of bootloaders from $(BR2_EXTERNAL)
  2014-10-07 21:29   ` Eric Le Bihan
@ 2014-10-07 22:29     ` Yann E. MORIN
  2014-10-08  8:09       ` Thomas Petazzoni
  0 siblings, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2014-10-07 22:29 UTC (permalink / raw)
  To: buildroot

Eric, Thomas, All,

On 2014-10-07 23:29 +0200, Eric Le Bihan spake thusly:
> On Tue, Oct 07, 2014 at 03:48:51PM +0200, Thomas Petazzoni wrote:
> > Dear Eric Le Bihan,
> >
> > On Tue,  7 Oct 2014 15:01:32 +0200, Eric Le Bihan wrote:
> > > If a package is based on "generic-package", pkg-generic.mk will compute
> > > the name of the Kconfig variable to use for checking if this package has
> > > been selected by the user.
> > >
> > > Unfortunately, this mechanism does not take into account the case where
> > > a bootloader is declared in a $(BR2_EXTERNAL)/boot directory.
> > >
> > > So, even if the bootloader has been selected, it will not be added to
> > > $(TARGETS) and will not be built.
> > >
> > > This patch fixes this issue, as well as handle toolchains.
[--SNIP--]
> > Well, this makes the assumption that $(BR2_EXTERNAL) bootloaders should
> > be in boot/ and $(BR2_EXTERNAL) toolchain stuff should be in toolchain/.
> >
> > In practice, the boot/ and toolchain/ organization in the main
> > Buildroot does not necessarily need to be reflected in $(BR2_EXTERNAL):
> > you can have a bootloader in $(BR2_EXTERNAL)/package/foobar/, just name
> > its option BR2_PACKAGE_FOOBAR and not BR2_TARGET_FOOBAR.
> 
> On the contrary, I find it confusing *not* to reflect the main Buildroot
> organization in $(BR2_EXTERNAL).
> 
> To me, the $(BR2_EXTERNAL)/package directory should contain Makefiles for
> programs to be deployed in the rootfs of the target or host tools to build
> them. But a bootloader is not to be deployed on the rootfs, but to be built
> next to it. So it is more sensible to put the Makefile for a new/proprietary
> bootloader in $(BR2_EXTERNAL)/boot.

I have to agree with Eric here: I think allowing (not enforcing) the
same layout in br2-external as in Buildroot is a good thing.

> For $(BR2_EXTERNAL)/toolchain, a user may want to use a proprietary toolchain
> that is not available as a tarball and thus not installable via the
> traditional means.

But does that even work at all?

Toolchains are not packages; br2-external was not designed to get new
toolchain definitions. That it works, if it does at all, is just merely
happenstance.

I wonder if/how we should/could get this to work.

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] 6+ messages in thread

* [Buildroot] [Patch v2 1/1] Fix selection of bootloaders from $(BR2_EXTERNAL)
  2014-10-07 22:29     ` Yann E. MORIN
@ 2014-10-08  8:09       ` Thomas Petazzoni
  2014-10-08  8:50         ` Eric Le Bihan
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2014-10-08  8:09 UTC (permalink / raw)
  To: buildroot

Yann, Eric,

On Wed, 8 Oct 2014 00:29:33 +0200, Yann E. MORIN wrote:

> > To me, the $(BR2_EXTERNAL)/package directory should contain Makefiles for
> > programs to be deployed in the rootfs of the target or host tools to build
> > them. But a bootloader is not to be deployed on the rootfs, but to be built
> > next to it. So it is more sensible to put the Makefile for a new/proprietary
> > bootloader in $(BR2_EXTERNAL)/boot.
> 
> I have to agree with Eric here: I think allowing (not enforcing) the
> same layout in br2-external as in Buildroot is a good thing.

Well, ok. So let's go with the patch, but maybe with an update to the
manual?

> > For $(BR2_EXTERNAL)/toolchain, a user may want to use a proprietary toolchain
> > that is not available as a tarball and thus not installable via the
> > traditional means.
> 
> But does that even work at all?
> 
> Toolchains are not packages; br2-external was not designed to get new
> toolchain definitions. That it works, if it does at all, is just merely
> happenstance.
> 
> I wonder if/how we should/could get this to work.

Clearly, that cannot work: how would one re-use the external toolchain
logic to copy the sysroot, the target libraries and so on. Plus, the
existing external toolchain mechanism already allows to use the tarball
of a toolchain.

Best regards,

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 v2 1/1] Fix selection of bootloaders from $(BR2_EXTERNAL)
  2014-10-08  8:09       ` Thomas Petazzoni
@ 2014-10-08  8:50         ` Eric Le Bihan
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Le Bihan @ 2014-10-08  8:50 UTC (permalink / raw)
  To: buildroot

On Wed, Oct 08, 2014 at 10:09:54AM +0200, Thomas Petazzoni wrote:
> Yann, Eric,
>
> On Wed, 8 Oct 2014 00:29:33 +0200, Yann E. MORIN wrote:
>
> > > To me, the $(BR2_EXTERNAL)/package directory should contain Makefiles for
> > > programs to be deployed in the rootfs of the target or host tools to build
> > > them. But a bootloader is not to be deployed on the rootfs, but to be built
> > > next to it. So it is more sensible to put the Makefile for a new/proprietary
> > > bootloader in $(BR2_EXTERNAL)/boot.
> >
> > I have to agree with Eric here: I think allowing (not enforcing) the
> > same layout in br2-external as in Buildroot is a good thing.
>
> Well, ok. So let's go with the patch, but maybe with an update to the
> manual?
>
> > > For $(BR2_EXTERNAL)/toolchain, a user may want to use a proprietary toolchain
> > > that is not available as a tarball and thus not installable via the
> > > traditional means.
> >
> > But does that even work at all?
> >
> > Toolchains are not packages; br2-external was not designed to get new
> > toolchain definitions. That it works, if it does at all, is just merely
> > happenstance.
> >
> > I wonder if/how we should/could get this to work.
>
> Clearly, that cannot work: how would one re-use the external toolchain
> logic to copy the sysroot, the target libraries and so on. Plus, the
> existing external toolchain mechanism already allows to use the tarball
> of a toolchain.

So, it would be best to remove the toolchain part of the patch, keeping only
the bootloader one. I'll add a comment about it in
docs/manual/customize-outside-br.txt and send a new version of the patch.

Best regards,
ELB

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

end of thread, other threads:[~2014-10-08  8:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-07 13:01 [Buildroot] [Patch v2 1/1] Fix selection of bootloaders from $(BR2_EXTERNAL) Eric Le Bihan
2014-10-07 13:48 ` Thomas Petazzoni
2014-10-07 21:29   ` Eric Le Bihan
2014-10-07 22:29     ` Yann E. MORIN
2014-10-08  8:09       ` Thomas Petazzoni
2014-10-08  8:50         ` Eric Le Bihan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox