[Buildroot] [PATCH] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Buildroot] [PATCH] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Yann E. MORIN]

Currently, calling foo-reconfigure for a kconfig-based package will not
re-trigger the configuration (kconfig-wise) step for the package.

This can be problematic when using an override-srcdir suring development
and the content of the veride-srcdir changes (e.g. on a rebase, a merge,
or during a bisect).

This is because the configuration (kconfig-wise) of the package is not
done in the _CONFIGURE_CMDS block, but as a separate action that is not
part of any step [0].

So, be sure to remove the stamp file .stamp_kconfig_fixup_done during
the foo-clean-for-reconfigure rule, so that the configuration is applied
again with the new source tree.

We use another rule, foo-clean-kconfig-for-reconfigure, because we do
not want to override the default foo-clean-for-reconfigure rule, and we
have no way to add conditional commands to it.

[0] The reasons it was not done are not entirely clear in my head, but
IIRC that was not working at the time we tried with Thomas DS.

Reported-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/pkg-kconfig.mk | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index b0f5178..ce9bf33 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -118,6 +118,12 @@ $$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config
 # Before running configure, the configuration file should be present and fixed
 $$($(2)_TARGET_CONFIGURE): $$($(2)_DIR)/.stamp_kconfig_fixup_done
 
+# Force olddefconfig again on -reconfigure
+$(1)-clean-for-reconfigure: $(1)-clean-kconfig-for-reconfigure
+
+$(1)-clean-kconfig-for-reconfigure:
+	rm -f $$($(2)_DIR)/.stamp_kconfig_fixup_done
+
 # Only enable the foo-*config targets when the package is actually enabled.
 # Note: the variable $(2)_KCONFIG_VAR is not related to the kconfig
 # infrastructure, but defined by pkg-generic.mk. The generic infrastructure is
-- 
2.7.4

[Buildroot] [PATCH] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Vivien Didelot]

Hi,

"Yann E. MORIN" <yann.morin.1998@free.fr> writes:

> Currently, calling foo-reconfigure for a kconfig-based package will not
> re-trigger the configuration (kconfig-wise) step for the package.
>
> This can be problematic when using an override-srcdir suring development
> and the content of the veride-srcdir changes (e.g. on a rebase, a merge,
> or during a bisect).
>
> This is because the configuration (kconfig-wise) of the package is not
> done in the _CONFIGURE_CMDS block, but as a separate action that is not
> part of any step [0].
>
> So, be sure to remove the stamp file .stamp_kconfig_fixup_done during
> the foo-clean-for-reconfigure rule, so that the configuration is applied
> again with the new source tree.
>
> We use another rule, foo-clean-kconfig-for-reconfigure, because we do
> not want to override the default foo-clean-for-reconfigure rule, and we
> have no way to add conditional commands to it.
>
> [0] The reasons it was not done are not entirely clear in my head, but
> IIRC that was not working at the time we tried with Thomas DS.
>
> Reported-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Without this patch, make linux-rebuild or make linux-reconfigure will
prompt new kernel symbols, which is painful when rebasing/bisecting.

With this patch, make linux-rebuild will still prompt new symbols, but
make linux-reconfigure will not, which is intuitive.

Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Thanks,

        Vivien

[Buildroot] [PATCH] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Arnout Vandecappelle]

On 14-09-16 00:35, Yann E. MORIN wrote:
> Currently, calling foo-reconfigure for a kconfig-based package will not
> re-trigger the configuration (kconfig-wise) step for the package.
> 
> This can be problematic when using an override-srcdir suring development
> and the content of the veride-srcdir changes (e.g. on a rebase, a merge,
> or during a bisect).
> 
> This is because the configuration (kconfig-wise) of the package is not
> done in the _CONFIGURE_CMDS block, but as a separate action that is not
> part of any step [0].
> 
> So, be sure to remove the stamp file .stamp_kconfig_fixup_done during
> the foo-clean-for-reconfigure rule, so that the configuration is applied
> again with the new source tree.
> 
> We use another rule, foo-clean-kconfig-for-reconfigure, because we do
> not want to override the default foo-clean-for-reconfigure rule, and we
> have no way to add conditional commands to it.

 Well, it could just be added to the -clean-for-reconfigure in the generic
