diff mbox

[2/3] Add support for BR2_EXTERNAL

Message ID OF916AAFB6.8E78891C-ON86257BE3.000A49F0-86257BE3.000B5AF0@rockwellcollins.com
State Not Applicable
Headers show

Commit Message

Ryan Barnett Sept. 11, 2013, 2:03 a.m. UTC
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on 09/08/2013 
08:15:28 AM:

> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  Config.in             |  6 ++++--
>  Makefile              | 29 +++++++++++++++++++++++++----
>  docs/manual/manual.mk |  2 +-

One idea that I have, is for the make file that is genreated for the an
external output directory, if we could preserve the the value of 
BR2_EXTERNAL
as follows. I don't think my logic is fully there but there is more of a 
sudo implementation:


Again this is only a suggestion but otherwise I'll ack the patchset.

Thanks,
-Ryan




Ryan J Barnett / Software Engineer / Platform SW 
MS 137-157, 855 35th St NE, Cedar Rapids, IA, 52498-3161, US
Phone: 319-263-3880 / VPN: 263-3880 
rjbarnet@rockwellcollins.com
www.rockwellcollins.com

Comments

Yann E. MORIN Sept. 11, 2013, 5:03 p.m. UTC | #1
Ryan, All,

On 2013-09-10 21:03 -0500, rjbarnet@rockwellcollins.com spake thusly:
> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on 09/08/2013 
> 08:15:28 AM:
> 
> > 
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > ---
> >  Config.in             |  6 ++++--
> >  Makefile              | 29 +++++++++++++++++++++++++----
> >  docs/manual/manual.mk |  2 +-
> 
> One idea that I have, is for the make file that is genreated for the an
> external output directory, if we could preserve the the value of 
> BR2_EXTERNAL
> as follows. I don't think my logic is fully there but there is more of a 
> sudo implementation:
> 
> diff --git a/support/scripts/mkmakefile b/support/scripts/mkmakefile
> --- a/support/scripts/mkmakefile
> +++ b/support/scripts/mkmakefile
> @@ -27,6 +27,7 @@ makedir := \$(dir \$(call lastword,\$(MAKEFILE_LIST)))
> 
>  MAKEARGS := -C $1
>  MAKEARGS += O=\$(if \$(patsubst /%,,\$(makedir)),\$(CURDIR)/)\$(patsubst 
> %/,%,\$(makedir))
> +MAKEARGS += BR2_EXTERNAL=\$(realpath \$(BR2_EXTERNAL))

Why realpath? I think we should pass the exact value of BR2_EXTERNAL.

Otherwise, I agree that this could be usefull.

Regards,
Yann E. MORIN.
Ryan Barnett Sept. 11, 2013, 5:12 p.m. UTC | #2
"Yann E. MORIN" <yann.morin.1998@gmail.com> wrote on 09/11/2013 12:03:40 
PM:

> > One idea that I have, is for the make file that is genreated for the 
an
> > external output directory, if we could preserve the the value of 
> > BR2_EXTERNAL
> > as follows. I don't think my logic is fully there but there is more of 
a 
> > sudo implementation:
> > 
> > diff --git a/support/scripts/mkmakefile b/support/scripts/mkmakefile
> > --- a/support/scripts/mkmakefile
> > +++ b/support/scripts/mkmakefile
> > @@ -27,6 +27,7 @@ makedir := \$(dir \$(call 
lastword,\$(MAKEFILE_LIST)))
> > 
> >  MAKEARGS := -C $1
> >  MAKEARGS += O=\$(if \$(patsubst 
/%,,\$(makedir)),\$(CURDIR)/)\$(patsubst 
> > %/,%,\$(makedir))
> > +MAKEARGS += BR2_EXTERNAL=\$(realpath \$(BR2_EXTERNAL))
> 
> Why realpath? I think we should pass the exact value of BR2_EXTERNAL.

I would agree, like I said when I suggested this I didn't think it all the
way through other than this feature would be very useful.

