diff mbox series

[RFC,1/3] support/kconfig/merge_config.sh: merge also buildroot config files

Message ID 1507668210-5427-2-git-send-email-angelo.compagnucci@gmail.com
State Changes Requested
Headers show
Series Adding software stacks | expand

Commit Message

Angelo Compagnucci Oct. 10, 2017, 8:43 p.m. UTC 
This patch adds a way to merge buildroot config file programmatically.
It adds an option (-b, buildroot mode) to manage buildroot config files.
The buildroot mode changes the way the script looks for configurations
entries using the BR2_ prefix and modify the call to the make command
to be buildroot friendly.

Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
 support/kconfig/merge_config.sh | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Arnout Vandecappelle Oct. 11, 2017, 9:43 p.m. UTC | #1 
On 10-10-17 22:43, Angelo Compagnucci wrote:
> This patch adds a way to merge buildroot config file programmatically.

 We already do that in test-pkg, without this patch. What are we doing wrong?

> It adds an option (-b, buildroot mode) to manage buildroot config files.

 I wouldn't bother with that option. It's only going to be used for Buildroot
config files (other packages using Kconfig will have a copy of the script
already). So we can just change it directly. And we already have a stack of
changes on the upstream kconfig.

> The buildroot mode changes the way the script looks for configurations
> entries using the BR2_ prefix and modify the call to the make command
> to be buildroot friendly.
> 
> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
[snip]
> @@ -81,7 +90,7 @@ INITFILE=$1
>  shift;
>  
>  MERGE_LIST=$*
> -SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
> +SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"

 So here you can just hardcode BR2_

>  TMP_FILE=$(mktemp -t .tmp.config.XXXXXXXXXX)
>  
>  echo "Using $INITFILE as base"
> @@ -131,7 +140,12 @@ fi
>  # Use the merged file as the starting point for:
>  # alldefconfig: Fills in any missing symbols with Kconfig default
>  # allnoconfig: Fills in any missing symbols with # CONFIG_* is not set
> +if [ "$BUILDROOT_MODE" = "false" ]; then
>  make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
> +else
> +cp $TMP_FILE $OUTPUT/.config
> +make $OUTPUT_ARG olddefconfig

 Why is this needed? alldefconfig works fine, no?

> +fi
>  
>  
>  # Check all specified config values took (might have missed-dependency issues)

 Since the kconfig stuff comes from upstream but is modified, we also maintain
the changes as a stack of patches in support/kconfig/patches. So you should
generate a new patch for this change and add it to the series file. I'm not sure
why we don't use a vendor branch and just merge, but that's the way we do it :-).


 Regards,
 Arnout
Angelo Compagnucci Oct. 12, 2017, 6:42 a.m. UTC | #2 
Dear Arnout Vandecappelle,

2017-10-11 23:43 GMT+02:00 Arnout Vandecappelle <arnout@mind.be>:
>
>
> On 10-10-17 22:43, Angelo Compagnucci wrote:
>> This patch adds a way to merge buildroot config file programmatically.
>
>  We already do that in test-pkg, without this patch. What are we doing wrong?

The actual support/kconfig/merge_config.sh looks for CONFIG_ symbols
via a regexp to print it's output and to check for inconsistencies.
Without the correct regexp this is impossible. Probably the right
commit message should be: "support/kconfig/merge_config.sh: check also
buildroot config files"

>> It adds an option (-b, buildroot mode) to manage buildroot config files.
>
>  I wouldn't bother with that option. It's only going to be used for Buildroot
> config files (other packages using Kconfig will have a copy of the script
> already). So we can just change it directly. And we already have a stack of
> changes on the upstream kconfig.
>
>> The buildroot mode changes the way the script looks for configurations
>> entries using the BR2_ prefix and modify the call to the make command
>> to be buildroot friendly.
>>
>> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> [snip]
>> @@ -81,7 +90,7 @@ INITFILE=$1
>>  shift;
>>
>>  MERGE_LIST=$*
>> -SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
>> +SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"
>
>  So here you can just hardcode BR2_

Ok

>
>>  TMP_FILE=$(mktemp -t .tmp.config.XXXXXXXXXX)
>>
>>  echo "Using $INITFILE as base"
>> @@ -131,7 +140,12 @@ fi
>>  # Use the merged file as the starting point for:
>>  # alldefconfig: Fills in any missing symbols with Kconfig default
>>  # allnoconfig: Fills in any missing symbols with # CONFIG_* is not set
>> +if [ "$BUILDROOT_MODE" = "false" ]; then
>>  make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
>> +else
>> +cp $TMP_FILE $OUTPUT/.config
>> +make $OUTPUT_ARG olddefconfig
>
>  Why is this needed? alldefconfig works fine, no?

Yes, indeed, this part could be removed.

>
>> +fi
>>
>>
>>  # Check all specified config values took (might have missed-dependency issues)
>
>  Since the kconfig stuff comes from upstream but is modified, we also maintain
> the changes as a stack of patches in support/kconfig/patches. So you should
> generate a new patch for this change and add it to the series file. I'm not sure
> why we don't use a vendor branch and just merge, but that's the way we do it :-).