infra, but keeping it in pkg-kconfig like this is probably better.

> 
> [0] The reasons it was not done are not entirely clear in my head, but
> IIRC that was not working at the time we tried with Thomas DS.
> 
> Reported-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> ---
>  package/pkg-kconfig.mk | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index b0f5178..ce9bf33 100644
> --- a/package/pkg-kconfig.mk
> +++ b/package/pkg-kconfig.mk
> @@ -118,6 +118,12 @@ $$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config
>  # Before running configure, the configuration file should be present and fixed
>  $$($(2)_TARGET_CONFIGURE): $$($(2)_DIR)/.stamp_kconfig_fixup_done
>  
> +# Force olddefconfig again on -reconfigure
> +$(1)-clean-for-reconfigure: $(1)-clean-kconfig-for-reconfigure
> +
> +$(1)-clean-kconfig-for-reconfigure:
> +	rm -f $$($(2)_DIR)/.stamp_kconfig_fixup_done
> +
>  # Only enable the foo-*config targets when the package is actually enabled.
>  # Note: the variable $(2)_KCONFIG_VAR is not related to the kconfig
>  # infrastructure, but defined by pkg-generic.mk. The generic infrastructure is
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Thomas De Schampheleire]

On Wed, Sep 14, 2016 at 12:35 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Currently, calling foo-reconfigure for a kconfig-based package will not
> re-trigger the configuration (kconfig-wise) step for the package.
>
> This can be problematic when using an override-srcdir suring development
> and the content of the veride-srcdir changes (e.g. on a rebase, a merge,
> or during a bisect).
>
> This is because the configuration (kconfig-wise) of the package is not
> done in the _CONFIGURE_CMDS block, but as a separate action that is not
> part of any step [0].
>
> So, be sure to remove the stamp file .stamp_kconfig_fixup_done during
> the foo-clean-for-reconfigure rule, so that the configuration is applied
> again with the new source tree.
>
> We use another rule, foo-clean-kconfig-for-reconfigure, because we do
> not want to override the default foo-clean-for-reconfigure rule, and we
> have no way to add conditional commands to it.
>
> [0] The reasons it was not done are not entirely clear in my head, but
> IIRC that was not working at the time we tried with Thomas DS.

This period is very blurry for me :-)

I can't recall having focused on the reconfigure step for the kconfig
infrastructure, I think I was looking primarily at configure, combined
with cleans and various other combinations. So it is very well
possible that this has always been broken until now.

/Thomas

[Buildroot] [PATCH] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Thomas Petazzoni]

Hello,

On Wed, 14 Sep 2016 09:27:25 +0200, Thomas De Schampheleire wrote:

> > This can be problematic when using an override-srcdir suring development
> > and the content of the veride-srcdir changes (e.g. on a rebase, a merge,
> > or during a bisect).
> >
> > This is because the configuration (kconfig-wise) of the package is not
> > done in the _CONFIGURE_CMDS block, but as a separate action that is not
> > part of any step [0].
> >
> > So, be sure to remove the stamp file .stamp_kconfig_fixup_done during
> > the foo-clean-for-reconfigure rule, so that the configuration is applied
> > again with the new source tree.
> >
> > We use another rule, foo-clean-kconfig-for-reconfigure, because we do
> > not want to override the default foo-clean-for-reconfigure rule, and we
> > have no way to add conditional commands to it.
> >
> > [0] The reasons it was not done are not entirely clear in my head, but
> > IIRC that was not working at the time we tried with Thomas DS.  
> 
> This period is very blurry for me :-)
> 
> I can't recall having focused on the reconfigure step for the kconfig
> infrastructure, I think I was looking primarily at configure, combined
> with cleans and various other combinations. So it is very well
> possible that this has always been broken until now.

