On Sat, Nov 2, 2013 at 9:56 AM, Stefano Lattarini <[email protected]> wrote: > On 11/02/2013 03:20 PM, Jim Meyering wrote:> On Fri, Nov 1, 2013 at 8:55 AM, > Stefano Lattarini >> <[email protected]> wrote: >>> This probably calls for a two patch series: the first introducing the test >>> as >>> an XFAIL, the second fixing the bug without touching the tests, and >>> verifying >>> that the test succeeds. >> >> That seems like overkill, and unnecessary churn in git. Usually, once I >> have a complete(including test case) and committed-but-not-pushed patch , >> I either arrange to run the test against the previous binary by replacing >> src/grep with the grep from my path, or (probably better) temporarily >> backing out the fix, e.g., with "git log -1 -p src/dfa.c|patch -R -p1" >> and ensuring that "make check" fails. >> > This nit I pointed out was admittedly minor, and in large part a matter > of personal preferences, so I have no problem with you disagreeing and > ignoring it. > >>> Maybe you could even amend the test to run with all of the default locale, >>> the >>> en_US.UTF-8 locale, and the C locale. Possibly overly paranoid, but the >>> enhancement would be trivial, so why not get the extra coverage anyway? >> >> That seems worthwhile. >> The default locale is set via tests/Makefile.am to LC_ALL=C, so I have >> done this: ... > I think you attached the wrong patch ;-)
Sigh, you're right. Here's the intended one (along with a NEWS update):
From ecb7452cd420b7a2e0cca186e0d6666261c16c46 Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Thu, 31 Oct 2013 21:10:02 -0700 Subject: [PATCH 1/2] maint: NEWS: document a release-related bug fix * NEWS (Bug fixes): Add an entry for a fix pulled from gnulib. --- NEWS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/NEWS b/NEWS index db9e365..161be50 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,13 @@ GNU grep NEWS -*- outline -*- * Noteworthy changes in release ?.? (????-??-??) [?] +** Bug fixes + + Fix gnulib-provided maint.mk so that the release procedure described + in README-release actually does what we want. Before that fix, that + procedure resulted in a grep-2.15 tarball that would lead to a grep + binary whose --version-reported version number was 2.14.51... + * Noteworthy changes in release 2.15 (2013-10-26) [stable] -- 1.8.4.2.564.g0d6cf24 From c4bacd524ca1e7fec97543542d6179c25a3c1506 Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Thu, 31 Oct 2013 20:20:30 -0700 Subject: [PATCH 2/2] grep: fix regression involving \s and \S Commit v2.14-40-g01ec90b made \s and \S work with multi-byte characters, but it made it so any use like \s*, \s+, \s?, \s{3} would malfunction in a multi-byte locale. * src/dfa.c (lex): Also reset laststart. * tests/backslash-s-and-repetition-operators: New file. * tests/Makefile.am (TESTS): Add it. * NEWS (Bug fixes): Mention it. * THANKS: Update. Reported by Mirraz Mirraz in http://bugs.gnu.org/15773. --- NEWS | 5 +++++ THANKS | 1 + src/dfa.c | 1 + tests/Makefile.am | 1 + tests/backslash-s-and-repetition-operators | 36 ++++++++++++++++++++++++++++++ 5 files changed, 44 insertions(+) create mode 100755 tests/backslash-s-and-repetition-operators diff --git a/NEWS b/NEWS index 161be50..262b74b 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,11 @@ GNU grep NEWS -*- outline -*- procedure resulted in a grep-2.15 tarball that would lead to a grep binary whose --version-reported version number was 2.14.51... + The fix to make \s and \S work with multi-byte white space broke + the use of each shortcut whenever followed by a repetition operator. + For example, \s*, \s+, \s? and \s{3} would all malfunction in a + multi-byte locale. [bug introduced in grep-2.14] + * Noteworthy changes in release 2.15 (2013-10-26) [stable] diff --git a/THANKS b/THANKS index 1a1901c..475c51e 100644 --- a/THANKS +++ b/THANKS @@ -66,6 +66,7 @@ Martin Rex <[email protected]> Michael Aichlmayr <[email protected]> Mike Frysinger <[email protected]> Miles Bader <[email protected]> +Mirraz Mirraz <[email protected]> Nelson H. F. Beebe <[email protected]> Olaf Kirch <[email protected]> Paolo Bonzini <[email protected]> diff --git a/src/dfa.c b/src/dfa.c index de6c671..92c410e 100644 --- a/src/dfa.c +++ b/src/dfa.c @@ -1473,6 +1473,7 @@ lex (void) POP_LEX_STATE (); + laststart = 0; return lasttok; case 'w': diff --git a/tests/Makefile.am b/tests/Makefile.am index a64a2d2..970a9de 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -36,6 +36,7 @@ TESTS = \ backref \ backref-multibyte-slow \ backref-word \ + backslash-s-and-repetition-operators \ backslash-s-vs-invalid-multitype \ big-hole \ big-match \ diff --git a/tests/backslash-s-and-repetition-operators b/tests/backslash-s-and-repetition-operators new file mode 100755 index 0000000..3b250d4 --- /dev/null +++ b/tests/backslash-s-and-repetition-operators @@ -0,0 +1,36 @@ +#! /bin/sh +# Ensure that \s and \S work with repetition operators. +# +# Copyright (C) 2013 Free Software Foundation, Inc. +# +# Copying and distribution of this file, with or without modification, +# are permitted in any medium without royalty provided the copyright +# notice and this notice are preserved. + +. "${srcdir=.}/init.sh"; path_prepend_ ../src + +require_en_utf8_locale_ + +fail=0 + +for loc in en_US.UTF-8 C; do + echo locale=$loc + LC_ALL=$loc + export LC_ALL + + printf ' \n' > in || framework_failure_ + + for re in '\s\+' '\s*' '\s\?' '\s\{1\}'; do + grep "^$re\$" in > out || fail=1 + compare in out || fail=1 + done + + printf 'X\n' > in || framework_failure_ + + for re in '\S\+' '\S*' '\S\?' '\S\{1\}'; do + grep "^$re\$" in > out || fail=1 + compare in out || fail=1 + done +done + +Exit $fail -- 1.8.4.2.564.g0d6cf24
