All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] Makefile: be careful what we remove from saved defconfig
@ 2020-10-21 20:08 Yann E. MORIN
  2020-10-22 11:18 ` Hervé CODINA
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Yann E. MORIN @ 2020-10-21 20:08 UTC (permalink / raw)
  To: buildroot

When we savedefconfig, we remove BR2_DEFCONFIG (f71a621d91e, savedefconfig:
Remove BR2_DEFCONFIG from saved defconfig file) and BR2_DL_DIR (36edacce9c2,
Makefile: exclude BR2_DL_DIR from savedefconfig), because their meaning
is only valid locally.

However, we were not careful to really match the exact variables, so we
could match arbitrary options.

For example, these config options would all be dropped:

    BR2_DEFCONFIG="toto"
    BR2_DL_DIR="titi"
    BR2_PACKAGE_SABR2_DEFCONFIG="tutu"
    BR2_PACKAGE_SABR2_DL_DIR=y

While the first two are indeed the ones we want to drop, the last two
are options (whatever their meaning or how poorly named they are) of the
hypothetical 'sabr2' package, and we want to keep those in a defconfig.

When cleaning the just-saved defconfig, be sure to anchor the patterns to
the beginning of the line.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Sven Oliver Moll <buildroot@svol.li>
Cc: Herve Codina <Herve.CODINA@celad.com>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index d1d19d8f40..d4751e278e 100644
--- a/Makefile
+++ b/Makefile
@@ -1027,8 +1027,8 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 	@$(COMMON_CONFIG_ENV) $< \
 		--savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
 		$(CONFIG_CONFIG_IN)
-	@$(SED) '/BR2_DEFCONFIG=/d' \
-		-e '/BR2_DL_DIR=/d' \
+	@$(SED) '/^BR2_DEFCONFIG=/d' \
+		-e '/^BR2_DL_DIR=/d' \
 		$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)
 
 .PHONY: defconfig savedefconfig update-defconfig
-- 
2.25.1

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

* [Buildroot] [PATCH] Makefile: be careful what we remove from saved defconfig
  2020-10-21 20:08 [Buildroot] [PATCH] Makefile: be careful what we remove from saved defconfig Yann E. MORIN
@ 2020-10-22 11:18 ` Hervé CODINA
  2020-10-23 17:23 ` Hervé CODINA
  2020-10-24 20:46 ` Thomas Petazzoni
  2 siblings, 0 replies; 6+ messages in thread
From: Hervé CODINA @ 2020-10-22 11:18 UTC (permalink / raw)
  To: buildroot



On 21/10/2020 22:08, Yann E. MORIN wrote:
> When we savedefconfig, we remove BR2_DEFCONFIG (f71a621d91e, savedefconfig:
> Remove BR2_DEFCONFIG from saved defconfig file) and BR2_DL_DIR (36edacce9c2,
> Makefile: exclude BR2_DL_DIR from savedefconfig), because their meaning
> is only valid locally.
> 
> However, we were not careful to really match the exact variables, so we
> could match arbitrary options.
> 
> For example, these config options would all be dropped:
> 
>      BR2_DEFCONFIG="toto"
>      BR2_DL_DIR="titi"
>      BR2_PACKAGE_SABR2_DEFCONFIG="tutu"
>      BR2_PACKAGE_SABR2_DL_DIR=y
> 
> While the first two are indeed the ones we want to drop, the last two
> are options (whatever their meaning or how poorly named they are) of the
> hypothetical 'sabr2' package, and we want to keep those in a defconfig.
> 
> When cleaning the just-saved defconfig, be sure to anchor the patterns to
> the beginning of the line.
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Sven Oliver Moll <buildroot@svol.li>
> Cc: Herve Codina <Herve.CODINA@celad.com>

Oups, my bad.

Acked-by: Herve Codina <Herve.CODINA@celad.com>

Best regards,
Herve Codina

> ---
>   Makefile | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index d1d19d8f40..d4751e278e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1027,8 +1027,8 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
>   	@$(COMMON_CONFIG_ENV) $< \
>   		--savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
>   		$(CONFIG_CONFIG_IN)
> -	@$(SED) '/BR2_DEFCONFIG=/d' \
> -		-e '/BR2_DL_DIR=/d' \
> +	@$(SED) '/^BR2_DEFCONFIG=/d' \
> +		-e '/^BR2_DL_DIR=/d' \
>   		$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)>   
>   .PHONY: defconfig savedefconfig update-defconfig
> 

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

