[PATCH] gen_build_files.sh: rewrite with sed

[Christopher Barry]

On Wed, 2010-11-17 at 18:36 -0500, Mike Frysinger wrote:
> On Tuesday, November 16, 2010 15:31:13 Cristian Ionescu-Idbohrn wrote:
> > 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.

IMHO, *ALL* variables should be in curly braces as a safe convention.

> 
> nothing superfluous about it.  your general usage of using carats instead of 
> properly quoting context however is obnoxious and not easily usable unless i 
> choose specifically monospace fonts in my e-mail client.

well, this is a convention mostly destroyed by LookOut.

> 
> > > +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
> 
> wrong.  the code works fine.

AFAIK, The only place where the simultaneous define and assignment
breaks down is where the assignment is like: 
local foo=$(bar baz) || err_handler
In that case, the 'local' keyword will always return true, causing the
err_handler routine to not be executed if bar fails.
For simple assignment, there is no issue at all. In fact, the code above
is the most efficient.

> 
> > > +	#chk "${dst}"
> > > +	(
> > 
> > 	^
> > 	this will open and run a subshell (fork); is that necessary?
> > 
> > Would not:
> > 
> > 	{
> > 		cmd1
> > 		cmd2
> > 		...
> > 	} | sed ...
> > 
> > workas well; forkless?
> 
> because of the pipe, the number of forks is the same.  try stracing:
> 	bash -c '( echo; ) | cat'
> 	bash -c '{ echo; } | cat
> 
> > > +		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
> 
> no idea what you're talking about.  i see no typo.
> 
> > > +		else
> > > +			if [ -n "${insert}" ]; then
> > 			     ^^
> > 			     useless, this:
> > 
> > 			if [ "$insert" ]; then
> > 
> > does the same job.
> 
> style wise, i prefer the braces to be *consistent* and not worry about random 
> inlining of vars with plain text.  so any feedback along these lines is 
> ignored.

One thing I would say is (in bash anyway) '[[' is a builtin, while '['
is not, and thus will incur less overhead. If you're using bash, you
should use this always (unless there is some good reason not to)


> 
> > >  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?
> 
> you cant make multiple statements and expect a single true/false response to 
> cover it.  backticks are not obsolete and they're not going anywhere.  
> obviously backticks are a pita to nest, but there is no nesting.  ive left the 
> existing code unchanged.
> 
> > > +generate \
> > > +	"$srctree/include/applets.src.h" \
> > > +	"include/applets.h" \
> > 	^                 ^
> > 	now, why does this need to be quoted?
> 
> consistency.  obviously $srctree needs quoting.
> 
> > >  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)?
> 
> ive let it alone.  dont care either way.
> -mike
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox