Opened 17 years ago

Closed 16 years ago

#2054 closed enhancement (fixed)

Grep-2.5.3

Reported by: Matthew Burgess Owned by: ken@…
Priority: normal Milestone: 7.0
Component: Book Version: SVN
Severity: normal Keywords:
Cc:

Description

New version. No announcement that I could see as of yet. Current redhat-fixes patch doesn't apply, unsurprisingly.

Attachments (1)

grep.patch (5.6 KB ) - added by Matthew Burgess 16 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 by robert@…, 17 years ago

There are 2-3 test suite failures in this Grep release. I searched for a fix and found none. Grep-cvs has the same failures. It may be why no distros have picked up this release.

comment:2 by alexander@…, 17 years ago

Also it may be due to the i18n patch (required for the LSB testsuite).

comment:3 by Matthew Burgess, 17 years ago

Grep-2.5.3 needs texinfo available in chapter 5 (the Makefile calls out to `makeinfo'). Should we update the host requirements, or move texinfo up in the buildorder?

comment:4 by alexander@…, 17 years ago

or sed out the rules from Makefile, so that documentation is not built/installed

in reply to:  4 comment:5 by Matthew Burgess, 17 years ago

This is actually caused by what looks like a bug in doc/Makefile:

if $(MAKEINFO) $(AM_MAKEINFOFLAGS) $(MAKEINFOFLAGS) -I $(srcdir) \
 -o $@ $<; \
then \
  rc=0; \
  cd $(srcdir); \
else \
  rc=$$?; \
  cd $(srcdir) && \
  $$restore $$backupdir/* `echo "./$@" | sed 's|[^/]*$$||'`; \
fi; \

This presumes that $(MAKEINFO) ('makeinfo' in our case), exists. Not sure what the correct approach here is for upstream...presumably fix the autoconf foo that is setting MAKEINFO to a non-existent binary? Anyway, it's been reported at http://lists.gnu.org/archive/html/bug-grep/2007-08/msg00013.html - no response so far though.

There also appears to be a packaging bug:

./configure --prefix=/tools \
  --disable-perl-regexp
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
/bin/bash: /home/matthew/sources/grep-2.5.3/missing: No such file or directory
configure: WARNING: `missing' script is too old or missing

This doesn't look like it's been reported yet, but I'm not sure how critical it is either.

in reply to:  1 comment:6 by ken@…, 16 years ago

Replying to robert@linuxfromscratch.org:

There are 2-3 test suite failures in this Grep release. I searched for a fix and found none. Grep-cvs has the same failures. It may be why no distros have picked up this release.

Mine only reports 3 of 14 tests failed, but the first one (foad1.sh) is a series of tests with -i and -o, and many with --color=always which mixes escape sequences into the results in the log and is pretty much unreadable. Tentatively, I think -i (ignore case) and -o (show only the match) aren't working together (-i works, -i and -o gives no output, at least if the match is in the other case), and colour highlighting of the match doesn't seem to be working if it's in the other case. The two tests after that also fail and looking at the results I don't have a clue what is going on in those two.

Fortunately, google knows a little about this - http://www.diy-linux.org/pipermail/diy-linux-dev/2007-August/001066.html shows much the same (in Greg's case fmbtest.sh was skipped, in my case it had failures. He points to http://lists.gnu.org/archive/html/bug-grep/2006-03/msg00004.html which implies these are long-standing new catch-all tests ready for when they can fix the problems.

https://savannah.gnu.org/bugs/?18666 seems to be similar

I think I've seen these failures on other architectures with my most recent clfs builds (never got around to doing more than noting there were 2 or 3 failures), which leads me to guess they aren't critical for normal use.

Some of the tests in foad1.sh seem to be in UTF-8, but they display as garbage in my log, and I can't type non-ascii in my chroot rxvt-unicode session. Curiously, when I try executing the program from outside chroot, and passing it a string containing UTF-8 cyrillic and accented latin letters as well as the ascii letters for which I'm searching, the new version seems to work correctly for -i -o --color=always. Ah, outside chroot I'm in en_GB.UTF-8. Unfortunately, setting LC_ALL in chroot doesn't make my simplified manual test succeed.

Looking at foad1.sh within chroot, there are many other tests which presumably do succeed, it's only the combination of -i with -o, and some or all of the UTF-8 case-insensitive matching that are failing (my log is garbling ALL of the UTF-8 it shows, so I can't recognise what it reports). Although I can't type non-ascii in that session, I can read it in files even in the C locale, but I can't read the output from the cs_CZ.UTF-8 tests - maybe the highlighting for --color=auto is trashing it.