* [Buildroot] [PATCH] Makefile: be careful what we remove from saved defconfig
  2020-10-21 20:08 [Buildroot] [PATCH] Makefile: be careful what we remove from saved defconfig Yann E. MORIN
  2020-10-22 11:18 ` Hervé CODINA
@ 2020-10-23 17:23 ` Hervé CODINA
  2020-10-23 17:35   ` Sven Oliver Moll
  2020-10-24 20:46 ` Thomas Petazzoni
  2 siblings, 1 reply; 6+ messages in thread
From: Hervé CODINA @ 2020-10-23 17:23 UTC (permalink / raw)
  To: buildroot



On 21/10/2020 22:08, Yann E. MORIN wrote:
> When we savedefconfig, we remove BR2_DEFCONFIG (f71a621d91e, savedefconfig:
> Remove BR2_DEFCONFIG from saved defconfig file) and BR2_DL_DIR (36edacce9c2,
> Makefile: exclude BR2_DL_DIR from savedefconfig), because their meaning
> is only valid locally.
> 
> However, we were not careful to really match the exact variables, so we
> could match arbitrary options.
> 
> For example, these config options would all be dropped:
> 
>      BR2_DEFCONFIG="toto"
>      BR2_DL_DIR="titi"
>      BR2_PACKAGE_SABR2_DEFCONFIG="tutu"
>      BR2_PACKAGE_SABR2_DL_DIR=y
> 
> While the first two are indeed the ones we want to drop, the last two
> are options (whatever their meaning or how poorly named they are) of the
> hypothetical 'sabr2' package, and we want to keep those in a defconfig.
> 
> When cleaning the just-saved defconfig, be sure to anchor the patterns to
> the beginning of the line.
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Sven Oliver Moll <buildroot@svol.li>
> Cc: Herve Codina <Herve.CODINA@celad.com>
> ---
>   Makefile | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index d1d19d8f40..d4751e278e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1027,8 +1027,8 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
>   	@$(COMMON_CONFIG_ENV) $< \
>   		--savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
>   		$(CONFIG_CONFIG_IN)
> -	@$(SED) '/BR2_DEFCONFIG=/d' \
> -		-e '/BR2_DL_DIR=/d' \
> +	@$(SED) '/^BR2_DEFCONFIG=/d' \
> +		-e '/^BR2_DL_DIR=/d' \
>   		$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)

Maybe I acked too quickly ...

What do you think about this pattern
@$(SED) '/^BR2_DEFCONFIG=/d' -e '/^# BR2_DEFCONFIG /
in order to match "BR2_DEFCONFIG=xxxxxxxx" and "# BR2_DEFCONFIG is not set" ?

Of course, same pattern for BR2_DL_DIR.

I am not sure that BR2_DEFCONFIG nor BR2_DL_DIR can be commented (ie can be not set)
but this will prevent any traces of BR2_DEFCONFIG or BR2_DL_DIR even if commented.

Herve Codina



>   
>   .PHONY: defconfig savedefconfig update-defconfig
> 

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

* [Buildroot] [PATCH] Makefile: be careful what we remove from saved defconfig
  2020-10-23 17:23 ` Hervé CODINA
@ 2020-10-23 17:35   ` Sven Oliver Moll
  2020-10-24 16:32     ` Hervé CODINA
  0 siblings, 1 reply; 6+ messages in thread
From: Sven Oliver Moll @ 2020-10-23 17:35 UTC (permalink / raw)
  To: buildroot

On Fri, 23 Oct 2020, Herv? CODINA wrote:

> Maybe I acked too quickly ...
>
> What do you think about this pattern
> @$(SED) '/^BR2_DEFCONFIG=/d' -e '/^# BR2_DEFCONFIG /
> in order to match "BR2_DEFCONFIG=xxxxxxxx" and "# BR2_DEFCONFIG is not set" ?
[...]

I don't think that this is necessary, because savedefconfig only saves the 
ones that are different from preset. So:
- BR2_DEFCONFIG is the name to save to, so an unset instance does not exist
   (a bit like chicken and egg)
- BR2_DL_DIR, when unset, will not appear as "# BR2_DL_DIR is unset" in the
   saved *_defconfig
...or am I missing something?


Greetings,
SvOlli
-- 
|  _______       |
| (  /\          |
|__)v\/lli a.k.a.| Be good, but not TOO good.
|Sven Oliver Moll|   -- Frumpy The Clown by Judd Winick (Final: June, 7th 1998)

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

* [Buildroot] [PATCH] Makefile: be careful what we remove from saved defconfig
  2020-10-23 17:35   ` Sven Oliver Moll