Well, the question is not so much about "reconfigure", but about
the configure step itself. Why isn't the configure step done inside
<pkg>_CONFIGURE_CMDS ? If it had been done inside the
<pkg>_CONFIGURE_CMDS, then those "hacks" to make reconfigure work
would not be needed.

I'm pretty sure that there's a solid reason for not doing the .config
preparation inside <pkg>_CONFIGURE_CMDS, but it'd be great to remember
why and document it somewhere :)

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

[Buildroot] [PATCH] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Thomas De Schampheleire]

On Wed, Sep 14, 2016 at 11:01 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Wed, 14 Sep 2016 09:27:25 +0200, Thomas De Schampheleire wrote:
>
>> > This can be problematic when using an override-srcdir suring development
>> > and the content of the veride-srcdir changes (e.g. on a rebase, a merge,
>> > or during a bisect).
>> >
>> > This is because the configuration (kconfig-wise) of the package is not
>> > done in the _CONFIGURE_CMDS block, but as a separate action that is not
>> > part of any step [0].
>> >
>> > So, be sure to remove the stamp file .stamp_kconfig_fixup_done during
>> > the foo-clean-for-reconfigure rule, so that the configuration is applied
>> > again with the new source tree.
>> >
>> > We use another rule, foo-clean-kconfig-for-reconfigure, because we do
>> > not want to override the default foo-clean-for-reconfigure rule, and we
>> > have no way to add conditional commands to it.
>> >
>> > [0] The reasons it was not done are not entirely clear in my head, but
>> > IIRC that was not working at the time we tried with Thomas DS.
>>
>> This period is very blurry for me :-)
>>
>> I can't recall having focused on the reconfigure step for the kconfig
>> infrastructure, I think I was looking primarily at configure, combined
>> with cleans and various other combinations. So it is very well
>> possible that this has always been broken until now.
>
> Well, the question is not so much about "reconfigure", but about
> the configure step itself. Why isn't the configure step done inside
> <pkg>_CONFIGURE_CMDS ? If it had been done inside the
> <pkg>_CONFIGURE_CMDS, then those "hacks" to make reconfigure work
> would not be needed.
>
> I'm pretty sure that there's a solid reason for not doing the .config
> preparation inside <pkg>_CONFIGURE_CMDS, but it'd be great to remember
> why and document it somewhere :)

I think the answer to this question is simple: one of the goals was to allow:

'make clean linux-menuconfig'

without this step first building all dependencies of linux (which is
quite a lot).
If the kconfig configuration would be part of CONFIGURE_CMDS, then
we'd end up in that situation.

Best regards,
Thomas

[Buildroot] [PATCH] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Yann E. MORIN]

Thomas?, All,

On 2016-09-14 19:42 +0200, Thomas De Schampheleire spake thusly:
> On Wed, Sep 14, 2016 at 11:01 AM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
> > On Wed, 14 Sep 2016 09:27:25 +0200, Thomas De Schampheleire wrote:
> >
> >> > This can be problematic when using an override-srcdir suring development
> >> > and the content of the veride-srcdir changes (e.g. on a rebase, a merge,
> >> > or during a bisect).
> >> >
> >> > This is because the configuration (kconfig-wise) of the package is not
> >> > done in the _CONFIGURE_CMDS block, but as a separate action that is not
> >> > part of any step [0].
> >> >
> >> > So, be sure to remove the stamp file .stamp_kconfig_fixup_done during
> >> > the foo-clean-for-reconfigure rule, so that the configuration is applied
> >> > again with the new source tree.
> >> >
> >> > We use another rule, foo-clean-kconfig-for-reconfigure, because we do
> >> > not want to override the default foo-clean-for-reconfigure rule, and we
> >> > have no way to add conditional commands to it.
> >> >
> >> > [0] The reasons it was not done are not entirely clear in my head, but
> >> > IIRC that was not working at the time we tried with Thomas DS.
> >>
> >> This period is very blurry for me :-)
> >>
> >> I can't recall having focused on the reconfigure step for the kconfig
> >> infrastructure, I think I was looking primarily at configure, combined
> >> with cleans and various other combinations. So it is very well
> >> possible that this has always been broken until now.
> >
> > Well, the question is not so much about "reconfigure", but about
> > the configure step itself. Why isn't the configure step done inside
> > <pkg>_CONFIGURE_CMDS ? If it had been done inside the
> > <pkg>_CONFIGURE_CMDS, then those "hacks" to make reconfigure work
> > would not be needed.
> >
> > I'm pretty sure that there's a solid reason for not doing the .config
> > preparation inside <pkg>_CONFIGURE_CMDS, but it'd be great to remember
> > why and document it somewhere :)
> 
> I think the answer to this question is simple: one of the goals was to allow:
> 
> 'make clean linux-menuconfig'
> 
> without this step first building all dependencies of linux (which is
> quite a lot).
> If the kconfig configuration would be part of CONFIGURE_CMDS, then
> we'd end up in that situation.