Ok.

>
>
>  Regards,
>  Arnout
>
> --
> 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
Arnout Vandecappelle Oct. 12, 2017, 2:41 p.m. UTC | #3 
On 12-10-17 08:42, Angelo Compagnucci wrote:
> Dear Arnout Vandecappelle,
> 
> 2017-10-11 23:43 GMT+02:00 Arnout Vandecappelle <arnout@mind.be>:
>>
>>
>> On 10-10-17 22:43, Angelo Compagnucci wrote:
>>> This patch adds a way to merge buildroot config file programmatically.
>>
>>  We already do that in test-pkg, without this patch. What are we doing wrong?
> 
> The actual support/kconfig/merge_config.sh looks for CONFIG_ symbols
> via a regexp to print it's output and to check for inconsistencies.
> Without the correct regexp this is impossible. Probably the right
> commit message should be: "support/kconfig/merge_config.sh: check also
> buildroot config files"

 Ah, and in test-pkg we don't need it because we check for that explicitly after
running merge_config.sh.

 But if you start from a full .config (which you do in the example of 'make
foo_defconfig; make bar_stack), isn't it going to give a warning for each and
every symbol defined in the stack?

[snip]
>>> @@ -131,7 +140,12 @@ fi
>>>  # Use the merged file as the starting point for:
>>>  # alldefconfig: Fills in any missing symbols with Kconfig default
>>>  # allnoconfig: Fills in any missing symbols with # CONFIG_* is not set
>>> +if [ "$BUILDROOT_MODE" = "false" ]; then
>>>  make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
>>> +else
>>> +cp $TMP_FILE $OUTPUT/.config
>>> +make $OUTPUT_ARG olddefconfig
>>
>>  Why is this needed? alldefconfig works fine, no?
> 
> Yes, indeed, this part could be removed.

 We only added alldefconfig a few months ago, perhaps your initial patch still
predates that.

 Regards,
 Arnout
Angelo Compagnucci Oct. 13, 2017, 8:39 a.m. UTC | #4 
Dear Arnout Vandecappelle ,

2017-10-12 16:41 GMT+02:00 Arnout Vandecappelle <arnout@mind.be>:
>
>
> On 12-10-17 08:42, Angelo Compagnucci wrote:
>> Dear Arnout Vandecappelle,
>>
>> 2017-10-11 23:43 GMT+02:00 Arnout Vandecappelle <arnout@mind.be>:
>>>
>>>
>>> On 10-10-17 22:43, Angelo Compagnucci wrote:
>>>> This patch adds a way to merge buildroot config file programmatically.
>>>
>>>  We already do that in test-pkg, without this patch. What are we doing wrong?
>>
>> The actual support/kconfig/merge_config.sh looks for CONFIG_ symbols
>> via a regexp to print it's output and to check for inconsistencies.
>> Without the correct regexp this is impossible. Probably the right
>> commit message should be: "support/kconfig/merge_config.sh: check also
>> buildroot config files"
>
>  Ah, and in test-pkg we don't need it because we check for that explicitly after
> running merge_config.sh.
>
>  But if you start from a full .config (which you do in the example of 'make
> foo_defconfig; make bar_stack), isn't it going to give a warning for each and
> every symbol defined in the stack?

No, this is the output:

$ make qemu_arm_versatile_defconfig && make lamp_stack
umask 0022 && make -C /media/angelo/WD/data/BUILDROOT/buildroot
O=/media/angelo/WD/data/BUILDROOT/br_qemu_arm/.
qemu_arm_versatile_defconfig
  GEN     /media/angelo/WD/data/BUILDROOT/br_qemu_arm/Makefile
#
# configuration written to /media/angelo/WD/data/BUILDROOT/br_qemu_arm/.config
#
umask 0022 && make -C /media/angelo/WD/data/BUILDROOT/buildroot
O=/media/angelo/WD/data/BUILDROOT/br_qemu_arm/. lamp_stack
/media/angelo/WD/data/BUILDROOT/buildroot/support/kconfig/merge_config.sh
-b -O /media/angelo/WD/data/BUILDROOT/br_qemu_arm
/media/angelo/WD/data/BUILDROOT/br_qemu_arm/.config
/media/angelo/WD/data/BUILDROOT/buildroot/stacks/lamp_stack
Using /media/angelo/WD/data/BUILDROOT/br_qemu_arm/.config as base
Merging /media/angelo/WD/data/BUILDROOT/buildroot/stacks/lamp_stack
Value of BR2_TOOLCHAIN_BUILDROOT_CXX is redefined by fragment
/media/angelo/WD/data/BUILDROOT/buildroot/stacks/lamp_stack:
Previous value: # BR2_TOOLCHAIN_BUILDROOT_CXX is not set
New value: BR2_TOOLCHAIN_BUILDROOT_CXX=y

Value of BR2_PACKAGE_PHP is redefined by fragment
/media/angelo/WD/data/BUILDROOT/buildroot/stacks/lamp_stack:
Previous value: # BR2_PACKAGE_PHP is not set
New value: BR2_PACKAGE_PHP=y

