git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Andreas Schwab <schwab@linux-m68k.org>
Subject: Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported
Date: Fri, 16 Jul 2010 21:19:40 +0000	[thread overview]
Message-ID: <AANLkTim4iI-MMkSCEU6ln9wrlC_X1GvfMHM5Qij3P591@mail.gmail.com> (raw)
In-Reply-To: <20100716210609.GA23044@burratino>

On Fri, Jul 16, 2010 at 21:06, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>
>> The failure is totally predicated on whether or not REG_STARTEND is
>> available on the system
> [...]
>> Then you could do:
>>
>>     test_expect_success REG_STARTEND 'git grep ile a' '
>>         git grep ile a
>>     '
>
> Sorry to harp on this, but no, that would not be right.  When
> REG_STARTEND is not available on a system, this is still a bug and
> we still want to know when it is fixed.  The test should not be
> skipped.
>
> So one could do:
>
>  if have_reg_startend
>  then
>        expectation=success
>  else
>        expectation=failure
>  fi
>  test_expect_$expectation 'git grep ile a' '
>        git grep ile a
>  '

Well, the assumption I was making that we would do something in the
make or ./configure process to set NO_REGEX=YesPlease if we found that
the system didn't support REG_STARTEND, and fail loudly if that
requirenment wasn't met.

However, what you posted above would be what we'd want if we wanted a
soft requirement. Maybe that's what we want, I don't know :)

>> The glibc one is probably pretty good as far as minimal POSIX DFA
>> engines go. Hopefully you can patch it up to get it to compile on
>> non-GNU systems.
>
> No promises, in particular because I don’t have any non-GNU
> installations handy to test on.  Probably gnulib’s copy will do,
> as Andreas suggested.

Probably, I didn't look closely at it.

>> Regarding regular expression implementations. We might want to look
>> into bundling one implementation and using it everywhere
>
> Please no. :)
>
> If we can do better than glibc, then glibc should be improved (yes,
> I know GNU grep does much better than glibc already).

I think engines like GNU grep get some of their speed by not
supporting some POSIX features. So it's not something you can
completely solve at the libc level, since regcomp/regexec should only
have POSIX semantics.

But I should find out if we need this before we discuss this any
further, if it's a big performance win when e.g. grep-ing through the
entire history of Linux it might be worth it.

>> Using NFA engines like that also gives you some performance guarantees
>
> Do you mean “using DFA engines”?  i.e. I thought GNU grep avoids
> backtracking by converting the NFA to a DFA, at least conceptually.

Yes, I always mix up my NFA and DFA at this time of night. I should
just start writing whatever I think I don't mean, and that'll be what
I mean :)

      reply	other threads:[~2010-07-16 21:19 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-08  0:42 [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported Ævar Arnfjörð Bjarmason
2010-07-08 19:40 ` Junio C Hamano
2010-07-08 20:09   ` Ævar Arnfjörð Bjarmason
2010-07-08 21:58     ` René Scharfe
2010-07-15 15:32       ` Ævar Arnfjörð Bjarmason
2010-07-15 17:47     ` Junio C Hamano
2010-07-15 18:44       ` Ævar Arnfjörð Bjarmason
     [not found]         ` <20100715220059.GA3312@burratino>
2010-07-16 13:58           ` [RFC/PATCH] Update compat/regex Ævar Arnfjörð Bjarmason
2010-07-16 14:17             ` Andreas Schwab
2010-08-15 11:08               ` Ævar Arnfjörð Bjarmason
2010-08-16 12:26                 ` Paolo Bonzini
2010-08-17  3:25                 ` [PATCH/RFC 0/3] " Ævar Arnfjörð Bjarmason
2010-08-17  3:25                 ` [PATCH/RFC 2/3] compat/regex: hacks to get the gawk regex engine to compile within git Ævar Arnfjörð Bjarmason
2010-08-17  3:35                   ` Jonathan Nieder
2010-08-17  3:25                 ` [PATCH/RFC 3/3] t/t7008-grep-binary.sh: un-TODO a test that needs REG_STARTEND Ævar Arnfjörð Bjarmason
     [not found]                 ` <1282015548-19074-2-git-send-email-avarab@gmail.com>
2010-08-17  3:37                   ` [PATCH/RFC 1/3] compat/regex: use the regex engine from gawk for compat Jonathan Nieder
2010-08-17  3:50                     ` Ævar Arnfjörð Bjarmason
2010-08-17  4:08                       ` Jonathan Nieder
2010-08-17  5:17                 ` [PATCH/RFC v2 0/3] Update compat/regex Ævar Arnfjörð Bjarmason
2010-08-17  8:03                   ` Jonathan Nieder
2010-08-17  9:24                     ` [PATCH 0/5] " Ævar Arnfjörð Bjarmason
2010-08-17 11:46                       ` Paolo Bonzini
2010-08-17 23:19                       ` Junio C Hamano
2010-08-17 23:50                         ` Jonathan Nieder
2010-08-18 10:41                           ` Ævar Arnfjörð Bjarmason
2010-08-17  9:24                     ` [PATCH 2/5] compat/regex: get the gawk regex engine to compile within git Ævar Arnfjörð Bjarmason
2010-08-17  9:24                     ` [PATCH 3/5] Change regerror() declaration from K&R style to ANSI C (C89) Ævar Arnfjörð Bjarmason
2010-08-17  9:24                     ` [PATCH 4/5] t/t7008-grep-binary.sh: un-TODO a test that needs REG_STARTEND Ævar Arnfjörð Bjarmason
2010-08-17  9:24                     ` [PATCH 5/5] autoconf: don't use platform regex if it lacks REG_STARTEND Ævar Arnfjörð Bjarmason
2010-08-17  5:17                 ` [PATCH/RFC v2 2/3] compat/regex: get the gawk regex engine to compile within git Ævar Arnfjörð Bjarmason
2010-08-17  5:17                 ` [PATCH/RFC v2 3/3] t/t7008-grep-binary.sh: un-TODO a test that needs REG_STARTEND Ævar Arnfjörð Bjarmason
2010-07-16 14:33         ` [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported Ævar Arnfjörð Bjarmason
2010-07-16 19:50           ` Jonathan Nieder
2010-07-16 20:51             ` Ævar Arnfjörð Bjarmason
2010-07-16 21:06               ` Jonathan Nieder
2010-07-16 21:19                 ` Ævar Arnfjörð Bjarmason [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AANLkTim4iI-MMkSCEU6ln9wrlC_X1GvfMHM5Qij3P591@mail.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=schwab@linux-m68k.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).