Message ID | 87lhm85xhy.fsf@redhat.com |
---|---|
State | New |
Headers | show |
On 12/15/2014 11:00 PM, Sergio Durigan Junior wrote: > +# Check if grep supports the '--text' option. > + > +GREP_TEXT_OPT="--text" > +if grep --text 2>&1 | grep "unrecognized option" > /dev/null 2>&1 ; then > + GREP_TEXT_OPT="" > +fi > + That assumes all greps output "unrecognized option" on unrecognized options. I don't think that can be counted on being portable? ISTM reversing the logic of the test would be better. IOW, test that --text actually works. Something like this perhaps: # If grep supports the '--text' option, use it. GREP_TEXT_OPT="" if echo foo | grep --text foo > /dev/null 2>&1 ; then GREP_TEXT_OPT="--text" fi Thanks, Pedro Alves
On Tue, Dec 16, 2014 at 09:36:33AM +0000, Pedro Alves wrote: > On 12/15/2014 11:00 PM, Sergio Durigan Junior wrote: > > +# Check if grep supports the '--text' option. > > + > > +GREP_TEXT_OPT="--text" > > +if grep --text 2>&1 | grep "unrecognized option" > /dev/null 2>&1 ; then > > + GREP_TEXT_OPT="" > > +fi > > + > > That assumes all greps output "unrecognized option" on > unrecognized options. I don't think that can be counted on being > portable? ISTM reversing the logic of the test would be better. > IOW, test that --text actually works. Something like this perhaps: > > # If grep supports the '--text' option, use it. > GREP_TEXT_OPT="" > if echo foo | grep --text foo > /dev/null 2>&1 ; then > GREP_TEXT_OPT="--text" > fi Even better include some non-text bytes around the foo string in the input you feed to grep. Also, perhaps better would be to use a GREP variable, initialized to GREP=grep or GREP="grep --text" and just invoke $GREP. Jakub
On Tuesday, December 16 2014, Jakub Jelinek wrote: > On Tue, Dec 16, 2014 at 09:36:33AM +0000, Pedro Alves wrote: >> On 12/15/2014 11:00 PM, Sergio Durigan Junior wrote: >> > +# Check if grep supports the '--text' option. >> > + >> > +GREP_TEXT_OPT="--text" >> > +if grep --text 2>&1 | grep "unrecognized option" > /dev/null 2>&1 ; then >> > + GREP_TEXT_OPT="" >> > +fi >> > + >> >> That assumes all greps output "unrecognized option" on >> unrecognized options. I don't think that can be counted on being >> portable? ISTM reversing the logic of the test would be better. >> IOW, test that --text actually works. Something like this perhaps: >> >> # If grep supports the '--text' option, use it. >> GREP_TEXT_OPT="" >> if echo foo | grep --text foo > /dev/null 2>&1 ; then >> GREP_TEXT_OPT="--text" >> fi > > Even better include some non-text bytes around the foo string in the input you feed to grep. > Also, perhaps better would be to use a GREP variable, initialized to > GREP=grep > or > GREP="grep --text" > and just invoke $GREP. Hm, right. Thank you Pedro and Jakub for the comments. I will work on another version of the patch.
diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh index a83c8e8..ebf93bf 100755 --- a/contrib/dg-extract-results.sh +++ b/contrib/dg-extract-results.sh @@ -127,13 +127,20 @@ do done test $ERROR -eq 0 || exit 1 +# Check if grep supports the '--text' option. + +GREP_TEXT_OPT="--text" +if grep --text 2>&1 | grep "unrecognized option" > /dev/null 2>&1 ; then + GREP_TEXT_OPT="" +fi + if [ -z "$TOOL" ]; then # If no tool was specified, all specified summary files must be for # the same tool. - CNT=`grep '=== .* tests ===' $SUM_FILES | $AWK '{ print $3 }' | sort -u | wc -l` + CNT=`grep $GREP_TEXT_OPT '=== .* tests ===' $SUM_FILES | $AWK '{ print $3 }' | sort -u | wc -l` if [ $CNT -eq 1 ]; then - TOOL=`grep '=== .* tests ===' $FIRST_SUM | $AWK '{ print $2 }'` + TOOL=`grep $GREP_TEXT_OPT '=== .* tests ===' $FIRST_SUM | $AWK '{ print $2 }'` else msg "${PROGNAME}: sum files are for multiple tools, specify a tool" msg "" @@ -144,7 +151,7 @@ else # Ignore the specified summary files that are not for this tool. This # should keep the relevant files in the same order. - SUM_FILES=`grep -l "=== $TOOL" $SUM_FILES` + SUM_FILES=`grep $GREP_TEXT_OPT -l "=== $TOOL" $SUM_FILES` if test -z "$SUM_FILES" ; then msg "${PROGNAME}: none of the specified files are results for $TOOL" exit 1 @@ -233,7 +240,7 @@ else VARIANTS="" for VAR in $VARS do - grep "Running target $VAR" $SUM_FILES > /dev/null && VARIANTS="$VARIANTS $VAR" + grep $GREP_TEXT_OPT "Running target $VAR" $SUM_FILES > /dev/null && VARIANTS="$VARIANTS $VAR" done fi