@ 2020-10-24 16:32     ` Hervé CODINA
  0 siblings, 0 replies; 6+ messages in thread
From: Hervé CODINA @ 2020-10-24 16:32 UTC (permalink / raw)
  To: buildroot



On 23/10/2020 19:35, Sven Oliver Moll wrote:
> On Fri, 23 Oct 2020, Herv? CODINA wrote:
> 
>> Maybe I acked too quickly ...
>>
>> What do you think about this pattern
>> @$(SED) '/^BR2_DEFCONFIG=/d' -e '/^# BR2_DEFCONFIG /
>> in order to match "BR2_DEFCONFIG=xxxxxxxx" and "# BR2_DEFCONFIG is not set" ?
> [...]
> 
> I don't think that this is necessary, because savedefconfig only saves the ones that are different from preset. So:
> - BR2_DEFCONFIG is the name to save to, so an unset instance does not exist
>  ? (a bit like chicken and egg)
> - BR2_DL_DIR, when unset, will not appear as "# BR2_DL_DIR is unset" in the
>  ? saved *_defconfig
> ...or am I missing something?
> 
> 
> Greetings,
> SvOlli

I didn't fully check if BR2_DEFCONFIG and BR2_DL_DIR can appear commented.
As some commented 'symbols' are present in defconfig files in /configs directory,
my remark was just a kind of belt and suspenders approach :-)

If BR2_DEFCONFIG and BR2_DL_DIR cannot appear commented, using just '/^BR2_DEFCONFIG=/d' and
'/^BR2_DL_DIR=/d' is ok for me.

Herve

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

* [Buildroot] [PATCH] Makefile: be careful what we remove from saved defconfig
  2020-10-21 20:08 [Buildroot] [PATCH] Makefile: be careful what we remove from saved defconfig Yann E. MORIN
  2020-10-22 11:18 ` Hervé CODINA
  2020-10-23 17:23 ` Hervé CODINA
@ 2020-10-24 20:46 ` Thomas Petazzoni
  2 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2020-10-24 20:46 UTC (permalink / raw)
  To: buildroot

On Wed, 21 Oct 2020 22:08:28 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> When we savedefconfig, we remove BR2_DEFCONFIG (f71a621d91e, savedefconfig:
> Remove BR2_DEFCONFIG from saved defconfig file) and BR2_DL_DIR (36edacce9c2,
> Makefile: exclude BR2_DL_DIR from savedefconfig), because their meaning
> is only valid locally.
> 
> However, we were not careful to really match the exact variables, so we
> could match arbitrary options.
> 
> For example, these config options would all be dropped:
> 
>     BR2_DEFCONFIG="toto"
>     BR2_DL_DIR="titi"
>     BR2_PACKAGE_SABR2_DEFCONFIG="tutu"
>     BR2_PACKAGE_SABR2_DL_DIR=y
> 
> While the first two are indeed the ones we want to drop, the last two
> are options (whatever their meaning or how poorly named they are) of the
> hypothetical 'sabr2' package, and we want to keep those in a defconfig.
> 
> When cleaning the just-saved defconfig, be sure to anchor the patterns to
> the beginning of the line.
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Sven Oliver Moll <buildroot@svol.li>
> Cc: Herve Codina <Herve.CODINA@celad.com>
> ---
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied to master, thanks.

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

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

end of thread, other threads:[~2020-10-24 20:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21 20:08 [Buildroot] [PATCH] Makefile: be careful what we remove from saved defconfig Yann E. MORIN
2020-10-22 11:18 ` Hervé CODINA
2020-10-23 17:23 ` Hervé CODINA
2020-10-23 17:35   ` Sven Oliver Moll
2020-10-24 16:32     ` Hervé CODINA
2020-10-24 20:46 ` Thomas Petazzoni

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.