Skip to content
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

Zig for manylinux compliance without docker #756

Merged
merged 8 commits into from
Dec 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
with:
python-version: "3.10.0"
- name: Install cffi and virtualenv
run: pip install cffi virtualenv
run: pip install cffi virtualenv ziglang~=0.9.0
- uses: actions-rs/toolchain@v1
id: rustup
with:
Expand Down Expand Up @@ -106,6 +106,14 @@ jobs:
pypy3 -m pip install --force-reinstall --no-index --find-links test-crates/pyo3-pure/target/wheels pyo3-pure
pypy3 -m pip install pytest
pypy3 -m pytest test-crates/pyo3-pure/test_pyo3_pure.py
- uses: actions/setup-python@v2
with:
python-version: "3.10.0"
- name: test cross compiling with zig
if: matrix.os != 'windows-latest'
run: |
rustup target add aarch64-unknown-linux-gnu
cargo run -- build --no-sdist -i python -m test-crates/pyo3-pure/Cargo.toml --target aarch64-unknown-linux-gnu --zig

test-auditwheel:
name: Test Auditwheel
Expand Down
5 changes: 5 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ classifiers = [
]
dependencies = ["toml~=0.10.2"]

[project.optional-dependencies]
zig = [
"ziglang~=0.9.0",
]