> Otherwise, I agree that this could be usefull.
> 
> Regards,
> Yann E. MORIN.
Arnout Vandecappelle Sept. 12, 2013, 9:05 p.m. UTC | #3
On 11/09/13 04:03, rjbarnet@rockwellcollins.com wrote:
> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on
> 09/08/2013 08:15:28 AM:
>
>  >
>  > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>  > ---
>  >  Config.in   |  6 ++++--
>  >  Makefile    | 29 +++++++++++++++++++++++++----
>  >  docs/manual/manual.mk |  2 +-
>
> One idea that I have, is for the make file that is genreated for the an
> external output directory, if we could preserve the the value of
> BR2_EXTERNAL
> as follows.

  Thomas's patch already does that, because BR2_EXTERNAL is stored in the 
config file.

> I don't think my logic is fully there but there is more of a
> sudo implementation:
>
> diff --git a/support/scripts/mkmakefile b/support/scripts/mkmakefile
> --- a/support/scripts/mkmakefile
> +++ b/support/scripts/mkmakefile
> @@ -27,6 +27,7 @@ makedir := \$(dir \$(call lastword,\$(MAKEFILE_LIST)))
>
>   MAKEARGS := -C $1
>   MAKEARGS += O=\$(if \$(patsubst
> /%,,\$(makedir)),\$(CURDIR)/)\$(patsubst %/,%,\$(makedir))
> +MAKEARGS += BR2_EXTERNAL=\$(realpath \$(BR2_EXTERNAL))
>
>   MAKEFLAGS += --no-print-directory
>
> Again this is only a suggestion but otherwise I'll ack the patchset.
>
> Thanks,
> -Ryan
>
> -------------------------------------------------------------------------
> 	*Ryan J Barnett* / Software Engineer / Platform SW
> MS 137-157, 855 35th St NE, Cedar Rapids, IA, 52498-3161, US
> Phone: 319-263-3880 / VPN: 263-3880 _
> __rjbarnet@rockwellcollins.com_ <mailto:rjbarnet@rockwellcollins.com>_
> __www.rockwellcollins.com_ <http://www.rockwellcollins.com/>
>
>
>
>
>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>
Ryan Barnett Sept. 12, 2013, 9:30 p.m. UTC | #4
Arnout Vandecappelle <arnout@mind.be> wrote on 09/12/2013 04:05:18 PM:

> On 11/09/13 04:03, rjbarnet@rockwellcollins.com wrote:
> > Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on
> > 09/08/2013 08:15:28 AM:
> >
> >  >
> >  > Signed-off-by: Thomas Petazzoni 
<thomas.petazzoni@free-electrons.com>
> >  > ---
> >  >  Config.in   |  6 ++++--
> >  >  Makefile    | 29 +++++++++++++++++++++++++----
> >  >  docs/manual/manual.mk |  2 +-
> >
> > One idea that I have, is for the make file that is genreated for the 
an
> > external output directory, if we could preserve the the value of
> > BR2_EXTERNAL
> > as follows.
> 
>   Thomas's patch already does that, because BR2_EXTERNAL is stored in 
the 
> config file.
> 

I don't believe it gets stored in the config since Config.in pulls
the variable in from the shell's or make's environment. 

+config BR2_EXTERNAL
+       string
+       option env="BR2_EXTERNAL"

In order to utilize the BR2_EXTERNAL feature I must have BR2_EXTERNAL
either exported into my environment or be called as 

'make BR2_EXTERNAL=/some/path <target>'


When utilizing the BR2_EXTERNAL I usual want to use some defconfig from
my BR2_EXTERNAL so it adding it the makefile that is generated in your 
build directory when specifying O= would be extermely useful as that 
way I can switch shells and my definition of BR2_EXTERNAL would still 
be present.

'make O=/build/path BR2_EXTERNAL=/path/to/external defconfig'