Value of BR2_PACKAGE_APACHE is redefined by fragment
/media/angelo/WD/data/BUILDROOT/buildroot/stacks/lamp_stack:
Previous value: # BR2_PACKAGE_APACHE is not set
New value: BR2_PACKAGE_APACHE=y

  GEN     /media/angelo/WD/data/BUILDROOT/br_qemu_arm/Makefile
#
# configuration written to /media/angelo/WD/data/BUILDROOT/br_qemu_arm/.config
#
Value requested for BR2_PACKAGE_ZLIB not in final .config
Requested value:  # BR2_PACKAGE_ZLIB is not set
Actual value:     BR2_PACKAGE_ZLIB=y

Value requested for BR2_PACKAGE_EXPAT not in final .config
Requested value:  # BR2_PACKAGE_EXPAT is not set
Actual value:     BR2_PACKAGE_EXPAT=y

Value requested for BR2_PACKAGE_APR not in final .config
Requested value:  # BR2_PACKAGE_APR is not set
Actual value:     BR2_PACKAGE_APR=y

Value requested for BR2_PACKAGE_APR_UTIL not in final .config
Requested value:  # BR2_PACKAGE_APR_UTIL is not set
Actual value:     BR2_PACKAGE_APR_UTIL=y

Value requested for BR2_PACKAGE_NCURSES not in final .config
Requested value:  # BR2_PACKAGE_NCURSES is not set
Actual value:     BR2_PACKAGE_NCURSES=y

Value requested for BR2_PACKAGE_PCRE not in final .config
Requested value:  # BR2_PACKAGE_PCRE is not set
Actual value:     BR2_PACKAGE_PCRE=y

Value requested for BR2_PACKAGE_READLINE not in final .config
Requested value:  # BR2_PACKAGE_READLINE is not set
Actual value:     BR2_PACKAGE_READLINE=y


>
> [snip]
>>>> @@ -131,7 +140,12 @@ fi
>>>>  # Use the merged file as the starting point for:
>>>>  # alldefconfig: Fills in any missing symbols with Kconfig default
>>>>  # allnoconfig: Fills in any missing symbols with # CONFIG_* is not set
>>>> +if [ "$BUILDROOT_MODE" = "false" ]; then
>>>>  make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
>>>> +else
>>>> +cp $TMP_FILE $OUTPUT/.config
>>>> +make $OUTPUT_ARG olddefconfig
>>>
>>>  Why is this needed? alldefconfig works fine, no?
>>
>> Yes, indeed, this part could be removed.
>
>  We only added alldefconfig a few months ago, perhaps your initial patch still
> predates that.

Nope, only thought that olddefconfig could be better suited.

>
>  Regards,
>  Arnout
>
> --
> 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
diff mbox series

Patch

diff --git a/support/kconfig/merge_config.sh b/support/kconfig/merge_config.sh
index 8a1708b..cb6e439 100755
--- a/support/kconfig/merge_config.sh
+++ b/support/kconfig/merge_config.sh
@@ -29,6 +29,7 @@  trap clean_up HUP INT TERM
 usage() {
 	echo "Usage: $0 [OPTIONS] [CONFIG [...]]"
 	echo "  -h    display this help text"
+	echo "  -b    buildroot mode (searches for BR2_ and uses a custom make command)"
 	echo "  -m    only merge the fragments, do not execute the make command"
 	echo "  -n    use allnoconfig instead of alldefconfig"
 	echo "  -r    list redundant entries when merging fragments"
@@ -39,6 +40,8 @@  MAKE=true
 ALLTARGET=alldefconfig
 WARNREDUN=false
 OUTPUT=.
+BUILDROOT_MODE=FALSE
+CONFIG_PREFIX=CONFIG_
 
 while true; do
 	case $1 in
@@ -71,6 +74,12 @@  while true; do
 		shift 2
 		continue
 		;;
+	"-b")
+		BUILDROOT_MODE=true
+		CONFIG_PREFIX=BR2_
+		shift
+		continue
+		;;
 	*)
 		break
 		;;
@@ -81,7 +90,7 @@  INITFILE=$1
 shift;
 
 MERGE_LIST=$*
-SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(CONFIG_[a-zA-Z0-9_]*\)[= ].*/\2/p"
+SED_CONFIG_EXP="s/^\(# \)\{0,1\}\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)[= ].*/\2/p"
 TMP_FILE=$(mktemp -t .tmp.config.XXXXXXXXXX)
 
 echo "Using $INITFILE as base"
@@ -131,7 +140,12 @@  fi
 # Use the merged file as the starting point for:
 # alldefconfig: Fills in any missing symbols with Kconfig default
 # allnoconfig: Fills in any missing symbols with # CONFIG_* is not set
+if [ "$BUILDROOT_MODE" = "false" ]; then
 make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
+else
+cp $TMP_FILE $OUTPUT/.config
+make $OUTPUT_ARG olddefconfig
+fi
 
 
 # Check all specified config values took (might have missed-dependency issues)