From d80ca3506f9fa457baec70ab0ee38c93ec4e99a5 Mon Sep 17 00:00:00 2001 From: Gregory Szorc Date: Sun, 11 Jun 2023 17:26:50 -0700 Subject: [PATCH] unix: build _crypt extension module as shared Modern Linux distributions are starting to remove `libcrypt.so.1` from the base disto. See #173 and #113 before it for more context. The `_crypt` extension module depends on `libcrypt.so.1` and our static linking of this extension is causing the full Python distribution to depend on `libcrypt.so.1`, causing our binaries to not load/run on these distributions. This commit adds support for building extension modules as shared libraries (the way CPython does things by default). We update our YAML config to build `_crypt` as a shared library. --- cpython-unix/build-cpython.sh | 10 ++- cpython-unix/build.py | 5 ++ cpython-unix/extension-modules.yml | 1 + .../patch-makesetup-deduplicate-objs.patch | 14 ++++ ...nsion-module-shared-libraries-legacy.patch | 25 ------- ...ve-extension-module-shared-libraries.patch | 24 ------- pythonbuild/cpython.py | 16 ++++- src/validation.rs | 65 ++++++++++++++++--- 8 files changed, 96 insertions(+), 64 deletions(-) create mode 100644 cpython-unix/patch-makesetup-deduplicate-objs.patch delete mode 100644 cpython-unix/patch-remove-extension-module-shared-libraries-legacy.patch delete mode 100644 cpython-unix/patch-remove-extension-module-shared-libraries.patch diff --git a/cpython-unix/build-cpython.sh b/cpython-unix/build-cpython.sh index d7d5d034..c43a2875 100755 --- a/cpython-unix/build-cpython.sh +++ b/cpython-unix/build-cpython.sh @@ -174,12 +174,10 @@ fi # invoke the host Python on our own. patch -p1 -i ${ROOT}/patch-write-python-for-build.patch -# We build all extensions statically. So remove the auto-generated make -# rules that produce shared libraries for them. -if [ -n "${PYTHON_MEETS_MINIMUM_VERSION_3_11}" ]; then - patch -p1 -i ${ROOT}/patch-remove-extension-module-shared-libraries.patch -else - patch -p1 -i ${ROOT}/patch-remove-extension-module-shared-libraries-legacy.patch +# Object files can get listed multiple times leading to duplicate symbols +# when linking. Prevent this. +if [ -n "${PYTHON_MEETS_MAXIMUM_VERSION_3_10}" ]; then + patch -p1 -i ${ROOT}/patch-makesetup-deduplicate-objs.patch fi # The default build rule for the macOS dylib doesn't pick up libraries diff --git a/cpython-unix/build.py b/cpython-unix/build.py index f111aaeb..5b1411d6 100755 --- a/cpython-unix/build.py +++ b/cpython-unix/build.py @@ -582,6 +582,11 @@ def python_build_info( "variant": d["variant"], } + if info.get("build-mode") == "shared": + shared_dir = extra_metadata["python_config_vars"]["DESTSHARED"].strip("/") + extension_suffix = extra_metadata["python_config_vars"]["EXT_SUFFIX"] + entry["shared_lib"] = "%s/%s%s" % (shared_dir, extension, extension_suffix) + add_licenses_to_extension_entry(entry) bi["extensions"].setdefault(extension, []).append(entry) diff --git a/cpython-unix/extension-modules.yml b/cpython-unix/extension-modules.yml index 07ebaa82..d59bc360 100644 --- a/cpython-unix/extension-modules.yml +++ b/cpython-unix/extension-modules.yml @@ -67,6 +67,7 @@ _contextvars: - _contextvarsmodule.c _crypt: + build-mode: shared sources: - _cryptmodule.c links-conditional: diff --git a/cpython-unix/patch-makesetup-deduplicate-objs.patch b/cpython-unix/patch-makesetup-deduplicate-objs.patch new file mode 100644 index 00000000..95706aea --- /dev/null +++ b/cpython-unix/patch-makesetup-deduplicate-objs.patch @@ -0,0 +1,14 @@ +diff --git a/Modules/makesetup b/Modules/makesetup +index 1a767838c9..9f6d2f4396 100755 +--- a/Modules/makesetup ++++ b/Modules/makesetup +@@ -253,6 +253,9 @@ sed -e 's/[ ]*#.*//' -e '/^[ ]*$/d' | + done + done + ++ # Deduplicate OBJS. ++ OBJS=$(echo $OBJS | tr ' ' '\n' | sort -u | xargs) ++ + case $SHAREDMODS in + '') ;; + *) DEFS="SHAREDMODS=$SHAREDMODS$NL$DEFS";; diff --git a/cpython-unix/patch-remove-extension-module-shared-libraries-legacy.patch b/cpython-unix/patch-remove-extension-module-shared-libraries-legacy.patch deleted file mode 100644 index e7330d7b..00000000 --- a/cpython-unix/patch-remove-extension-module-shared-libraries-legacy.patch +++ /dev/null @@ -1,25 +0,0 @@ -diff --git a/Modules/makesetup b/Modules/makesetup ---- a/Modules/makesetup -+++ b/Modules/makesetup -@@ -241,18 +241,11 @@ sed -e 's/[ ]*#.*//' -e '/^[ ]*$/d' | - case $doconfig in - yes) OBJS="$OBJS $objs";; - esac -- for mod in $mods -- do -- file="$srcdir/$mod\$(EXT_SUFFIX)" -- case $doconfig in -- no) SHAREDMODS="$SHAREDMODS $file";; -- esac -- rule="$file: $objs" -- rule="$rule; \$(BLDSHARED) $objs $libs $ExtraLibs -o $file" -- echo "$rule" >>$rulesf -- done - done - -+ # Deduplicate OBJS. -+ OBJS=$(echo $OBJS | tr ' ' '\n' | sort -u | xargs) -+ - case $SHAREDMODS in - '') ;; - *) DEFS="SHAREDMODS=$SHAREDMODS$NL$DEFS";; diff --git a/cpython-unix/patch-remove-extension-module-shared-libraries.patch b/cpython-unix/patch-remove-extension-module-shared-libraries.patch deleted file mode 100644 index f4749fc4..00000000 --- a/cpython-unix/patch-remove-extension-module-shared-libraries.patch +++ /dev/null @@ -1,24 +0,0 @@ -diff --git a/Modules/makesetup b/Modules/makesetup -index 08303814c8..f24b380b85 100755 ---- a/Modules/makesetup -+++ b/Modules/makesetup -@@ -273,19 +273,6 @@ sed -e 's/[ ]*#.*//' -e '/^[ ]*$/d' | - case $doconfig in - yes) OBJS="$OBJS $objs";; - esac -- for mod in $mods -- do -- file="$srcdir/$mod\$(EXT_SUFFIX)" -- case $doconfig in -- no) -- SHAREDMODS="$SHAREDMODS $file" -- BUILT_SHARED="$BUILT_SHARED $mod" -- ;; -- esac -- rule="$file: $objs" -- rule="$rule; \$(BLDSHARED) $objs $libs $ExtraLibs -o $file" -- echo "$rule" >>$rulesf -- done - done - - case $SHAREDMODS in diff --git a/pythonbuild/cpython.py b/pythonbuild/cpython.py index 88dce468..ad21fb91 100644 --- a/pythonbuild/cpython.py +++ b/pythonbuild/cpython.py @@ -14,6 +14,7 @@ EXTENSION_MODULE_SCHEMA = { "type": "object", "properties": { + "build-mode": {"type": "string"}, "config-c-only": {"type": "boolean"}, "defines": {"type": "array", "items": {"type": "string"}}, "defines-conditional": { @@ -228,6 +229,9 @@ def derive_setup_local( python_version, info.get("maximum-python-version", "100.0") ) + if info.get("build-mode") not in (None, "shared", "static"): + raise Exception("unsupported build-mode for extension module %s" % name) + if not (python_min_match and python_max_match): log(f"ignoring extension module {name} because Python version incompatible") ignored.add(name) @@ -387,6 +391,7 @@ def derive_setup_local( section_lines = { "disabled": [], + "shared": [], "static": [], } @@ -427,7 +432,13 @@ def derive_setup_local( enabled_extensions[name]["setup_line"] = name.encode("ascii") continue - section = "static" + # musl is static only. Ignore build-mode override. + if "musl" in target_triple: + section = "static" + else: + section = info.get("build-mode", "static") + + enabled_extensions[name]["build-mode"] = section # Presumably this means the extension comes from the distribution's # Setup. Lack of sources means we don't need to derive a Setup.local @@ -549,6 +560,9 @@ def derive_setup_local( dest_lines = [] for section, lines in sorted(section_lines.items()): + if not lines: + continue + dest_lines.append(b"\n*%s*\n" % section.encode("ascii")) dest_lines.extend(lines) diff --git a/src/validation.rs b/src/validation.rs index ba0f1ebe..23bb74c3 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -707,6 +707,9 @@ const GLOBAL_EXTENSIONS_WINDOWS: &[&str] = &[ /// Extension modules not present in Windows static builds. const GLOBAL_EXTENSIONS_WINDOWS_NO_STATIC: &[&str] = &["_testinternalcapi", "_tkinter"]; +/// Extension modules that should be built as shared libraries. +const SHARED_LIBRARY_EXTENSIONS: &[&str] = &["_crypt"]; + const PYTHON_VERIFICATIONS: &str = include_str!("verify_distribution.py"); fn allowed_dylibs_for_triple(triple: &str) -> Vec { @@ -1742,13 +1745,49 @@ fn validate_distribution( } } - // Validate extension module initialization functions are present. - // - // Note that we export PyInit_* functions from libpython on POSIX whereas these - // aren't exported from official Python builds. We may want to consider changing - // this. + // Validate extension module metadata. for (name, variants) in json.as_ref().unwrap().build_info.extensions.iter() { for ext in variants { + if let Some(shared) = &ext.shared_lib { + if !seen_paths.contains(&PathBuf::from("python").join(shared)) { + context.errors.push(format!( + "extension module {} references missing shared library path {}", + name, shared + )); + } + } + + // Static builds never have shared library extension modules. + let want_shared = if is_static { + false + // Extension modules in libpython core are never shared libraries. + } else if ext.in_core { + false + // All remaining extensions are shared on Windows. + } else if triple.contains("windows") { + true + // On POSIX platforms we maintain a list. + } else { + SHARED_LIBRARY_EXTENSIONS.contains(&name.as_str()) + }; + + if want_shared && ext.shared_lib.is_none() { + context.errors.push(format!( + "extension module {} does not have a shared library", + name + )); + } else if !want_shared && ext.shared_lib.is_some() { + context.errors.push(format!( + "extension module {} contains a shared library unexpectedly", + name + )); + } + + // Ensure initialization functions are exported. + + // Note that we export PyInit_* functions from libpython on POSIX whereas these + // aren't exported from official Python builds. We may want to consider changing + // this. if ext.init_fn == "NULL" { continue; } @@ -1756,10 +1795,20 @@ fn validate_distribution( let exported = context.libpython_exported_symbols.contains(&ext.init_fn); // Static distributions never export symbols. + let wanted = if is_static { + false + // For some strange reason _PyWarnings_Init is exported as part of the ABI. + } else if name == "_warnings" { + true // Windows dynamic doesn't export extension module init functions. - // And for some strange reason _PyWarnings_Init is exported as part of the ABI. - let wanted = - !(is_static || triple.contains("-windows-")) || (!is_static && name == "_warnings"); + } else if triple.contains("-windows-") { + false + // Presence of a shared library extension implies no export. + } else if ext.shared_lib.is_some() { + false + } else { + true + }; if exported != wanted { context.errors.push(format!(