Yes, that was the basis for our thinking at the time.

Yet, maybe we could just move the call to the $(2)_REGEN_DOT_CONFIG
macro into the CONFIGURE_CMDS (or in a pre-configure hook).

I'll try to test that tonight...

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] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Yann E. MORIN]

Thomas?, All,

On 2016-09-14 20:32 +0200, Yann E. MORIN spake thusly:
> On 2016-09-14 19:42 +0200, Thomas De Schampheleire spake thusly:
> > On Wed, Sep 14, 2016 at 11:01 AM, Thomas Petazzoni
> > <thomas.petazzoni@free-electrons.com> wrote:
> > > Well, the question is not so much about "reconfigure", but about
> > > the configure step itself. Why isn't the configure step done inside
> > > <pkg>_CONFIGURE_CMDS ? If it had been done inside the
> > > <pkg>_CONFIGURE_CMDS, then those "hacks" to make reconfigure work
> > > would not be needed.
> > >
> > > I'm pretty sure that there's a solid reason for not doing the .config
> > > preparation inside <pkg>_CONFIGURE_CMDS, but it'd be great to remember
> > > why and document it somewhere :)
> > 
> > I think the answer to this question is simple: one of the goals was to allow:
> > 
> > 'make clean linux-menuconfig'
> > 
> > without this step first building all dependencies of linux (which is
> > quite a lot).
> > If the kconfig configuration would be part of CONFIGURE_CMDS, then
> > we'd end up in that situation.
> 
> Yes, that was the basis for our thinking at the time.
> 
> Yet, maybe we could just move the call to the $(2)_REGEN_DOT_CONFIG

I meant: $(2)_FIXUP_DOT_CONFIG

> macro into the CONFIGURE_CMDS (or in a pre-configure hook).

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] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Thomas Petazzoni]

Hello,

On Wed, 14 Sep 2016 00:35:17 +0200, Yann E. MORIN wrote:

> +# Force olddefconfig again on -reconfigure
> +$(1)-clean-for-reconfigure: $(1)-clean-kconfig-for-reconfigure
> +
> +$(1)-clean-kconfig-for-reconfigure:
> +	rm -f $$($(2)_DIR)/.stamp_kconfig_fixup_done

I was about to apply this, but in fact, I'm not sure I agree.

<foo>-reconfigure is supposed to re-do the configuration step entirely.
For example, with an autotools package, if I change the value of
<pkg>_CONF_OPTS and then do make <foo>-reconfigure, the configuration
is done again, with the new <pkg>_CONF_OPTS.

Here, what you're doing is that you're only re-doing the "fixup" of
the .config, but you're not re-loading the configuration from the
original defconfig or full config file. This means that if the user
changes the defconfig and does "make linux-reconfigure", it won't
reload the defconfig.

