unix: switch Linux 3.10 builds from readline to libedit

indygreg · indygreg · commit 1148eb932709 · 2021-09-27T20:06:08.000-07:00
CPython 3.10 adds mostly working support for libedit outside of
macOS. I think it is good enough to use by default.

This commit rewrites the build logic to use libedit and only libedit
on CPython 3.10+.
diff --git a/cpython-unix/build-cpython.sh b/cpython-unix/build-cpython.sh
@@ -493,24 +493,22 @@ diff --git a/Lib/ctypes/__init__.py b/Lib/ctypes/__init__.py
  if _os.name == "nt":
 EOF
 
-# libedit on non-macOS requires various hacks because readline.c assumes
-# libedit is only used on macOS and its readline/libedit detection code
-# makes various assumptions about the macOS environment.
+# CPython 3.10 added proper support for building against libedit outside of
+# macOS. On older versions, we need to patch readline.c and distribute
+# multiple extension module variants.
 #
 # USE_LIBEDIT comes from our static-modules file.
-#
-# TODO make upstream patches to readline.c to properly support libedit
-# on other platforms.
-cp Modules/readline.c Modules/readline-libedit.c
+if [[ "${PYTHON_MAJMIN_VERSION}" = "3.8" || "${PYTHON_MAJMIN_VERSION}" = "3.9" ]]; then
+    cp Modules/readline.c Modules/readline-libedit.c
 
-# readline.c assumes that a modern readline API version has a free_history_entry().
-# but libedit does not. Change the #ifdef accordingly.
-#
-# Similarly, we invoke configure using readline, which sets
-# HAVE_RL_COMPLETION_SUPPRESS_APPEND improperly. So hack that. This is a bug
-# in our build system, as we should probably be invoking configure again when
-# using libedit.
-patch -p1 << EOF
+    # readline.c assumes that a modern readline API version has a free_history_entry().
+    # but libedit does not. Change the #ifdef accordingly.
+    #
+    # Similarly, we invoke configure using readline, which sets
+    # HAVE_RL_COMPLETION_SUPPRESS_APPEND improperly. So hack that. This is a bug
+    # in our build system, as we should probably be invoking configure again when
+    # using libedit.
+    patch -p1 << EOF
 diff --git a/Modules/readline-libedit.c b/Modules/readline-libedit.c
 index 1e74f997b0..56a36e26e6 100644
 --- a/Modules/readline-libedit.c
@@ -543,6 +541,7 @@ index 1e74f997b0..56a36e26e6 100644
  #endif
                  rl_cleanup_after_signal();
 EOF
+fi
 
 # iOS doesn't have system(). Teach posixmodule.c about that.
 if [ "${PYTHON_MAJMIN_VERSION}" != "3.8" ]; then
@@ -572,6 +571,7 @@ fi
 # So we need to set both.
 CFLAGS="${EXTRA_TARGET_CFLAGS} -fPIC -I${TOOLS_PATH}/deps/include -I${TOOLS_PATH}/deps/include/ncursesw"
 LDFLAGS="${EXTRA_TARGET_LDFLAGS} -L${TOOLS_PATH}/deps/lib"
+EXTRA_CONFIGURE_FLAGS=
 
 if [ "${PYBUILD_PLATFORM}" = "macos" ]; then
     CFLAGS="${CFLAGS} -I${TOOLS_PATH}/deps/include/uuid"
@@ -580,14 +580,27 @@ if [ "${PYBUILD_PLATFORM}" = "macos" ]; then
     CFLAGS="${CFLAGS} -Werror=unguarded-availability-new"
 fi
 
