Re: [pcre-dev] PCRE autotools patch drg3

Startseite
Nachricht löschen
Autor: Bob Rossi
Datum:  
To: Craig Silverstein
CC: pcre-dev, skunk
Betreff: Re: [pcre-dev] PCRE autotools patch drg3
On Wed, Feb 28, 2007 at 05:43:25PM -0800, Craig Silverstein wrote:
> } This was working before your changes. Do you have any idea what
> } would have caused this?
>
> Sounds to me like the patch didn't apply cleanly, probably because of
> the \r's in RunGrepTest.in. Others were complaining about this, which
> is why he uuencoded the patch last time. Maybe you still had trouble?
>
> Look at your RunGrepTest.in and make sure it has a line like this:
>
>    echo "---------------------------- Test 36 -----------------------------" >>testtry
>    $valgrind ./pcregrep -L -r --include=grepinput --exclude 'grepinput$' 'fox' $testdata | sort >>testtry

     echo "---------------------------- Test 36 -----------------------------" >>testtry
     $valgrind ./pcregrep -L -r --include=grepinput --exclude 'grepinput$' 'fox' $testdata | sort >>testtry
     echo "RC=$?" >>testtry


Looks like I do. The patch applied cleanly actually.

> } -cf=diff
> } +cf="diff -u"
> }
> } Is this also a change you wanted to make to get the test suite passing?
>
> This just makes the output easier to read when the test fails.
>
> } -    valgrind) valgrind="valgrind -q --leak-check=no";; 
> } +    valgrind) valgrind="valgrind -q --leak-check=no";;
> }
> } Why did you change the above line?

>
> It sounds like he uses a pretty over-eager editor, so I wouldn't be
> surprised if it just went and stripped whitespace from the ends of the
> lines automatically. I agree with you that's not generally a good
> thing to put in a patch: it just causes clutter.


OK, can these be removed from the diff? I wouldn't expect an editor to
make changes like this. Seems rather inconvienent.

> } -$valgrind ./pcregrep --newline=cr -F "def
> } jkl" $testdata/grepinputx >>testtry
> } +pattern=`printf 'def\rjkl'`
> } +$valgrind ./pcregrep --newline=cr -F "$pattern" $testdata/grepinputx >>testtry
> }
> } Now, did you mean to make this change? Was the ^M in there on purpose?
> } Same for the others in the patch.
>
> Yes: the whole point of this change, actually, is to get rid of the ^M
> (notice the ^M is in the '-' section).


OK, well, after I ran uuencode on the file, the patch that was created
had the ^M's in it. What should be done?

Thanks,
Bob Rossi