Unless my analysis is wrong, I think the patch should be changed to
re-do the configuration step entirely.

Thanks,

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

[Buildroot] [PATCH] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Thomas Petazzoni]

Hello,

On Wed, 14 Sep 2016 19:42:30 +0200, Thomas De Schampheleire wrote:

> I think the answer to this question is simple: one of the goals was to allow:
> 
> 'make clean linux-menuconfig'
> 
> without this step first building all dependencies of linux (which is
> quite a lot).
> If the kconfig configuration would be part of CONFIGURE_CMDS, then
> we'd end up in that situation.

Gaah, yes, indeed, makes sense. Can we write this somewhere?

Thanks,

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

[Buildroot] [PATCH] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Yann E. MORIN]

Thomas, All,

On 2016-09-16 19:05 +0200, Thomas Petazzoni spake thusly:
> On Wed, 14 Sep 2016 00:35:17 +0200, Yann E. MORIN wrote:
> > +# Force olddefconfig again on -reconfigure
> > +$(1)-clean-for-reconfigure: $(1)-clean-kconfig-for-reconfigure
> > +
> > +$(1)-clean-kconfig-for-reconfigure:
> > +	rm -f $$($(2)_DIR)/.stamp_kconfig_fixup_done
> 
> I was about to apply this, but in fact, I'm not sure I agree.
> 
> <foo>-reconfigure is supposed to re-do the configuration step entirely.
> For example, with an autotools package, if I change the value of
> <pkg>_CONF_OPTS and then do make <foo>-reconfigure, the configuration
> is done again, with the new <pkg>_CONF_OPTS.
> 
> Here, what you're doing is that you're only re-doing the "fixup" of
> the .config, but you're not re-loading the configuration from the
> original defconfig or full config file. This means that if the user
> changes the defconfig and does "make linux-reconfigure", it won't
> reload the defconfig.
> 
> Unless my analysis is wrong, I think the patch should be changed to
> re-do the configuration step entirely.

Which is by far non-trivial, and something I've been working on the past
two evenings...

But I now have "something" that offloads most of the configuration as a
configure command step.

It is darn ugly and execissvely complex, though, and I am still looking
whether we still cover all the corner-cases that we used to cover
previously, plus this new use-case.

In the end, I'm not even sure I'd post that solution at all, because it
is definitely not elegant, makes the code much more complex and is not
fool-proof (not that I use to always provide fool-proof code, but I
refrain from doing so when I notice! ;-] ).

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] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Yann E. MORIN]

Thomas, All,

On 2016-09-16 19:05 +0200, Thomas Petazzoni spake thusly:
> On Wed, 14 Sep 2016 00:35:17 +0200, Yann E. MORIN wrote:
> 
> > +# Force olddefconfig again on -reconfigure
> > +$(1)-clean-for-reconfigure: $(1)-clean-kconfig-for-reconfigure
> > +
> > +$(1)-clean-kconfig-for-reconfigure:
> > +	rm -f $$($(2)_DIR)/.stamp_kconfig_fixup_done
> 
> I was about to apply this, but in fact, I'm not sure I agree.
> 
> <foo>-reconfigure is supposed to re-do the configuration step entirely.
> For example, with an autotools package, if I change the value of
> <pkg>_CONF_OPTS and then do make <foo>-reconfigure, the configuration
> is done again, with the new <pkg>_CONF_OPTS.
> 
> Here, what you're doing is that you're only re-doing the "fixup" of
> the .config, but you're not re-loading the configuration from the
> original defconfig or full config file. This means that if the user
> changes the defconfig and does "make linux-reconfigure", it won't
> reload the defconfig.
> 
> Unless my analysis is wrong, I think the patch should be changed to
> re-do the configuration step entirely.

After having a flash of genius (worth mentioning!), it occurred to me
that this use-case already works!

And this change does not modify this behaviour. More testing in
progress; results and explanations to come a bit later tonight...

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] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Yann E. MORIN]

Thomas, All,