+# CPython 3.10 introduced proper support for libedit on all platforms. Link against
+# libedit by default because it isn't GPL.
+#
+# Ideally we wouldn't need to adjust global compiler and linker flags. But configure
+# performs detection of readline features and sets some preprocessor defines accordingly.
+# So we define these accordingly.
+if [[ "${PYBUILD_PLATFORM}" != "macos" && "${PYTHON_MAJMIN_VERSION}" != "3.8" && "${PYTHON_MAJMIN_VERSION}" != "3.9" ]]; then
+    CFLAGS="${CFLAGS} -I${TOOLS_PATH}/deps/libedit/include"
+    LDFLAGS="${LDFLAGS} -L${TOOLS_PATH}/deps/libedit/lib"
+    EXTRA_CONFIGURE_FLAGS="${EXTRA_CONFIGURE_FLAGS} --with-readline=editline"
+fi
+
 CPPFLAGS=$CFLAGS
 
 CONFIGURE_FLAGS="
     --build=${BUILD_TRIPLE} \
     --host=${TARGET_TRIPLE}
     --prefix=/install
     --with-openssl=${TOOLS_PATH}/deps
-    --without-ensurepip"
+    --without-ensurepip
+    ${EXTRA_CONFIGURE_FLAGS}"
 
 if [ "${CC}" = "musl-clang" ]; then
     CFLAGS="${CFLAGS} -static"
diff --git a/cpython-unix/build.py b/cpython-unix/build.py
@@ -754,7 +754,7 @@ def build_cpython(
                 musl="musl" in target_triple,
             )
 
-        packages = target_needs(TARGETS_CONFIG, target_triple)
+        packages = target_needs(TARGETS_CONFIG, target_triple, python_version)
         # Toolchain packages are handled specially.
         packages.discard("binutils")
         packages.discard("clang")
diff --git a/cpython-unix/static-modules.3.10.aarch64-unknown-linux-gnu b/cpython-unix/static-modules.3.10.aarch64-unknown-linux-gnu
@@ -35,9 +35,4 @@ _xxsubinterpreters _xxsubinterpretersmodule.c
 _xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c
 ossaudiodev ossaudiodev.c
 pyexpat pyexpat.c expat/xmlparse.c expat/xmlrole.c expat/xmltok.c -DHAVE_EXPAT_CONFIG_H=1 -DXML_POOR_ENTROPY=1 -DUSE_PYEXPAT_CAPI -IModules/expat
-# readline variant needs to come first because libreadline is in /tools/deps and is
-# picked up by build. We /could/ make libedit first. But since we employ a hack to
-# coerce use of libedit on Linux, it seems prudent for the build system to pick
-# up readline.
-readline VARIANT=readline readline.c -I/tools/deps/include -I/tools/deps/include/ncursesw -L/tools/deps/lib -lreadline -lncursesw
-readline VARIANT=libedit readline-libedit.c -DUSE_LIBEDIT=1 -I/tools/deps/libedit/include -I/tools/deps/libedit/include/ncursesw -L/tools/deps/libedit/lib -ledit -lncursesw
+readline readline.c -ledit -lncursesw
diff --git a/cpython-unix/static-modules.3.10.armv7-unknown-linux-gnueabi b/cpython-unix/static-modules.3.10.armv7-unknown-linux-gnueabi
@@ -35,9 +35,4 @@ _xxsubinterpreters _xxsubinterpretersmodule.c
 _xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c
 ossaudiodev ossaudiodev.c
 pyexpat pyexpat.c expat/xmlparse.c expat/xmlrole.c expat/xmltok.c -DHAVE_EXPAT_CONFIG_H=1 -DXML_POOR_ENTROPY=1 -DUSE_PYEXPAT_CAPI -IModules/expat