> > I don't think my logic is fully there but there is more of a
> > sudo implementation:
> >
> > diff --git a/support/scripts/mkmakefile b/support/scripts/mkmakefile
> > --- a/support/scripts/mkmakefile
> > +++ b/support/scripts/mkmakefile
> > @@ -27,6 +27,7 @@ makedir := \$(dir \$(call 
lastword,\$(MAKEFILE_LIST)))
> >
> >   MAKEARGS := -C $1
> >   MAKEARGS += O=\$(if \$(patsubst
> > /%,,\$(makedir)),\$(CURDIR)/)\$(patsubst %/,%,\$(makedir))
> > +MAKEARGS += BR2_EXTERNAL=\$(realpath \$(BR2_EXTERNAL))
> >
> >   MAKEFLAGS += --no-print-directory
> >
> > Again this is only a suggestion but otherwise I'll ack the patchset.
> >
Arnout Vandecappelle Sept. 12, 2013, 9:41 p.m. UTC | #5
On 12/09/13 23:30, Ryan Barnett wrote:
> Arnout Vandecappelle <arnout@mind.be> wrote on 09/12/2013 04:05:18 PM:
>
>  > On 11/09/13 04:03, rjbarnet@rockwellcollins.com wrote:
>  > > Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on
>  > > 09/08/2013 08:15:28 AM:
>  > >
>  > >  >
>  > >  > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>  > >  > ---
>  > >  >  Config.in   |  6 ++++--
>  > >  >  Makefile    | 29 +++++++++++++++++++++++++----
>  > >  >  docs/manual/manual.mk |  2 +-
>  > >
>  > > One idea that I have, is for the make file that is genreated for the an
>  > > external output directory, if we could preserve the the value of
>  > > BR2_EXTERNAL
>  > > as follows.
>  >
>  >   Thomas's patch already does that, because BR2_EXTERNAL is stored in the
>  > config file.
>  >
>
> I don't believe it gets stored in the config since Config.in pulls
> the variable in from the shell's or make's environment.
>
> +config BR2_EXTERNAL
> +       string
> +       option env="BR2_EXTERNAL"
>
> In order to utilize the BR2_EXTERNAL feature I must have BR2_EXTERNAL
> either exported into my environment or be called as

  The Makefile exports it in the environment of the Kconfig targets. So 
once it is in the .config, it will stay there. Try it and you'll see :-)

>
> 'make BR2_EXTERNAL=/some/path <target>'
>
>
> When utilizing the BR2_EXTERNAL I usual want to use some defconfig from
> my BR2_EXTERNAL so it adding it the makefile that is generated in your
> build directory when specifying O= would be extermely useful as that
> way I can switch shells and my definition of BR2_EXTERNAL would still
> be present.
>
> 'make O=/build/path BR2_EXTERNAL=/path/to/external defconfig'

  In this particular case, you're not using the Makefile in the output 
dir but only in the buildroot dir so it's not relevant.

  The only case where it would be relevant is when you have an existing 
output directory, you remove the .config first, and then you do a
make foo_defconfig. Seems a bit an unlikely scenario.

  Regards,
  Arnout

>
>  > > I don't think my logic is fully there but there is more of a
>  > > sudo implementation:
>  > >
>  > > diff --git a/support/scripts/mkmakefile b/support/scripts/mkmakefile
>  > > --- a/support/scripts/mkmakefile
>  > > +++ b/support/scripts/mkmakefile
>  > > @@ -27,6 +27,7 @@ makedir := \$(dir \$(call lastword,\$(MAKEFILE_LIST)))
>  > >
>  > >   MAKEARGS := -C $1
>  > >   MAKEARGS += O=\$(if \$(patsubst
>  > > /%,,\$(makedir)),\$(CURDIR)/)\$(patsubst %/,%,\$(makedir))
>  > > +MAKEARGS += BR2_EXTERNAL=\$(realpath \$(BR2_EXTERNAL))
>  > >
>  > >   MAKEFLAGS += --no-print-directory
>  > >
>  > > Again this is only a suggestion but otherwise I'll ack the patchset.
>  > >
>
Ryan Barnett Sept. 12, 2013, 9:51 p.m. UTC | #6
Arnout Vandecappelle <arnout@mind.be> wrote on 09/12/2013 04:41:49 PM:

