-
Notifications
You must be signed in to change notification settings - Fork 979
Modify the handing of Frameworks on Apple platforms #5606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
cf315b3
9f0aac5
5fc325b
3820fa3
ca120b7
55a1d95
4ed808d
4a77d15
4bc523d
20d4cb3
f2260c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| If the `sysconfigdata` defines `PYTHONFRAMEWORK`, and the build is configured to link the against Python library, the link will be performed against that framework, using `PYTHONFRAMEWORKPREFIX` as a framework search path. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,6 +98,11 @@ pub struct InterpreterConfig { | |
| /// Serialized to `version`. | ||
| pub version: PythonVersion, | ||
|
|
||
| /// The name of the Python framework (if available) | ||
| /// | ||
| /// Serialized to `framework`. | ||
| pub framework: Option<String>, | ||
|
|
||
| /// Whether link library is shared. | ||
| /// | ||
| /// Serialized to `shared`. | ||
|
|
@@ -242,6 +247,10 @@ def print_if_set(varname, value): | |
| if value is not None: | ||
| print(varname, value) | ||
|
|
||
| def print_if_not_empty(varname, value): | ||
| if value: | ||
| print(varname, value) | ||
|
|
||
| # Windows always uses shared linking | ||
| WINDOWS = platform.system() == "Windows" | ||
|
|
||
|
|
@@ -255,6 +264,7 @@ SHARED = bool(get_config_var("Py_ENABLE_SHARED")) | |
| print("implementation", platform.python_implementation()) | ||
| print("version_major", sys.version_info[0]) | ||
| print("version_minor", sys.version_info[1]) | ||
| print_if_not_empty("framework", get_config_var("PYTHONFRAMEWORK")) | ||
| print("shared", PYPY or GRAALPY or ANACONDA or WINDOWS or FRAMEWORK or SHARED) | ||
| print("python_framework_prefix", FRAMEWORK_PREFIX) | ||
| print_if_set("ld_version", get_config_var("LDVERSION")) | ||
|
|
@@ -293,6 +303,7 @@ print("gil_disabled", get_config_var("Py_GIL_DISABLED")) | |
| ); | ||
| }; | ||
|
|
||
| let framework = map.get("framework").cloned(); | ||
| let shared = map["shared"].as_str() == "True"; | ||
| let python_framework_prefix = map.get("python_framework_prefix").cloned(); | ||
|
|
||
|
|
@@ -360,6 +371,7 @@ print("gil_disabled", get_config_var("Py_GIL_DISABLED")) | |
| Ok(InterpreterConfig { | ||
| version, | ||
| implementation, | ||
| framework, | ||
| shared, | ||
| abi3, | ||
| lib_name: Some(lib_name), | ||
|
|
@@ -403,10 +415,10 @@ print("gil_disabled", get_config_var("Py_GIL_DISABLED")) | |
| _ => bail!("expected a bool (1/true/True or 0/false/False) for Py_ENABLE_SHARED"), | ||
| }; | ||
| // macOS framework packages use shared linking (PYTHONFRAMEWORK is the framework name, hence the empty check) | ||
| let framework = match sysconfigdata.get_value("PYTHONFRAMEWORK") { | ||
| Some(s) => !s.is_empty(), | ||
| _ => false, | ||
| }; | ||
| let framework = get_key!(sysconfigdata, "PYTHONFRAMEWORK") | ||
| .ok() | ||
| .filter(|s| !s.is_empty()) | ||
| .map(str::to_string); | ||
| let python_framework_prefix = sysconfigdata | ||
| .get_value("PYTHONFRAMEWORKPREFIX") | ||
| .map(str::to_string); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should do something similar to filter out cases where the prefix is an empty string? Maybe if that is semantically no different from
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'd lean to filtering out empty PYTHONFRAMEWORKPREFIX examples; I can't speak with much Rust experience, but as a Python user, an explicit "None" makes more sense to me than ignoring an empty string. I'll push an update to that effect. |
||
|
|
@@ -429,11 +441,13 @@ print("gil_disabled", get_config_var("Py_GIL_DISABLED")) | |
| .map(|bytes_width: u32| bytes_width * 8) | ||
| .ok(); | ||
| let build_flags = BuildFlags::from_sysconfigdata(sysconfigdata); | ||
| let shared = shared || framework.is_some(); | ||
|
|
||
| Ok(InterpreterConfig { | ||
| implementation, | ||
| version, | ||
| shared: shared || framework, | ||
| framework, | ||
| shared, | ||
| abi3, | ||
| lib_dir, | ||
| lib_name, | ||
|
|
@@ -510,6 +524,7 @@ print("gil_disabled", get_config_var("Py_GIL_DISABLED")) | |
|
|
||
| let mut implementation = None; | ||
| let mut version = None; | ||
| let mut framework = None; | ||
| let mut shared = None; | ||
| let mut abi3 = None; | ||
| let mut lib_name = None; | ||
|
|
@@ -535,6 +550,7 @@ print("gil_disabled", get_config_var("Py_GIL_DISABLED")) | |
| match key { | ||
| "implementation" => parse_value!(implementation, value), | ||
| "version" => parse_value!(version, value), | ||
| "framework" => parse_value!(framework, value), | ||
| "shared" => parse_value!(shared, value), | ||
| "abi3" => parse_value!(abi3, value), | ||
| "lib_name" => parse_value!(lib_name, value), | ||
|
|
@@ -561,6 +577,7 @@ print("gil_disabled", get_config_var("Py_GIL_DISABLED")) | |
| Ok(InterpreterConfig { | ||
| implementation, | ||
| version, | ||
| framework, | ||
| shared: shared.unwrap_or(true), | ||
| abi3, | ||
| lib_name, | ||
|
|
@@ -674,6 +691,7 @@ print("gil_disabled", get_config_var("Py_GIL_DISABLED")) | |
|
|
||
| write_line!(implementation)?; | ||
| write_line!(version)?; | ||
| write_option_line!(framework)?; | ||
| write_line!(shared)?; | ||
| write_line!(abi3)?; | ||
| write_option_line!(lib_name)?; | ||
|
|
@@ -1622,6 +1640,7 @@ fn default_cross_compile(cross_compile_config: &CrossCompileConfig) -> Result<In | |
| Ok(InterpreterConfig { | ||
| implementation, | ||
| version, | ||
| framework: None, | ||
| shared: true, | ||
| abi3, | ||
| lib_name: Some(lib_name), | ||
|
|
@@ -1665,6 +1684,7 @@ fn default_abi3_config(host: &Triple, version: PythonVersion) -> Result<Interpre | |
| Ok(InterpreterConfig { | ||
| implementation, | ||
| version, | ||
| framework: None, | ||
| shared: true, | ||
| abi3, | ||
| lib_name, | ||
|
|
@@ -2055,6 +2075,7 @@ mod tests { | |
| implementation: PythonImplementation::CPython, | ||
| lib_name: Some("lib_name".into()), | ||
| lib_dir: Some("lib_dir".into()), | ||
| framework: Some("Python".into()), | ||
| shared: true, | ||
| version: MINIMUM_SUPPORTED_VERSION, | ||
| suppress_build_script_link_lines: true, | ||
|
|
@@ -2081,6 +2102,7 @@ mod tests { | |
| implementation: PythonImplementation::PyPy, | ||
| lib_dir: None, | ||
| lib_name: None, | ||
| framework: None, | ||
| shared: true, | ||
| version: PythonVersion { | ||
| major: 3, | ||
|
|
@@ -2106,6 +2128,7 @@ mod tests { | |
| implementation: PythonImplementation::CPython, | ||
| lib_name: Some("lib_name".into()), | ||
| lib_dir: Some("lib_dir\\n".into()), | ||
| framework: None, | ||
| shared: true, | ||
| version: MINIMUM_SUPPORTED_VERSION, | ||
| suppress_build_script_link_lines: true, | ||
|
|
@@ -2128,6 +2151,7 @@ mod tests { | |
| InterpreterConfig { | ||
| version: PythonVersion { major: 3, minor: 7 }, | ||
| implementation: PythonImplementation::CPython, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: false, | ||
| lib_name: None, | ||
|
|
@@ -2151,6 +2175,7 @@ mod tests { | |
| InterpreterConfig { | ||
| version: PythonVersion { major: 3, minor: 7 }, | ||
| implementation: PythonImplementation::CPython, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: false, | ||
| lib_name: None, | ||
|
|
@@ -2259,6 +2284,7 @@ mod tests { | |
| implementation: PythonImplementation::CPython, | ||
| lib_dir: Some("/usr/lib".into()), | ||
| lib_name: Some("python3.7m".into()), | ||
| framework: None, | ||
| shared: true, | ||
| version: PythonVersion::PY37, | ||
| suppress_build_script_link_lines: false, | ||
|
|
@@ -2289,6 +2315,7 @@ mod tests { | |
| implementation: PythonImplementation::CPython, | ||
| lib_dir: Some("/usr/lib".into()), | ||
| lib_name: Some("python3.7m".into()), | ||
| framework: Some("Python".into()), | ||
| shared: true, | ||
| version: PythonVersion::PY37, | ||
| suppress_build_script_link_lines: false, | ||
|
|
@@ -2316,6 +2343,7 @@ mod tests { | |
| implementation: PythonImplementation::CPython, | ||
| lib_dir: Some("/usr/lib".into()), | ||
| lib_name: Some("python3.7m".into()), | ||
| framework: None, | ||
| shared: false, | ||
| version: PythonVersion::PY37, | ||
| suppress_build_script_link_lines: false, | ||
|
|
@@ -2335,6 +2363,7 @@ mod tests { | |
| InterpreterConfig { | ||
| implementation: PythonImplementation::CPython, | ||
| version: PythonVersion { major: 3, minor: 7 }, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: true, | ||
| lib_name: Some("python3".into()), | ||
|
|
@@ -2359,6 +2388,7 @@ mod tests { | |
| InterpreterConfig { | ||
| implementation: PythonImplementation::CPython, | ||
| version: PythonVersion { major: 3, minor: 9 }, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: true, | ||
| lib_name: None, | ||
|
|
@@ -2394,6 +2424,7 @@ mod tests { | |
| InterpreterConfig { | ||
| implementation: PythonImplementation::CPython, | ||
| version: PythonVersion { major: 3, minor: 7 }, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: false, | ||
| lib_name: Some("python37".into()), | ||
|
|
@@ -2429,6 +2460,7 @@ mod tests { | |
| InterpreterConfig { | ||
| implementation: PythonImplementation::CPython, | ||
| version: PythonVersion { major: 3, minor: 8 }, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: false, | ||
| lib_name: Some("python38".into()), | ||
|
|
@@ -2464,6 +2496,7 @@ mod tests { | |
| InterpreterConfig { | ||
| implementation: PythonImplementation::CPython, | ||
| version: PythonVersion { major: 3, minor: 9 }, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: false, | ||
| lib_name: Some("python3.9".into()), | ||
|
|
@@ -2501,6 +2534,7 @@ mod tests { | |
| major: 3, | ||
| minor: 11 | ||
| }, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: false, | ||
| lib_name: Some("pypy3.11-c".into()), | ||
|
|
@@ -2894,6 +2928,7 @@ mod tests { | |
| implementation: PythonImplementation::CPython, | ||
| lib_dir: None, | ||
| lib_name: None, | ||
| framework: None, | ||
| shared: true, | ||
| version: PythonVersion { major: 3, minor: 7 }, | ||
| suppress_build_script_link_lines: false, | ||
|
|
@@ -2917,6 +2952,7 @@ mod tests { | |
| implementation: PythonImplementation::CPython, | ||
| lib_dir: None, | ||
| lib_name: None, | ||
| framework: None, | ||
| shared: true, | ||
| version: PythonVersion { major: 3, minor: 7 }, | ||
| suppress_build_script_link_lines: false, | ||
|
|
@@ -2982,6 +3018,7 @@ mod tests { | |
| implementation: PythonImplementation::CPython, | ||
| lib_dir: interpreter_config.lib_dir.to_owned(), | ||
| lib_name: interpreter_config.lib_name.to_owned(), | ||
| framework: None, | ||
| shared: true, | ||
| version: interpreter_config.version, | ||
| suppress_build_script_link_lines: false, | ||
|
|
@@ -3112,6 +3149,7 @@ mod tests { | |
| major: 3, | ||
| minor: 11, | ||
| }, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: false, | ||
| lib_name: Some("python3".into()), | ||
|
|
@@ -3156,6 +3194,7 @@ mod tests { | |
| let interpreter_config = InterpreterConfig { | ||
| implementation: PythonImplementation::CPython, | ||
| version: PythonVersion { major: 3, minor: 9 }, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: true, | ||
| lib_name: Some("python3".into()), | ||
|
|
@@ -3204,6 +3243,7 @@ mod tests { | |
| major: 3, | ||
| minor: 13, | ||
| }, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: false, | ||
| lib_name: Some("python3".into()), | ||
|
|
@@ -3238,6 +3278,7 @@ mod tests { | |
| let interpreter_config = InterpreterConfig { | ||
| implementation: PythonImplementation::CPython, | ||
| version: PythonVersion { major: 3, minor: 7 }, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: false, | ||
| lib_name: Some("python3".into()), | ||
|
|
@@ -3293,6 +3334,7 @@ mod tests { | |
| let mut config = InterpreterConfig { | ||
| implementation: PythonImplementation::CPython, | ||
| version: PythonVersion { major: 3, minor: 9 }, | ||
| framework: None, | ||
| shared: true, | ||
| abi3: false, | ||
| lib_name: None, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -150,7 +150,9 @@ fn emit_link_config(build_config: &BuildConfig) -> Result<()> { | |
|
|
||
| println!( | ||
| "cargo:rustc-link-lib={link_model}{alias}{lib_name}", | ||
| link_model = if interpreter_config.shared { | ||
| link_model = if interpreter_config.framework.is_some() { | ||
| "framework=" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the 3.11 CI failure has found a problem with this approach. Seems like Not sure what implications this has for the iOS build either? Seems like it's fundamentally impossible to use frameworks and link the right version. I wonder, maybe we don't need this PR? If cross-compile env has the correct
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Which failure is this? I'm seeing 11 skips, 32 successful tests in the most recent CI pass.
So - macOS frameworks and iOS frameworks (and every other non-macOS framework) are structured slightly differently. macOS frameworks are versioned - there's a single top-level framework, with a "Versions" subfolder; each version of the framework is then contained in the top level framework. A
So... that doesn't currently work, because iOS doesn't ship a copy of
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think I attempted this yesterday and had no luck. Possibly because
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added CI-build-full label to this PR so any further pushes will run the full suite of versions we run in the merge queue. |
||
| } else if interpreter_config.shared { | ||
| "" | ||
| } else { | ||
| "static=" | ||
|
|
@@ -160,11 +162,20 @@ fn emit_link_config(build_config: &BuildConfig) -> Result<()> { | |
| } else { | ||
| "" | ||
| }, | ||
| lib_name = interpreter_config.lib_name.as_ref().ok_or( | ||
| "attempted to link to Python shared library but config does not contain lib_name" | ||
| )?, | ||
| lib_name = if let Some(framework) = &interpreter_config.framework { | ||
| framework | ||
| } else { | ||
| interpreter_config.lib_name.as_ref().ok_or( | ||
| "attempted to link to Python shared library but config does not contain lib_name", | ||
| )? | ||
| }, | ||
| ); | ||
|
|
||
| if interpreter_config.framework.is_some() { | ||
| if let Some(framework_prefix) = &interpreter_config.python_framework_prefix { | ||
| println!("cargo:rustc-link-search=framework={framework_prefix}"); | ||
| } | ||
| } | ||
| if let Some(lib_dir) = &interpreter_config.lib_dir { | ||
| println!("cargo:rustc-link-search=native={lib_dir}"); | ||
| } else if matches!(build_config.source, BuildConfigSource::CrossCompile) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should note that if this is set, this will be used for linking instead of
sharedandlib_name.