On Fri, May 16, 2014 at 12:46 AM, Jeff Sipek <jeffpc@xxxxxxxxxxxxxx> wrote:
> On Tue, May 13, 2014 at 10:30:48PM +0200, Per Cederqvist wrote:
>> If you run something like "guilt header '.*'" the command would crash,
>> because the grep comand that tries to ensure that the patch exist
>> would detect a match, but the later code expected the match to be
>> exact.
>>
>> Fixed by comparing exact strings.
>>
>> And as a creeping feature "guilt header" will now try to use the
>> supplied patch name as an unachored regexp if no exact match was
>> found. If the regexp yields a unique match, it is used; if more than
>> one patch matches, the names of all patches are listed and the command
>> fails. (Exercise left to the reader: generalized this so that "guilt
>> push" also accepts a unique regular expression.)
>>
>> Signed-off-by: Per Cederqvist <cederp@xxxxxxxxx>
>> ---
>> guilt-header | 28 +++++++++++++++++++++++++---
>> 1 file changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/guilt-header b/guilt-header
>> index 41e00cc..4701b31 100755
>> --- a/guilt-header
>> +++ b/guilt-header
>> @@ -45,10 +45,32 @@ esac
>> [ -z "$patch" ] && die "No patches applied."
>>
>> # check that patch exists in the series
>> -ret=`get_full_series | grep -e "^$patch\$" | wc -l`
>> -if [ $ret -eq 0 ]; then
>> - die "Patch $patch is not in the series"
>> +full_series=`get_tmp_file series`
>> +get_full_series > "$full_series"
>> +found_patch=
>> +while read x; do
>> + if [ "$x" = "$patch" ]; then
>> + found_patch="$patch"
>> + break
>> + fi
>> +done < "$full_series"
>
> We have to use a temp file instead of a 'get_full_series | while read x; do ...'
> because that'd create a subshell, correct?
Yes. Also (and probably less importantly) we sometimes need to run grep on
the same output (see the creation of TMP_MATCHES below) and it would
be a bit wasteful to run get_full_series twice. (The assumption is that it is
cheaper to create a temp file than to recompute the value. I have not measured
this, though.)
>> +if [ -z "$found_patch" ]; then
>> + TMP_MATCHES=`get_tmp_file series`
>> + grep "$patch" < "$full_series" > "$TMP_MATCHES"
>> + nr=`wc -l < $TMP_MATCHES`
>> + if [ $nr -gt 1 ]; then
>> + echo "$patch does not uniquely identify a patch. Did you mean any of these?" >&2
>> + sed 's/^/ /' "$TMP_MATCHES" >&2
>> + rm -f "$TMP_MATCHES"
>> + exit 1
>> + elif [ $nr -eq 0 ]; then
>> + rm -f "$TMP_MATCHES"
>> + die "Patch $patch is not in the series"
>> + fi
>> + found_patch=`cat $TMP_MATCHES`
>> + rm -f "$TMP_MATCHES"
>> fi
>> +patch="$found_patch"
>
> Do we not delete $full_series?
Good catch. Will fix in the next version of the series.
I'll also rename the variable $TMP_FULL_SERIES to adhere
to the apparent coding style. (But I will not fix guilt-patchbomb
that uses $dir as a temporary variable.)
/ceder
>>
>> # FIXME: warn if we're editing an applied patch
>>
>> --
>> 1.8.3.1
>>
>
> --
> OpenIndiana ibdm: 8 cores, 12 GB RAM
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html