> On 12/09/13 23:30, Ryan Barnett wrote:
> > Arnout Vandecappelle <arnout@mind.be> wrote on 09/12/2013 04:05:18 PM:
> >
> >  > On 11/09/13 04:03, rjbarnet@rockwellcollins.com wrote:
> >  > > Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on
> >  > > 09/08/2013 08:15:28 AM:
> >  > >
> >  > >  >
> >  > >  > Signed-off-by: Thomas Petazzoni 
<thomas.petazzoni@free-electrons.com>
> >  > >  > ---
> >  > >  >  Config.in   |  6 ++++--
> >  > >  >  Makefile    | 29 +++++++++++++++++++++++++----
> >  > >  >  docs/manual/manual.mk |  2 +-
> >  > >
> >  > > One idea that I have, is for the make file that is genreated for 
the an
> >  > > external output directory, if we could preserve the the value of
> >  > > BR2_EXTERNAL
> >  > > as follows.
> >  >
> >  >   Thomas's patch already does that, because BR2_EXTERNAL is stored 
in the
> >  > config file.
> >  >
> >
> > I don't believe it gets stored in the config since Config.in pulls
> > the variable in from the shell's or make's environment.
> >
> > +config BR2_EXTERNAL
> > +       string
> > +       option env="BR2_EXTERNAL"
> >
> > In order to utilize the BR2_EXTERNAL feature I must have BR2_EXTERNAL
> > either exported into my environment or be called as
> 
>   The Makefile exports it in the environment of the Kconfig targets. So 
> once it is in the .config, it will stay there. Try it and you'll see :-)

I have looked as I'm experiment with this and it definitely isn't there :) 

Also according to the documentation you need to define it every time :)

> >
> > 'make BR2_EXTERNAL=/some/path <target>'
> >
> >
> > When utilizing the BR2_EXTERNAL I usual want to use some defconfig 
from
> > my BR2_EXTERNAL so it adding it the makefile that is generated in your
> > build directory when specifying O= would be extermely useful as that
> > way I can switch shells and my definition of BR2_EXTERNAL would still
> > be present.
> >
> > 'make O=/build/path BR2_EXTERNAL=/path/to/external defconfig'
> 
>   In this particular case, you're not using the Makefile in the output 
> dir but only in the buildroot dir so it's not relevant.
> 
>   The only case where it would be relevant is when you have an existing 
> output directory, you remove the .config first, and then you do a
> make foo_defconfig. Seems a bit an unlikely scenario.
> 
>   Regards,
>   Arnout
Arnout Vandecappelle Sept. 12, 2013, 9:57 p.m. UTC | #7
On 12/09/13 23:51, Ryan Barnett wrote:
>  > > In order to utilize the BR2_EXTERNAL feature I must have BR2_EXTERNAL
>  > > either exported into my environment or be called as
>  >
>  >   The Makefile exports it in the environment of the Kconfig targets. So
>  > once it is in the .config, it will stay there. Try it and you'll see :-)
>
> I have looked as I'm experiment with this and it definitely isn't there :)
> Also according to the documentation you need to define it every time :)
>

  Ah, yes indeed, sorry about that - I was trying to be too smart.

  To save it in the .config, it needs a prompt, and to be able to take it 
from the environment it needs a helper variable. Like BR2_DEFCONFIG.


  Regards,
  Arnout
Ryan Barnett Sept. 12, 2013, 10:11 p.m. UTC | #8
Arnout Vandecappelle <arnout@mind.be> wrote on 09/12/2013 04:57:04 PM:

> On 12/09/13 23:51, Ryan Barnett wrote:
> >  > > In order to utilize the BR2_EXTERNAL feature I must have 
BR2_EXTERNAL
> >  > > either exported into my environment or be called as
> >  >
> >  >   The Makefile exports it in the environment of the Kconfig 
targets. So
> >  > once it is in the .config, it will stay there. Try it and you'll 
see :-)
> >
> > I have looked as I'm experiment with this and it definitely isn't 
there :)
> > Also according to the documentation you need to define it every time 
:)
> >
> 
>   Ah, yes indeed, sorry about that - I was trying to be too smart.

Happens to the best of us.

>   To save it in the .config, it needs a prompt, and to be able to take 
it 
> from the environment it needs a helper variable. Like BR2_DEFCONFIG.