On 2016-09-16 19:05 +0200, Thomas Petazzoni spake thusly:
> On Wed, 14 Sep 2016 00:35:17 +0200, Yann E. MORIN wrote:
> 
> > +# Force olddefconfig again on -reconfigure
> > +$(1)-clean-for-reconfigure: $(1)-clean-kconfig-for-reconfigure
> > +
> > +$(1)-clean-kconfig-for-reconfigure:
> > +	rm -f $$($(2)_DIR)/.stamp_kconfig_fixup_done
> 
> I was about to apply this, but in fact, I'm not sure I agree.
> 
> <foo>-reconfigure is supposed to re-do the configuration step entirely.
> For example, with an autotools package, if I change the value of
> <pkg>_CONF_OPTS and then do make <foo>-reconfigure, the configuration
> is done again, with the new <pkg>_CONF_OPTS.
> 
> Here, what you're doing is that you're only re-doing the "fixup" of
> the .config, but you're not re-loading the configuration from the
> original defconfig or full config file. This means that if the user
> changes the defconfig and does "make linux-reconfigure", it won't
> reload the defconfig.
> 
> Unless my analysis is wrong, I think the patch should be changed to
> re-do the configuration step entirely.

So, as I said previously, this use-case is alredy covered by the current
set of dependencies, and this patch does not change the behaviour for
this use-case.

The reason is that .stamp_kconfig_fixup_done depends on the .config file.

In turn, the .config file depends on the base (def)config and fragments.

So, touching any of the base (def)config or fragments will trigger a
full reconfiguration, even without this patch. You can try this:

    $ make defconfig; make menuconfig  # Enable a pre-built toolchain
    $ make busybox-build
    $ touch touch package/busybox/busybox.config
    $ make V=1 busybox-build

You'll notice that, in the second busybox-build, the very first command
to be run, right after the removal of .stmap files, is to copy the base
busybox config file, followed by a call to the merge-config script:

    [...]
    rm -f /home/ymorin/dev/buildroot/outoput/build/busybox-1.25.0/.stamp_kconfig_fixup_done
    rm -f /home/ymorin/dev/buildroot/outoput/build/busybox-1.25.0/.stamp_configured
    cp package/busybox/busybox.config /home/ymorin/dev/buildroot/outoput/build/busybox-1.25.0/.config
    support/kconfig/merge_config.sh -m -O /home/ymorin/dev/buildroot/outoput/build/busybox-1.25.0 /home/ymorin/dev/buildroot/outoput/build/busybox-1.25.0/.config
    Using /home/ymorin/dev/buildroot/outoput/build/busybox-1.25.0/.config as base
    [...]

Now, with this patch applied, you'll notice this behaviour is kept, and
also occurs for the busybox-reconfigure action.

So, I'd like to argue that this patch fixes the reported issue and covers
the use-case you pointed to.

Please apply, as you seemed keen on doing so initially. ;-)

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] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Thomas Petazzoni]

Hello,

On Fri, 16 Sep 2016 21:02:44 +0200, Yann E. MORIN wrote:

> The reason is that .stamp_kconfig_fixup_done depends on the .config file.
> 
> In turn, the .config file depends on the base (def)config and fragments.
> 
> So, touching any of the base (def)config or fragments will trigger a
> full reconfiguration, even without this patch. You can try this:
> 
>     $ make defconfig; make menuconfig  # Enable a pre-built toolchain
>     $ make busybox-build
>     $ touch touch package/busybox/busybox.config
>     $ make V=1 busybox-build
> 
> You'll notice that, in the second busybox-build, the very first command
> to be run, right after the removal of .stmap files, is to copy the base
> busybox config file, followed by a call to the merge-config script:
> 
>     [...]
>     rm -f /home/ymorin/dev/buildroot/outoput/build/busybox-1.25.0/.stamp_kconfig_fixup_done
>     rm -f /home/ymorin/dev/buildroot/outoput/build/busybox-1.25.0/.stamp_configured
>     cp package/busybox/busybox.config /home/ymorin/dev/buildroot/outoput/build/busybox-1.25.0/.config
>     support/kconfig/merge_config.sh -m -O /home/ymorin/dev/buildroot/outoput/build/busybox-1.25.0 /home/ymorin/dev/buildroot/outoput/build/busybox-1.25.0/.config
>     Using /home/ymorin/dev/buildroot/outoput/build/busybox-1.25.0/.config as base
>     [...]
> 
> Now, with this patch applied, you'll notice this behaviour is kept, and
> also occurs for the busybox-reconfigure action.
> 
> So, I'd like to argue that this patch fixes the reported issue and covers
> the use-case you pointed to.

