[pacman-dev] [PATCH 2/3] makepkg: use extglob in place of regex

[Allan McRae]

On 06/09/11 20:03, Dave Reisner wrote:
> On Tue, Sep 06, 2011 at 05:23:06PM +1000, Allan McRae wrote:
>> On 05/09/11 14:57, Dave Reisner wrote:
>>> We seem to enjoy using bash regex capabilities, but never referencing
>>> the result with BASH_REMATCH. Replace almost all regexes with equivalent
>>> extglobs which are faster and functionally equivalent in these cases.
>>>
>>> Signed-off-by: Dave Reisner<dreisner at archlinux.org>
>>
>> I have no objections to this.  So ack with minor comments below.
>>
>>> ---
>>> The only remaining regexes are in lib{depends,provides} logic and I'm not
>>> really comfortable touching those. There's a lot of cleanup that should
>>> be done separately for that code where removal of those regexes can be
>>> handled accordingly...
>>
>> One day I will get to finishing my adjustments there...  honest!
>>
>>>
>>>   scripts/makepkg.sh.in |   16 +++++++++-------
>>>   1 files changed, 9 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>>> index 2f06b9b..75d168b 100644
>>> --- a/scripts/makepkg.sh.in
>>> +++ b/scripts/makepkg.sh.in
>>> @@ -81,6 +81,8 @@ FORCE_VER=""
>>>
>>>   PACMAN_OPTS=
>>>
>>> +shopt -s extglob
>>> +
>>>   ### SUBROUTINES ###
>>>
>>>   plain() {
>>> @@ -341,7 +343,7 @@ in_array() {
>>>   source_has_signatures(){
>>>   	local file
>>>   	for file in "${source[@]}"; do
>>> -		if [[ $file =~ \.(sig|asc)$ ]]; then
>>> +		if [[ $file = *.@(sig|asc) ]]; then
>>>   			return 0
>>>   		fi
>>>   	done
>>> @@ -420,7 +422,7 @@ download_file() {
>>>   run_pacman() {
>>>   	local cmd
>>>   	printf -v cmd "%q " "$PACMAN" $PACMAN_OPTS "$@"
>>> -	if (( ! ASROOT ))&&   [[ ! $1 =~ ^-(T|Qq)$ ]]; then
>>> +	if (( ! ASROOT ))&&   [[ ! $1 = -@(T|Qq) ]]; then
>>>   		if type -p sudo>/dev/null; then
>>>   			cmd="sudo $cmd"
>>>   		else
>>> @@ -709,7 +711,7 @@ check_pgpsigs() {
>>>
>>>   	for file in "${source[@]}"; do
>>>   		file="$(get_filename "$file")"
>>> -		if [[ ! $file =~ \.(sig|asc)$ ]]; then
>>> +		if [[ ! $file = *.@(sig|asc) ]]; then
>>>   			continue
>>>   		fi
>>>
>>> @@ -1451,7 +1453,7 @@ check_sanity() {
>>>   	awk -F'=' '/^[[:space:]]*pkgver=/ { $1=""; print $0 }' "$BUILDFILE" |
>>>   	while read -r i; do
>>>   		eval i=\"$(sed 's/^\(['\''"]\)\(.*\)\1$/\2/'<<<   "$i")\"
>>> -		if [[ $i =~ [[:space:]:-] ]]; then
>>> +		if [[ $i = *+([[:space:]:-])* ]]; then
>>>   			error "$(gettext "%s is not allowed to contain colons, hyphens or whitespace.")" "pkgver"
>>>   			return 1
>>>   		fi
>>> @@ -1460,7 +1462,7 @@ check_sanity() {
>>>   	awk -F'=' '/^[[:space:]]*pkgrel=/ { $1=""; print $0 }' "$BUILDFILE" |
>>>   	while read -r i; do
>>>   		eval i=\"$(sed 's/^\(['\''"]\)\(.*\)\1$/\2/'<<<   "$i")\"
>>> -		if [[ $i =~ [[:space:]-] ]]; then
>>> +		if [[ $i = *+([[:space:]-])* ]]; then
>>>   			error "$(gettext "%s is not allowed to contain hyphens or whitespace.")" "pkgrel"
>>>   			return 1
>>>   		fi
>>> @@ -1469,7 +1471,7 @@ check_sanity() {
>>>   	awk -F'=' '/^[[:space:]]*epoch=/ { $1=""; print $0 }' "$BUILDFILE" |
>>>   	while read -r i; do
>>>   		eval i=\"$(sed 's/^\(['\''"]\)\(.*\)\1$/\2/'<<<   "$i")\"
>>> -		if [[ ! $i =~ ^[0-9]*$ ]]; then
>>> +		if [[ $i&&   $i != +([0-9]) ]]; then
>>
>> Do we need the test for $i there given it should be non-null for us
>> not to exit the while loop?  Also, should we use [:digit:] rather
>> than [0-9]?
>>
>
> You're correct that the -n test can go. [0-9] and [:digit:] are
> actually equivalent -- locale isn't an issue here, but I'll change it in
> favor of consistency/safety.
>
>>>   			error "$(gettext "%s must be an integer.")" "epoch"
>>>   			return 1
>>>   		fi
>>> @@ -1526,7 +1528,7 @@ check_sanity() {
>>>   		sed -e "s/optdepends=/optdepends_list+=/" -e "s/#.*//" -e 's/\\$//')
>>>   	for i in "${optdepends_list[@]}"; do
>>>   		local pkg=${i%%:*}
>>> -		if [[ ! $pkg =~ ^[[:alnum:]\>\<\=\.\+\_\-]+$ ]]; then
>>> +		if [[ $pkg != +([[:alnum:]><=.+_-]) ]]; then
>>
>> Any chance you want to fix this for versioned optdepends containing
>> an epoch at the same time?  The optdepends work requires a colon
>> followed by a space before the description.  So I guess:
>>
>> local pkg=${i%%: *}
>> if [[ $pkg != +([[:alnum:]><=.+_-:]) ]]; then
>>
>> should do that?
>>
>
> Seems reasonable. We probably have some packages that don't fit this
> criteria of having a colon followed by a whitespace and description
> (e.g. extra/weechat). Are we going to add that to our sanity checks?
>

Having no description is fine too.   We just are enforcing that if there 
is a description, then it comes after ": ".

Allan