bug#19874: simplifying Emacs configure.ac

bug#19874: simplifying Emacs configure.ac

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

On 1/18/20 5:46 AM, Bruno Haible wrote:

> This code in Emacs configure.ac:
> 
> # Use the system putenv even if it lacks GNU features, as we don't need them,
> # and the gnulib replacement runs afoul of a FreeBSD 10.1 bug; see Bug#19874.
> AC_CHECK_FUNCS_ONCE([putenv])
> AC_DEFUN([gl_FUNC_PUTENV],
>    [test "$ac_cv_func_putenv" = yes || REPLACE_PUTENV=1])
> 
> appears to be extra convoluted. All platforms have the putenv function.
> Therefore REPLACE_PUTENV=1 is never executed here. If Emacs does not need
> the putenv override, the simpler way is to invoke gnulib-tool with
> '--avoid=putenv'.

Thanks for suggesting that. I installed the attached patch to Emacs master to 
implement something along the lines you suggested. I am cc'ing Ashish Shukla who 
reported Bug#19874 ("25.0.50; encode-time not working as expected"), as well as 
Wolfgang Jenkner who helped debug that, to give them a heads-up that the fix for 
Bug#19874 has changed.

[-- Attachment #2: 0001-Remove-Gnulib-putenv-code.patch --]
[-- Type: text/x-patch, Size: 11943 bytes --]

From 35a1a007bb7506c72ee6d9757a79014c679e7bae Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 18 Jan 2020 20:54:29 -0800
Subject: [PATCH] Remove Gnulib putenv code
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It’s not needed, since Emacs always uses the system putenv and all
platforms have putenv.  This improves on the fix for Bug#19874.
Suggested by Bruno Haible.
* admin/merge-gnulib (GNULIB_MODULES): Remove putenv.
* configure.ac: Remove workarounds for Gnulib putenv module.
* lib/gnulib.mk.in, m4/gnulib-comp.m4: Regenerate.
* lib/putenv.c, m4/putenv.m4: Remove.
---
 admin/merge-gnulib |   2 +-
 configure.ac       |   5 --
 lib/gnulib.mk.in   |  12 ---
 lib/putenv.c       | 194 ---------------------------------------------
 m4/gnulib-comp.m4  |   9 ---
 m4/putenv.m4       |  60 --------------
 6 files changed, 1 insertion(+), 281 deletions(-)
 delete mode 100644 lib/putenv.c
 delete mode 100644 m4/putenv.m4

diff --git a/admin/merge-gnulib b/admin/merge-gnulib
index 3dee0b72b3..48c81e61e2 100755
--- a/admin/merge-gnulib
+++ b/admin/merge-gnulib
@@ -38,7 +38,7 @@ GNULIB_MODULES=
   getloadavg getopt-gnu gettime gettimeofday gitlog-to-changelog
   ieee754-h ignore-value intprops largefile lstat
   manywarnings memmem-simple mempcpy memrchr minmax mkostemp mktime nstrftime
-  pathmax pipe2 pselect pthread_sigmask putenv
+  pathmax pipe2 pselect pthread_sigmask
   qcopy-acl readlink readlinkat regex
   sig2str socklen stat-time std-gnu11 stdalign stddef stdio
   stpcpy strnlen strtoimax symlink sys_stat sys_time
diff --git a/configure.ac b/configure.ac
index 8a13b36e94..d7b4d0352c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -882,11 +882,6 @@ AC_DEFUN
   test $func = pthread_sigmask || AS_VAR_APPEND([funcs], [" $func"])
 done
 ac_func_list=$funcs
-# Use the system putenv even if it lacks GNU features, as we don't need them,
-# and the gnulib replacement runs afoul of a FreeBSD 10.1 bug; see Bug#19874.
-AC_CHECK_FUNCS_ONCE([putenv])
-AC_DEFUN([gl_FUNC_PUTENV],
-  [test "$ac_cv_func_putenv" = yes || REPLACE_PUTENV=1])
 # Emacs does not use the wchar or wctype-h modules.
 AC_DEFUN([gt_TYPE_WINT_T],
   [GNULIB_OVERRIDES_WINT_T=0
diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in
index ed80f88a87..1c8085b1a4 100644
--- a/lib/gnulib.mk.in
+++ b/lib/gnulib.mk.in
@@ -127,7 +127,6 @@
 #  pipe2 \
 #  pselect \
 #  pthread_sigmask \
-#  putenv \
 #  qcopy-acl \
 #  readlink \
 #  readlinkat \
@@ -2162,17 +2161,6 @@ EXTRA_libgnu_a_SOURCES += pthread_sigmask.c
 endif
 ## end   gnulib module pthread_sigmask
 
-## begin gnulib module putenv
-ifeq (,$(OMIT_GNULIB_MODULE_putenv))
-
-
-EXTRA_DIST += putenv.c
-
-EXTRA_libgnu_a_SOURCES += putenv.c
-
-endif
-## end   gnulib module putenv
-
 ## begin gnulib module qcopy-acl
 ifeq (,$(OMIT_GNULIB_MODULE_qcopy-acl))
 
diff --git a/lib/putenv.c b/lib/putenv.c
deleted file mode 100644
index 9e862e63d3..0000000000
--- a/lib/putenv.c
+++ /dev/null
@@ -1,194 +0,0 @@
-/* Copyright (C) 1991, 1994, 1997-1998, 2000, 2003-2020 Free Software
-   Foundation, Inc.
-
-   NOTE: The canonical source of this file is maintained with the GNU C
-   Library.  Bugs can be reported to bug-glibc@prep.ai.mit.edu.
-
-   This program 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 any
-   later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <https://www.gnu.org/licenses/>.  */
-
-#include <config.h>
-
-/* Specification.  */
-#include <stdlib.h>
-
-#include <stddef.h>
-
-/* Include errno.h *after* sys/types.h to work around header problems
-   on AIX 3.2.5.  */
-#include <errno.h>
-#ifndef __set_errno
-# define __set_errno(ev) ((errno) = (ev))
-#endif
-
-#include <string.h>
-#include <unistd.h>
-
-#if defined _WIN32 && ! defined __CYGWIN__
-# define WIN32_LEAN_AND_MEAN
-# include <windows.h>
-#endif
-
-#if _LIBC
-# if HAVE_GNU_LD
-#  define environ __environ
-# else
-extern char **environ;
-# endif
-#endif
-
-#if _LIBC
-/* This lock protects against simultaneous modifications of 'environ'.  */
-# include <bits/libc-lock.h>
-__libc_lock_define_initialized (static, envlock)
-# define LOCK   __libc_lock_lock (envlock)
-# define UNLOCK __libc_lock_unlock (envlock)
-#else
-# define LOCK
-# define UNLOCK
-#endif
-
-static int
-_unsetenv (const char *name)
-{
-  size_t len;
-#if !HAVE_DECL__PUTENV
-  char **ep;
-#endif
-
-  if (name == NULL || *name == '\0' || strchr (name, '=') != NULL)
-    {
-      __set_errno (EINVAL);
-      return -1;
-    }
-
-  len = strlen (name);
-
-#if HAVE_DECL__PUTENV
-  {
-    int putenv_result, putenv_errno;
-    char *name_ = malloc (len + 2);
-    memcpy (name_, name, len);
-    name_[len] = '=';
-    name_[len + 1] = 0;
-    putenv_result = _putenv (name_);
-    putenv_errno = errno;
-    free (name_);
-    __set_errno (putenv_errno);
-    return putenv_result;
-  }
-#else
-
-  LOCK;
-
-  ep = environ;
-  while (*ep != NULL)
-    if (!strncmp (*ep, name, len) && (*ep)[len] == '=')
-      {
-        /* Found it.  Remove this pointer by moving later ones back.  */
-        char **dp = ep;
-
-        do
-          dp[0] = dp[1];
-        while (*dp++);
-        /* Continue the loop in case NAME appears again.  */
-      }
-    else
-      ++ep;
-
-  UNLOCK;
-
-  return 0;
-#endif
-}
-
-
-/* Put STRING, which is of the form "NAME=VALUE", in the environment.
-   If STRING contains no '=', then remove STRING from the environment.  */
-int
-putenv (char *string)
-{
-  const char *name_end = strchr (string, '=');
-  char **ep;
-
-  if (name_end == NULL)
-    {
-      /* Remove the variable from the environment.  */
-      return _unsetenv (string);
-    }
-
-#if HAVE_DECL__PUTENV
-  /* Rely on _putenv to allocate the new environment.  If other
-     parts of the application use _putenv, the !HAVE_DECL__PUTENV code
-     would fight over who owns the environ vector, causing a crash.  */
-  if (name_end[1])
-    return _putenv (string);
-  else
-    {
-      /* _putenv ("NAME=") unsets NAME, so invoke _putenv ("NAME= ")
-         to allocate the environ vector and then replace the new
-         entry with "NAME=".  */
-      int putenv_result, putenv_errno;
-      char *name_x = malloc (name_end - string + sizeof "= ");
-      if (!name_x)
-        return -1;
-      memcpy (name_x, string, name_end - string + 1);
-      name_x[name_end - string + 1] = ' ';
-      name_x[name_end - string + 2] = 0;
-      putenv_result = _putenv (name_x);
-      putenv_errno = errno;
-      for (ep = environ; *ep; ep++)
-        if (strcmp (*ep, name_x) == 0)
-          {
-            *ep = string;
-            break;
-          }
-# if defined _WIN32 && ! defined __CYGWIN__
-      if (putenv_result == 0)
-        {
-          /* _putenv propagated "NAME= " into the subprocess environment;
-             fix that by calling SetEnvironmentVariable directly.  */
-          name_x[name_end - string] = 0;
-          putenv_result = SetEnvironmentVariable (name_x, "") ? 0 : -1;
-          putenv_errno = ENOMEM; /* ENOMEM is the only way to fail.  */
-        }
-# endif
-      free (name_x);
-      __set_errno (putenv_errno);
-      return putenv_result;
-    }
-#else
-  for (ep = environ; *ep; ep++)
-    if (strncmp (*ep, string, name_end - string) == 0
-        && (*ep)[name_end - string] == '=')
-      break;
-
-  if (*ep)
-    *ep = string;
-  else
-    {
-      static char **last_environ = NULL;
-      size_t size = ep - environ;
-      char **new_environ = malloc ((size + 2) * sizeof *new_environ);
-      if (! new_environ)
-        return -1;
-      new_environ[0] = string;
-      memcpy (new_environ + 1, environ, (size + 1) * sizeof *new_environ);
-      free (last_environ);
-      last_environ = new_environ;
-      environ = new_environ;
-    }
-
-  return 0;
-#endif
-}
diff --git a/m4/gnulib-comp.m4 b/m4/gnulib-comp.m4
index cb3b89dd72..48d8030f53 100644
--- a/m4/gnulib-comp.m4
+++ b/m4/gnulib-comp.m4
@@ -133,7 +133,6 @@ AC_DEFUN
   # Code from module pipe2:
   # Code from module pselect:
   # Code from module pthread_sigmask:
-  # Code from module putenv:
   # Code from module qcopy-acl:
   # Code from module readlink:
   # Code from module readlinkat:
@@ -368,12 +367,6 @@ AC_DEFUN
     gl_PREREQ_PTHREAD_SIGMASK
   fi
   gl_SIGNAL_MODULE_INDICATOR([pthread_sigmask])
-  gl_FUNC_PUTENV
-  if test $REPLACE_PUTENV = 1; then
-    AC_LIBOBJ([putenv])
-    gl_PREREQ_PUTENV
-  fi
-  gl_STDLIB_MODULE_INDICATOR([putenv])
   gl_FUNC_READLINK
   if test $HAVE_READLINK = 0 || test $REPLACE_READLINK = 1; then
     AC_LIBOBJ([readlink])
@@ -973,7 +966,6 @@ AC_DEFUN
   lib/pipe2.c
   lib/pselect.c
   lib/pthread_sigmask.c
-  lib/putenv.c
   lib/qcopy-acl.c
   lib/readlink.c
   lib/readlinkat.c
@@ -1112,7 +1104,6 @@ AC_DEFUN
   m4/pipe2.m4
   m4/pselect.m4
   m4/pthread_sigmask.m4
-  m4/putenv.m4
   m4/readlink.m4
   m4/readlinkat.m4
   m4/regex.m4
diff --git a/m4/putenv.m4 b/m4/putenv.m4
deleted file mode 100644
index e38f8c5694..0000000000
--- a/m4/putenv.m4
+++ /dev/null
@@ -1,60 +0,0 @@
-# putenv.m4 serial 24
-dnl Copyright (C) 2002-2020 Free Software Foundation, Inc.
-dnl This file is free software; the Free Software Foundation
-dnl gives unlimited permission to copy and/or distribute it,
-dnl with or without modifications, as long as this notice is preserved.
-
-dnl From Jim Meyering.
-dnl
-dnl Check whether putenv ("FOO") removes FOO from the environment.
-dnl The putenv in libc on at least SunOS 4.1.4 does *not* do that.
-
-AC_DEFUN([gl_FUNC_PUTENV],
-[
-  AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
-  AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
-  AC_CACHE_CHECK([for putenv compatible with GNU and SVID],
-   [gl_cv_func_svid_putenv],
-   [AC_RUN_IFELSE([AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],[[
-    /* Put it in env.  */
-    if (putenv ("CONFTEST_putenv=val"))
-      return 1;
-
-    /* Try to remove it.  */
-    if (putenv ("CONFTEST_putenv"))
-      return 2;
-
-    /* Make sure it was deleted.  */
-    if (getenv ("CONFTEST_putenv") != 0)
-      return 3;
-
-    return 0;
-              ]])],
-             gl_cv_func_svid_putenv=yes,
-             gl_cv_func_svid_putenv=no,
-             dnl When crosscompiling, assume putenv is broken.
-             [case "$host_os" in
-                               # Guess yes on glibc systems.
-                *-gnu* | gnu*) gl_cv_func_svid_putenv="guessing yes" ;;
-                               # Guess yes on musl systems.
-                *-musl*)       gl_cv_func_svid_putenv="guessing yes" ;;
-                               # Guess no on native Windows.
-                mingw*)        gl_cv_func_svid_putenv="guessing no" ;;
-                               # If we don't know, obey --enable-cross-guesses.
-                *)             gl_cv_func_svid_putenv="$gl_cross_guess_normal" ;;
-              esac
-             ])
-   ])
-  case "$gl_cv_func_svid_putenv" in
-    *yes) ;;
-    *)
-      REPLACE_PUTENV=1
-      ;;
-  esac
-])
-
-# Prerequisites of lib/putenv.c.
-AC_DEFUN([gl_PREREQ_PUTENV],
-[
-  AC_CHECK_DECLS([_putenv])
-])
-- 
2.17.1