I like to view BR2_EXTERNAL as something that is similar to "O=" which 
means
that I have the ability to change it at a moments notice. However, like it 

is done in mkmakefile when doing the following from within buildroot 
toplevel
source directory:

make O=/path/to/build defconfig

O= is stored in the generated makefile (as you know). However, it gets 
very
annoying to type BR2_EXTERNAL=/path/to/external and I don't always want to
export it as shell environment variable. Thus, adding that line to 
mkmakefile is handy since usually when you specify O= you are doing a 
specific build which BR2_EXTERNAL would definitely be a part of.

Thanks,
-Ryan
Arnout Vandecappelle Sept. 13, 2013, 8:56 p.m. UTC | #9
On 13/09/13 00:11, Ryan Barnett wrote:
> Arnout Vandecappelle <arnout@mind.be> wrote on 09/12/2013 04:57:04 PM:
>
>  > On 12/09/13 23:51, Ryan Barnett wrote:
>  > >  > > In order to utilize the BR2_EXTERNAL feature I must have
> BR2_EXTERNAL
>  > >  > > either exported into my environment or be called as
>  > >  >
>  > >  >   The Makefile exports it in the environment of the Kconfig
> targets. So
>  > >  > once it is in the .config, it will stay there. Try it and you'll
> see :-)
>  > >
>  > > I have looked as I'm experiment with this and it definitely isn't
> there :)
>  > > Also according to the documentation you need to define it every time :)
>  > >
>  >
>  >   Ah, yes indeed, sorry about that - I was trying to be too smart.
>
> Happens to the best of us.
>
>  >   To save it in the .config, it needs a prompt, and to be able to take it
>  > from the environment it needs a helper variable. Like BR2_DEFCONFIG.
>
> I like to view BR2_EXTERNAL as something that is similar to "O=" which means
> that I have the ability to change it at a moments notice. However, like it
> is done in mkmakefile when doing the following from within buildroot
> toplevel
> source directory:
>
> make O=/path/to/build defconfig
>
> O= is stored in the generated makefile (as you know). However, it gets very
> annoying to type BR2_EXTERNAL=/path/to/external and I don't always want to
> export it as shell environment variable. Thus, adding that line to
> mkmakefile is handy since usually when you specify O= you are doing a
> specific build which BR2_EXTERNAL would definitely be a part of.

  My point is: it should (and can) be made part of the .config. It really 
has to be part of the .config, because some options in the .config will 
refer to the packages that are defined in the external directory.

  Regards,
  Arnout
Thomas Petazzoni Sept. 14, 2013, 5:29 a.m. UTC | #10
Dear Arnout Vandecappelle,

On Fri, 13 Sep 2013 22:56:59 +0200, Arnout Vandecappelle wrote:

> > O= is stored in the generated makefile (as you know). However, it gets very
> > annoying to type BR2_EXTERNAL=/path/to/external and I don't always want to
> > export it as shell environment variable. Thus, adding that line to
> > mkmakefile is handy since usually when you specify O= you are doing a
> > specific build which BR2_EXTERNAL would definitely be a part of.
> 
>   My point is: it should (and can) be made part of the .config. It really 
> has to be part of the .config, because some options in the .config will 
> refer to the packages that are defined in the external directory.

I believe it cannot be part of the .config, because there is a
chicken-and-egg problem due to the fact that running 'make menuconfig'
requires to already know the BR2_EXTERNAL value.

Thomas
diff mbox

Patch

diff --git a/support/scripts/mkmakefile b/support/scripts/mkmakefile
--- a/support/scripts/mkmakefile
+++ b/support/scripts/mkmakefile
@@ -27,6 +27,7 @@  makedir := \$(dir \$(call lastword,\$(MAKEFILE_LIST)))

 MAKEARGS := -C $1
 MAKEARGS += O=\$(if \$(patsubst /%,,\$(makedir)),\$(CURDIR)/)\$(patsubst 
%/,%,\$(makedir))
+MAKEARGS += BR2_EXTERNAL=\$(realpath \$(BR2_EXTERNAL))

 MAKEFLAGS += --no-print-directory