all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: 32194@debbugs.gnu.org
Subject: bug#32194: [PATCH] Use Gnulib regex for lib-src
Date: Sat, 04 Aug 2018 18:41:38 +0300	[thread overview]
Message-ID: <83a7q2t9zh.fsf@gnu.org> (raw)
In-Reply-To: <15823bbe-8298-0d69-c7a6-edf2001e4513@cs.ucla.edu> (message from Paul Eggert on Tue, 31 Jul 2018 11:21:38 -0700)

> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: 32194@debbugs.gnu.org
> Date: Tue, 31 Jul 2018 11:21:38 -0700
> 
> Eli Zaretskii wrote:
> 
> > would it be possible to let the configure
> >     script also check -lregex for those functions?  This could be
> >     important on non-glibc systems.
> 
> It would no doubt be possible. However, we haven't found a need in other 
> applications that use Gnulib. Gnulib code tends to be more up-to-date 
> than system-supplied libraries, and in practice it's OK if the system 
> puts the regex code in a place where 'configure' won't find it, as Emacs 
> will just fall back on the better Gnulib version in that case.

The idea is that the system-supplied regex library is also GNU regex,
but one tuned better to the specific platform.  The configure script
could check whether that library is up to speed, before using it.

Obviously, not a terribly important issue, but I've seen projects that
do this.

> > 2. Do we really need to rename src/regex.c?  We could have 2 regex.c in
> >     separate directories, couldn't we?  If possible, I'd prefer not to
> >     rename files, as that makes VCS forensics more complicated.
> 
> Renaming src/regex.h works around problems with having two include files 
> src/regex.h and lib/regex.h that fight each other. I tried not renaming 
> src/regex.h at first, and then ran into problems and gave up; although I 
> surely could get it to work eventually, I figured that we'd continue to 
> have ongoing problems and so it would be better to bite the bullet.

Too bad, but I guess we have no choice.  I'd only ask that the
renaming be done as a separate commit before the rest of the changes
in those two files, so that all the changes in src/regex.[ch] will be
after the rename, as that will make future forensics easier.

> > 3. Is it really necessary to pull mbrtowc, locale_charset,
> >     nl_langinfo, and their dependencies?
> 
> I didn't know, so in my first cut I did the easiest thing and pulled 
> them all in. It's pretty easy to omit them; we can always add some back 
> if we find they're needed after all. Revised patch attached; it is the 
> first patch in the attached series. (The later patches are some of the 
> simplifications mentioned above.)

OK, thanks.

> > P.S. Is it true that this version will no longer support a build with
> > WIDE_CHAR_SUPPORT undefined, i.e. those which have only the C locale?
> 
> I don't see any issues with such a build. What sort of problem do you 
> have in mind?

AFAIU, you suggest removing the !WIDE_CHAR_SUPPORT code, but we
previously supported platforms that don't have all the prerequisites
for using that code.  So platforms which don't have WIDE_CHAR_SUPPORT
will now be required to provide it, or will fail to compile/run.  Am I
wrong?

> OK, revised patchset attached. This keeps the debugging code in question. I took 
> the liberty of renaming DEBUG (which clashes with other DEBUGs) to 
> EMACS_REGEX_DEBUG.

Fine with me.

> (malloc, realloc, free): Do not redefine.  Adjust all callers
> to use xmalloc, xrealloc, xfree instead.

Is this wise?  xmalloc etc. are tuned to allocate Lisp data and
aligned objects, something that isn't necessarily needed in regex
library.  Why not use the normal memory allocation routines?

Thanks.

P.S. I think we should compare the performance of etags before and
after the switch, just to be sure we aren't going to suffer a
performance penalty.





  parent reply	other threads:[~2018-08-04 15:41 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-17 23:47 bug#32194: [PATCH] Use Gnulib regex for lib-src Paul Eggert
2018-07-18 16:34 ` Eli Zaretskii
2018-07-31 18:21   ` Paul Eggert
2018-08-01  0:29     ` Noam Postavsky
2018-08-01  0:59       ` Paul Eggert
2018-08-01  1:10         ` Noam Postavsky
2018-08-01  1:27           ` Paul Eggert
2018-08-02  0:58             ` Noam Postavsky
2018-08-02  1:49               ` Paul Eggert
2018-08-02  1:55                 ` Noam Postavsky
2018-08-02  2:42               ` Eli Zaretskii
2018-08-02  3:19                 ` Paul Eggert
2018-08-02 10:28                   ` Noam Postavsky
2018-08-02 14:05     ` Eli Zaretskii
2018-08-02 14:41       ` Paul Eggert
2018-08-02 14:55         ` Eli Zaretskii
2018-08-02 15:41           ` Paul Eggert
2018-08-04 15:41     ` Eli Zaretskii [this message]
2018-08-04 23:34       ` Paul Eggert
2018-08-05 17:58         ` Eli Zaretskii
2018-08-06  2:40           ` Paul Eggert
2018-08-06 10:59             ` Andy Moreton
2018-08-06 14:52               ` Eli Zaretskii
2018-08-06 15:40                 ` Andy Moreton
2018-08-06 14:54             ` Eli Zaretskii

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=83a7q2t9zh.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=32194@debbugs.gnu.org \
    --cc=eggert@cs.ucla.edu \
    /path/to/YOUR_REPLY

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

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

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.