Skip to content

Commit

Permalink
unix: build _crypt extension module as shared
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
indygreg committed Jul 19, 2023
1 parent 31e1732 commit 031d8a1
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 66 deletions.
8 changes: 0 additions & 8 deletions cpython-unix/build-cpython.sh
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,6 @@ 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
fi

# The default build rule for the macOS dylib doesn't pick up libraries
# from modules / makesetup. So patch it accordingly.
patch -p1 -i ${ROOT}/patch-macos-link-extension-modules.patch
Expand Down
5 changes: 5 additions & 0 deletions cpython-unix/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions cpython-unix/extension-modules.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ _contextvars:
- _contextvarsmodule.c

_crypt:
build-mode: shared
sources:
- _cryptmodule.c
links-conditional:
Expand Down

This file was deleted.

24 changes: 0 additions & 24 deletions cpython-unix/patch-remove-extension-module-shared-libraries.patch

This file was deleted.

16 changes: 15 additions & 1 deletion pythonbuild/cpython.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -387,6 +391,7 @@ def derive_setup_local(

section_lines = {
"disabled": [],
"shared": [],
"static": [],
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
65 changes: 57 additions & 8 deletions src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<MachOAllowedDylib> {
Expand Down Expand Up @@ -1742,24 +1745,70 @@ 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;
}

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!(
Expand Down

0 comments on commit 031d8a1

Please sign in to comment.