Message ID | 1507668210-5427-2-git-send-email-angelo.compagnucci@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Series | Adding software stacks | expand |
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
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
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
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 --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)
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(-)