[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Julian Foad
Subject: Re: --with-included-foo={yes,no} and which foo.h to use?
Date: Thu, 07 Jul 2005 11:29:00 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050511

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?  

        * 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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]