linux: compile readline extension against both libreadline and libedit

Now that we have support for variants in the PYTHON.json file,
we can start to put it to use.

This commit teaches the Linux build system to build the
readline extension with both libreadline and libedit.

In order to accomplish this, we hacked up static-modules to
support custom syntax which is translated to custom build
rules. We then taught the distribution "parsing" code to
handle extension variants.

This is all a bit hacky. But it gets the job done.

As part of this, Python's configure and the installable
distribution now revert back to readline instead of libedit.
While there are GPLv3 issues, I think this is acceptable. We
have to pick 1 and readline is the only supported "backend"
on Linux. So I think readline makes sense over libedit.

Author: indygreg

cpython-linux/build-cpython.sh

@@ -28,13 +28,15 @@ mv Setup.local Python-${PYTHON_VERSION}/Modules/Setup.local
 
 pushd Python-${PYTHON_VERSION}
 
+cp Modules/readline.c Modules/readline-libedit.c
+
 # Python supports using libedit instead of readline. But Modules/readline.c
 # has all of this behind ``#ifdef __APPLE__`` instead of a more specific
 # feature flag. All occurrences of __APPLE__ in that file are related to
 # libedit. So we just replace the content. USE_LIBEDIT comes from our
 # static-modules file.
 # TODO make changes upstream to allow libedit to more easily be used
-sed -i s/__APPLE__/USE_LIBEDIT/g Modules/readline.c
+sed -i s/__APPLE__/USE_LIBEDIT/g Modules/readline-libedit.c
 
 # Most bits look at CFLAGS. But setup.py only looks at CPPFLAGS.
 # So we need to set both.
