git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
JSJosef Sipek <jsipek@fsl.cs.sunysb.edu>
Date
Jul 31, 2007, 04:05 UTC
Message-ID
<20070731040510.GD12918@filer.fsl.cs.sunysb.edu>
In-Reply-To
<1185851481271-git-send-email-eclesh@ucla.edu>

On Mon, Jul 30, 2007 at 08:11:18PM -0700, Eric Lesh wrote: ...

Show 8 quoted lines
> diff --git a/Documentation/guilt-guard.txt b/Documentation/guilt-guard.txt
> new file mode 100644
> index 0000000..6290bf7
> --- /dev/null
> +++ b/Documentation/guilt-guard.txt
> @@ -0,0 +1,40 @@
> +guilt-guard(1)
> +===============
Extra = ;)
Show 28 quoted lines
> +
> +NAME
> +----
> +guilt-guard - Assign guards to patches
> +
> +SYNOPSIS
> +--------
> +include::usage-guilt-guard.txt[]
> +
> +DESCRIPTION
> +-----------
> +Assign guards to the specified patch, or to the patch on top of the
> +stack if no patch is given on the command line.
> +
> +An unguarded patch is always pushed.
> +
> +A positive guard begins with a +. A patch with a positive guard is
> +pushed *only if* the guard is selected.
> +
> +A negative guard begins with a -. A patch with a negative guard is
> +always pushed, *unless* the guard is selected.
> +
> +OPTIONS
> +-------
> +-l|--list::
> +        List all patches and their guards
> +-n|--none::
> +        Remove all guards from a patch

I generally try to keep an empty line between the options. I don't think it affects rendering, but just for consistency.

Show 38 quoted lines
> +
> +Author
> +------
> +Written by Eric Lesh <eclesh@ucla.edu>
> +
> +Documentation
> +-------------
> +Documentation by Eric Lesh <eclesh@ucla.edu>
> +
> +include::footer.txt[]
> diff --git a/guilt b/guilt
> index 700c167..6af590c 100755
> --- a/guilt
> +++ b/guilt
> @@ -187,6 +187,72 @@ get_series()
>  		" $series
>  }
>  
> +get_guarded_series()
> +{
> +	get_series | while read p
> +	do
> +		check_guards "$p" && echo "$p"
> +	done
> +}
> +
> +# usage: check_guards <patch>
> +# Returns 0 if the patch should be pushed
> +check_guards()
> +{
> +	get_guards "$1" | while read guard
> +	do
> +		pos=`printf %s $guard | grep -e "^+"`
> +		guard=`printf %s $guard | sed -e 's/^[+-]//'`
> +		if [ "$pos" ]; then
> +			# Push +guard *only if* guard selected
> +			push=`grep -e "^$guard\$" "$guards_file" >/dev/null 2>/dev/null; echo $?`
> +			[ $push -ne 0 ] && return 1
			grep -e "^$guard\$" "$guards_file" >/dev/null 2>/dev/null
			[ $? -ne 0 ] && return 1
Much cleaner looking :) Or even:
			grep -e ....... || return 1
> +		else
> +			# Push -guard *unless* guard selected
> +			push=`grep -e "^$guard\$" "$guards_file" >/dev/null 2>/dev/null; echo $?`
> +			[ $push -eq 0 ] && return 1
Ditto.
> +		fi
> +		return 0
> +	done
> +	return $?

I'd throw in a small comment above the outter return to make sure no one tries to clean things up and remove it.

Show 12 quoted lines
> +}
> +
> +# usage: get_guards <patch>
> +get_guards()
> +{
> +	sed -n -e "\,^$1[[:space:]]*#, {
> +		s,^$1[[:space:]]*,,
> +		s,#[^+-]*,,g
> +
> +		p
> +		}
> +		" $series
Three things...
1) I generally preserve the sed script indentation
2) I like having the whole script start at col 0, syntax highlighting makes
it readable enough.
3) quote the "$series" in case there's whitespace in the repo path or branch
name

This is what I'd make it look like if I were to write it, but that's me just nitpicking at this point :)

	sed -n -e "
