Skip to content

Commit

Permalink
pyoxidizer: target appropriate Apple SDK when compiling custom config.c
Browse files Browse the repository at this point in the history
Before this commit, our invocation of the cc crate would likely pick up
the default Apple SDK. This would work most of the time. However, during
scenarios like cross-compiling, it may not work. For example, if the
default SDK is a 10.15 SDK and we are compiling for aarch64, which is
only supported by the 11.0 SDK. In this case, you'd get a compiler
error about the supported target not being supported. Even if things
did work, building of the final binary would trigger full Apple SDK
checks and this would reveal an error.

This commit teaches the config.c compiling to use the same logic as
the Rust project / final binary building code for finding and validating
the Apple SDK. It should enable errors to be detected sooner. It
should also hopefully prevent hard-to-debug compiler errors from
occurring, instead presenting an actionable error about the Apple
SDK.
  • Loading branch information
indygreg committed May 7, 2021
1 parent c97fdcf commit 589a25c
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 8 deletions.
11 changes: 11 additions & 0 deletions pyoxidizer/docs/pyoxidizer_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,17 @@ Bug Fixes
properly update the ``PATH`` environment variable if that installation option
is active. This affects PyOxidizer's own MSI installers.

Other Relevant Changes
^^^^^^^^^^^^^^^^^^^^^^

* When building for Apple platforms, we now check for a compatible Apple SDK earlier
during binary building (when compiling a custom ``config.c`` for a custom
``libpython``). This should surface missing dependencies sooner in the build
and potentially replace cryptic compiler error messages with an actionable one
about the Apple SDK. Related to this, we now target a specific Apple SDK when
compiling the aforementioned source file to ensure that the same, validated SDK
is consistently used.

.. _version_0_15_0:

0.15.0
Expand Down
2 changes: 1 addition & 1 deletion pyoxidizer/src/project_building.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ pub fn build_executable_with_rust_project<'a>(

// Derive and write the artifacts needed to build a binary embedding Python.
let embedded_data = exe
.to_embedded_python_context(logger, opt_level)
.to_embedded_python_context(logger, env, opt_level)
.context("obtaining embedded python context")?;
embedded_data
.write_files(&artifacts_path)
Expand Down
2 changes: 2 additions & 0 deletions pyoxidizer/src/py_packaging/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Defining and manipulating binaries embedding Python.