@@ -71,10 +73,14 @@ for d in Modules Objects Parser Programs Python; do
     cp -av $d/*.o /build/out/python/build/$d/
 done
 
+# Also copy extension variant metadata files.
+cp -av Modules/VARIANT-*.data /build/out/python/build/Modules/
+
 # The object files need to be linked against library dependencies. So copy
 # library files as well.
 mkdir /build/out/python/build/lib
 cp -av /tools/deps/lib/*.a /build/out/python/build/lib/
+cp -av /tools/deps/libedit/lib/*.a /build/out/python/build/lib/
 
 # config.c defines _PyImport_Inittab and extern references to modules, which
 # downstream consumers may want to strip. We bundle config.c and config.c.in so

cpython-linux/build-libedit.sh

@@ -17,16 +17,17 @@ pushd libedit-${LIBEDIT_VERSION}
 
 cflags="${EXTRA_TARGET_CFLAGS} -fPIC -I/tools/deps/include -I/tools/deps/include/ncurses"
 
+# Install to /tools/deps/libedit so it doesn't conflict with readline's files.
 CLFAGS="${cflags}" CPPFLAGS="${cflags}" LDFLAGS="-L/tools/deps/lib" \
     ./configure \
         --build=x86_64-unknown-linux-gnu \
         --host=${TARGET} \
-        --prefix=/tools/deps \
+        --prefix=/tools/deps/libedit \
 
 make -j `nproc`
 make -j `nproc` install DESTDIR=/build/out
 
 # Alias readline/{history.h, readline.h} for readline compatibility.
-mkdir /build/out/tools/deps/include/readline
-ln -s ../editline/readline.h /build/out/tools/deps/include/readline/readline.h
-ln -s ../editline/readline.h /build/out/tools/deps/include/readline/history.h
+mkdir /build/out/tools/deps/libedit/include/readline
+ln -s ../editline/readline.h /build/out/tools/deps/libedit/include/readline/readline.h
+ln -s ../editline/readline.h /build/out/tools/deps/libedit/include/readline/history.h

cpython-linux/build.py

@@ -443,14 +443,14 @@ def python_build_info(container, config_c_in, setup_dist, setup_local):
 
     # Extension data is derived by "parsing" the Setup.dist and Setup.local files.
 
-    def process_setup_line(line):
-        d = parse_setup_line(line)
+    def process_setup_line(line, variant=None):
+        d = parse_setup_line(line, variant)
 
         if not d:
             return
 
         extension = d['extension']
-        log('processing extension %s' % extension)
+        log('processing extension %s (variant %s)' % (extension, d['variant']))
 
         objs = []
 
@@ -481,13 +481,13 @@ def process_setup_line(line):
                     'system': True,
                 })
 
-        bi['extensions'][extension] = [{
+        bi['extensions'].setdefault(extension, []).append({
             'in_core': False,
             'init_fn': 'PyInit_%s' % extension,
             'links': links,
             'objs': objs,
-            'variant': 'default',
-        }]
+            'variant': d['variant'],
+        })
 
 
     found_start = False
@@ -511,19 +511,37 @@ def process_setup_line(line):
 
         process_setup_line(line)
 
+    # Extension variants are denoted by the presence of
+    # Modules/VARIANT-<extension>-<variant>.data files that describe the
+    # extension. Find those files and process them.
+    data, stat = container.get_archive('/build/out/python/build/Modules')
+    data = io.BytesIO(b''.join(data))
+
+    tf = tarfile.open(fileobj=data)
+
+    for ti in tf:
+        basename = os.path.basename(ti.name)
+
+        if not basename.startswith('VARIANT-') or not basename.endswith('.data'):
+            continue
+
+        variant = basename[:-5].split('-')[2]
+        line = tf.extractfile(ti).read().strip()
+        process_setup_line(line, variant=variant)
+
     # There are also a setup of built-in extensions defined in config.c.in which
     # aren't built using the Setup.* files and are part of the core libpython
     # distribution. Define extensions entries for these so downstream consumers
     # can register their PyInit_ functions.
     for name, init_fn in sorted(config_c_in.items()):
         log('adding in-core extension %s' % name)
-        bi['extensions'][name] = [{
+        bi['extensions'].setdefault(name, []).append({
             'in_core': True,
             'init_fn': init_fn,
             'links': [],
             'objs': [],
             'variant': 'default',
-        }]
+        })
 
     # Any paths left in modules_objs are not part of any extension and are
     # instead part of the core distribution.
@@ -553,11 +571,11 @@ def build_cpython(client, image, platform, optimized=False):
         # TODO support bdb/gdbm toggle
         install_tools_archive(container, BUILD / ('bdb-%s.tar' % platform))
         install_tools_archive(container, BUILD / ('bzip2-%s.tar' % platform))
-        # TODO support libedit/libreadline toggle
         install_tools_archive(container, BUILD / ('libedit-%s.tar' % platform))
         install_tools_archive(container, BUILD / ('libffi-%s.tar' % platform))
         install_tools_archive(container, BUILD / ('ncurses-%s.tar' % platform))
         install_tools_archive(container, BUILD / ('openssl-%s.tar' % platform))
+        install_tools_archive(container, BUILD / ('readline-%s.tar' % platform))
         install_tools_archive(container, BUILD / ('sqlite-%s.tar' % platform))
         # tk requires a bunch of X11 stuff.
         #install_tools_archive(container, BUILD / ('tcltk-%s.tar' % platform))

cpython-linux/static-modules

@@ -32,5 +32,10 @@ _uuid _uuidmodule.c -I/tools/deps/include/uuid -luuid
 _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 readline.c -DUSE_LIBEDIT=1 -I/tools/deps/include -I/tools/deps/include/ncurses -L/tools/deps/lib -ledit -lncurses
-xxlimited xxlimited.c -DPy_LIMITED_API=0x03050000
+# 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/ncurses -L/tools/deps/lib -lreadline -lncurses
+readline VARIANT=libedit readline-libedit.c -DUSE_LIBEDIT=1 -I/tools/deps/libedit/include -I/tools/deps/libedit/include/ncurses -L/tools/deps/libedit/lib -ledit -lncurses
+xxlimited xxlimited.c -DPy_LIMITED_API=0x03050000

pythonbuild/cpython.py

@@ -67,7 +67,7 @@
 }
 
 
-def parse_setup_line(line: bytes):
+def parse_setup_line(line: bytes, variant: str):
     """Parse a line in a ``Setup.*`` file."""
     if b'#' in line:
         line = line[:line.index(b'#')].rstrip()
@@ -89,7 +89,13 @@ def parse_setup_line(line: bytes):
             # Object files are named according to the basename: parent
             # directories they may happen to reside in are stripped out.
             source_path = pathlib.Path(word.decode('ascii'))
-            obj_path = pathlib.Path('Modules/%s' % source_path.with_suffix('.o').name)
+
+            if variant:
+                obj_path = pathlib.Path('Modules/VARIANT-%s-%s-%s' % (
+                    extension, variant, source_path.with_suffix('.o').name))
+            else:
+                obj_path = pathlib.Path('Modules/%s' % source_path.with_suffix('.o').name)
+
             objs.add(obj_path)
 
         # Arguments looking like link libraries are converted to library
@@ -105,6 +111,7 @@ def parse_setup_line(line: bytes):
         'posix_obj_paths': objs,
         'links': links,
         'frameworks': frameworks,
+        'variant': variant or 'default',
     }
 
 
@@ -160,8 +167,82 @@ def derive_setup_local(static_modules_lines, cpython_source_archive, disabled=No
             dest_lines.append(line)
 
     RE_DEFINE = re.compile(b'-D[^=]+=[^\s]+')
+    RE_VARIANT = re.compile(b'VARIANT=([^\s]+)\s')
+
+    seen_variants = set()
 
     for line in static_modules_lines:
+        # We supplement the format to support declaring extension variants.
+        # A variant results in multiple compiles of a given extension.
+        # However, the CPython build system doesn't take kindly to this because
+        # a) we can only have a single extension with a given name
+        # b) it assumes the init function matches the extension name
+        # c) attempting to link multiple variants into the same binary can often
+        #    result in duplicate symbols when variants use different libraries
+        #    implementing the same API.
+        #
+        # When we encounter a variant, we only pass the 1st variant through to
+        # Setup.local. We then supplement the Makefile with rules to build
+        # remaining variants.
+        m = RE_VARIANT.search(line)
+        if m:
+            line = RE_VARIANT.sub(b'', line)
+            variant = m.group(1)
+            extension = line.split()[0]
+
+            cflags = []
+            ldflags = []
+
+            for w in line.split()[1:]:
+                if w.startswith((b'-I', b'-D')):
+                    cflags.append(w)
+                elif w.startswith((b'-L', b'-l')):
+                    ldflags.append(w)
+                elif w.endswith(b'.c'):
+                    pass
+                else:
+                    raise ValueError('unexpected content in Setup variant line: %s' % w)
+
+            if extension in seen_variants:
+                sources = [w for w in line.split() if w.endswith(b'.c')]
+                object_files = []
+                for source in sources:
+                    basename = os.path.basename(source)[:-2]
+
+                    # Use a unique name to ensure there aren't collisions
+                    # across extension variants.
+                    object_file = b'Modules/VARIANT-%s-%s-%s.o' % (
+                        extension, variant, basename)
+                    object_files.append(object_file)
+                    make_lines.append(
+                        b'%s: $(srcdir)/Modules/%s; '
+                        b'$(CC) $(PY_BUILTIN_MODULE_CFLAGS) '
+                        b'%s -c $(srcdir)/Modules/%s -o %s' % (
+                            object_file, source, b' '.join(cflags),
+                            source, object_file))
+
+                extension_target = b'Modules/%s-VARIANT-%s$(EXT_SUFFIX)' % (
+                    extension, variant)
+
+                make_lines.append(b'%s: %s' % (
+                    extension_target, b' '.join(object_files)))
+                make_lines.append(
+                    b'\t$(BLDSHARED) %s %s -o Modules/%s-VARIANT-%s$(EXT_SUFFIX)' %
+                    (b' '.join(object_files), b' '.join(ldflags),
+                     extension, variant))
+                make_lines.append(
+                    b'\techo "%s" > Modules/VARIANT-%s-%s.data' % (
+                        line, extension, variant))
+
+                # Add dependencies to $(LIBRARY) target to force compilation of
+                # extension variant.
+                make_lines.append(b'$(LIBRARY): %s' % extension_target)
+
+                continue
+
+            else:
+                seen_variants.add(extension)
+
         # makesetup parses lines with = as extra config options. There appears
         # to be no easy way to define e.g. -Dfoo=bar in Setup.local. We hack
         # around this by detecting the syntax we'd like to support and move the