Failing configure tests due to missing space

[allsey87]

Bug report

Bug description:

When building CPython from source, I noticed some suspicious errors that seem to be related to a missing spaces in the configure script. It seems that most uses of as_fn_append correctly include a leading space before appending to a variable, however, there are several cases where this space has not been added which can lead to two arguments being concatenated:

• https://github.com/python/cpython/blob/main/configure#L9617
• https://github.com/python/cpython/blob/main/configure#L9735
• https://github.com/python/cpython/blob/main/configure#L9780
• https://github.com/python/cpython/blob/main/configure#L9821
• https://github.com/python/cpython/blob/main/configure#L9862
• https://github.com/python/cpython/blob/main/configure#L9903
• https://github.com/python/cpython/blob/main/configure#L9944
• https://github.com/python/cpython/blob/main/configure#L9996

When compiling to Emscripten in Bazel, for example, I end up with -sRELOCATABLE=1-Wstrict-prototypes in one of my tests:

configure:9880: checking if we can enable /home/developer/.cache/bazel/_bazel_developer/25e07d78077dfe1eca932359d50e41ef/sandbox/processwrapper-sandbox/29/execroot/_main/external/emsdk/emscripten_toolchain/emcc.sh strict-prototypes warning
configure:9900: /home/developer/.cache/bazel/_bazel_developer/25e07d78077dfe1eca932359d50e41ef/sandbox/processwrapper-sandbox/29/execroot/_main/external/emsdk/emscripten_toolchain/emcc.sh -c --sysroot=/home/developer/.cache/bazel/_bazel_developer/25e07d78077dfe1eca932359d50e41ef/sandbox/processwrapper-sandbox/29/execroot/_main/external/emscripten_bin_linux/emscripten/cache/sysroot -fdiagnostics-color -fno-strict-aliasing -funsigned-char -no-canonical-prefixes -Wall -iwithsysroot/include/c++/v1 -iwithsysroot/include/compat -iwithsysroot/include -isystem /home/developer/.cache/bazel/_bazel_developer/25e07d78077dfe1eca932359d50e41ef/sandbox/processwrapper-sandbox/29/execroot/_main/external/emscripten_bin_linux/lib/clang/19/include -Wno-builtin-macro-redefined -D__DATE__=redacted -D__TIMESTAMP__=redacted -D__TIME__=redacted -O2 -g0 -fwasm-exceptions -pthread -sRELOCATABLE=1-Wstrict-prototypes -Werror -I/home/developer/.cache/bazel/_bazel_developer/25e07d78077dfe1eca932359d50e41ef/sandbox/processwrapper-sandbox/29/execroot/_main/bazel-out/k8-fastbuild/bin/python/libpython.ext_build_deps/libffi/include conftest.c >&5
emcc: error: setting `RELOCATABLE` expects `bool` but got `str`

CPython versions tested on:

3.12

Operating systems tested on:

Other

Linked PRs

• gh-120671: Fix as_fn_append CFLAGS #120822
• [3.13] gh-120671: Fix PY_CHECK_CC_WARNING() in configure.ac (GH-120822) #120985
• [3.12] gh-120671: Fix PY_CHECK_CC_WARNING() in configure.ac (GH-120822) #120986

[allsey87]

Actually this issue is all caused by a single space missing in the configure.ac file. I propose the following:

diff --git a/configure b/configure
index 99dd1fe595..3f043be4f2 100755
--- a/configure
+++ b/configure
@@ -9506,7 +9506,7 @@ then :
 else $as_nop
 
     py_cflags=$CFLAGS
-    as_fn_append CFLAGS "-Wextra -Werror"
+    as_fn_append CFLAGS " -Wextra -Werror"
     cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
@@ -9624,7 +9624,7 @@ then :
 else $as_nop
 
     py_cflags=$CFLAGS
-    as_fn_append CFLAGS "-Wunused-result -Werror"
+    as_fn_append CFLAGS " -Wunused-result -Werror"
     cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
@@ -9669,7 +9669,7 @@ then :
 else $as_nop
 
     py_cflags=$CFLAGS
-    as_fn_append CFLAGS "-Wunused-parameter -Werror"
+    as_fn_append CFLAGS " -Wunused-parameter -Werror"
     cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
@@ -9710,7 +9710,7 @@ then :
 else $as_nop
 
     py_cflags=$CFLAGS
-    as_fn_append CFLAGS "-Wint-conversion -Werror"
+    as_fn_append CFLAGS " -Wint-conversion -Werror"
     cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
@@ -9751,7 +9751,7 @@ then :
 else $as_nop
 
     py_cflags=$CFLAGS
-    as_fn_append CFLAGS "-Wmissing-field-initializers -Werror"
+    as_fn_append CFLAGS " -Wmissing-field-initializers -Werror"
     cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
@@ -9792,7 +9792,7 @@ then :
 else $as_nop
 
     py_cflags=$CFLAGS
-    as_fn_append CFLAGS "-Wsign-compare -Werror"
+    as_fn_append CFLAGS " -Wsign-compare -Werror"
     cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
@@ -9833,7 +9833,7 @@ then :
 else $as_nop
 
     py_cflags=$CFLAGS
-    as_fn_append CFLAGS "-Wunreachable-code -Werror"
+    as_fn_append CFLAGS " -Wunreachable-code -Werror"
     cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
@@ -9885,7 +9885,7 @@ then :
 else $as_nop
 
     py_cflags=$CFLAGS
-    as_fn_append CFLAGS "-Wstrict-prototypes -Werror"
+    as_fn_append CFLAGS " -Wstrict-prototypes -Werror"
     cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
diff --git a/configure.ac b/configure.ac
index bd2be94b47..61255c2acd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2363,7 +2363,7 @@ AC_DEFUN([PY_CHECK_CC_WARNING], [
   AS_VAR_PUSHDEF([py_var], [ac_cv_$1_]m4_normalize($2)[_warning])
   AC_CACHE_CHECK([m4_ifblank([$3], [if we can $1 $CC $2 warning], [$3])], [py_var], [
     AS_VAR_COPY([py_cflags], [CFLAGS])
-    AS_VAR_APPEND([CFLAGS], ["-W$2 -Werror"])
+    AS_VAR_APPEND([CFLAGS], [" -W$2 -Werror"])
     AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
                       [AS_VAR_SET([py_var], [yes])],
                       [AS_VAR_SET([py_var], [no])])

[zware]

Would you like to submit a pull request to that effect?

[allsey87]

Will do!

[eli-schwartz]

In commit 76d14fa (GH-29485), some code was refactored to be more compact and use more advanced autoconf tricks. In the process, it switched

AS_VAR_SET([XXX], ["$XXX other values"])

to

AS_VAR_APPEND([XXX], ["other values"])

This was a functional logic break, since it elided the space in between the existing and appended values -- and that space is mandatory for command-line flags stuffed into a variable.

The same problem occurred for LDFLAGS, but was silently fixed as a side effect of commit bb8b931 (GH-32229).

[allsey87]

@eli-schwartz should there also be a space at configure.ac#L7548?

[vstinner]

@eli-schwartz should there also be a space at configure.ac#L7548?

I think that this line is fine since added line ends with a newline.

[vstinner]

Fixed by 2106c9b. Thanks @allsey87!