-# readline variant needs to come first because libreadline is in /tools/deps and is
-# picked up by build. We /could/ make libedit first. But since we employ a hack to
-# coerce use of libedit on Linux, it seems prudent for the build system to pick
-# up readline.
-readline VARIANT=readline readline.c -I/tools/deps/include -I/tools/deps/include/ncursesw -L/tools/deps/lib -lreadline -lncursesw
-readline VARIANT=libedit readline-libedit.c -DUSE_LIBEDIT=1 -I/tools/deps/libedit/include -I/tools/deps/libedit/include/ncursesw -L/tools/deps/libedit/lib -ledit -lncursesw
+readline readline.c -ledit -lncursesw
diff --git a/cpython-unix/static-modules.3.10.armv7-unknown-linux-gnueabihf b/cpython-unix/static-modules.3.10.armv7-unknown-linux-gnueabihf
@@ -35,9 +35,4 @@ _xxsubinterpreters _xxsubinterpretersmodule.c
 _xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c
 ossaudiodev ossaudiodev.c
 pyexpat pyexpat.c expat/xmlparse.c expat/xmlrole.c expat/xmltok.c -DHAVE_EXPAT_CONFIG_H=1 -DXML_POOR_ENTROPY=1 -DUSE_PYEXPAT_CAPI -IModules/expat
-# readline variant needs to come first because libreadline is in /tools/deps and is
-# picked up by build. We /could/ make libedit first. But since we employ a hack to
-# coerce use of libedit on Linux, it seems prudent for the build system to pick
-# up readline.
-readline VARIANT=readline readline.c -I/tools/deps/include -I/tools/deps/include/ncursesw -L/tools/deps/lib -lreadline -lncursesw
-readline VARIANT=libedit readline-libedit.c -DUSE_LIBEDIT=1 -I/tools/deps/libedit/include -I/tools/deps/libedit/include/ncursesw -L/tools/deps/libedit/lib -ledit -lncursesw
+readline readline.c -ledit -lncursesw
diff --git a/cpython-unix/static-modules.3.10.i686-unknown-linux-gnu b/cpython-unix/static-modules.3.10.i686-unknown-linux-gnu
@@ -34,9 +34,4 @@ _xxsubinterpreters _xxsubinterpretersmodule.c
 _xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c
 ossaudiodev ossaudiodev.c
 pyexpat pyexpat.c expat/xmlparse.c expat/xmlrole.c expat/xmltok.c -DHAVE_EXPAT_CONFIG_H=1 -DXML_POOR_ENTROPY=1 -DUSE_PYEXPAT_CAPI -IModules/expat
-# readline variant needs to come first because libreadline is in /tools/deps and is
-# picked up by build. We /could/ make libedit first. But since we employ a hack to
-# coerce use of libedit on Linux, it seems prudent for the build system to pick
-# up readline.
-readline VARIANT=readline readline.c -I/tools/deps/include -I/tools/deps/include/ncursesw -L/tools/deps/lib -lreadline -lncursesw
-readline VARIANT=libedit readline-libedit.c -DUSE_LIBEDIT=1 -I/tools/deps/libedit/include -I/tools/deps/libedit/include/ncursesw -L/tools/deps/libedit/lib -ledit -lncursesw
+readline readline.c -ledit -lncursesw
diff --git a/cpython-unix/static-modules.3.10.linux64 b/cpython-unix/static-modules.3.10.linux64
@@ -35,9 +35,4 @@ _xxsubinterpreters _xxsubinterpretersmodule.c
 _xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c
 ossaudiodev ossaudiodev.c
 pyexpat pyexpat.c expat/xmlparse.c expat/xmlrole.c expat/xmltok.c -DHAVE_EXPAT_CONFIG_H=1 -DXML_POOR_ENTROPY=1 -DUSE_PYEXPAT_CAPI -IModules/expat
