svn commit: r328240 - in head: etc/mtree lib lib/libc/regex lib/libc/tests/regex lib/libregex lib/libregex/tests share/mk

[Konstantin Belousov]

On Mon, Jan 22, 2018 at 11:43:34AM -0600, Kyle Evans wrote:
> On Mon, Jan 22, 2018 at 9:53 AM, Konstantin Belousov
> <kostikbel at gmail.com> wrote:
> > On Mon, Jan 22, 2018 at 08:05:30AM -0600, Kyle Evans wrote:
> >> On Mon, Jan 22, 2018 at 4:26 AM, Konstantin Belousov
> >> <kostikbel at gmail.com> wrote:
> >> > On Mon, Jan 22, 2018 at 02:44:41AM +0000, Kyle Evans wrote:
> >> >> Author: kevans
> >> >> Date: Mon Jan 22 02:44:41 2018
> >> >> New Revision: 328240
> >> >> URL: https://svnweb.freebsd.org/changeset/base/328240
> >> >>
> >> >> Log:
> >> >>   Add libregex, connect it to the build
> >> >>
> >> >>   libregex is a regex(3) implementation intended to feature GNU extensions and
> >> >>   any other non-POSIX compliant extensions that are deemed worthy.
> >> >>
> >> >>   These extensions are separated out into a separate library for the sake of
> >> >>   not cluttering up libc further with them as well as not deteriorating the
> >> >>   speed (or lack thereof) of the libc implementation.
> >> >>
> >> >>   libregex is implemented as a build of the libc implementation with LIBREGEX
> >> >>   defined to distinguish this from a libc build. The reasons for
> >> >>   implementation like this are two-fold:
> >> >>
> >> >>   1.) Maintenance- This reduces the overhead induced by adding yet another
> >> >>   regex implementation to base.
> >> >>
> >> >>   2.) Ease of use- Flipping on GNU extensions will be as simple as linking
> >> >>   against libregex, and POSIX-compliant compilations can be guaranteed with a
> >> >>   REG_POSIX cflag that should be ignored by libc/regex and disables extensions
> >> >>   in libregex. It is also easier to keep REG_POSIX sane and POSIX pure when
> >> >>   implemented in this fashion.
> >> > You are doing very fragile and unmaintainable trick on all consumers
> >> > there.  Your libregex.so exports the same symbols under the same version
> >> > as the libc does. In other words, we now provide two binary-incompatible
> >> > callable symbols, and selection of the symbol by the consumer depends on
> >> > the DT_NEEDED order and interposing.  For instance, if some program loads
> >> > a module linked to your libregex, the program behaviour suddenly changes.
> >> >
> >> > Since the library provides incompatible implementation, it must use
> >> > different versions for the symbols, at least to save others time to
> >> > debug the mess.
> >>
> >> What's the best way that you see, going forward?
> >>
> >> I'm inclined to throw a Symbol.map into libregex using FBSD_1.1...
> >> these interfaces are otherwise stable stable within the two respective
> >> libraries, so I don't see that causing too much pain in the future
> >> because symbol version changes should be rare.
> > I do not think this is wise to create contention on the standard FreeBSD'
> > version namespace.
> >
> >>
> >> On the other hand, I could see wanting to use something more like
> >> FBSD_LIBREGEX_1.0 so that if the situation does come up one doesn't
> >> need to double-check that they're not colliding with the other
> >> implementation.
> > I like this more.  We still have to carry that symbols with the current
> > behaviour forever, but at least they would no longer conflict with the
> > libc' symbols for dynamic linking.
> 
> Right- that makes sense. Would you object to the following patch? The
> versioning was botched in the first place because of no VERSION_DEF.
> I've also dropped the FBSD_ prefix from my previous mention of it
> because there seems to be no good justification or precedent for it
> elsewhere in the tree that I've found.
This looks fine.

I would suggest to add the explanation why a different version name was
used there, in particular, to highlight that the symbols otherwise
conflict with the symbols from libc.
> 
> diff --git a/lib/libregex/Makefile b/lib/libregex/Makefile
> index eb6b0b64650..8ac496dd9f7 100644
> --- a/lib/libregex/Makefile
> +++ b/lib/libregex/Makefile
> @@ -9,7 +9,8 @@ SHLIB_MINOR=    0
> 
>  CFLAGS+=       -DLIBREGEX
>  LIBC_SRCTOP=   ${.CURDIR:H}/libc
> -SYMBOL_MAPS=   ${SYM_MAPS}
> +VERSION_DEF=   ${.CURDIR}/Versions.def
> +SYMBOL_MAPS=   ${.CURDIR}/Symbol.map
>  WARNS?=        2
> 
>  #HAS_TESTS=
> diff --git a/lib/libregex/Symbol.map b/lib/libregex/Symbol.map
> new file mode 100644
> index 00000000000..1b77bd85aca
> --- /dev/null
> +++ b/lib/libregex/Symbol.map
> @@ -0,0 +1,10 @@
> +/*
> + * $FreeBSD$
> + */
> +
> +LIBREGEX_1.0 {
> +       regcomp;
> +       regerror;
> +       regexec;
> +       regfree;
> +};
> diff --git a/lib/libregex/Versions.def b/lib/libregex/Versions.def
> new file mode 100644
> index 00000000000..b6c84200762
> --- /dev/null
> +++ b/lib/libregex/Versions.def
> @@ -0,0 +1,5 @@
> +# $FreeBSD$
> +
> +LIBREGEX_1.0 {
> +};
> +