Re: [PATCH 1/3] scripts: kconfig: merge_config.sh: refactor from shell/sed/grep to awk

[Mikko Rapeli <mikko.rapeli@linaro.org>]

Hi,

On Tue, Dec 30, 2025 at 01:55:49PM -0700, Nathan Chancellor wrote:
> Hi Mikko,
> 
> I don't really know awk well so I won't give as much review on that
> part.
> 
> On Mon, Dec 29, 2025 at 01:44:45PM +0200, Mikko Rapeli wrote:
> > From: Anders Roxell <anders.roxell@linaro.org>
> > 
> > merge_config.sh shell/sed/grep loop scales poorly and is slow.
> > With Yocto genericarm64 kernel and around 190 config fragments
> > the script takes more than 20 minutes to run on a fast build machine.
> > Re-implementation with awk does the same job in 10 seconds.
> > Using awk since it is likely available in the build environments
> > and using perl, python etc would introduce more complex runtime
> > dependencies. awk is good enough and lot better than shell/sed/grep.
> > 
> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> > Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> > ---
> >  scripts/kconfig/merge_config.sh | 161 ++++++++++++++++++++++++--------
> >  1 file changed, 123 insertions(+), 38 deletions(-)
> > 
> > diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
> > index 79c09b378be8..46397d7c6957 100755
> > --- a/scripts/kconfig/merge_config.sh
> > +++ b/scripts/kconfig/merge_config.sh
> > @@ -16,8 +16,8 @@
> >  set -e
> >  
> >  clean_up() {
> > -	rm -f $TMP_FILE
> > -	rm -f $MERGE_FILE
> > +	rm -f "${TMP_FILE}"
> > +	rm -f "${TMP_FILE}.new"
> >  }
> >  
> >  usage() {
> > @@ -121,7 +121,6 @@ SED_CONFIG_EXP1="s/^\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)=.*/\1/p"
> >  SED_CONFIG_EXP2="s/^# \(${CONFIG_PREFIX}[a-zA-Z0-9_]*\) is not set$/\1/p"
> 
> The SED_CONFIG_EXP variables are unused now so they can be removed.

Right, removing in v2.

> >  TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
> > -MERGE_FILE=$(mktemp ./.merge_tmp.config.XXXXXXXXXX)
> >  
> >  echo "Using $INITFILE as base"
> >  
> > @@ -136,42 +135,128 @@ for ORIG_MERGE_FILE in $MERGE_LIST ; do
> >  		echo "The merge file '$ORIG_MERGE_FILE' does not exist.  Exit." >&2
> >  		exit 1
> >  	fi
> > -	cat $ORIG_MERGE_FILE > $MERGE_FILE
> > -	CFG_LIST=$(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $MERGE_FILE)
> > -
> > -	for CFG in $CFG_LIST ; do
> > -		grep -q -w $CFG $TMP_FILE || continue
> > -		PREV_VAL=$(grep -w $CFG $TMP_FILE)
> > -		NEW_VAL=$(grep -w $CFG $MERGE_FILE)
> > -		BUILTIN_FLAG=false
> > -		if [ "$BUILTIN" = "true" ] && [ "${NEW_VAL#CONFIG_*=}" = "m" ] && [ "${PREV_VAL#CONFIG_*=}" = "y" ]; then
> > -			${WARNOVERRIDE} Previous  value: $PREV_VAL
> > -			${WARNOVERRIDE} New value:       $NEW_VAL
> > -			${WARNOVERRIDE} -y passed, will not demote y to m
> > -			${WARNOVERRIDE}
> > -			BUILTIN_FLAG=true
> > -		elif [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then
> > -			${WARNOVERRIDE} Value of $CFG is redefined by fragment $ORIG_MERGE_FILE:
> > -			${WARNOVERRIDE} Previous  value: $PREV_VAL
> > -			${WARNOVERRIDE} New value:       $NEW_VAL
> > -			${WARNOVERRIDE}
> > -			if [ "$STRICT" = "true" ]; then
> > -				STRICT_MODE_VIOLATED=true
> > -			fi
> > -		elif [ "$WARNREDUN" = "true" ]; then
> > -			${WARNOVERRIDE} Value of $CFG is redundant by fragment $ORIG_MERGE_FILE:
> > -		fi
> > -		if [ "$BUILTIN_FLAG" = "false" ]; then
> > -			sed -i "/$CFG[ =]/d" $TMP_FILE
> > -		else
> > -			sed -i "/$CFG[ =]/d" $MERGE_FILE
> > -		fi
> > -	done
> > -	# In case the previous file lacks a new line at the end
> > -	echo >> $TMP_FILE
> > -	cat $MERGE_FILE >> $TMP_FILE
> > -done
> > +	# Use awk for single-pass processing instead of per-symbol grep/sed
> > +	if ! awk -v prefix="$CONFIG_PREFIX" \
> 
> Please use the AWK variable from Kbuild here:
> 
>   if ${AWK} -v prefix="$CONFIG_PREFIX" \
> 
> You can do something like:
> 
>   if [ -z "${AWK}" ]; then
>       AWK=awk
>   fi
> 
> towards the top of the script to have the script continue to work
> outside of Kbuild as well.

Ok will add this in v2.

> > +		-v warnoverride="$WARNOVERRIDE" \
> > +		-v strict="$STRICT" \
> > +		-v builtin="$BUILTIN" \
> > +		-v warnredun="$WARNREDUN" '
> > +	BEGIN {
> > +		strict_violated = 0
> > +		cfg_regex = "^" prefix "[a-zA-Z0-9_]+"
> > +		notset_regex = "^# " prefix "[a-zA-Z0-9_]+ is not set$"
> > +	}
> > +
> > +	# Extract config name from a line, returns "" if not a config line
> > +	function get_cfg(line) {
> > +		if (match(line, cfg_regex)) {
> > +			return substr(line, RSTART, RLENGTH)
> > +		} else if (match(line, notset_regex)) {
> > +			# Extract CONFIG_FOO from "# CONFIG_FOO is not set"
> > +			sub(/^# /, "", line)
> > +			sub(/ is not set$/, "", line)
> > +			return line
> > +		}
> > +		return ""
> > +	}
> > +
> > +	# Normalize: strip trailing comments, convert "is not set" to "=n"
> > +	function normalize(line) {
> > +		if (line == "") return ""
> > +		sub(/[[:space:]]+#.*/, "", line)
> > +		if (line ~ / is not set$/) {
> > +			sub(/^# /, "", line)
> > +			sub(/ is not set$/, "=n", line)
> > +		}
> > +		return line
> > +	}
> > +
> > +	function warn_builtin(cfg, prev, new) {
> > +		if (warnoverride == "true") return
> > +		print cfg ": -y passed, will not demote y to m" > "/dev/stderr"
> > +		print "  Previous value: " prev > "/dev/stderr"
> > +		print "  New value:	 " new > "/dev/stderr"
> > +		print "" > "/dev/stderr"
> > +	}
> > +
> > +	function warn_redefined(cfg, prev, new) {
> > +		if (warnoverride == "true") return
> > +		print "Value of " cfg " is redefined by fragment " mergefile ":" > "/dev/stderr"
> > +		print "  Previous value: " prev > "/dev/stderr"
> > +		print "  New value:	 " new > "/dev/stderr"
> > +		print "" > "/dev/stderr"
> > +	}
> > +
> > +	function warn_redundant(cfg) {
> > +		if (warnredun != "true" || warnoverride == "true") return
> > +		print "Value of " cfg " is redundant by fragment " mergefile ":" > "/dev/stderr"
> > +	}
> > +
> > +	# First pass: read merge file, store all lines and index
> > +	FILENAME == ARGV[1] {
> > +	        mergefile = FILENAME
> > +		merge_lines[FNR] = $0
> > +		merge_total = FNR
> > +		cfg = get_cfg($0)
> > +		if (cfg != "") {
> > +			merge_cfg[cfg] = $0
> > +			merge_cfg_line[cfg] = FNR
> > +		}
> > +		next
> > +	}
> >  
> > +	# Second pass: process base file (TMP_FILE)
> > +		cfg = get_cfg($0)
> > +
> > +		# Not a config or not in merge file - keep it
> > +		if (cfg == "" || !(cfg in merge_cfg)) {
> 
>   I get an error on this line when trying to use this script via the arm64
>   virtconfig target:
> 
>   $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux- clean virtconfig
>   awk: cmd. line:70:              if (cfg == "" || !(cfg in merge_cfg)) {
>   awk: cmd. line:70:              ^ syntax error
>   make[2]: *** [arch/arm64/Makefile:222: virtconfig] Error 1
>   ...
> 
>   $ awk --version | head -1
>   GNU Awk 5.3.2, API 4.0, PMA Avon 8-g1, (GNU MPFR 4.2.2, GNU MP 6.3.0)

Oops. Sorry, there is a curly brace missing:

@@ -216,6 +214,7 @@ for ORIG_MERGE_FILE in $MERGE_LIST ; do
        }

        # Second pass: process base file (TMP_FILE)
+       {
                cfg = get_cfg($0)

                # Not a config or not in merge file - keep it

but this is fatal so I had been testing an old version of the patch
in my yocto/bitbake setup. I will double check this with v2.

Cheers,

-Mikko