-# readline variant needs to come first because libreadline is in /tools/deps and is
-# picked up by build. We /could/ make libedit first. But since we employ a hack to
-# coerce use of libedit on Linux, it seems prudent for the build system to pick
-# up readline.
-readline VARIANT=readline readline.c -I/tools/deps/include -I/tools/deps/include/ncursesw -L/tools/deps/lib -lreadline -lncursesw
-readline VARIANT=libedit readline-libedit.c -DUSE_LIBEDIT=1 -I/tools/deps/libedit/include -I/tools/deps/libedit/include/ncursesw -L/tools/deps/libedit/lib -ledit -lncursesw
+readline readline.c -ledit -lncursesw
diff --git a/cpython-unix/static-modules.3.10.mips-unknown-linux-gnu b/cpython-unix/static-modules.3.10.mips-unknown-linux-gnu
@@ -35,9 +35,4 @@ _xxsubinterpreters _xxsubinterpretersmodule.c
 _xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c
 ossaudiodev ossaudiodev.c
 pyexpat pyexpat.c expat/xmlparse.c expat/xmlrole.c expat/xmltok.c -DHAVE_EXPAT_CONFIG_H=1 -DXML_POOR_ENTROPY=1 -DUSE_PYEXPAT_CAPI -IModules/expat
-# readline variant needs to come first because libreadline is in /tools/deps and is
-# picked up by build. We /could/ make libedit first. But since we employ a hack to
-# coerce use of libedit on Linux, it seems prudent for the build system to pick
-# up readline.
-readline VARIANT=readline readline.c -I/tools/deps/include -I/tools/deps/include/ncursesw -L/tools/deps/lib -lreadline -lncursesw
-readline VARIANT=libedit readline-libedit.c -DUSE_LIBEDIT=1 -I/tools/deps/libedit/include -I/tools/deps/libedit/include/ncursesw -L/tools/deps/libedit/lib -ledit -lncursesw
+readline readline.c -ledit -lncursesw
diff --git a/cpython-unix/static-modules.3.10.mipsel-unknown-linux-gnu b/cpython-unix/static-modules.3.10.mipsel-unknown-linux-gnu
@@ -35,9 +35,4 @@ _xxsubinterpreters _xxsubinterpretersmodule.c
 _xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c
 ossaudiodev ossaudiodev.c
 pyexpat pyexpat.c expat/xmlparse.c expat/xmlrole.c expat/xmltok.c -DHAVE_EXPAT_CONFIG_H=1 -DXML_POOR_ENTROPY=1 -DUSE_PYEXPAT_CAPI -IModules/expat
-# readline variant needs to come first because libreadline is in /tools/deps and is
-# picked up by build. We /could/ make libedit first. But since we employ a hack to
-# coerce use of libedit on Linux, it seems prudent for the build system to pick
-# up readline.
-readline VARIANT=readline readline.c -I/tools/deps/include -I/tools/deps/include/ncursesw -L/tools/deps/lib -lreadline -lncursesw
-readline VARIANT=libedit readline-libedit.c -DUSE_LIBEDIT=1 -I/tools/deps/libedit/include -I/tools/deps/libedit/include/ncursesw -L/tools/deps/libedit/lib -ledit -lncursesw
+readline readline.c -ledit -lncursesw
diff --git a/cpython-unix/static-modules.3.10.s390x-unknown-linux-gnu b/cpython-unix/static-modules.3.10.s390x-unknown-linux-gnu
@@ -35,9 +35,4 @@ _xxsubinterpreters _xxsubinterpretersmodule.c
 _xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c
 ossaudiodev ossaudiodev.c
 pyexpat pyexpat.c expat/xmlparse.c expat/xmlrole.c expat/xmltok.c -DHAVE_EXPAT_CONFIG_H=1 -DXML_POOR_ENTROPY=1 -DUSE_PYEXPAT_CAPI -IModules/expat
