Re: --with-included-foo={yes,no} and which foo.h to use?

[Julian Foad]

Charles Levert wrote:
> Updating to glibc's regex.c will allow us to
> progress in fixing RE_ICASE bugs now, and maybe
> others as well.  That why I can't see one being
> a priority without the other.
[...]
> Please comment.  I'd really like to see this go
> in before 2.6, for the above reasons.

Thanks for working on this.

I'm generally in favour of getting this in to Grep soon, unless it has the 
potential to destabilise Grep by changing the behaviour in subtle ways or 
creating a risk of bugs in the way we interface to it.  Do you see any such risks?

> 2005-07-07  Charles Levert  <address@hidden>
>
>         Big regex.[ch] update from latest glibc CVS libc/posix/.

It's not appropriate to use the latest development version of some code.  Use 
the latest officially released version.  (Maybe that's what you meant, but it 
doesn't sound like it.)  If the latest released version isn't useable, please 
discuss.  Not using officially released versions of software nearly always 
leads to trouble at some time.

>         This should be an enabler for fixing pending bugs.
>         * lib/regex.h, lib/posix, lib/posix/.cvsignore,
>           lib/posix/Makefile.am, lib/posix/regex.h: Removed.
>         * lib/regex/regex.h,
>           lib/regex/regex_internal.h, lib/regex/regex_internal.c,
>           lib/regex/regcomp.c, lib/regex/regexec.c: New from glibc CVS,
>           all include files (even the *.c) in their own directory so
>           they won't be found if not configured (especially regex.h).
>         * lib/regex/.cvsignore, lib/regex/Makefile.am: New.
>         * lib/regex.c: Replaced with glibc CVS version.
>         * m4/regex.m4: Replaced with gnulib CVS version.
>         * m4/onceonly.m4, m4/restrict.m4: New from latest gnulib CVS.
>         * m4/Makefile.am: Add previous two files.
>         * Makefile.am: Add m4/onceonly.m4 and m4/restrict.m4.
>         * lib/regex/regex_internal.h: Do not re#define __mempcpy is it's

"is it's" -> "if it's"

>           already a cpp macro (maybe even to a gcc internal).

Does the same argument not apply to all the other __foo macros defined there?  
Why?

>         * tests/spencer1.tests: 'a[b-a]' now has an 'Invalid range end'.

That sentence didn't tell me what you'd changed.  How about "Expect an error 
('Invalid range end') for 'a[b-a]'."

>         * m4/regex.m4: Add a configure-time test of the system regex

It's confusing to have two separate log entries for this file, one saying that 
it's been replaced and the other saying that it's been changed.  Please combine 
them into a single entry.

Ditto for lib/regex/regex_internal.h, though that's harder because it was 
listed along with lots of others.  I suggest moving the following sentence up 
into the overall description part of the log message:

  All the regex include files (even the *.c) go in their own directory so
  they won't be found if not configured (especially regex.h).

and then writing:

  * lib/regex/regex.h, lib/regex/regex_internal.c,
    lib/regex/regcomp.c, lib/regex/regexec.c: New from glibc CVS.
  * lib/regex/regex_internal.h: New from glibc CVS. Do not redefine __mempcpy
    if it's already a cpp macro (maybe even to a gcc internal).

>           for this last error to be correctly reported, number invalid
>           return codes from 1 to 7, add AC_SUBST(REGEX_INCLUDE, ...),
>           attempt to use RE_ICASE so compilation will fail if missing.

That whole description is too compressed for me to be able to understand it. 
Please could you show us the diff between the GnuLib version and your version 
if that will help.  (Most of my questions are: What is the consequence of that 
new test?  What are "invalid return codes", and why are you numbering them? 
Compilation of what will fail - do you mean Grep now requires RE_ICASE so 
configure should fail if it's missing?)

>         * bootstrap/Makefile.try: Add -I$(libdir)/regex to CFLAGS.
>         * configure.in: Now uses gl_INCLUDED_REGEX, improved
>           --without-included-regex warning, generates lib/regex/Makefile
>           but no longer lib/posix/Makefile.
>         * lib/Makefile.am: SUBDIRS now has regex but no longer posix,
>           INCLUDES has @REGEX_INCLUDE@, regex.h gone from noinst_HEADERS.
>         * src/Makefile.am: INCLUDES has @address@hidden

- Julian