Re: [GUILT PATCH 2/4] guilt-guard: Assign guards to patches in series

From: Josef Sipek <jsipek@fsl.cs.sunysb.edu>
Date: Jul 30, 2007, 19:28 UTC
Message-ID: <20070730192815.GC17253@filer.fsl.cs.sunysb.edu>
In-Reply-To: <87k5sics0f.fsf@hubert.paunchy.net>

On Sun, Jul 29, 2007 at 11:41:52PM -0700, Eric Lesh wrote:

Show 35 quoted lines

> Josef Sipek <jsipek@fsl.cs.sunysb.edu> writes:
> 
> [...]
> 
> >> +get_guarded_series()
> >> +{
> >> +	get_series | while read p
> >> +	do
> >> +		[ -z `check_guards $p` ] && echo "$p"
> >
> > Having check_guards return 0 or 1 makes things cleaner:
> >
> > check_guards "$p" && echo "$p"
> >
> >> +	done
> >> +}
> >> +
> >> +# usage: check_guards <patch>
> >> +# Returns t if the patch should be skipped
> >> +check_guards()
> >> +{
> >> +        get_guards "$1" | while read guard
> >> +        do
> >> +                pos=`echo $guard | grep -e "^+"`
> >> +                guard=`echo $guard | sed -e 's/[+-]//'`
> >> +                if [ $pos ]; then
> >> +                        # Push +guard *only if* guard selected
> >> +                        push=`grep -e "^$guard\$" "$guards_file" > /dev/null; echo $?`
> >> +                        [ $push -ne 0 ] && echo t
> >
> > 			   [ $push -ne 0 ] && return 1
> >
> 
> This returns from the subshell created by the pipe and the while loop,
> right?

Right, sorry.

Show 18 quoted lines

> So I'm using:
> 
> check_guards()
> {
> 	get_guards "$1" | while read guard
> 	do
> 		pos=`echo $guard | grep -e "^+"`
> 		guard=`echo $guard | sed -e 's/^[+-]//'`
> 		if [ $pos ]; then
> 			# Push +guard *only if* guard selected
> 			push=`grep -e "^$guard\$" "$guards_file" > /dev/null; echo $?`
> 			[ $push -ne 0 ] && return 1
> 		else
> 			# Push -guard *unless* guard selected
> 			push=`grep -e "^$guard\$" "$guards_file" > /dev/null; echo $?`
> 			[ $push -eq 0 ] && return 1
> 		fi
>                 return 0

Beware of whitespace :)

Show 13 quoted lines

> 	done
> 	return $?
> }
> 
> where 1 means push.
> 
> >> +# usage: get_guards <patch>
> >> +get_guards()
> >> +{
> >> +	grep -e "^$1[[:space:]]*#" < "$series" | sed -e "s/^$1 //" -e 's/#[^+-]*//g'
> >> +}
> 
> Should this also be one sed script instead of a grep + sed?

I'm all for more complex sed/awk scripts to replace lots of forks and pipes.

Show 23 quoted lines

> >> +
> >> +# usage: set_guards <patch> <guards>
> >
> > I'd try to make it clearer that multiple guards can be specified.
> >
> 
> Done with <guards...> now.
> 
> >> +set_guards()
> >> +{
> >> +	p="$1"
> >> +	shift
> >> +	for x in "$@"; do
> >> +		if [ -z $(echo "$x" | grep -e "^[+-]") ]; then
> >
> > Is that the only restriction on the guard name?
> >
> 
> Yes.  On patches, you put a '+guard' or '-guard'.  When selecting with
> guilt-select, it's just 'guard'.  The + or - just means 'apply when
> selected' or 'apply unless selected'.  You can edit things manually to
> make guards with a space in the name, but the mechanism will work even
> in that case.

I am thinking that it _might_ make sense to have some validate_guard_name function - I am not sure if it would be used enough to make it useful instead of just obfuscating the code.

Show 13 quoted lines

> >> +			echo "'$x' is not a valid guard name"
> >> +		else
> >> +			sed -i -e "s/^\($p[[:space:]]*.*\)$/\1 #$x/" "$series"
> >> +		fi
> >> +	done
> >> +}
> >> +
> >> +# usage: unset_guards <patch> <guards>
> >
> 
> [...]
> 
> The rest I'll do.  Thanks for the review.

Thanks for the patches :)

Jeff.

-- 
The obvious mathematical breakthrough would be development of an easy way to
factor large prime numbers.
		- Bill Gates, The Road Ahead, pg. 265