-# readline variant needs to come first because libreadline is in /tools/deps and is
-# picked up by build. We /could/ make libedit first. But since we employ a hack to
-# coerce use of libedit on Linux, it seems prudent for the build system to pick
-# up readline.
-readline VARIANT=readline readline.c -I/tools/deps/include -I/tools/deps/include/ncursesw -L/tools/deps/lib -lreadline -lncursesw
-readline VARIANT=libedit readline-libedit.c -DUSE_LIBEDIT=1 -I/tools/deps/libedit/include -I/tools/deps/libedit/include/ncursesw -L/tools/deps/libedit/lib -ledit -lncursesw
+readline readline.c -ledit -lncursesw
diff --git a/docs/quirks.rst b/docs/quirks.rst
@@ -208,3 +208,19 @@ build/version of Clang is supported. Use at your own risk.
 
 See https://jonnyzzz.com/blog/2018/06/05/link-error-2/ and
 https://jonnyzzz.com/blog/2018/06/13/link-error-3/ for more on this topic.
+
+.. _quirk_linux_libedit:
+
+Use of ``libedit`` on Linux
+===========================
+
+Python 3.10+ Linux distributions link against ``libedit`` (as opposed to
+``readline``) by default, as ``libedit`` is supported on 3.10+ outside of
+macOS.
+
+Most Python builds on Linux will link against ``readline`` because ``readline``
+is the dominant library on Linux.
+
+Some functionality may behave subtly differently as a result of our choice
+to link ``libedit`` by default. (We choose ``libedit`` by default to
+avoid GPL licensing requirements of ``readline``.)
diff --git a/docs/technotes.rst b/docs/technotes.rst
@@ -71,16 +71,19 @@ distribute because end-users often want software to respect their
 terminal databases. But for that to work, ``ncurses`` needs to be compiled
 in a way that respects the user's environment.
 
-On macOS, we statically link a ``libedit`` we compile ourselves. We
-dynamically link against ``libncurses``, which is provided by the
-system, typically in ``/usr/lib``.
-
-On Linux, we produce ``readline`` extension module variants compiled
-against both ``libreadline`` and ``libedit``, which are statically linked
-against libraries built ourselves. These libraries each statically link
-against a ``libncurses`` built ourselves. The ``readline`` extension
-module variant is the default, as Python compiles against ``readline``
-by default.
+On macOS, we use the system ``libedit`` and ``libncurses``, which is
+typically provided in ``/usr/lib``.
+
+On Linux, Python 3.10+ distributions have a ``readline`` extension
+module compiled and statically linked against ``libedit`` and
+``libncurses``, both of which we be build ourselves.
+
+On Linux, older Python versions produce ``readline`` extension module
+variants compiled against both ``libreadline`` and ``libedit``, which
+are statically linked against libraries built ourselves. These libraries
+each statically link against a ``libncurses`` built ourselves. The
+``readline`` extension module variant is the default, as Python compiles
+against ``readline`` by default.
 
 gettext / locale Module
 -----------------------
diff --git a/pythonbuild/utils.py b/pythonbuild/utils.py
@@ -49,11 +49,18 @@ def supported_targets(yaml_path: pathlib.Path):
     return targets
 
 
-def target_needs(yaml_path: pathlib.Path, target: str):
+def target_needs(yaml_path: pathlib.Path, target: str, python_version: str):
     """Obtain the dependencies needed to build the specified target."""
     settings = get_targets(yaml_path)[target]
 
-    return set(settings["needs"])
+    needs = set(settings["needs"])
+
+    # We only ship libedit linked readline extension on 3.10+ to avoid a GPL
+    # dependency.
+    if not python_version.startswith(("3.8", "3.9")):
+        needs.discard("readline")
+
+    return needs
 
 
 def release_tag_from_git():

Original file line number	Diff line number	Diff line change
`@@ -754,7 +754,7 @@ def build_cpython(`
`754`	`754`	`musl="musl" in target_triple,`
`755`	`755`	`)`
`756`	`756`
`757`		`- packages = target_needs(TARGETS_CONFIG, target_triple)`
	`757`	`+ packages = target_needs(TARGETS_CONFIG, target_triple, python_version)`
`758`	`758`	`# Toolchain packages are handled specially.`
`759`	`759`	`packages.discard("binutils")`
`760`	`760`	`packages.discard("clang")`