Seems to fail similarly on the host system.

in reply to:  3 comment:7 by ken@…, 16 years ago

Replying to matthew@linuxfromscratch.org:

Grep-2.5.3 needs texinfo available in chapter 5 (the Makefile calls out to `makeinfo'). Should we update the host requirements, or move texinfo up in the buildorder?

My preference would be to upgrade the host requirements.

comment:8 by Matthew Burgess, 16 years ago

r8372 added Texinfo due to Binutils-2.18 requiring it, so I don't think anything needs to be done for that part of this issue. For the rest of the failures, I remember reading a lot on bug-grep about the interaction of -i and -o behaving badly. I'm therefore happy if you want to add Grep-2.5.3 and put a 'make -k check' in there with an explanation that it's to avoid known upstream bugs.

in reply to:  8 ; comment:9 by ken@…, 16 years ago

Replying to matthew@linuxfromscratch.org:

r8372 added Texinfo due to Binutils-2.18 requiring it, so I don't think anything needs to be done for that part of this issue. For the rest of the failures, I remember reading a lot on bug-grep about the interaction of -i and -o behaving badly. I'm therefore happy if you want to add Grep-2.5.3 and put a 'make -k check' in there with an explanation that it's to avoid known upstream bugs.

I'd forgotten taxinfo was already in. I'm looking at debian at the moment (trying to work out what they are _actually_ doing (compared to which patches they ship), and what exactly they changed, does my head in), but I don't think the testsuite needs make -k, it didn't seem to be like coreutils or gcc which die at the first failure.

Will recheck later.

in reply to:  9 ; comment:10 by ken@…, 16 years ago

Replying to ken@linuxfromscratch.org:

Replying to matthew@linuxfromscratch.org:

I don't think the testsuite needs make -k, it didn't seem to be like coreutils or gcc which die at the first failure.

Will recheck later.

Rechecked (and no nearer a fix) - all tests run, and the make stops with cc 2 wheter or not '-k' is used.

comment:11 by ken@…, 16 years ago

Played with the patches from debian and mandriva. If I understood the debian changelog correctly, they have dropped their patch 65 (make dfa optional, by setting an environment variable to use it) again. Trying various of their other patches singly or in combinations didn't get past the headline '3 of 14 tests failed'.

Looked at mandriva cooker. They have a one-hunk patch carried forward from 2.5.1 which doesn't affect the test results. They also have a patch which changes some of the expected results for yesno, and unsets GREP_COLOR within foad1. On its own, that silences yesno, but foad1 is unchanged (still 58 failures within it) and fmbtest still fails. They also use the debian patches (interestingly, one of these doesn't apply for me, it chokes on a #@@ line in front of the @@ of the sixth hunk, which has been added by the debian maintainer as a comment - have to remove that). The whole-set variants have fewer failures in foad1 (but still more than 40), and additional failures in fmbtest.

I conclude that the change to unset GREP_COLOR is only useful with the debian patches. I could take the rest of that patch and use it to silence yesno (the reasoning in it sounds plausible), but we would still have a few failures in fmbtest and a vast number in foad1, so I don't really see the point.

comment:12 by ken@…, 16 years ago

Owner: changed from lfs-book@… to ken@…
Status: newassigned

Just for a laugh, added these tests to 2.5.1a (both with and without the redhat fixes). All 3 are indeed reported as failing, but the number of failures is greater: 80 for foad1, 17 for fmbtest, 18 for yesno. So, 2.5.3 is an improvement.

I think I'll go back to the debian bugs noted in their changelog, to see if I can find test cases, and try anything relevant against vanilla 2.5.3 - there are an awful lot of patches floating around. And one day, my build will resume. Honest.

by Matthew Burgess, 16 years ago

Attachment: grep.patch added

comment:13 by Matthew Burgess, 16 years ago

Ken,

Here's the fairly trivial patch I've just built against. I couldn't see any other way of handling the exit status from the test suite failures. Is this anything like what you've got so far?

in reply to:  13 comment:14 by ken@…, 16 years ago

Replying to matthew@linuxfromscratch.org:

Ken,

Here's the fairly trivial patch I've just built against. I couldn't see any other way of handling the exit status from the test suite failures. Is this anything like what you've got so far?

Thanks - my own scripts have a whole lot more overhead, and can be made to continue whatever happens. But that is useful if jhalfs will need it - I was just going to mention that some tests are known to fail. See my next comment for progress on patches.

comment:15 by ken@…, 16 years ago

There are a couple of obscure bugs reported against debian : http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=294367 and http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=406259 which are both reproducible with the LFS version of 2.5.1a and fixed in 2.5.3. This is good.

But then I found an uncomfortable testcase - in multibyte locales, grepping through a big file takes a lot longer (on my example file, 6.4 seconds for en_US.UTF-8, <0.02s for en_US and C (typical values, some variation across runs). There are a whole series of debian bugs about this, all the way back to 2.5.1. This is based on http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=442882 but using a different text file about 150KB long which has zero matches and lets me omit the pipe to /dev/null.

In our 2.5.1a with the redhat fixes, this problem is not present

Back to playing with the patches - discovered that the mandriva 2.5.1a-mbcset patch plus debian patches 55,60-1,63-67 (everything except the mandriva patch to the testscript and the patches to the man pages to drop references to "not-free" texinfo) has the best test results I've seen (42 failures in foad1, 2 in fmbtest, 3 in yesno). It also runs my big file test in <0.020s in all three locales. The down-side to that is that debian seems to have stopped using patch 65 (maybe I'm misreading), and in any case it makes use of 'dfa' (an algorithm) optional in multibyte locales - you have to set an environment variable to a non-zero integer to use it). If I set that variable, I'm back to 48 failures (better than vanilla) in foad 1, 14 in fmbtest (much worse), and 3 in yesno (same). Setting the environment variable also makes my test in en_US.UTF-8 take more than 6 seconds again. thinks, looks at the 2.5.1a-redhat_fixes-2 patch : ok, we were already using egf-speedup (64) and dfa-optional (65) in some form.

I think this is close to what I'm going to use, even though the "headline" test failures (3/14) are unchanged. Might as well look at adding the 'new' test for -w from 2.5.1a, and take another look at mandriva's patch to the testsuite.

Also, time to look at the individual fedora patches for 2.5.1a, in case any are still useful. I don't know, I thought I was only going to do minimal patching to this. AfI think this is close to what I'm going to use, even though the "headline" test failures (3/14) are unchanged. Might as well look at adding the 'new' test for -w from 2.5.1a, and take another look at mandriva's patch to the testsuite.

After all, only 3 tests failed, and at best I can reduce that to 2!

/me hopes the next release will be more friendly to multi-byte locales.

comment:16 by ken@…, 16 years ago

Looking at fedora:

no testcases for -fgrep (and it doesn't apply),

no testcase for -oi (some of this is present, some maybe not),

-manpage is present,

-color doesn't apply,

-bracket is present in a slightly different form,

-skip is present,

-w is not needed https://bugzilla.redhat.com/show_bug.cgi?id=179698,

-P needs pcre to be compiled in,

-empty-pattern https://bugzilla.redhat.com/show_bug.cgi?id=204255 is already fixed in 2.5.3 but present in 2.5.1a

busy-loop problem (unsure which patch fixes it) https://bugzilla.redhat.com/show_bug.cgi?id=169524 already fixed in 2.5.3 but present in 2.5.1a,

grep -Fw problem in non UTF-8 https://bugzilla.redhat.com/show_bug.cgi?id=189580 - cannot reproduce this.

-mem-exhausted https://bugzilla.redhat.com/show_bug.cgi?id=198165 : didn't look to see if this applies, the problem is caused by large files containing no line-feed, with a test case that begins "create a sparse file 100G in size" - I don't see any reason to care, at least until somebody complains that grep has used up all their memory.

That only leaves the -tests patch which adds fmbtest.sh. Interestingly, it's different from upstream's version (it has test 5, which upstream omits, and slightly different looping. With 2.5.3, using this produces more failures in the test than the shipped version.

I still need to test combinations of debian and mandriva patches to see how few of them we can get away with, and to re-review the fix-tests patch.

comment:17 by Matthew Burgess, 16 years ago

Ken,

Thanks very much for your exhaustive analysis of the outstanding test failures in this version of Grep. I'm not sure whether you had given it any thought, but it would be very useful if you could submit whatever patch you are happy with to upstream. Development did seem to pick up a while back, though I'm not sure how long it might take them to review your patch.

As it is, I've just stumbled across the grep CVS commits list. I wonder whether simply backporting http://lists.gnu.org/archive/html/grep-commit/2008-02/msg00005.html and http://lists.gnu.org/archive/html/grep-commit/2008-02/msg00004.html and anything similar would be the quickest option for now, while upstream work on fixing things up for Grep-2.5.4?

Thanks again,

Matt.

in reply to:  17 comment:18 by ken@…, 16 years ago

As it is, I've just stumbled across the grep CVS commits list. I wonder whether simply backporting http://lists.gnu.org/archive/html/grep-commit/2008-02/msg00005.html and http://lists.gnu.org/archive/html/grep-commit/2008-02/msg00004.html and anything similar would be the quickest option for now, while upstream work on fixing things up for Grep-2.5.4?

Thanks for that. I couldn't get 00004 to apply (line-wrapping didn't help), but I found a link to the daily snapshots and applied the changes to foad1 and yesno from that. I'll take the yesno change, thank you! For foad1, with an armful of debian patches, I seem to get an extra failure with the patch. Ah, it's the (new) -H -h test and the output is "(standard input):wA wB/" instead of "wA wB/" - the marker is a warning somewhere in the debian patchset. Will try to fix the expected result.

There are also documentation updates for "-e PATTERN" which we might as well have.

Current proposal - let's take all the debian patches except their manpage changes (they omit reference to info pages because they remove them under the dfsg), plus upstream docs, yesno, and (adjusted) foad1. This includes debian's 55-bigfile patch - I can't find a testcase, and I don't intend to create a file bigger than 4GB to search through (I suspect this might be 'bigfile' as in 'more than 32 bits long'), but it's definitely playing in the multibyte charset space. So, a debian patch and an upstream patch (with a note that it is adapted to play nice with the debian patch).

Omit the mandriva patches. I don't have a testcase for their mbcset change. The change to the test scripts is mostly replaced by the yesno patch.

This should leave us with a headline "2 of 14 tests failed", and 42 and 2 failures within them (we started with "3 of 14" and 58, 2, 3). It should also fix the excessive slowness in multibyte locales.

I'll try and get the patches prepared this weekend, and retest them.

in reply to:  17 comment:19 by ken@…, 16 years ago

Replying to matthew@linuxfromscratch.org:

I'm not sure whether you had given it any thought, but it would be very useful if you could submit whatever patch you are happy with to upstream. Development did seem to pick up a while back, though I'm not sure how long it might take them to review your patch.

On Sunday, Tony Abou-Assaleh (the new maintainer) said he had asked for copyright assignment on an i18n patch, but I couldn't find the one he referred to and so far haven't seen a reply to my question about it. I'm not the author of the debian patches, so we'll have to wait and see.

in reply to:  10 ; comment:20 by ken@…, 16 years ago

Replying to ken@linuxfromscratch.org:

Replying to ken@linuxfromscratch.org:

Replying to matthew@linuxfromscratch.org:

I don't think the testsuite needs make -k, it didn't seem to be like coreutils or gcc which die at the first failure.

Will recheck later.

Rechecked (and no nearer a fix) - all tests run, and the make stops with cc 2 wheter or not '-k' is used.

With my current patches, make check ends with with cc 0 even though it shows 2 tests failed.

in reply to:  20 comment:21 by ken@…, 16 years ago

Replying to ken@linuxfromscratch.org:

With my current patches, make check ends with with cc 0 even though it shows 2 tests failed.

Hmm, I'm no longer confident that is true, I set a "check failed" message in my stamps, so at that point the status wasn't zero.

comment:22 by ken@…, 16 years ago

Resolution: fixed
Status: assignedclosed

Fixed in r8487

Note: See TracTickets for help on using tickets.