[PATCH] gen_build_files.sh: rewrite with sed

Cristian Ionescu-Idbohrn cristian.ionescu-idbohrn at axis.com
Tue Nov 16 20:31:13 UTC 2010


On Tue, 16 Nov 2010, Mike Frysinger wrote:
>
> The shell parsing of files is incredibly slow on many systems.  With
> one report, the process was taking a minute or two which made people
> thing the build was hung.  So rewrite the craziness with sed and proper
> shell functions.  On an idle system, this cut the runtime by half.

Sounds good.  Still, you can surely explain the inconsistant and totally
superfluous curly brackets around variable names.

> Signed-off-by: Mike Frysinger <vapier at gentoo.org>
> ---
>  scripts/gen_build_files.sh |  100 ++++++++++++++++++++-----------------------
>  1 files changed, 47 insertions(+), 53 deletions(-)
>
> diff --git a/scripts/gen_build_files.sh b/scripts/gen_build_files.sh
> index 09a95b5..1c1edcc 100755
> --- a/scripts/gen_build_files.sh
> +++ b/scripts/gen_build_files.sh
> @@ -9,43 +9,54 @@ mkdir include 2>/dev/null
>
>  srctree="$1"
>
> +status() { printf '  %-8s%s\n' "$1" "$2"; }
> +gen() { status "GEN" "$@"; }
> +chk() { status "CHK" "$@"; }
> +
> +generate()
> +{
> +	local src="$1" dst="$2" header="$3" insert="$4"
	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
All of it or parts of it may go wrong if the default shell is dash, as
word word splitting and/or variable expansion in that shell works in
misterious ways :(  Keeping variable declarations separate from variable
assignments may get you around the problem.  Ex.

	local src dst header insert
	src=$1
	dst=$2
	header=$3
	insert=$4

> +	#chk "${dst}"
> +	(
	^
	this will open and run a subshell (fork); is that necessary?

Would not:

	{
		cmd1
		cmd2
		...
	} | sed ...

workas well; forkless?

> +		echo "${header}"
> +		if grep -qs '^INSERT$' "${src}"; then
> +			sed -n '1,/^INSERT$/p' "${src}"
> +			echo "${insert}"
> +			sed -n '/^INSERT$/,$p' "${src}"
			                   ^
			                   may be a typo

> +		else
> +			if [ -n "${insert}" ]; then
			     ^^
			     useless, this:

			if [ "$insert" ]; then

does the same job.

> +				echo "ERROR: INSERT line missing in: ${src}" 1>&2
> +			fi

and this:

			[ -z "$insert" ] ||
				echo "ERROR: INSERT line missing in: $src" 1>&2

does the same thing, using less electrons :)

> +			cat "${src}"
> +		fi
> +	) | sed '/^INSERT$/d' > "${dst}.tmp"
> +	if ! cmp -s "${dst}" "${dst}.tmp"; then
> +		gen "${dst}"
> +		mv "${dst}.tmp" "${dst}"
> +	else
> +		rm -f "${dst}.tmp"
> +	fi
> +}
> +
>  # (Re)generate include/applets.h
> -src="$srctree/include/applets.src.h"
> -dst="include/applets.h"
>  s=`sed -n 's@^//applet:@@p' -- "$srctree"/*/*.c "$srctree"/*/*/*.c`

I read somewhere backtick is obsolete, and easier nestable $(...) should
be used instead.  Is it true?

> -old=`cat "$dst" 2>/dev/null`
> -# Why "IFS='' read -r REPLY"??
> -# This atrocity is needed to read lines without mangling.
> -# IFS='' prevents whitespace trimming,
> -# -r suppresses backslash handling.
> -new=`echo "/* DO NOT EDIT. This file is generated from applets.src.h */"
> -while IFS='' read -r REPLY; do
> -	test x"$REPLY" = x"INSERT" && REPLY="$s"
> -	printf "%s\n" "$REPLY"
> -done <"$src"`
> -if test x"$new" != x"$old"; then
> -	echo "  GEN     $dst"
> -	printf "%s\n" "$new" >"$dst"
> -fi
> +generate \
> +	"$srctree/include/applets.src.h" \
> +	"include/applets.h" \
	^                 ^
	now, why does this need to be quoted?

and more of the above followed.

>  exit 0

What's the purpose with that 'exit 0'?  If some or all the above failed,
pretend it succeeded (as I don't see errexit is used)?


Cheers,

-- 
Cristian


More information about the busybox mailing list