use {
super::{config::PyembedPythonInterpreterConfig, distribution::AppleSdkInfo},
crate::environment::Environment,
anyhow::{anyhow, Context, Result},
python_packaging::{
policy::PythonPackagingPolicy,
Expand Down Expand Up @@ -401,6 +402,7 @@ pub trait PythonBinaryBuilder {
fn to_embedded_python_context(
&self,
logger: &slog::Logger,
env: &Environment,
opt_level: &str,
) -> Result<EmbeddedPythonContext>;
}
Expand Down
23 changes: 22 additions & 1 deletion pyoxidizer/src/py_packaging/libpython.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ Building a native binary containing Python.
*/

use {
anyhow::{anyhow, Result},
crate::{environment::Environment, py_packaging::distribution::AppleSdkInfo},
anyhow::{anyhow, Context, Result},
python_packaging::libpython::LibPythonBuildContext,
slog::warn,
std::{
Expand Down Expand Up @@ -56,13 +57,16 @@ pub struct LibpythonInfo {
/// Create a static libpython from a Python distribution.
///
/// Returns a vector of cargo: lines that can be printed in build scripts.
#[allow(clippy::too_many_arguments)]
pub fn link_libpython(
logger: &slog::Logger,
env: &Environment,
context: &LibPythonBuildContext,
out_dir: &Path,
host_triple: &str,
target_triple: &str,
opt_level: &str,
apple_sdk_info: Option<&AppleSdkInfo>,
) -> Result<LibpythonInfo> {
let mut cargo_metadata: Vec<String> = Vec::new();

Expand Down Expand Up @@ -112,6 +116,23 @@ pub fn link_libpython(
}
}

// The cc crate will pick up the default Apple SDK by default. There could be a mismatch
// between it and what we want. For example, if we're building for aarch64 but the default
// SDK is a 10.15 SDK that doesn't support ARM. We attempt to mitigate this by resolving
// a compatible Apple SDK and pointing the compiler invocation at it via compiler flags.
if target_triple.contains("-apple-") {
let sdk_info = apple_sdk_info.ok_or_else(|| {
anyhow!("Apple SDK info should be defined when targeting Apple platforms")
})?;

let sdk = env
.resolve_apple_sdk(logger, sdk_info)
.context("resolving Apple SDK to use")?;

build.flag("-isysroot");
build.flag(&format!("{}", sdk.path.display()));
}

build
.out_dir(out_dir)
.host(host_triple)
Expand Down
11 changes: 8 additions & 3 deletions pyoxidizer/src/py_packaging/standalone_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use {
},
standalone_distribution::StandaloneDistribution,
},
crate::environment::Environment,
anyhow::{anyhow, Context, Result},
once_cell::sync::Lazy,
python_packaging::{
Expand Down Expand Up @@ -308,6 +309,7 @@ impl StandalonePythonExecutableBuilder {
fn resolve_python_linking_info(
&self,
logger: &slog::Logger,
env: &Environment,
opt_level: &str,
) -> Result<PythonLinkingInfo> {
let libpythonxy_filename;
Expand Down Expand Up @@ -336,11 +338,13 @@ impl StandalonePythonExecutableBuilder {

let library_info = link_libpython(
logger,
env,
&LibPythonBuildContext::merge(&link_contexts),
&temp_dir_path,
&self.host_triple,
&self.target_triple,
opt_level,
self.apple_sdk_info(),
)?;

libpythonxy_filename =
Expand Down Expand Up @@ -878,6 +882,7 @@ impl PythonBinaryBuilder for StandalonePythonExecutableBuilder {
fn to_embedded_python_context(
&self,
logger: &slog::Logger,
env: &Environment,
opt_level: &str,
) -> Result<EmbeddedPythonContext> {
let mut file_seen = false;
Expand Down Expand Up @@ -985,7 +990,7 @@ impl PythonBinaryBuilder for StandalonePythonExecutableBuilder {
}
}

let linking_info = self.resolve_python_linking_info(logger, opt_level)?;
let linking_info = self.resolve_python_linking_info(logger, env, opt_level)?;

if self.link_mode == LibpythonLinkMode::Dynamic {
if let Some(p) = &self.target_distribution.libpython_shared_library {
Expand Down Expand Up @@ -1307,7 +1312,7 @@ pub mod tests {
let logger = get_logger()?;
let options = StandalonePythonExecutableBuilderOptions::default();
let exe = options.new_builder()?;
let embedded = exe.to_embedded_python_context(&logger, "0")?;
let embedded = exe.to_embedded_python_context(&logger, &get_env()?, "0")?;

let temp_dir = tempfile::Builder::new()
.prefix("pyoxidizer-test")
Expand All @@ -1329,7 +1334,7 @@ pub mod tests {
exe.resources_load_mode =
PackedResourcesLoadMode::BinaryRelativePathMemoryMapped("resources".into());

let embedded = exe.to_embedded_python_context(&logger, "0")?;
let embedded = exe.to_embedded_python_context(&logger, &get_env()?, "0")?;

assert_eq!(
&embedded.config.packed_resources,
Expand Down
8 changes: 5 additions & 3 deletions pyoxidizer/src/starlark/python_embedded_resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,11 @@ impl PythonEmbeddedResourcesValue {
output_path.display()
);

let embedded = self
.exe
.to_embedded_python_context(context.logger(), &context.build_opt_level)?;
let embedded = self.exe.to_embedded_python_context(
context.logger(),
context.env(),
&context.build_opt_level,
)?;

std::fs::create_dir_all(&output_path)
.with_context(|| format!("creating output directory: {}", output_path.display()))?;
Expand Down

0 comments on commit 589a25c

Please sign in to comment.