That was not the use-case I pointed to, what I pointed to what the
following use-case:

 1. Create a Buildroot configuration, with Linux enabled, using the
    omap2plus_defconfig

 2. Build your system.

 3. Go in menuconfig, and change the Linux defconfig to
    mvebu_v7_defconfig.

 4. Run "make linux-reconfigure"

I would expect the newly defined Linux configuration to be taken into
account, but it's not, it only re-does the fixups and doesn't reload
the configuration from mvebu_v7_defconfig.

Now, we can discuss whether this is the behavior that we want or not.
But at least,  that's the behavior I was referring to, and which your
patch doesn't address.

*But*, I'll apply your patch nonetheless because it fixes other issues,
and doesn't change the behavior I'm describing.

Thanks!

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

[Buildroot] [PATCH] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Yann E. MORIN]

Thomas, All,

On 2016-09-17 14:42 +0200, Thomas Petazzoni spake thusly:
> On Fri, 16 Sep 2016 21:02:44 +0200, Yann E. MORIN wrote:
[--SNIP--]
> > So, I'd like to argue that this patch fixes the reported issue and covers
> > the use-case you pointed to.
> 
> That was not the use-case I pointed to, what I pointed to what the
> following use-case:
> 
>  1. Create a Buildroot configuration, with Linux enabled, using the
>     omap2plus_defconfig
> 
>  2. Build your system.
> 
>  3. Go in menuconfig, and change the Linux defconfig to
>     mvebu_v7_defconfig.
> 
>  4. Run "make linux-reconfigure"
> 
> I would expect the newly defined Linux configuration to be taken into
> account, but it's not, it only re-does the fixups and doesn't reload
> the configuration from mvebu_v7_defconfig.

Ah, I did not understand that. Indeed, this use-case is not covered,
neitehr currently nor with this patch applied.

However, I wonder how we could detect that condition...

> Now, we can discuss whether this is the behavior that we want or not.
> But at least,  that's the behavior I was referring to, and which your
> patch doesn't address.

Indeed, that's not covered. But neither is it a regression...

> *But*, I'll apply your patch nonetheless because it fixes other issues,
> and doesn't change the behavior I'm describing.

Thanks!

I'll see if we can do something to cover your use-case...

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] infra/pkg-kconfig: Be sure to reconfigure the package on foo-reconfigure

[Thomas Petazzoni]

Hello,

On Sat, 17 Sep 2016 14:53:47 +0200, Yann E. MORIN wrote:

> > I would expect the newly defined Linux configuration to be taken into
> > account, but it's not, it only re-does the fixups and doesn't reload
> > the configuration from mvebu_v7_defconfig.  
> 
> Ah, I did not understand that. Indeed, this use-case is not covered,
> neitehr currently nor with this patch applied.
> 
> However, I wonder how we could detect that condition...

Well, it's not so much about "detecting": it's about
"<pkg>-reconfigure" having the semantic of "I'm doing the entire
configure step again".

> I'll see if we can do something to cover your use-case...

Again, I am not sure at all it is worth trying to cover my use case.

On one side, <pkg>-reconfigure should "reconfigure" the package. But on
the other hand, Buildroot does not guarantee at all that changes done
in menuconfig are taken into account without re-doing a full "make
clean".

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