line 1	{
	line 2
	line 4
}
" "$series"
Show 6 quoted lines
> +}
> +
> +# usage: set_guards <patch> <guards...>
> +set_guards()
> +{
> +	p="$1"
Again, be careful about namespace polution.
> +	shift
> +	for x in "$@"; do
> +		if [ -z $(printf %s "$x" | grep -e "^[+-]") ]; then
Out of curiosity, why printf and not echo?
> +			echo "'$x' is not a valid guard name"
> +		else
> +			sed -i -e "s,^\($p[[:space:]]*.*\)$,\1 #$x," "$series"

The regexp is in double quotes, so you should escape the $ (EOL), as well as all the \. Yep, this is shell scripting at its worst.

Show 8 quoted lines
> +		fi
> +	done
> +}
> +
> +# usage: unset_guards <patch> <guards...>
> +unset_guards()
> +{
> +	p="$1"
Namespace..
Show 27 quoted lines
> +	shift
> +	for x in "$@"; do
> +		sed -i -e "/^$p[[:space:]]/s/ #$x//" "$series"
> +	done
> +}
> +
>  # usage: do_make_header <hash>
>  do_make_header()
>  {
> diff --git a/guilt-guard b/guilt-guard
> new file mode 100755
> index 0000000..a0cac2e
> --- /dev/null
> +++ b/guilt-guard
> @@ -0,0 +1,69 @@
> +#!/bin/sh
> +#
> +# Copyright (c) Eric Lesh, 2007
> +#
> +
> +USAGE="[-l|--list|-n|--none|[<patchname>] [(+|-)<guard>...]]"
> +. `dirname $0`/guilt
> +
> +print_guards()
> +{
> +	guards=`get_guards "$1"`
> +	echo "$1: $guards"
	echo "$1: `get_guards \"$1\"`"

Eliminates assignment & namespace polution :) Not really a problem here as this is a guilt-guard only function.

Show 22 quoted lines
> +}
> +
> +if [ "$1" == "-l" ] || [ "$1" == "--list" ]; then
> +	get_series | while read patch; do
> +		print_guards "$patch"
> +	done
> +	exit 0
> +elif [ "$1" == "-n" ] || [ "$1" == "--none" ]; then
> +	patch="$2"
> +	if [ -z "$patch" ]; then
> +		patch=`get_top`
> +	fi
> +	unset_guards "$patch" `get_guards "$patch"`
> +	exit 0
> +fi
> +
> +case $# in
> +	0)
> +		if [ ! -s "$applied" ]; then
> +			die "No patches applied."
> +		fi
> +		print_guards `get_top`

Quote the `get_top` in case the topmost patch has whitespace in it. Yeah, it's annoying, but safety first :)

Show 37 quoted lines
> +		;;
> +	1)
> +		if [ -z $(printf %s "$1" | grep -e '^[+-]') ]; then
> +			if [ -z $(get_series | grep -e "^$1\$") ]; then
> +				die "Patch $1 does not exist."
> +			else
> +				print_guards "$1"
> +			fi
> +		else
> +			patch=`get_top`
> +			if [ -z "$patch" ]; then
> +				die "You must specify a patch."
> +			fi
> +			unset_guards "$patch" `get_guards "$patch"`
> +			set_guards "$patch" "$1"
> +		fi
> +		;;
> +	*)
> +		if [ -z $(printf %s "$1" | grep -e '^[+-]') ]; then
> +			if [ -z $(get_series | grep -e "^$1\$") ]; then
> +				die "Patch $1 does not exist."
> +			else
> +				patch="$1"
> +			fi
> +			shift
> +		else
> +			patch=`get_top`
> +			if [ -z "$patch" ]; then
> +				die "You must specify a patch."
> +			fi
> +		fi
> +		unset_guards "$patch" `get_guards "$patch"`
> +		set_guards "$patch" "$@"
> +		;;
> +esac
> -- 
> 1.5.2
Josef 'Jeff' Sipek, falling asleep after a loooong day.
-- 
We have joy, we have fun, we have Linux on a Sun...
Previous: Eric LeshNext: Eric Lesh
Message 5 of 16 in “Add guards to guilt”
  1. 0/5 Add guards to guiltEric Lesh, Jul 31, 2007
  2. 1/5 get_series: Remove comments from end of series linesEric Lesh, Jul 31, 2007
  3. Josef SipekJul 31, 2007
  4. 2/5 guilt-guard: Assign guards to patches in seriesEric Lesh, Jul 31, 2007
  5. Josef SipekJul 31, 2007
  6. Eric LeshAug 9, 2007
  7. David KastrupAug 9, 2007
  8. Thomas AdamAug 9, 2007
  9. David KastrupAug 9, 2007
  10. Eric LeshAug 9, 2007
  11. Eric LeshAug 9, 2007
  12. Josef SipekAug 9, 2007
  13. 3/5 guilt-select: Select guards to apply when pushing patchesEric Lesh, Jul 31, 2007
  14. 4/5 get_series: return guarded patches onlyEric Lesh, Jul 31, 2007
  15. 5/5 Guards test suiteEric Lesh, Jul 31, 2007
  16. Josef SipekJul 31, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.