bug#19874: simplifying Emacs configure.ac

[Ashish SHUKLA]

On Jan 19, 2020, at 06:09, Paul Eggert <eggert@cs.ucla.edu> wrote:
> 
> On 1/18/20 5:46 AM, Bruno Haible wrote:
> 
>> This code in Emacs configure.ac:
>> # Use the system putenv even if it lacks GNU features, as we don't need them,
>> # and the gnulib replacement runs afoul of a FreeBSD 10.1 bug; see Bug#19874.
>> AC_CHECK_FUNCS_ONCE([putenv])
>> AC_DEFUN([gl_FUNC_PUTENV],
>>   [test "$ac_cv_func_putenv" = yes || REPLACE_PUTENV=1])
>> appears to be extra convoluted. All platforms have the putenv function.
>> Therefore REPLACE_PUTENV=1 is never executed here. If Emacs does not need
>> the putenv override, the simpler way is to invoke gnulib-tool with
>> '--avoid=putenv'.
> 
> Thanks for suggesting that. I installed the attached patch to Emacs master to implement something along the lines you suggested. I am cc'ing Ashish Shukla who reported Bug#19874 ("25.0.50; encode-time not working as expected"), as well as Wolfgang Jenkner who helped debug that, to give them a heads-up that the fix for Bug#19874 has changed.
> <0001-Remove-Gnulib-putenv-code.patch>

Hi

I just tried Emacs (git revision “140eb90bc5”) on FreeBSD 12.1-RELEASE-p1 (amd64) which includes your commit, and it’s working as expected.

Thanks!
--
Ashish | GPG: F682 CDCC 39DC 0FEA E116  20B6 C746 CFA9 E74F A4B0

“Sometimes even to live is an act of courage.” (Seneca)