* [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported @ 2010-07-08 0:42 Ævar Arnfjörð Bjarmason 2010-07-08 19:40 ` Junio C Hamano 0 siblings, 1 reply; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-08 0:42 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason Change the git grep test that utilizes the REG_STARTEND flag so that it doesn't TODO pass on platforms where REG_STARTEND is supported. Git's own harness doesn't care, but a TAP harness will report all TODO tests that pass. Having t7008-grep-binary.sh be the only test (aside from the test-lib.sh test) that passes a TODO test is distracting. Before prove(1)'s test summary looked like this: All tests successful. Test Summary Report ------------------- ./t0000-basic.sh (Wstat: 0 Tests: 46 Failed: 0) TODO passed: 5 ./t7008-grep-binary.sh (Wstat: 0 Tests: 18 Failed: 0) TODO passed: 11 Files=476, Tests=6071, [...] Result: PASS And now it'll give: All tests successful. Test Summary Report ------------------- ./t0000-basic.sh (Wstat: 0 Tests: 46 Failed: 0) TODO passed: 5 Files=476, Tests=6071, Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t7008-grep-binary.sh | 15 ++++++++++----- 1 files changed, 10 insertions(+), 5 deletions(-) diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh index eb8ca88..6fd2b40 100755 --- a/t/t7008-grep-binary.sh +++ b/t/t7008-grep-binary.sh @@ -59,11 +59,16 @@ test_expect_success 'git grep -Fi iLE a' ' git grep -Fi iLE a ' -# This test actually passes on platforms where regexec() supports the -# flag REG_STARTEND. -test_expect_failure 'git grep ile a' ' - git grep ile a -' +if git grep ile a +then + # This only passes on platforms where regexec() supports the + # REG_STARTEND flag. + test_expect_success 'git grep ile a' 'git grep ile a' +else + # On platforms where REG_STARTEND isn't supported we mark the + # failure as a TODO. + test_expect_failure 'git grep ile a' 'git grep ile a' +fi test_expect_failure 'git grep .fi a' ' git grep .fi a -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported 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 0 siblings, 1 reply; 36+ messages in thread From: Junio C Hamano @ 2010-07-08 19:40 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > +if git grep ile a > +then > + test_expect_success 'git grep ile a' 'git grep ile a' > +else > + test_expect_failure 'git grep ile a' 'git grep ile a' > +fi So if command "X" is known to succeed, we run it inside expect_success and if not we run it inside expect_failure? What kind of idiocy is that, I have to wonder... ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported 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 17:47 ` Junio C Hamano 0 siblings, 2 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-08 20:09 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Thu, Jul 8, 2010 at 19:40, Junio C Hamano <gitster@pobox.com> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> +if git grep ile a >> +then >> + test_expect_success 'git grep ile a' 'git grep ile a' >> +else >> + test_expect_failure 'git grep ile a' 'git grep ile a' >> +fi > > So if command "X" is known to succeed, we run it inside expect_success > and if not we run it inside expect_failure? > > What kind of idiocy is that, I have to wonder... Well, the point is to normalize the test suite so that we never have passing TODO tests if everything's OK. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported 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 1 sibling, 1 reply; 36+ messages in thread From: René Scharfe @ 2010-07-08 21:58 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git Am 08.07.2010 22:09, schrieb Ævar Arnfjörð Bjarmason: > On Thu, Jul 8, 2010 at 19:40, Junio C Hamano <gitster@pobox.com> wrote: >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> >>> +if git grep ile a >>> +then >>> + test_expect_success 'git grep ile a' 'git grep ile a' >>> +else >>> + test_expect_failure 'git grep ile a' 'git grep ile a' >>> +fi >> >> So if command "X" is known to succeed, we run it inside expect_success >> and if not we run it inside expect_failure? >> >> What kind of idiocy is that, I have to wonder... > > Well, the point is to normalize the test suite so that we never have > passing TODO tests if everything's OK. In this particular case the tested functionality is not provided accross all platforms supported by git. It would be better to fix the actual issue, namely that the fallback regex lib we provide doesn't support REG_STARTEND. René ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported 2010-07-08 21:58 ` René Scharfe @ 2010-07-15 15:32 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-15 15:32 UTC (permalink / raw) To: René Scharfe; +Cc: Junio C Hamano, git On Thu, Jul 8, 2010 at 21:58, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote: > In this particular case the tested functionality is not provided accross > all platforms supported by git. It would be better to fix the actual > issue, namely that the fallback regex lib we provide doesn't support > REG_STARTEND. We're currently providing the GNU regex library, it seems Mac OS X and FreeBSD ship with a regex library that has REG_STARTEND (according to `git log --reverse compat/regex` the current library was added because of incompatibilities on OSX, but those were since fixed in 310e0216c8). If the FreeBSD and Mac OS X implementations are good enough we could just swap the GNU one for the FreeBSD one (http://www.freebsd.org/cgi/cvsweb.cgi/src/lib/libc/regex/). Is there any reason not to do this, or would a different regex library be a better fit? ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported 2010-07-08 20:09 ` Ævar Arnfjörð Bjarmason 2010-07-08 21:58 ` René Scharfe @ 2010-07-15 17:47 ` Junio C Hamano 2010-07-15 18:44 ` Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 36+ messages in thread From: Junio C Hamano @ 2010-07-15 17:47 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > On Thu, Jul 8, 2010 at 19:40, Junio C Hamano <gitster@pobox.com> wrote: >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> >>> +if git grep ile a >>> +then >>> + test_expect_success 'git grep ile a' 'git grep ile a' >>> +else >>> + test_expect_failure 'git grep ile a' 'git grep ile a' >>> +fi >> >> So if command "X" is known to succeed, we run it inside expect_success >> and if not we run it inside expect_failure? >> >> What kind of idiocy is that, I have to wonder... > > Well, the point is to normalize the test suite so that we never have > passing TODO tests if everything's OK. I do not consider a test that passes under some condition but doesn't under some other condition "everything is OK". Marking the test as "expect failure" as René originally did makes a lot of sense to me. The quoted patch is even worse as it will _actively_ prevent you from catching a new error you just introduced while futzing "git grep" on a platform that used to work. Your "if" statement will say "ah, grep is broken", and you will use expect-failure, not because your platform does not support REG_STARTEND, but because you broke "git grep". The point of having tests is to help you catch your bugs while you develop. A test that turns itself off when the feature it is testing is broken helps nobody. So forget about "passing TODO tests", whatever a "TODO test" is. The change in question is actively _wrong_. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported 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 14:33 ` [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported Ævar Arnfjörð Bjarmason 0 siblings, 2 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-15 18:44 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Thu, Jul 15, 2010 at 17:47, Junio C Hamano <gitster@pobox.com> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> On Thu, Jul 8, 2010 at 19:40, Junio C Hamano <gitster@pobox.com> wrote: >>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >>> >>>> +if git grep ile a >>>> +then >>>> + test_expect_success 'git grep ile a' 'git grep ile a' >>>> +else >>>> + test_expect_failure 'git grep ile a' 'git grep ile a' >>>> +fi >>> >>> So if command "X" is known to succeed, we run it inside expect_success >>> and if not we run it inside expect_failure? >>> >>> What kind of idiocy is that, I have to wonder... >> >> Well, the point is to normalize the test suite so that we never have >> passing TODO tests if everything's OK. > > I do not consider a test that passes under some condition but doesn't > under some other condition "everything is OK". Marking the test as > "expect failure" as René originally did makes a lot of sense to me. > > The quoted patch is even worse as it will _actively_ prevent you from > catching a new error you just introduced while futzing "git grep" on a > platform that used to work. Your "if" statement will say "ah, grep is > broken", and you will use expect-failure, not because your platform does > not support REG_STARTEND, but because you broke "git grep". > > The point of having tests is to help you catch your bugs while you > develop. A test that turns itself off when the feature it is testing is > broken helps nobody. > > So forget about "passing TODO tests", whatever a "TODO test" is. The > change in question is actively _wrong_. I was under the impression that REG_STARTEND was considered purely icing on the cake, i.e. that the tests should be passing whether or not it was present. I guess my reasoning at the time was that if that wasn't the case, reporting an unexpected pass by default, as opposed to a failing TODO on platforms without REG_STARTEND. Since only TAP will report this, I thought that was just an omission. Anyway, since REG_STARTEND *isn't* obviously considered icing you're of course right, but the test is still broken as-is. Now it reports an abnormal condition if REG_STARTEND is present (passing TODO test), it should instead have a failing TODO test where REG_STARTEND isn't present. I'll come up with a patch to fix that. We should also just upgrade the GNU regex library in compat/regex to the version that supports REG_STARTEND. Unfortunately that seems easier said than done, since the library is now part of glibc, and has aquired a lot of glibc specific macros and other constructs that would need to be #defined away or otherwise worked around. Thanks for the review. ^ permalink raw reply [flat|nested] 36+ messages in thread
[parent not found: <20100715220059.GA3312@burratino>]
* Re: [RFC/PATCH] Update compat/regex [not found] ` <20100715220059.GA3312@burratino> @ 2010-07-16 13:58 ` Ævar Arnfjörð Bjarmason 2010-07-16 14:17 ` Andreas Schwab 0 siblings, 1 reply; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-16 13:58 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Junio C Hamano, git On Thu, Jul 15, 2010 at 22:00, Jonathan Nieder <jrnieder@gmail.com> wrote: > The old glibc regular expression library git uses does not support > REG_STARTEND. Borrow the latest version from the GNU C library > (version 2.12, license is LGPL 2.1 or later). > > Changes from glibc version: update the FSF address in the license > header, use _LIBC_REGEX macro to allow building outside of glibc, > add MAX, bool, true, and false macros to allow building with a C89 > compiler. > > Reintroduces warnings that were fixed in git before. They can be > defeated again separately. Nevertheless this should be an > improvement. Those should be funneled upstream anyway, not just fixed in our tree. > Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> > --- > Ęvar Arnfjörš Bjarmason wrote: > >> Unfortunately that seems >> easier said than done, since the library is now part of glibc, and has >> aquired a lot of glibc specific macros and other constructs > > Oh, come on. :) > > Completely untested. This patch has all the glibc-specific stuff that makes it break hard if you don't have the GNU C library. Writing macros/definitions to fix all that stuff up was the "easier said than done" part I was referring to. It's probably not that hard (although I wouldn't put it past GNU to have e.g. GCC-specific stuff in the code, but I haven't checked), just tedious. on Solaris: In file included from compat/regex/regex.c:73: compat/regex/regex_internal.c:40: error: syntax error before "re_string_allocate" compat/regex/regex_internal.c:42: warning: return type defaults to `int' compat/regex/regex_internal.c:68: error: syntax error before "re_string_construct" compat/regex/regex_internal.c:70: warning: return type defaults to `int' compat/regex/regex_internal.c:131: error: syntax error before "re_string_realloc_buffers" compat/regex/regex_internal.c:132: warning: return type defaults to `int' compat/regex/regex_internal.c:132: error: conflicting types for 're_string_realloc_buffers' compat/regex/regex_internal.h:392: error: previous declaration of 're_string_realloc_buffers' was here compat/regex/regex_internal.c:132: error: conflicting types for 're_string_realloc_buffers' compat/regex/regex_internal.h:392: error: previous declaration of 're_string_realloc_buffers' was here compat/regex/regex_internal.c:572: error: syntax error before "re_string_reconstruct" compat/regex/regex_internal.c:573: warning: return type defaults to `int' compat/regex/regex_internal.c: In function `re_string_reconstruct': compat/regex/regex_internal.c:687: warning: unused variable `prev_valid_len' compat/regex/regex_internal.c: At top level: compat/regex/regex_internal.c:962: error: syntax error before "re_node_set_alloc" compat/regex/regex_internal.c:963: warning: return type defaults to `int' compat/regex/regex_internal.c:974: error: syntax error before "re_node_set_init_1" compat/regex/regex_internal.c:975: warning: return type defaults to `int' compat/regex/regex_internal.c:990: error: syntax error before "re_node_set_init_2" compat/regex/regex_internal.c:991: warning: return type defaults to `int' compat/regex/regex_internal.c:1020: error: syntax error before "re_node_set_init_copy" compat/regex/regex_internal.c:1021: warning: return type defaults to `int' compat/regex/regex_internal.c:1045: error: syntax error before "re_node_set_add_intersect" compat/regex/regex_internal.c:1047: warning: return type defaults to `int' compat/regex/regex_internal.c:1136: error: syntax error before "re_node_set_init_union" compat/regex/regex_internal.c:1138: warning: return type defaults to `int' compat/regex/regex_internal.c:1189: error: syntax error before "re_node_set_merge" compat/regex/regex_internal.c:1190: warning: return type defaults to `int' compat/regex/regex_internal.c:1272: error: syntax error before "re_node_set_insert" compat/regex/regex_internal.c:1273: warning: return type defaults to `int' compat/regex/regex_internal.c:1329: error: syntax error before "re_node_set_insert_last" compat/regex/regex_internal.c:1330: warning: return type defaults to `int' compat/regex/regex_internal.c: In function `re_dfa_add_node': compat/regex/regex_internal.c:1406: warning: unused variable `type' compat/regex/regex_internal.c: At top level: compat/regex/regex_internal.c:1472: error: syntax error before "re_acquire_state" compat/regex/regex_internal.c:1474: warning: return type defaults to `int' compat/regex/regex_internal.c: In function `re_acquire_state': compat/regex/regex_internal.c:1493: warning: return makes integer from pointer without a cast compat/regex/regex_internal.c:1501: warning: return makes integer from pointer without a cast compat/regex/regex_internal.c: At top level: compat/regex/regex_internal.c:1516: error: syntax error before "re_acquire_state_context" compat/regex/regex_internal.c:1518: warning: return type defaults to `int' compat/regex/regex_internal.c: In function `re_acquire_state_context': compat/regex/regex_internal.c:1537: warning: return makes integer from pointer without a cast compat/regex/regex_internal.c:1544: warning: return makes integer from pointer without a cast compat/regex/regex_internal.c: At top level: compat/regex/regex_internal.c:1553: error: syntax error before "register_state" compat/regex/regex_internal.c:1555: warning: return type defaults to `int' compat/regex/regex_internal.c:1608: error: syntax error before "create_ci_newstate" compat/regex/regex_internal.c:1610: warning: return type defaults to `int' compat/regex/regex_internal.c:1610: error: conflicting types for 'create_ci_newstate' compat/regex/regex_internal.c:27: error: previous declaration of 'create_ci_newstate' was here compat/regex/regex_internal.c:1610: error: conflicting types for 'create_ci_newstate' compat/regex/regex_internal.c:27: error: previous declaration of 'create_ci_newstate' was here compat/regex/regex_internal.c: In function `create_ci_newstate': compat/regex/regex_internal.c:1650: warning: return makes integer from pointer without a cast compat/regex/regex_internal.c: At top level: compat/regex/regex_internal.c:1658: error: syntax error before "create_cd_newstate" compat/regex/regex_internal.c:1660: warning: return type defaults to `int' compat/regex/regex_internal.c:1660: error: conflicting types for 'create_cd_newstate' compat/regex/regex_internal.c:31: error: previous declaration of 'create_cd_newstate' was here compat/regex/regex_internal.c:1660: error: conflicting types for 'create_cd_newstate' compat/regex/regex_internal.c:31: error: previous declaration of 'create_cd_newstate' was here compat/regex/regex_internal.c: In function `create_cd_newstate': compat/regex/regex_internal.c:1726: warning: return makes integer from pointer without a cast In file included from compat/regex/regex.c:74: compat/regex/regcomp.c: In function `create_initial_state': compat/regex/regcomp.c:1013: warning: assignment makes pointer from integer without a cast compat/regex/regcomp.c:1020: warning: assignment makes pointer from integer without a cast compat/regex/regcomp.c:1022: warning: assignment makes pointer from integer without a cast compat/regex/regcomp.c:1026: warning: assignment makes pointer from integer without a cast In file included from compat/regex/regex.c:75: compat/regex/regexec.c: In function `regexec': compat/regex/regexec.c:230: warning: unused variable `dfa' compat/regex/regexec.c: In function `re_search_stub': compat/regex/regexec.c:418: warning: unused variable `dfa' compat/regex/regexec.c: At top level: compat/regex/regexec.c:629: error: syntax error before "re_search_internal" compat/regex/regexec.c:631: warning: return type defaults to `int' compat/regex/regexec.c:631: error: conflicting types for 're_search_internal' compat/regex/regexec.c:43: error: previous declaration of 're_search_internal' was here compat/regex/regexec.c:631: error: conflicting types for 're_search_internal' compat/regex/regexec.c:43: error: previous declaration of 're_search_internal' was here compat/regex/regexec.c:956: error: syntax error before "prune_impossible_nodes" compat/regex/regexec.c:957: warning: return type defaults to `int' compat/regex/regexec.c:957: error: conflicting types for 'prune_impossible_nodes' compat/regex/regexec.c:56: error: previous declaration of 'prune_impossible_nodes' was here compat/regex/regexec.c:957: error: conflicting types for 'prune_impossible_nodes' compat/regex/regexec.c:56: error: previous declaration of 'prune_impossible_nodes' was here compat/regex/regexec.c: In function `acquire_init_state_context': compat/regex/regexec.c:1074: warning: return makes pointer from integer without a cast compat/regex/regexec.c: At top level: compat/regex/regexec.c:1095: error: syntax error before "check_matching" compat/regex/regexec.c:1097: warning: return type defaults to `int' compat/regex/regexec.c:1097: error: conflicting types for 'check_matching' compat/regex/regexec.c:58: error: previous declaration of 'check_matching' was here compat/regex/regexec.c:1097: error: conflicting types for 'check_matching' compat/regex/regexec.c:58: error: previous declaration of 'check_matching' was here compat/regex/regexec.c:1368: error: syntax error before "push_fail_stack" compat/regex/regexec.c:1370: warning: return type defaults to `int' compat/regex/regexec.c:1370: error: conflicting types for 'push_fail_stack' compat/regex/regexec.c:69: error: previous declaration of 'push_fail_stack' was here compat/regex/regexec.c:1370: error: conflicting types for 'push_fail_stack' compat/regex/regexec.c:69: error: previous declaration of 'push_fail_stack' was here compat/regex/regexec.c:1415: error: syntax error before "set_regs" compat/regex/regexec.c:1417: warning: return type defaults to `int' compat/regex/regexec.c:1417: error: conflicting types for 'set_regs' compat/regex/regexec.c:73: error: previous declaration of 'set_regs' was here compat/regex/regexec.c:1417: error: conflicting types for 'set_regs' compat/regex/regexec.c:73: error: previous declaration of 'set_regs' was here compat/regex/regexec.c:1669: error: syntax error before "build_sifted_states" compat/regex/regexec.c:1671: warning: return type defaults to `int' compat/regex/regexec.c:1671: error: conflicting types for 'build_sifted_states' compat/regex/regexec.c:89: error: previous declaration of 'build_sifted_states' was here compat/regex/regexec.c:1671: error: conflicting types for 'build_sifted_states' compat/regex/regexec.c:89: error: previous declaration of 'build_sifted_states' was here compat/regex/regexec.c: In function `merge_state_array': compat/regex/regexec.c:1772: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c: In function `update_cur_sifted_state': compat/regex/regexec.c:1815: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c: At top level: compat/regex/regexec.c:1831: error: syntax error before "add_epsilon_src_nodes" compat/regex/regexec.c:1833: warning: return type defaults to `int' compat/regex/regexec.c:1833: error: conflicting types for 'add_epsilon_src_nodes' compat/regex/regexec.c:98: error: previous declaration of 'add_epsilon_src_nodes' was here compat/regex/regexec.c:1833: error: conflicting types for 'add_epsilon_src_nodes' compat/regex/regexec.c:98: error: previous declaration of 'add_epsilon_src_nodes' was here compat/regex/regexec.c: In function `add_epsilon_src_nodes': compat/regex/regexec.c:1837: warning: initialization makes pointer from integer without a cast compat/regex/regexec.c: At top level: compat/regex/regexec.c:2144: error: syntax error before "sift_states_bkref" compat/regex/regexec.c:2146: warning: return type defaults to `int' compat/regex/regexec.c:2146: error: conflicting types for 'sift_states_bkref' compat/regex/regexec.c:120: error: previous declaration of 'sift_states_bkref' was here compat/regex/regexec.c:2146: error: conflicting types for 'sift_states_bkref' compat/regex/regexec.c:120: error: previous declaration of 'sift_states_bkref' was here compat/regex/regexec.c:2274: error: syntax error before "transit_state" compat/regex/regexec.c:2276: warning: return type defaults to `int' compat/regex/regexec.c:2276: error: conflicting types for 'transit_state' compat/regex/regexec.c:129: error: previous declaration of 'transit_state' was here compat/regex/regexec.c:2276: error: conflicting types for 'transit_state' compat/regex/regexec.c:129: error: previous declaration of 'transit_state' was here compat/regex/regexec.c: In function `transit_state': compat/regex/regexec.c:2303: warning: return makes integer from pointer without a cast compat/regex/regexec.c:2314: warning: return makes integer from pointer without a cast compat/regex/regexec.c:2316: warning: return makes integer from pointer without a cast compat/regex/regexec.c: In function `merge_state_with_log': compat/regex/regexec.c:2375: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c: In function `transit_state_bkref': compat/regex/regexec.c:2648: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c:2665: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c: At top level: compat/regex/regexec.c:2699: error: syntax error before "get_subexp" compat/regex/regexec.c:2700: warning: return type defaults to `int' compat/regex/regexec.c:2700: error: conflicting types for 'get_subexp' compat/regex/regexec.c:153: error: previous declaration of 'get_subexp' was here compat/regex/regexec.c:2700: error: conflicting types for 'get_subexp' compat/regex/regexec.c:153: error: previous declaration of 'get_subexp' was here compat/regex/regexec.c:2899: error: syntax error before "check_arrival" compat/regex/regexec.c:2901: warning: return type defaults to `int' compat/regex/regexec.c:2901: error: conflicting types for 'check_arrival' compat/regex/regexec.c:164: error: previous declaration of 'check_arrival' was here compat/regex/regexec.c:2901: error: conflicting types for 'check_arrival' compat/regex/regexec.c:164: error: previous declaration of 'check_arrival' was here compat/regex/regexec.c: In function `check_arrival': compat/regex/regexec.c:2974: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c:3025: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c: At top level: compat/regex/regexec.c:3060: error: syntax error before "check_arrival_add_next_nodes" compat/regex/regexec.c:3062: warning: return type defaults to `int' compat/regex/regexec.c:3062: error: conflicting types for 'check_arrival_add_next_nodes' compat/regex/regexec.c:169: error: previous declaration of 'check_arrival_add_next_nodes' was here compat/regex/regexec.c:3062: error: conflicting types for 'check_arrival_add_next_nodes' compat/regex/regexec.c:169: error: previous declaration of 'check_arrival_add_next_nodes' was here compat/regex/regexec.c: In function `check_arrival_add_next_nodes': compat/regex/regexec.c:3066: warning: unused variable `err' compat/regex/regexec.c: At top level: compat/regex/regexec.c:3192: error: syntax error before "check_arrival_expand_ecl_sub" compat/regex/regexec.c:3194: warning: return type defaults to `int' compat/regex/regexec.c:3194: error: conflicting types for 'check_arrival_expand_ecl_sub' compat/regex/regexec.c:177: error: previous declaration of 'check_arrival_expand_ecl_sub' was here compat/regex/regexec.c:3194: error: conflicting types for 'check_arrival_expand_ecl_sub' compat/regex/regexec.c:177: error: previous declaration of 'check_arrival_expand_ecl_sub' was here compat/regex/regexec.c:3236: error: syntax error before "expand_bkref_cache" compat/regex/regexec.c:3238: warning: return type defaults to `int' compat/regex/regexec.c:3238: error: conflicting types for 'expand_bkref_cache' compat/regex/regexec.c:181: error: previous declaration of 'expand_bkref_cache' was here compat/regex/regexec.c:3238: error: conflicting types for 'expand_bkref_cache' compat/regex/regexec.c:181: error: previous declaration of 'expand_bkref_cache' was here compat/regex/regexec.c: In function `expand_bkref_cache': compat/regex/regexec.c:3309: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c: In function `build_trtable': compat/regex/regexec.c:3434: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c:3442: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c:3450: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c: At top level: compat/regex/regexec.c:4102: error: syntax error before "extend_buffers" compat/regex/regexec.c:4103: warning: return type defaults to `int' compat/regex/regexec.c:4103: error: conflicting types for 'extend_buffers' compat/regex/regexec.c:202: error: previous declaration of 'extend_buffers' was here compat/regex/regexec.c:4103: error: conflicting types for 'extend_buffers' compat/regex/regexec.c:202: error: previous declaration of 'extend_buffers' was here compat/regex/regexec.c:4165: error: syntax error before "match_ctx_init" compat/regex/regexec.c:4166: warning: return type defaults to `int' compat/regex/regexec.c:4166: error: conflicting types for 'match_ctx_init' compat/regex/regexec.c:22: error: previous declaration of 'match_ctx_init' was here compat/regex/regexec.c:4166: error: conflicting types for 'match_ctx_init' compat/regex/regexec.c:22: error: previous declaration of 'match_ctx_init' was here compat/regex/regexec.c:4238: error: syntax error before "match_ctx_add_entry" compat/regex/regexec.c:4240: warning: return type defaults to `int' compat/regex/regexec.c:4240: error: conflicting types for 'match_ctx_add_entry' compat/regex/regexec.c:27: error: previous declaration of 'match_ctx_add_entry' was here compat/regex/regexec.c:4240: error: conflicting types for 'match_ctx_add_entry' compat/regex/regexec.c:27: error: previous declaration of 'match_ctx_add_entry' was here compat/regex/regexec.c:4310: error: syntax error before "match_ctx_add_subtop" compat/regex/regexec.c:4311: warning: return type defaults to `int' compat/regex/regexec.c:4311: error: conflicting types for 'match_ctx_add_subtop' compat/regex/regexec.c:31: error: previous declaration of 'match_ctx_add_subtop' was here compat/regex/regexec.c:4311: error: conflicting types for 'match_ctx_add_subtop' compat/regex/regexec.c:31: error: previous declaration of 'match_ctx_add_subtop' was here compat/regex/regex_internal.h:392: warning: 're_string_realloc_buffers' declared `static' but never defined compat/regex/regex_internal.c:27: warning: 'create_ci_newstate' declared `static' but never defined compat/regex/regex_internal.c:31: warning: 'create_cd_newstate' declared `static' but never defined compat/regex/regexec.c:22: warning: 'match_ctx_init' declared `static' but never defined compat/regex/regexec.c:27: warning: 'match_ctx_add_entry' declared `static' but never defined compat/regex/regexec.c:31: warning: 'match_ctx_add_subtop' declared `static' but never defined compat/regex/regexec.c:43: warning: 're_search_internal' declared `static' but never defined compat/regex/regexec.c:56: warning: 'prune_impossible_nodes' declared `static' but never defined compat/regex/regexec.c:58: warning: 'check_matching' declared `static' but never defined compat/regex/regexec.c:69: warning: 'push_fail_stack' declared `static' but never defined compat/regex/regexec.c:73: warning: 'set_regs' declared `static' but never defined compat/regex/regexec.c:89: warning: 'build_sifted_states' declared `static' but never defined compat/regex/regexec.c:98: warning: 'add_epsilon_src_nodes' declared `static' but never defined compat/regex/regexec.c:120: warning: 'sift_states_bkref' declared `static' but never defined compat/regex/regexec.c:129: warning: 'transit_state' declared `static' but never defined compat/regex/regexec.c:153: warning: 'get_subexp' declared `static' but never defined compat/regex/regexec.c:164: warning: 'check_arrival' declared `static' but never defined compat/regex/regexec.c:169: warning: 'check_arrival_add_next_nodes' declared `static' but never defined compat/regex/regexec.c:177: warning: 'check_arrival_expand_ecl_sub' declared `static' but never defined compat/regex/regexec.c:181: warning: 'expand_bkref_cache' declared `static' but never defined compat/regex/regexec.c:202: warning: 'extend_buffers' declared `static' but never defined gmake: *** [compat/regex/regex.o] Error 1 And on FreeBSD: In file included from git-compat-util.h:93, from cache.h:4, from thread-utils.c:1: compat/regex/regex.h:367: warning: declaration does not declare anything CC compat/memmem.o In file included from compat/../git-compat-util.h:93, from compat/memmem.c:1: compat/regex/regex.h:367: warning: declaration does not declare anything CC compat/regex/regex.o In file included from compat/regex/regex.c:63: compat/regex/regex_internal.h:421:20: error: alloca.h: No such file or directory In file included from compat/regex/regex.c:73: compat/regex/regex_internal.c:40: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_string_allocate' compat/regex/regex_internal.c:68: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_string_construct' compat/regex/regex_internal.c:131: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_string_realloc_buffers' compat/regex/regex_internal.c:572: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_string_reconstruct' compat/regex/regex_internal.c:962: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_node_set_alloc' compat/regex/regex_internal.c:974: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_node_set_init_1' compat/regex/regex_internal.c:990: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_node_set_init_2' compat/regex/regex_internal.c:1020: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_node_set_init_copy' compat/regex/regex_internal.c:1045: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_node_set_add_intersect' compat/regex/regex_internal.c:1136: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_node_set_init_union' compat/regex/regex_internal.c:1189: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_node_set_merge' compat/regex/regex_internal.c:1272: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_node_set_insert' compat/regex/regex_internal.c:1329: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_node_set_insert_last' compat/regex/regex_internal.c: In function 're_dfa_add_node': compat/regex/regex_internal.c:1406: warning: unused variable 'type' compat/regex/regex_internal.c: At top level: compat/regex/regex_internal.c:1472: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_acquire_state' compat/regex/regex_internal.c:1516: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_acquire_state_context' compat/regex/regex_internal.c:1553: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'register_state' compat/regex/regex_internal.c:1608: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'create_ci_newstate' compat/regex/regex_internal.c:1658: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'create_cd_newstate' In file included from compat/regex/regex.c:74: compat/regex/regcomp.c: In function 're_compile_internal': compat/regex/regcomp.c:778: warning: implicit declaration of function 're_string_construct' compat/regex/regcomp.c: In function 'create_initial_state': compat/regex/regcomp.c:969: warning: implicit declaration of function 're_node_set_init_copy' compat/regex/regcomp.c:1002: warning: implicit declaration of function 're_node_set_merge' compat/regex/regcomp.c:1013: warning: implicit declaration of function 're_acquire_state_context' compat/regex/regcomp.c:1013: warning: assignment makes pointer from integer without a cast compat/regex/regcomp.c:1020: warning: assignment makes pointer from integer without a cast compat/regex/regcomp.c:1022: warning: assignment makes pointer from integer without a cast compat/regex/regcomp.c:1026: warning: assignment makes pointer from integer without a cast compat/regex/regcomp.c: In function 'link_nfa_nodes': compat/regex/regcomp.c:1408: warning: implicit declaration of function 're_node_set_init_2' compat/regex/regcomp.c:1415: warning: implicit declaration of function 're_node_set_init_1' compat/regex/regcomp.c: In function 'duplicate_node_closure': compat/regex/regcomp.c:1459: warning: implicit declaration of function 're_node_set_insert' compat/regex/regcomp.c: In function 'calc_inveclosure': compat/regex/regcomp.c:1590: warning: implicit declaration of function 're_node_set_insert_last' compat/regex/regcomp.c: In function 'calc_eclosure_iter': compat/regex/regcomp.c:1653: warning: implicit declaration of function 're_node_set_alloc' In file included from compat/regex/regex.c:75: compat/regex/regexec.c: In function 'regexec': compat/regex/regexec.c:230: warning: unused variable 'dfa' compat/regex/regexec.c: In function 're_search_stub': compat/regex/regexec.c:418: warning: unused variable 'dfa' compat/regex/regexec.c: At top level: compat/regex/regexec.c:629: error: expected '=', ',', ';', 'asm' or '__attribute__' before 're_search_internal' compat/regex/regexec.c:636: error: expected identifier or '(' before '{' token compat/regex/regexec.c:956: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'prune_impossible_nodes' compat/regex/regexec.c:958: error: expected identifier or '(' before '{' token compat/regex/regexec.c: In function 'acquire_init_state_context': compat/regex/regexec.c:1074: warning: return makes pointer from integer without a cast compat/regex/regexec.c: At top level: compat/regex/regexec.c:1095: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'check_matching' compat/regex/regexec.c:1368: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'push_fail_stack' compat/regex/regexec.c:1415: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'set_regs' compat/regex/regexec.c:1669: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'build_sifted_states' compat/regex/regexec.c: In function 'merge_state_array': compat/regex/regexec.c:1768: warning: implicit declaration of function 're_node_set_init_union' compat/regex/regexec.c:1772: warning: implicit declaration of function 're_acquire_state' compat/regex/regexec.c:1772: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c: In function 'update_cur_sifted_state': compat/regex/regexec.c:1815: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c: At top level: compat/regex/regexec.c:1831: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'add_epsilon_src_nodes' compat/regex/regexec.c: In function 'sub_epsilon_src_nodes': compat/regex/regexec.c:1884: warning: implicit declaration of function 're_node_set_add_intersect' compat/regex/regexec.c: At top level: compat/regex/regexec.c:2144: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'sift_states_bkref' compat/regex/regexec.c:2274: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'transit_state' compat/regex/regexec.c: In function 'merge_state_with_log': compat/regex/regexec.c:2375: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c: In function 'transit_state_bkref': compat/regex/regexec.c:2648: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c:2665: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c: At top level: compat/regex/regexec.c:2699: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'get_subexp' compat/regex/regexec.c:2899: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'check_arrival' compat/regex/regexec.c:3060: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'check_arrival_add_next_nodes' compat/regex/regexec.c:3192: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'check_arrival_expand_ecl_sub' compat/regex/regexec.c:3236: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'expand_bkref_cache' compat/regex/regexec.c: In function 'build_trtable': compat/regex/regexec.c:3434: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c:3442: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c:3450: warning: assignment makes pointer from integer without a cast compat/regex/regexec.c: At top level: compat/regex/regexec.c:4102: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'extend_buffers' compat/regex/regexec.c:4165: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'match_ctx_init' compat/regex/regexec.c:4238: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'match_ctx_add_entry' compat/regex/regexec.c:4310: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'match_ctx_add_subtop' gmake: *** [compat/regex/regex.o] Error 1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC/PATCH] Update compat/regex 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 0 siblings, 1 reply; 36+ messages in thread From: Andreas Schwab @ 2010-07-16 14:17 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Jonathan Nieder, Junio C Hamano, git Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > This patch has all the glibc-specific stuff that makes it break hard > if you don't have the GNU C library. Writing macros/definitions to fix all that > stuff up was the "easier said than done" part I was referring to. You might want to try out the gnulib version instead. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC/PATCH] Update compat/regex 2010-07-16 14:17 ` Andreas Schwab @ 2010-08-15 11:08 ` Ævar Arnfjörð Bjarmason 2010-08-16 12:26 ` Paolo Bonzini ` (7 more replies) 0 siblings, 8 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-15 11:08 UTC (permalink / raw) To: Andreas Schwab; +Cc: Jonathan Nieder, Junio C Hamano, git On Fri, Jul 16, 2010 at 14:17, Andreas Schwab <schwab@linux-m68k.org> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> This patch has all the glibc-specific stuff that makes it break hard >> if you don't have the GNU C library. Writing macros/definitions to fix all that >> stuff up was the "easier said than done" part I was referring to. > > You might want to try out the gnulib version instead. I fiddled a bit with gnulib for both the regex engine and libintl, but I can't get it to do what I want. The assumption with gnulib seems to be that you're including the libraries in a GNU program that only uses the autotools, it seems to be about as easy to just copy/paste things from glibc if you're adding libraries to a program like Git that uses its own build system. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC/PATCH] Update compat/regex 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 ` (6 subsequent siblings) 7 siblings, 0 replies; 36+ messages in thread From: Paolo Bonzini @ 2010-08-16 12:26 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Andreas Schwab, Jonathan Nieder, Junio C Hamano, git On 08/15/2010 01:08 PM, Ævar Arnfjörð Bjarmason wrote: > On Fri, Jul 16, 2010 at 14:17, Andreas Schwab<schwab@linux-m68k.org> wrote: >> Ævar Arnfjörð Bjarmason<avarab@gmail.com> writes: >> >>> This patch has all the glibc-specific stuff that makes it break hard >>> if you don't have the GNU C library. Writing macros/definitions to fix all that >>> stuff up was the "easier said than done" part I was referring to. >> >> You might want to try out the gnulib version instead. > > I fiddled a bit with gnulib for both the regex engine and libintl, but > I can't get it to do what I want. > > The assumption with gnulib seems to be that you're including the > libraries in a GNU program that only uses the autotools, it seems to > be about as easy to just copy/paste things from glibc if you're adding > libraries to a program like Git that uses its own build system. Andreas is right, the glibc code is not meant to be portable. It is really simpler if you start from the version in gnulib, which is the one that is included in most GNU packages nowadays. You should download GNU grep 2.6.x and (starting from lib/reg*) add headers from its lib/ directory until it compiles. Alternatively try out gawk, as it does not use gnulib but has the same set of sanitizations. Paolo ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH/RFC 0/3] Update compat/regex 2010-08-15 11:08 ` Ævar Arnfjörð Bjarmason 2010-08-16 12:26 ` Paolo Bonzini @ 2010-08-17 3:25 ` Æ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 ` (5 subsequent siblings) 7 siblings, 0 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-17 3:25 UTC (permalink / raw) To: git Cc: Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Junio C Hamano, Ævar Arnfjörð Bjarmason On Mon, Aug 16, 2010 at 12:26, Paolo Bonzini <bonzini@gnu.org> wrote: > Alternatively try out gawk, as it does not use gnulib but has the same set > of sanitizations. Why didn't you say that earlier? :) Here's a RFC patch series that uses the gawk regex engine from the gawk-devel branch of gawk CVS. It compiles on Linux/FreeBSD and Solaris, with only a single warning on FreeBSD due to an unused variable (upstream bug). Ævar Arnfjörð Bjarmason (3): compat/regex: use the regex engine from gawk for compat compat/regex: hacks to get the gawk regex engine to compile within git t/t7008-grep-binary.sh: un-TODO a test that needs REG_STARTEND Makefile | 4 + compat/regex/COPYING | 674 ++++++ compat/regex/mbsupport.h | 59 + compat/regex/regcomp.c | 3892 ++++++++++++++++++++++++++++++++ compat/regex/regex.c | 5003 +---------------------------------------- compat/regex/regex.h | 462 +++-- compat/regex/regex_internal.c | 1744 ++++++++++++++ compat/regex/regex_internal.h | 810 +++++++ compat/regex/regexec.c | 4377 +++++++++++++++++++++++++++++++++++ t/t7008-grep-binary.sh | 2 +- 10 files changed, 11921 insertions(+), 5106 deletions(-) create mode 100644 compat/regex/COPYING create mode 100644 compat/regex/mbsupport.h create mode 100644 compat/regex/regcomp.c create mode 100644 compat/regex/regex_internal.c create mode 100644 compat/regex/regex_internal.h create mode 100644 compat/regex/regexec.c -- 1.7.2.1.389.gc3d0b ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH/RFC 2/3] compat/regex: hacks to get the gawk regex engine to compile within git 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 ` Æ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 ` (4 subsequent siblings) 7 siblings, 1 reply; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-17 3:25 UTC (permalink / raw) To: git Cc: Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Junio C Hamano, Ævar Arnfjörð Bjarmason The gawk regex engine didn't include stdio.h, and only include stddef.h if HAVE_STDDEF_H is set. Adding -DHAVE_STDDEF_H caused some internal errors in by /usr/include headers, so change the regex.h code to include it unconditionally. We also need to define -DGAWK so that e.g. "bool", "MAX" and other similar things used inside gawk get defined. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 4 ++++ compat/regex/regex.h | 7 +++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index b4745a5..6704780 100644 --- a/Makefile +++ b/Makefile @@ -1443,6 +1443,10 @@ ifdef UNRELIABLE_FSTAT BASIC_CFLAGS += -DUNRELIABLE_FSTAT endif ifdef NO_REGEX + # TODO: How do I compile just regex.o with this flag, not the + # whole of Git? + BASIC_CFLAGS += -DGAWK + COMPAT_CFLAGS += -Icompat/regex COMPAT_OBJS += compat/regex/regex.o endif diff --git a/compat/regex/regex.h b/compat/regex/regex.h index de93327..508bc80 100644 --- a/compat/regex/regex.h +++ b/compat/regex/regex.h @@ -22,9 +22,12 @@ #ifndef _REGEX_H #define _REGEX_H 1 -#ifdef HAVE_STDDEF_H +#include <stdio.h> +/* + Git: Was in `#ifdef HAVE_STDDEF_H` in gawk, adding -DHAVE_STDDEF_H makes a + *lot* of other things break +*/ #include <stddef.h> -#endif #ifdef HAVE_SYS_TYPES_H #include <sys/types.h> -- 1.7.2.1.389.gc3d0b ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH/RFC 2/3] compat/regex: hacks to get the gawk regex engine to compile within git 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 0 siblings, 0 replies; 36+ messages in thread From: Jonathan Nieder @ 2010-08-17 3:35 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Paolo Bonzini, Andreas Schwab, Junio C Hamano Hi! Ævar Arnfjörð Bjarmason wrote: > + # TODO: How do I compile just regex.o with this flag, not the > + # whole of Git? > + BASIC_CFLAGS += -DGAWK gawk.o: EXTRA_CPPFLAGS = -DGAWK See v1.7.1-rc0~60 and v1.7.0-rc0~90^2~2. > --- a/compat/regex/regex.h > +++ b/compat/regex/regex.h > @@ -22,9 +22,12 @@ > #ifndef _REGEX_H > #define _REGEX_H 1 > > -#ifdef HAVE_STDDEF_H > +#include <stdio.h> > +/* > + Git: Was in `#ifdef HAVE_STDDEF_H` in gawk, adding -DHAVE_STDDEF_H makes a > + *lot* of other things break > +*/ > #include <stddef.h> > -#endif Maybe #if 1 #include "git-compat-util.h" #else ... would be simpler. ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH/RFC 3/3] t/t7008-grep-binary.sh: un-TODO a test that needs REG_STARTEND 2010-08-15 11:08 ` Ævar Arnfjörð Bjarmason ` (2 preceding siblings ...) 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:25 ` Ævar Arnfjörð Bjarmason [not found] ` <1282015548-19074-2-git-send-email-avarab@gmail.com> ` (3 subsequent siblings) 7 siblings, 0 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-17 3:25 UTC (permalink / raw) To: git Cc: Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Junio C Hamano, Ævar Arnfjörð Bjarmason Now that we have a regex engine that supports REG_STARTEND this test should fail if "git grep" can't grep NULL characters. Platforms that don't have a POSIX regex engine that supports REG_STARTEND should always define NO_REGEX=YesPlease when compiling. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t7008-grep-binary.sh | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh index eb8ca88..c0f9f3f 100755 --- a/t/t7008-grep-binary.sh +++ b/t/t7008-grep-binary.sh @@ -61,7 +61,7 @@ test_expect_success 'git grep -Fi iLE a' ' # This test actually passes on platforms where regexec() supports the # flag REG_STARTEND. -test_expect_failure 'git grep ile a' ' +test_expect_success 'git grep ile a' ' git grep ile a ' -- 1.7.2.1.389.gc3d0b ^ permalink raw reply related [flat|nested] 36+ messages in thread
[parent not found: <1282015548-19074-2-git-send-email-avarab@gmail.com>]
* Re: [PATCH/RFC 1/3] compat/regex: use the regex engine from gawk for compat [not found] ` <1282015548-19074-2-git-send-email-avarab@gmail.com> @ 2010-08-17 3:37 ` Jonathan Nieder 2010-08-17 3:50 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 36+ messages in thread From: Jonathan Nieder @ 2010-08-17 3:37 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Paolo Bonzini, Andreas Schwab, Junio C Hamano Ævar Arnfjörð Bjarmason wrote: > +++ b/compat/regex/mbsupport.h > @@ -0,0 +1,59 @@ > +/* > + * mbsupport.h --- Localize determination of whether we have multibyte stuff. > + */ > + > +/* > + * Copyright (C) 2004, 2005 the Free Software Foundation, Inc. > + * > + * This file is part of GAWK, the GNU implementation of the > + * AWK Programming Language. > + * > + * GAWK is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 3 of the License, or > + * (at your option) any later version. Worrisome. (Most of git is still GPL-2 only, which would not be compatible.) ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH/RFC 1/3] compat/regex: use the regex engine from gawk for compat 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 0 siblings, 1 reply; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-17 3:50 UTC (permalink / raw) To: Jonathan Nieder; +Cc: git, Paolo Bonzini, Andreas Schwab, Junio C Hamano On Tue, Aug 17, 2010 at 03:37, Jonathan Nieder <jrnieder@gmail.com> wrote: > Ævar Arnfjörð Bjarmason wrote: > >> +++ b/compat/regex/mbsupport.h >> @@ -0,0 +1,59 @@ >> +/* >> + * mbsupport.h --- Localize determination of whether we have multibyte stuff. >> + */ >> + >> +/* >> + * Copyright (C) 2004, 2005 the Free Software Foundation, Inc. >> + * >> + * This file is part of GAWK, the GNU implementation of the >> + * AWK Programming Language. >> + * >> + * GAWK is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 3 of the License, or >> + * (at your option) any later version. > > Worrisome. (Most of git is still GPL-2 only, which would not be > compatible.) Actually it's LGPL-2.1 (all the copyright headers in the *.[ch] files agree), I just copied the wrong license. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH/RFC 1/3] compat/regex: use the regex engine from gawk for compat 2010-08-17 3:50 ` Ævar Arnfjörð Bjarmason @ 2010-08-17 4:08 ` Jonathan Nieder 0 siblings, 0 replies; 36+ messages in thread From: Jonathan Nieder @ 2010-08-17 4:08 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Paolo Bonzini, Andreas Schwab, Junio C Hamano Ævar Arnfjörð Bjarmason wrote: > On Tue, Aug 17, 2010 at 03:37, Jonathan Nieder <jrnieder@gmail.com> wrote: >> Ævar Arnfjörð Bjarmason wrote: >>> +++ b/compat/regex/mbsupport.h >>> @@ -0,0 +1,59 @@ >>> +/* >>> + * mbsupport.h --- Localize determination of whether we have multibyte stuff. >>> + */ >>> + >>> +/* >>> + * Copyright (C) 2004, 2005 the Free Software Foundation, Inc. >>> + * >>> + * This file is part of GAWK, the GNU implementation of the >>> + * AWK Programming Language. >>> + * >>> + * GAWK is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License as published by >>> + * the Free Software Foundation; either version 3 of the License, or >>> + * (at your option) any later version. >> >> Worrisome. (Most of git is still GPL-2 only, which would not be >> compatible.) > > Actually it's LGPL-2.1 (all the copyright headers in the *.[ch] files > agree), I just copied the wrong license. Thanks for checking. That’s good to hear. ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH/RFC v2 0/3] Update compat/regex 2010-08-15 11:08 ` Ævar Arnfjörð Bjarmason ` (4 preceding siblings ...) [not found] ` <1282015548-19074-2-git-send-email-avarab@gmail.com> @ 2010-08-17 5:17 ` Ævar Arnfjörð Bjarmason 2010-08-17 8:03 ` Jonathan Nieder 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 7 siblings, 1 reply; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-17 5:17 UTC (permalink / raw) To: git Cc: Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Junio C Hamano, Ævar Arnfjörð Bjarmason This has the following changes: * Supply the custom regex.o flags only to regex.o as suggested by Jonathan Nieder: +ifdef NO_REGEX +compat/regex/regex.o: EXTRA_CPPFLAGS = -DGAWK -DNO_MBSUPPORT +endif * The code is LGPL-2.1, not GPL-3 * Don't include mbsupport.h, we don't need it, and it can be un-included with a flag. * Simplify our modifications to regex.h, just include two headers at the very top, don't modify any gawk code. * Update commit messages Ævar Arnfjörð Bjarmason (3): compat/regex: use the regex engine from gawk for compat compat/regex: get the gawk regex engine to compile within git t/t7008-grep-binary.sh: un-TODO a test that needs REG_STARTEND Makefile | 4 + compat/regex/regcomp.c | 3892 ++++++++++++++++++++++++++++++++ compat/regex/regex.c | 5003 +---------------------------------------- compat/regex/regex.h | 462 +++-- compat/regex/regex_internal.c | 1744 ++++++++++++++ compat/regex/regex_internal.h | 810 +++++++ compat/regex/regexec.c | 4377 +++++++++++++++++++++++++++++++++++ t/t7008-grep-binary.sh | 2 +- 8 files changed, 11188 insertions(+), 5106 deletions(-) create mode 100644 compat/regex/regcomp.c create mode 100644 compat/regex/regex_internal.c create mode 100644 compat/regex/regex_internal.h create mode 100644 compat/regex/regexec.c -- 1.7.2.1.389.gc3d0b ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH/RFC v2 0/3] Update compat/regex 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 ` (4 more replies) 0 siblings, 5 replies; 36+ messages in thread From: Jonathan Nieder @ 2010-08-17 8:03 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Paolo Bonzini, Andreas Schwab, Junio C Hamano Ævar Arnfjörð Bjarmason wrote: > Ævar Arnfjörð Bjarmason (3): > compat/regex: use the regex engine from gawk for compat > compat/regex: get the gawk regex engine to compile within git > t/t7008-grep-binary.sh: un-TODO a test that needs REG_STARTEND For what it’s worth: Acked-by: Jonathan Nieder <jrnieder@gmail.com> Thanks. Here’s a patch to go on top. I am not sure what platforms support REG_STARTEND, but we can always update the makefile as reports come in. -- 8< -- Subject: autoconf: don't use platform regex if it lacks REG_STARTEND If the platform regex cannot match null bytes, we might as well use the glibc version instead. Cc: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Cc: René Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- config.mak.in | 1 + configure.ac | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 0 deletions(-) diff --git a/config.mak.in b/config.mak.in index b4e65c3..67dbd3b 100644 --- a/config.mak.in +++ b/config.mak.in @@ -58,6 +58,7 @@ NO_INET_NTOP=@NO_INET_NTOP@ NO_INET_PTON=@NO_INET_PTON@ NO_ICONV=@NO_ICONV@ OLD_ICONV=@OLD_ICONV@ +NO_REGEX=@NO_REGEX@ NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@ INLINE=@INLINE@ SOCKLEN_T=@SOCKLEN_T@ diff --git a/configure.ac b/configure.ac index 5601e8b..71ac89f 100644 --- a/configure.ac +++ b/configure.ac @@ -706,6 +706,27 @@ else fi AC_SUBST(NO_C99_FORMAT) # +# Define NO_REGEX if you have no or inferior regex support in your C library. +AC_CACHE_CHECK([whether the platform regex can handle null bytes], + [ac_cv_c_excellent_regex], [ +AC_EGREP_CPP(yippeeyeswehaveit, + AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT +#include <regex.h> +], +[#ifdef REG_STARTEND +yippeeyeswehaveit +#endif +]), + [ac_cv_c_excellent_regex=yes], + [ac_cv_c_excellent_regex=yes]) +]) +if test $ac_cv_c_excellent_regex = yes; then + NO_REGEX= +else + NO_REGEX=YesPlease +fi +AC_SUBST(NO_REGEX) +# # Define FREAD_READS_DIRECTORIES if your are on a system which succeeds # when attempting to read from an fopen'ed directory. AC_CACHE_CHECK([whether system succeeds to read fopen'ed directory], -- 1.7.2.1.544.ga752d.dirty ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 0/5] Update compat/regex 2010-08-17 8:03 ` Jonathan Nieder @ 2010-08-17 9:24 ` Ævar Arnfjörð Bjarmason 2010-08-17 11:46 ` Paolo Bonzini 2010-08-17 23:19 ` Junio C Hamano 2010-08-17 9:24 ` [PATCH 2/5] compat/regex: get the gawk regex engine to compile within git Ævar Arnfjörð Bjarmason ` (3 subsequent siblings) 4 siblings, 2 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-17 9:24 UTC (permalink / raw) To: git Cc: Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Junio C Hamano, Frank Li, Marius Storm-Olsen, Johannes Sixt, Ævar Arnfjörð Bjarmason Here's a (hopefully) final version of this series. Changes since the v2 RFC: * Re-apply Frank Li's regerror() patch already in Git as v1.6.5-rc2~23. There was no need to apply another msvc fix, v1.7.0-rc0~15, because it had already been fixed upstream. * Include Jonathan Nieder's autoconf patch and add his Acked-by to the rest, add my Tested-by to his. * Fix text alignment in one of the commit messages. Note: This patch is intentionally diff --check unclean so we don't diverge from upstream. This series can also be pulled from http://github.com/avar/git/tree/update-fallback-regex-engine-v3 if the whitespace causes issues with git-am. Frank Li (1): Change regerror() declaration from K&R style to ANSI C (C89) Jonathan Nieder (1): autoconf: don't use platform regex if it lacks REG_STARTEND Ævar Arnfjörð Bjarmason (3): compat/regex: use the regex engine from gawk for compat compat/regex: get the gawk regex engine to compile within git t/t7008-grep-binary.sh: un-TODO a test that needs REG_STARTEND Makefile | 4 + compat/regex/regcomp.c | 3889 ++++++++++++++++++++++++++++++++ compat/regex/regex.c | 5003 +---------------------------------------- compat/regex/regex.h | 462 +++-- compat/regex/regex_internal.c | 1744 ++++++++++++++ compat/regex/regex_internal.h | 810 +++++++ compat/regex/regexec.c | 4377 +++++++++++++++++++++++++++++++++++ config.mak.in | 1 + configure.ac | 21 + t/t7008-grep-binary.sh | 2 +- 10 files changed, 11207 insertions(+), 5106 deletions(-) create mode 100644 compat/regex/regcomp.c create mode 100644 compat/regex/regex_internal.c create mode 100644 compat/regex/regex_internal.h create mode 100644 compat/regex/regexec.c -- 1.7.2.1.389.gc3d0b ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5] Update compat/regex 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 1 sibling, 0 replies; 36+ messages in thread From: Paolo Bonzini @ 2010-08-17 11:46 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Junio C Hamano, Frank Li, Marius Storm-Olsen, Johannes Sixt On 08/17/2010 11:24 AM, Ævar Arnfjörð Bjarmason wrote: > compat/regex: use the regex engine from gawk for compat > compat/regex: get the gawk regex engine to compile within git Should these two be squashed to ensure bisectability over a wide range of host systems? Paolo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5] Update compat/regex 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 1 sibling, 1 reply; 36+ messages in thread From: Junio C Hamano @ 2010-08-17 23:19 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Frank Li, Marius Storm-Olsen, Johannes Sixt Hmm, is there [1/5] in the series? ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5] Update compat/regex 2010-08-17 23:19 ` Junio C Hamano @ 2010-08-17 23:50 ` Jonathan Nieder 2010-08-18 10:41 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 36+ messages in thread From: Jonathan Nieder @ 2010-08-17 23:50 UTC (permalink / raw) To: Junio C Hamano Cc: Ævar Arnfjörð Bjarmason, git, Paolo Bonzini, Andreas Schwab, Frank Li, Marius Storm-Olsen, Johannes Sixt Junio C Hamano wrote: > Hmm, is there [1/5] in the series? Yes, but it probably missed the list because of vger's message length limits. For convenience, here is the series. I also think the first two patches should be combined, but this does not do so because others might disagree. Patch 5 (autoconf) is the v2 version. The following changes since commit 452c6d506b1a6dcf24d4ceaa592afc39c1c1a60e: push: mention "git pull" in error message for non-fast forwards (2010-08-12 18:06:07 -0700) are available in the git repository at: git://repo.or.cz/git/jrn.git ab/compat-regex Frank Li (1): Change regerror() declaration from K&R style to ANSI C (C89) Jonathan Nieder (1): autoconf: don't use platform regex if it lacks REG_STARTEND Ævar Arnfjörð Bjarmason (3): compat/regex: use the regex engine from gawk for compat compat/regex: get the gawk regex engine to compile within git t/t7008-grep-binary.sh: un-TODO a test that needs REG_STARTEND Makefile | 4 + compat/regex/regcomp.c | 3889 ++++++++++++++++++++++++++++++++ compat/regex/regex.c | 5003 +---------------------------------------- compat/regex/regex.h | 462 +++-- compat/regex/regex_internal.c | 1744 ++++++++++++++ compat/regex/regex_internal.h | 810 +++++++ compat/regex/regexec.c | 4377 +++++++++++++++++++++++++++++++++++ config.mak.in | 1 + configure.ac | 21 + t/t7008-grep-binary.sh | 2 +- 10 files changed, 11207 insertions(+), 5106 deletions(-) create mode 100644 compat/regex/regcomp.c create mode 100644 compat/regex/regex_internal.c create mode 100644 compat/regex/regex_internal.h create mode 100644 compat/regex/regexec.c ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5] Update compat/regex 2010-08-17 23:50 ` Jonathan Nieder @ 2010-08-18 10:41 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-18 10:41 UTC (permalink / raw) To: Jonathan Nieder Cc: Junio C Hamano, git, Paolo Bonzini, Andreas Schwab, Frank Li, Marius Storm-Olsen, Johannes Sixt 2010/8/17 Jonathan Nieder <jrnieder@gmail.com>: > Junio C Hamano wrote: > >> Hmm, is there [1/5] in the series? > > Yes, but it probably missed the list because of vger's message length > limits. > > For convenience, here is the series. I also think the first two > patches should be combined, but this does not do so because others > might disagree. Patch 5 (autoconf) is the v2 version. Pedantic: If there's going to be squashing that should probably include the regerror change too, otherwise it can't be bisected on MSVC either. Personally I think nothing should be squashed, because the history of having 1 commit be the unaltered gawk library and the rest being the changes we need on top of it will help future developers when we need to update the regex library again. And since most people hacking git probably do so on GNU or the BSD's which don't need NO_REGEX=YesPlease the practical downside of doing so is *very* little. ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/5] compat/regex: get the gawk regex engine to compile within git 2010-08-17 8:03 ` Jonathan Nieder 2010-08-17 9:24 ` [PATCH 0/5] " Ævar Arnfjörð Bjarmason @ 2010-08-17 9:24 ` Æ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 ` (2 subsequent siblings) 4 siblings, 0 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-17 9:24 UTC (permalink / raw) To: git Cc: Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Junio C Hamano, Frank Li, Marius Storm-Olsen, Johannes Sixt, Ævar Arnfjörð Bjarmason We need to define -DGAWK -DNO_MBSUPPORT so that the gawk regex engine will compile, and include stdio.h and stddef.h in regex.h. Gawk itself includes these headers before it includes the regex.h header. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Acked-by: Jonathan Nieder <jrnieder@gmail.com> --- Makefile | 4 ++++ compat/regex/regex.h | 3 +++ 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/Makefile b/Makefile index b4745a5..23a9f0d 100644 --- a/Makefile +++ b/Makefile @@ -1879,6 +1879,10 @@ ifdef NO_EXPAT http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT endif +ifdef NO_REGEX +compat/regex/regex.o: EXTRA_CPPFLAGS = -DGAWK -DNO_MBSUPPORT +endif + git-%$X: %.o $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) diff --git a/compat/regex/regex.h b/compat/regex/regex.h index de93327..61c9683 100644 --- a/compat/regex/regex.h +++ b/compat/regex/regex.h @@ -1,3 +1,6 @@ +#include <stdio.h> +#include <stddef.h> + /* Definitions for data structures and routines for the regular expression library. Copyright (C) 1985,1989-93,1995-98,2000,2001,2002,2003,2005,2006,2008 -- 1.7.2.1.389.gc3d0b ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 3/5] Change regerror() declaration from K&R style to ANSI C (C89) 2010-08-17 8:03 ` Jonathan Nieder 2010-08-17 9:24 ` [PATCH 0/5] " Æ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 ` Æ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 4 siblings, 0 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-17 9:24 UTC (permalink / raw) To: git Cc: Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Junio C Hamano, Frank Li, Marius Storm-Olsen, Johannes Sixt, Ævar Arnfjörð Bjarmason From: Frank Li <lznuaa@gmail.com> The MSVC headers typedef errcode as int, and thus confused the compiler in the K&R style definition. ANSI style deconfuses it. This patch was originally applied as v1.6.5-rc2~23 but needs to be re-applied since compat/regex was overwritten by Ævar Arnfjörð Bjarmason with the gawk regex engine. Signed-off-by: Frank Li <lznuaa@gmail.com> Signed-off-by: Marius Storm-Olsen <mstormo@gmail.com> Acked-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- compat/regex/regcomp.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c index 5115d7a..647c22a 100644 --- a/compat/regex/regcomp.c +++ b/compat/regex/regcomp.c @@ -546,11 +546,8 @@ weak_alias (__regcomp, regcomp) from either regcomp or regexec. We don't use PREG here. */ size_t -regerror (errcode, preg, errbuf, errbuf_size) - int errcode; - const regex_t *__restrict preg; - char *__restrict errbuf; - size_t errbuf_size; +regerror(int errcode, const regex_t *__restrict preg, + char *__restrict errbuf, size_t errbuf_size) { const char *msg; size_t msg_size; -- 1.7.2.1.389.gc3d0b ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 4/5] t/t7008-grep-binary.sh: un-TODO a test that needs REG_STARTEND 2010-08-17 8:03 ` Jonathan Nieder ` (2 preceding siblings ...) 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 ` Æ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 4 siblings, 0 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-17 9:24 UTC (permalink / raw) To: git Cc: Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Junio C Hamano, Frank Li, Marius Storm-Olsen, Johannes Sixt, Ævar Arnfjörð Bjarmason Now that we have a regex engine that supports REG_STARTEND this test should fail if "git grep" can't grep NULL characters. Platforms that don't have a POSIX regex engine which supports REG_STARTEND should always define NO_REGEX=YesPlease when compiling. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Acked-by: Jonathan Nieder <jrnieder@gmail.com> --- t/t7008-grep-binary.sh | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh index eb8ca88..c0f9f3f 100755 --- a/t/t7008-grep-binary.sh +++ b/t/t7008-grep-binary.sh @@ -61,7 +61,7 @@ test_expect_success 'git grep -Fi iLE a' ' # This test actually passes on platforms where regexec() supports the # flag REG_STARTEND. -test_expect_failure 'git grep ile a' ' +test_expect_success 'git grep ile a' ' git grep ile a ' -- 1.7.2.1.389.gc3d0b ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 5/5] autoconf: don't use platform regex if it lacks REG_STARTEND 2010-08-17 8:03 ` Jonathan Nieder ` (3 preceding siblings ...) 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 ` Ævar Arnfjörð Bjarmason 4 siblings, 0 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-17 9:24 UTC (permalink / raw) To: git Cc: Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Junio C Hamano, Frank Li, Marius Storm-Olsen, Johannes Sixt, Ævar Arnfjörð Bjarmason, René Scharfe From: Jonathan Nieder <jrnieder@gmail.com> If the platform regex cannot match null bytes, we might as well use the glibc version instead. Cc: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Cc: René Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Tested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- config.mak.in | 1 + configure.ac | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 0 deletions(-) diff --git a/config.mak.in b/config.mak.in index b4e65c3..67dbd3b 100644 --- a/config.mak.in +++ b/config.mak.in @@ -58,6 +58,7 @@ NO_INET_NTOP=@NO_INET_NTOP@ NO_INET_PTON=@NO_INET_PTON@ NO_ICONV=@NO_ICONV@ OLD_ICONV=@OLD_ICONV@ +NO_REGEX=@NO_REGEX@ NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@ INLINE=@INLINE@ SOCKLEN_T=@SOCKLEN_T@ diff --git a/configure.ac b/configure.ac index 5601e8b..71ac89f 100644 --- a/configure.ac +++ b/configure.ac @@ -706,6 +706,27 @@ else fi AC_SUBST(NO_C99_FORMAT) # +# Define NO_REGEX if you have no or inferior regex support in your C library. +AC_CACHE_CHECK([whether the platform regex can handle null bytes], + [ac_cv_c_excellent_regex], [ +AC_EGREP_CPP(yippeeyeswehaveit, + AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT +#include <regex.h> +], +[#ifdef REG_STARTEND +yippeeyeswehaveit +#endif +]), + [ac_cv_c_excellent_regex=yes], + [ac_cv_c_excellent_regex=yes]) +]) +if test $ac_cv_c_excellent_regex = yes; then + NO_REGEX= +else + NO_REGEX=YesPlease +fi +AC_SUBST(NO_REGEX) +# # Define FREAD_READS_DIRECTORIES if your are on a system which succeeds # when attempting to read from an fopen'ed directory. AC_CACHE_CHECK([whether system succeeds to read fopen'ed directory], -- 1.7.2.1.389.gc3d0b ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH/RFC v2 2/3] compat/regex: get the gawk regex engine to compile within git 2010-08-15 11:08 ` Ævar Arnfjörð Bjarmason ` (5 preceding siblings ...) 2010-08-17 5:17 ` [PATCH/RFC v2 0/3] Update compat/regex Ævar Arnfjörð Bjarmason @ 2010-08-17 5:17 ` Æ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 7 siblings, 0 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-17 5:17 UTC (permalink / raw) To: git Cc: Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Junio C Hamano, Ævar Arnfjörð Bjarmason We need to define -DGAWK -DNO_MBSUPPORT so that the gawk regex engine will compile, and include stdio.h and stddef.h in regex.h. Gawk itself includes these headers before it includes the regex.h header. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 4 ++++ compat/regex/regex.h | 3 +++ 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/Makefile b/Makefile index b4745a5..23a9f0d 100644 --- a/Makefile +++ b/Makefile @@ -1879,6 +1879,10 @@ ifdef NO_EXPAT http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT endif +ifdef NO_REGEX +compat/regex/regex.o: EXTRA_CPPFLAGS = -DGAWK -DNO_MBSUPPORT +endif + git-%$X: %.o $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) diff --git a/compat/regex/regex.h b/compat/regex/regex.h index de93327..61c9683 100644 --- a/compat/regex/regex.h +++ b/compat/regex/regex.h @@ -1,3 +1,6 @@ +#include <stdio.h> +#include <stddef.h> + /* Definitions for data structures and routines for the regular expression library. Copyright (C) 1985,1989-93,1995-98,2000,2001,2002,2003,2005,2006,2008 -- 1.7.2.1.389.gc3d0b ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH/RFC v2 3/3] t/t7008-grep-binary.sh: un-TODO a test that needs REG_STARTEND 2010-08-15 11:08 ` Ævar Arnfjörð Bjarmason ` (6 preceding siblings ...) 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 ` Ævar Arnfjörð Bjarmason 7 siblings, 0 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-08-17 5:17 UTC (permalink / raw) To: git Cc: Paolo Bonzini, Andreas Schwab, Jonathan Nieder, Junio C Hamano, Ævar Arnfjörð Bjarmason Now that we have a regex engine that supports REG_STARTEND this test should fail if "git grep" can't grep NULL characters. Platforms that don't have a POSIX regex engine which supports REG_STARTEND should always define NO_REGEX=YesPlease when compiling. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t7008-grep-binary.sh | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh index eb8ca88..c0f9f3f 100755 --- a/t/t7008-grep-binary.sh +++ b/t/t7008-grep-binary.sh @@ -61,7 +61,7 @@ test_expect_success 'git grep -Fi iLE a' ' # This test actually passes on platforms where regexec() supports the # flag REG_STARTEND. -test_expect_failure 'git grep ile a' ' +test_expect_success 'git grep ile a' ' git grep ile a ' -- 1.7.2.1.389.gc3d0b ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported 2010-07-15 18:44 ` Ævar Arnfjörð Bjarmason [not found] ` <20100715220059.GA3312@burratino> @ 2010-07-16 14:33 ` Ævar Arnfjörð Bjarmason 2010-07-16 19:50 ` Jonathan Nieder 1 sibling, 1 reply; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-16 14:33 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Thu, Jul 15, 2010 at 18:44, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Thu, Jul 15, 2010 at 17:47, Junio C Hamano <gitster@pobox.com> wrote: >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> >>> On Thu, Jul 8, 2010 at 19:40, Junio C Hamano <gitster@pobox.com> wrote: >>>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >>>> >>>>> +if git grep ile a >>>>> +then >>>>> + test_expect_success 'git grep ile a' 'git grep ile a' >>>>> +else >>>>> + test_expect_failure 'git grep ile a' 'git grep ile a' >>>>> +fi >>>> >>>> So if command "X" is known to succeed, we run it inside expect_success >>>> and if not we run it inside expect_failure? >>>> >>>> What kind of idiocy is that, I have to wonder... >>> >>> Well, the point is to normalize the test suite so that we never have >>> passing TODO tests if everything's OK. >> >> I do not consider a test that passes under some condition but doesn't >> under some other condition "everything is OK". Marking the test as >> "expect failure" as René originally did makes a lot of sense to me. >> >> The quoted patch is even worse as it will _actively_ prevent you from >> catching a new error you just introduced while futzing "git grep" on a >> platform that used to work. Your "if" statement will say "ah, grep is >> broken", and you will use expect-failure, not because your platform does >> not support REG_STARTEND, but because you broke "git grep". >> >> The point of having tests is to help you catch your bugs while you >> develop. A test that turns itself off when the feature it is testing is >> broken helps nobody. >> >> So forget about "passing TODO tests", whatever a "TODO test" is. The >> change in question is actively _wrong_. > > I was under the impression that REG_STARTEND was considered purely > icing on the cake, i.e. that the tests should be passing whether or > not it was present. > > I guess my reasoning at the time was that if that wasn't the case, > reporting an unexpected pass by default, as opposed to a failing TODO > on platforms without REG_STARTEND. Since only TAP will report this, I > thought that was just an omission. > > Anyway, since REG_STARTEND *isn't* obviously considered icing you're > of course right, but the test is still broken as-is. Now it reports an > abnormal condition if REG_STARTEND is present (passing TODO test), it > should instead have a failing TODO test where REG_STARTEND isn't > present. I'll come up with a patch to fix that. Well to clarify: The TAP is arguably right, although semantically these sort of tests should probably be a SKIP on unsupported platforms, not a passing TODO. prove(1) also features passing TODO tests a bit too prominently for my tastes. I've filed a bug for that: https://rt.cpan.org/Public/Bug/Display.html?id=59428 > We should also just upgrade the GNU regex library in compat/regex to > the version that supports REG_STARTEND. Unfortunately that seems > easier said than done, since the library is now part of glibc, and has > aquired a lot of glibc specific macros and other constructs that would > need to be #defined away or otherwise worked around. This is what we should be focusing on, the patch by Jonathan Nieder is a good start. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported 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 0 siblings, 1 reply; 36+ messages in thread From: Jonathan Nieder @ 2010-07-16 19:50 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git Ævar Arnfjörð Bjarmason wrote: > Well to clarify: The TAP is arguably right, although semantically > these sort of tests should probably be a SKIP on unsupported > platforms, not a passing TODO. No, we support all platforms people are willing to fix without uglifying the code too much. So a bug is a bug. Test prerequisites get used for behavior that is either out of scope (Posix-style permissions on Windows) or hard to test (signal delivery to child process in t7502-commit). The semantic problem you are describing here is that we have no separate way to mark bugs that are not consistently reproducible. A “fixed” test_expect_failure is sometimes a fluke, like in this example. If lucky, you can find an appropriate condition and use test_expect_success or test_expect_failure as appropriate. In the general case, that is not always easy. Better to eliminate the unreproducible bugs. > On Thu, Jul 15, 2010 at 18:44, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: >> We should also just upgrade the GNU regex library in compat/regex to >> the version that supports REG_STARTEND. [...] > This is what we should be focusing on By the way, I have no preference for choice of regex library here. If something else is easier to get working correctly, that would be great. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported 2010-07-16 19:50 ` Jonathan Nieder @ 2010-07-16 20:51 ` Ævar Arnfjörð Bjarmason 2010-07-16 21:06 ` Jonathan Nieder 0 siblings, 1 reply; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-16 20:51 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Junio C Hamano, git On Fri, Jul 16, 2010 at 19:50, Jonathan Nieder <jrnieder@gmail.com> wrote: > Ævar Arnfjörð Bjarmason wrote: > >> Well to clarify: The TAP is arguably right, although semantically >> these sort of tests should probably be a SKIP on unsupported >> platforms, not a passing TODO. > > No, we support all platforms people are willing to fix without > uglifying the code too much. So a bug is a bug. Test > prerequisites get used for behavior that is either out of scope > (Posix-style permissions on Windows) or hard to test (signal > delivery to child process in t7502-commit). > > The semantic problem you are describing here is that we have no > separate way to mark bugs that are not consistently reproducible. > A “fixed” test_expect_failure is sometimes a fluke, like in this > example. > > If lucky, you can find an appropriate condition and use > test_expect_success or test_expect_failure as appropriate. In > the general case, that is not always easy. Better to eliminate the > unreproducible bugs. The failure is totally predicated on whether or not REG_STARTEND is available on the system, so in a perfect world (where we couldn't provide a compat regex library) we should detect whether REG_STARTEND is defined in regex.h, and then stick it in GIT-BUILD-OPTIONS. Then you could do: test_expect_success REG_STARTEND 'git grep ile a' ' git grep ile a ' But it's much easier to just fix the replacement regex library so the test works everywhere instead of detecting for REG_STARTEND (e.g. using a helper). >> On Thu, Jul 15, 2010 at 18:44, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > >>> We should also just upgrade the GNU regex library in compat/regex to >>> the version that supports REG_STARTEND. > [...] >> This is what we should be focusing on > > By the way, I have no preference for choice of regex library here. If > something else is easier to get working correctly, that would be great. 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. More generally, the regex use in Git is something I've wanted to look at more closely. Right now a bunch of Git tools use regexes in one form or another, but they don't do so consistently: config.c:847: !regexec(store.value_regex, value, 0, NULL, 0))); config.c:1155: if (regcomp(store.value_regex, value_regex, builtin/remote.c:1432: if (regcomp(&old_regex, oldurl, REG_EXTENDED)) builtin/remote.c:1436: if (!regexec(&old_regex, urlset[i], 0, NULL, 0)) builtin/blame.c:1963: if (!(reg_error = regcomp(®exp, spec + 1, REG_NEWLINE)) && builtin/blame.c:1964: !(reg_error = regexec(®exp, line, 1, match, 0))) { builtin/config.c:104: if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0)) builtin/config.c:107: (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0))) builtin/config.c:177: if (regcomp(key_regexp, key, REG_EXTENDED)) { builtin/config.c:190: if (regcomp(regexp, regex_, REG_EXTENDED)) { builtin/apply.c:579: if (regcomp(stamp, stamp_regexp, REG_EXTENDED)) { builtin/apply.c:586: status = regexec(stamp, timestamp, ARRAY_SIZE(m), m, 0); sha1_name.c:703: if (regcomp(®ex, prefix, REG_EXTENDED)) sha1_name.c:729: if (!regexec(®ex, p + 2, 0, NULL, 0)) { diff.c:779: if (!regexec(word_regex, buffer->ptr + *begin, 1, match, 0)) { diff.c:2011: if (regcomp(ecbdata.diff_words->word_regex, xdiff-interface.c:276: if (!regexec(®->re, line_buffer, 2, pmatch, 0)) { xdiff-interface.c:323: if (regcomp(®->re, expression, cflags)) diffcore-pickaxe.c:29: while (*data && !regexec(regexp, data, 1, ®match, flags)) { diffcore-pickaxe.c:62: err = regcomp(®ex, needle, REG_EXTENDED | REG_NEWLINE); grep.c:73: err = regcomp(&p->regexp, p->pattern, opt->regflags); grep.c:375: return regexec(preg, line, 1, match, eflags); http-backend.c:567: if (regcomp(&re, c->pattern, REG_EXTENDED)) http-backend.c:569: if (!regexec(&re, dir, 1, out, 0)) { Some of these are supplying REG_EXTENDED, some (like git-grep) allow for passing REG_ICASE, some don't. There's no way to do e.g. do a case insensitive git blame -L'/start/', other than -L'/[sS][tT][aA][rR][tT]/' that is. It would be nice if we could consistently pass regex flags, like -L'/start/i' in this case. This also applies to features like the new ':/string' feature added by Linus, maybe that should optionally be ':/string/i' (or within the scope of the current implementation, ':!i/string' or ':!/string/i'). Regarding regular expression implementations. We might want to look into bundling one implementation and using it everywhere, but I haven't tested that to see if there are significant wins to be had. There are regex implementations (notably GNU awk and GNU grep) that are probably better fits for what Git does, a GNU grep backend for git-log would probably search through revisions with a regex faster, but I haven't tested it so I don't know if it's fast enough to bother with it. Using NFA engines like that also gives you some performance guarantees (see [1][2]). Although that mostly matters in pathological situations. But having Git make that guarantee might be useful, e.g. so Gitweb can offer an interface to git-grep without having the CPU on the host burned up by a pattern like (a*|a*)*. The NFA engines we could use to get those features include Google's RE2 (it's in C++, but we might fall back on e.g. the Plan9 engine), GNU awk/grep, TRE, and probably some others. But maybe this isn't something that's wanted or needed. Does anyone else want a more unified regex interface in Git, or a determanistic regex engine built-in? 1. http://swtch.com/~rsc/regexp/regexp1.html 2. http://stackoverflow.com/questions/1178173/regex-implementation-that-can-handle-machine-generated-regexs-non-backtracking ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported 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 0 siblings, 1 reply; 36+ messages in thread From: Jonathan Nieder @ 2010-07-16 21:06 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Junio C Hamano, git, Andreas Schwab Æ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 ' > 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. > 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). > 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. Thanks for the food for thought, Jonathan ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] grep: Don't pass a TODO test if REG_STARTEND is supported 2010-07-16 21:06 ` Jonathan Nieder @ 2010-07-16 21:19 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 36+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-07-16 21:19 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Junio C Hamano, git, Andreas Schwab 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 :) ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2010-08-18 10:41 UTC | newest] Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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
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).