[project.urls]
"Source Code" = "https://github.com/PyO3/maturin"
Issues = "https://github.com/PyO3/maturin/issues"
Expand Down
3 changes: 2 additions & 1 deletion src/auditwheel/audit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,8 @@ pub fn auditwheel_rs(
policy.fixup_musl_libc_so_name(target.target_arch());

if let Some(highest_policy) = highest_policy {
if policy.priority < highest_policy.priority {
// Don't recommend manylinux1 because rust doesn't support it anymore
if policy.priority < highest_policy.priority && highest_policy.name != "manylinux_2_5" {
println!(
"πŸ“¦ Wheel is eligible for a higher priority tag. \
You requested {} but this wheel is eligible for {}",
Expand Down
2 changes: 1 addition & 1 deletion src/auditwheel/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl Display for Policy {
f.write_str(&self.name)
} else {
f.write_fmt(format_args!(
"{}(aka {})",
"{} (aka {})",
&self.name,
self.aliases.join(",")
))
Expand Down
4 changes: 3 additions & 1 deletion src/build_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,10 @@ pub struct BuildContext {
pub release: bool,
/// Strip the library for minimum file size
pub strip: bool,
/// Whether to skip checking the linked libraries for manylinux/musllinux compliance
/// Skip checking the linked libraries for manylinux/musllinux compliance
pub skip_auditwheel: bool,
/// When compiling for manylinux, use zig as linker to ensure glibc version compliance
pub zig: bool,
/// Whether to use the the manylinux/musllinux or use the native linux tag (off)
pub platform_tag: Option<PlatformTag>,
/// Extra arguments that will be passed to cargo as `cargo rustc [...] [arg1] [arg2] --`
Expand Down
34 changes: 27 additions & 7 deletions src/build_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ pub struct BuildOptions {
/// Don't check for manylinux compliance
#[structopt(long = "skip-auditwheel")]
pub skip_auditwheel: bool,
/// For manylinux targets, use zig to ensure compliance for the chosen manylinux version
///
/// Default to manylinux2010/manylinux_2_12 if you do not specify an `--compatibility`
///
/// Make sure you installed zig with `pip install maturin[zig]`
#[structopt(long)]
pub zig: bool,
/// The --target option for cargo
#[structopt(long, name = "TRIPLE", env = "CARGO_BUILD_TARGET")]
pub target: Option<String>,
Expand Down Expand Up @@ -96,6 +103,7 @@ impl Default for BuildOptions {
manifest_path: PathBuf::from("Cargo.toml"),
out: None,
skip_auditwheel: false,
zig: false,
target: None,
cargo_extra_args: Vec::new(),
rustc_extra_args: Vec::new(),
Expand Down Expand Up @@ -271,14 +279,25 @@ impl BuildOptions {
let strip = pyproject.map(|x| x.strip()).unwrap_or_default() || strip;
let skip_auditwheel =
pyproject.map(|x| x.skip_auditwheel()).unwrap_or_default() || self.skip_auditwheel;
let platform_tag = self.platform_tag.or_else(|| {
pyproject.and_then(|x| {
if x.compatibility().is_some() {
args_from_pyproject.push("compatibility");
}
x.compatibility()
let platform_tag = self
.platform_tag
.or_else(|| {
pyproject.and_then(|x| {
if x.compatibility().is_some() {
args_from_pyproject.push("compatibility");
}
x.compatibility()
})
})
});
.or(
// With zig we can compile to any glibc version that we want, so we pick the lowest
// one supported by the rust compiler
if self.zig && !target.is_musl_target() {
Some(target.get_minimum_manylinux_tag())
} else {
None
},
);
if platform_tag == Some(PlatformTag::manylinux1()) {
eprintln!("⚠️ Warning: manylinux1 is unsupported by the Rust compiler.");
}
Expand All @@ -302,6 +321,7 @@ impl BuildOptions {
release,
strip,
skip_auditwheel,
zig: self.zig,
platform_tag,
cargo_extra_args,
rustc_extra_args,
Expand Down
113 changes: 110 additions & 3 deletions src/compile.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
use crate::build_context::BridgeModel;
use crate::python_interpreter::InterpreterKind;
use crate::BuildContext;
use crate::PythonInterpreter;
use crate::target::Arch;
use crate::{BuildContext, PlatformTag, PythonInterpreter};
use anyhow::{anyhow, bail, Context, Result};
use fat_macho::FatWriter;
use fs_err::{self as fs, File};
use std::collections::HashMap;
use std::env;
use std::io::{BufReader, Read};
#[cfg(target_family = "unix")]
use std::fs::OpenOptions;
use std::io::{BufReader, Read, Write};
#[cfg(target_family = "unix")]
use std::os::unix::fs::OpenOptionsExt;
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};
use std::str;
Expand Down Expand Up @@ -101,11 +105,100 @@ fn compile_universal2(
Ok(result)
}

/// We want to use `zig cc` as linker and c compiler. We want to call `python -m ziglang cc`, but
/// cargo only accepts a path to an executable as linker, so we add a wrapper script. We then also
/// use the wrapper script to pass arguments and substitute an unsupported argument.
///
/// We create different files for different args because otherwise cargo might skip recompiling even
/// if the linker target changed
fn prepare_zig_linker(context: &BuildContext) -> Result<(PathBuf, PathBuf)> {
let target = &context.target;
let arch = if target.cross_compiling() {
if matches!(target.target_arch(), Arch::Armv7L) {
"armv7".to_string()
} else {
target.target_arch().to_string()
}
} else {
"native".to_string()
};
let (zig_cc, zig_cxx, cc_args) = match context.platform_tag {
// Not sure branch even has any use case, but it doesn't hurt to support it
None | Some(PlatformTag::Linux) => (
"./zigcc-gnu.sh".to_string(),
"./zigcxx-gnu.sh".to_string(),
format!("{}-linux-gnu", arch),
),
Some(PlatformTag::Musllinux { x, y }) => {
println!("⚠️ Warning: zig with musl is unstable");
(
format!("./zigcc-musl-{}-{}.sh", x, y),
format!("./zigcxx-musl-{}-{}.sh", x, y),
format!("{}-linux-musl", arch),
)
}
Some(PlatformTag::Manylinux { x, y }) => (
format!("./zigcc-gnu-{}-{}.sh", x, y),
format!("./zigcxx-gnu-{}-{}.sh", x, y),
// https://github.com/ziglang/zig/issues/10050#issuecomment-956204098
format!(
"${{@/-lgcc_s/-lunwind}} -target {}-linux-gnu.{}.{}",
arch, x, y
),
),
};

let zig_linker_dir = dirs::cache_dir()
// If the really is no cache dir, cwd will also do
.unwrap_or_else(|| PathBuf::from("."))
.join(env!("CARGO_PKG_NAME"))
.join(env!("CARGO_PKG_VERSION"));
fs::create_dir_all(&zig_linker_dir)?;
let zig_cc = zig_linker_dir.join(zig_cc);
let zig_cxx = zig_linker_dir.join(zig_cxx);

let mut zig_cc_file = create_linker_script(&zig_cc)?;
writeln!(&mut zig_cc_file, "#!/bin/bash")?;
writeln!(&mut zig_cc_file, "python -m ziglang cc {}", cc_args)?;
drop(zig_cc_file);

let mut zig_cxx_file = create_linker_script(&zig_cxx)?;
writeln!(&mut zig_cxx_file, "#!/bin/bash")?;
writeln!(&mut zig_cxx_file, "python -m ziglang c++ {}", cc_args)?;
drop(zig_cxx_file);

Ok((zig_cc, zig_cxx))
}

#[cfg(target_family = "unix")]
fn create_linker_script(path: &Path) -> Result<std::fs::File> {
let custom_linker_file = OpenOptions::new()
.create(true)
.write(true)
.truncate(true)
.mode(0o700)
.open(path)?;
Ok(custom_linker_file)
}

#[cfg(not(target_family = "unix"))]
fn create_linker_script(path: &Path) -> Result<File> {
Ok(File::create(path)?)
}

fn compile_target(
context: &BuildContext,
python_interpreter: Option<&PythonInterpreter>,
bindings_crate: &BridgeModel,
) -> Result<HashMap<String, PathBuf>> {
let (zig_cc, zig_cxx) = if context.zig && context.target.is_linux() {
let (cc, cxx) =
prepare_zig_linker(context).context("Failed to create zig linker wrapper")?;
(Some(cc), Some(cxx))
} else {
(None, None)
};

let mut shared_args = vec!["--manifest-path", context.manifest_path.to_str().unwrap()];

shared_args.extend(context.cargo_extra_args.iter().map(String::as_str));
Expand Down Expand Up @@ -207,6 +300,20 @@ fn compile_target(
// but forwarding stderr is still useful in case there some non-json error
.stderr(Stdio::inherit());

// Also set TARGET_CC and TARGET_CXX for cc-rs and cmake-rs
if let Some(zig_cc) = zig_cc {
let env_target = context
.target
.target_triple()
.to_uppercase()
.replace("-", "_");
build_command.env("TARGET_CC", &zig_cc);
build_command.env(format!("CARGO_TARGET_{}_LINKER", env_target), &zig_cc);
}
if let Some(zig_cxx) = zig_cxx {
build_command.env("TARGET_CXX", &zig_cxx);
}

if let BridgeModel::BindingsAbi3(_, _) = bindings_crate {
let is_pypy = python_interpreter
.map(|p| p.interpreter_kind == InterpreterKind::PyPy)
Expand Down
3 changes: 2 additions & 1 deletion src/develop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pub fn develop(
manifest_path: manifest_file.to_path_buf(),
out: Some(wheel_dir.path().to_path_buf()),
skip_auditwheel: false,
zig: false,
target: None,
cargo_extra_args,
rustc_extra_args,
Expand Down Expand Up @@ -113,7 +114,7 @@ pub fn develop(
// Y U NO accept windows path prefix, pip?
// Anyways, here's shepmasters stack overflow solution
// https://stackoverflow.com/a/50323079/3549270
#[cfg(not(target_os = "windows"))]
#[cfg(target_family = "unix")]
fn adjust_canonicalization(p: impl AsRef<Path>) -> String {
p.as_ref().display().to_string()
}
Expand Down
6 changes: 3 additions & 3 deletions src/module_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ use fs_err::File;
use sha2::{Digest, Sha256};
use std::collections::HashMap;
use std::ffi::OsStr;
#[cfg(not(target_os = "windows"))]
#[cfg(target_family = "unix")]
use std::fs::OpenOptions;
use std::io;
use std::io::{Read, Write};
#[cfg(not(target_os = "windows"))]
#[cfg(target_family = "unix")]
use std::os::unix::fs::OpenOptionsExt;
use std::path::{Path, PathBuf};
use std::process::{Command, Output};
Expand Down Expand Up @@ -164,7 +164,7 @@ impl ModuleWriter for PathWriter {

// We only need to set the executable bit on unix
let mut file = {
#[cfg(not(target_os = "windows"))]
#[cfg(target_family = "unix")]
{
OpenOptions::new()
.create(true)
Expand Down
16 changes: 13 additions & 3 deletions tests/common/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub fn test_integration(
package: impl AsRef<Path>,
bindings: Option<String>,
unique_name: &str,
zig: bool,
) -> Result<()> {
maybe_mock_cargo();

Expand All @@ -32,17 +33,21 @@ pub fn test_integration(
&cargo_extra_args,
"--out",
&shed,
"--compatibility",
"linux",
];

if let Some(ref bindings) = bindings {
cli.push("--bindings");
cli.push(bindings);
}

let options: BuildOptions = BuildOptions::from_iter_safe(cli)?;
if zig {
cli.push("--zig")
} else {
cli.push("--compatibility");
cli.push("linux");
}

let options: BuildOptions = BuildOptions::from_iter_safe(cli)?;
let build_context = options.into_build_context(false, cfg!(feature = "faster-tests"), false)?;
let wheels = build_context.build_wheels()?;

Expand Down Expand Up @@ -70,6 +75,11 @@ pub fn test_integration(
// We can do this since we know that wheels are built and returned in the
// order they are in the build context
for ((filename, supported_version), python_interpreter) in wheels.iter().zip(interpreter) {
if zig && build_context.target.is_linux() && !build_context.target.is_musl_target() {
assert!(filename
.to_string_lossy()
.ends_with("manylinux_2_12_x86_64.manylinux2010_x86_64.whl"))
konstin marked this conversation as resolved.
Show resolved Hide resolved
}
let venv_name = if supported_version == "py3" {
format!("{}-cffi", test_name)
} else {
Expand Down
2 changes: 1 addition & 1 deletion tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub mod other;
// Y U NO accept windows path prefix, pip?
// Anyways, here's shepmasters stack overflow solution
// https://stackoverflow.com/a/50323079/3549270
#[cfg(not(target_os = "windows"))]
#[cfg(target_family = "unix")]
pub fn adjust_canonicalization(p: impl AsRef<Path>) -> String {
p.as_ref().display().to_string()
}
Expand Down
Loading