Make macOS univeral2 a command line option

PyO3 · Feb 2, 2021 · 8a2efa0 · 8a2efa0
1 parent aa12ce0
commit 8a2efa0
Show file tree

Hide file tree

Showing 10 changed files with 96 additions and 51 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -35,6 +35,7 @@ jobs:
         with:
           profile: minimal
           toolchain: stable
+          target: aarch64-apple-darwin
           override: true
       #- name: Cache cargo registry
       #  uses: actions/cache@v2
@@ -51,6 +52,16 @@ jobs:
       #  with:
       #    path: target
       #    key: ${{ runner.os }}-${{ steps.rustup.outputs.rustc_hash }}-target-${{ hashFiles('**/Cargo.lock') }}
+      - name: Setup Xcode env
+        if: matrix.os == 'macos-latest'
+        shell: bash
+        run: |
+          echo "PYO3_CROSS_LIB_DIR=/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/lib" >> "${GITHUB_ENV}"
+          sudo xcode-select -s /Applications/Xcode_12.3.app
+          bindir="$(xcode-select --print-path)/Toolchains/XcodeDefault.xctoolchain/usr/bin"
+          echo "CC=${bindir}/clang" >> "${GITHUB_ENV}"
+          echo "CXX=${bindir}/clang++" >> "${GITHUB_ENV}"
+          echo "SDKROOT=$(xcrun --sdk macosx --show-sdk-path)" >> "${GITHUB_ENV}"
       - name: cargo test
         uses: actions-rs/cargo@v1
         with:

diff --git a/src/build_context.rs b/src/build_context.rs
@@ -118,6 +118,8 @@ pub struct BuildContext {
     pub interpreter: Vec<PythonInterpreter>,
     /// Cargo.toml as resolved by [cargo_metadata]
     pub cargo_metadata: Metadata,
+    /// Whether to use universal2 or use the native macOS tag (off)
+    pub universal2: bool,
 }
 
 type BuiltWheelMetadata = (PathBuf, String);
@@ -171,7 +173,9 @@ impl BuildContext {
         // otherwise it's none
         let artifact = self.compile_cdylib(self.interpreter.get(0), Some(&self.module_name))?;
 
-        let platform = self.target.get_platform_tag(&self.manylinux);
+        let platform = self
+            .target
+            .get_platform_tag(&self.manylinux, self.universal2);
         let tag = format!("cp{}{}-abi3-{}", major, min_minor, platform);
 
         let mut writer = WheelWriter::new(
@@ -218,7 +222,7 @@ impl BuildContext {
             let artifact =
                 self.compile_cdylib(Some(&python_interpreter), Some(&self.module_name))?;
 
-            let tag = python_interpreter.get_tag(&self.manylinux);
+            let tag = python_interpreter.get_tag(&self.manylinux, self.universal2);
 
             let mut writer = WheelWriter::new(
                 &tag,
@@ -300,7 +304,9 @@ impl BuildContext {
     pub fn build_cffi_wheel(&self) -> Result<PathBuf> {
         let artifact = self.compile_cdylib(None, None)?;
 
-        let (tag, tags) = self.target.get_universal_tags(&self.manylinux);
+        let (tag, tags) = self
+            .target
+            .get_universal_tags(&self.manylinux, self.universal2);
 
         let mut builder =
             WheelWriter::new(&tag, &self.out, &self.metadata21, &self.scripts, &tags)?;
@@ -338,7 +344,9 @@ impl BuildContext {
         auditwheel_rs(&artifact, &self.target, &self.manylinux)
             .context(format!("Failed to ensure {} compliance", self.manylinux))?;
 
-        let (tag, tags) = self.target.get_universal_tags(&self.manylinux);
+        let (tag, tags) = self
+            .target
+            .get_universal_tags(&self.manylinux, self.universal2);
 
         if !self.scripts.is_empty() {
             bail!("Defining entrypoints and working with a binary doesn't mix well");

diff --git a/src/build_options.rs b/src/build_options.rs
@@ -68,6 +68,9 @@ pub struct BuildOptions {
     /// Use as `--rustc-extra-args="--my-arg"`
     #[structopt(long = "rustc-extra-args")]
     pub rustc_extra_args: Vec<String>,
+    /// Control whether to build universal2 wheel for macOS or not
+    #[structopt(long)]
+    pub universal2: bool,
 }
 
 impl Default for BuildOptions {
@@ -82,6 +85,7 @@ impl Default for BuildOptions {
             target: None,
             cargo_extra_args: Vec::new(),
             rustc_extra_args: Vec::new(),
+            universal2: true,
         }
     }
 }
@@ -191,6 +195,7 @@ impl BuildOptions {
             rustc_extra_args,
             interpreter,
             cargo_metadata,
+            universal2: self.universal2,
         })
     }
 }

diff --git a/src/compile.rs b/src/compile.rs
@@ -19,7 +19,7 @@ pub fn compile(
     python_interpreter: Option<&PythonInterpreter>,
     bindings_crate: &BridgeModel,
 ) -> Result<HashMap<String, PathBuf>> {
-    if context.target.is_macos_universal2() {
+    if context.target.is_macos() && context.universal2 {
         let build_type = match bindings_crate {
             BridgeModel::Bin => "bin",
             _ => "cdylib",
@@ -30,7 +30,7 @@ pub fn compile(
             bindings_crate,
             Some("aarch64-apple-darwin"),
         )
-        .context("Failed to build a arm64 library through cargo")?
+        .context("Failed to build a aarch64 library through cargo")?
         .get(build_type)
         .cloned()
         .ok_or_else(|| {
@@ -64,21 +64,21 @@ pub fn compile(
         })?;
 
         // Create an universal dylib
-        let cdylib_path = arm64_artifact
+        let output_path = aarch64_artifact
             .display()
             .to_string()
             .replace("aarch64-apple-darwin/", "");
         let mut writer = FatWriter::new();
-        let arm64_file = fs::read(arm64_artifact)?;
+        let aarch64_file = fs::read(aarch64_artifact)?;
         let x86_64_file = fs::read(x86_64_artifact)?;
         writer
-            .add(&arm64_file)
-            .map_err(|e| anyhow!("Failed to add arm64 cdylib: {:?}", e))?;
+            .add(&aarch64_file)
+            .map_err(|e| anyhow!("Failed to add aarch64 cdylib: {:?}", e))?;
         writer
             .add(&x86_64_file)
             .map_err(|e| anyhow!("Failed to add x86_64 cdylib: {:?}", e))?;
         writer
-            .write_to_file(&cdylib_path)
+            .write_to_file(&output_path)
             .map_err(|e| anyhow!("Failed to create unversal cdylib: {:?}", e))?;
 
         let mut result = HashMap::new();

diff --git a/src/develop.rs b/src/develop.rs
@@ -36,6 +36,7 @@ pub fn develop(
         target: None,
         cargo_extra_args,
         rustc_extra_args,
+        universal2: true,
     };
 
     let build_context = build_options.into_build_context(release, strip)?;
@@ -117,16 +118,18 @@ pub fn develop(
     // Write dist-info directory so pip can interact with it
     let tags = match build_context.bridge {
         BridgeModel::Bindings(_) => {
-            vec![build_context.interpreter[0].get_tag(&build_context.manylinux)]
+            vec![build_context.interpreter[0]
+                .get_tag(&build_context.manylinux, build_context.universal2)]
         }
         BridgeModel::BindingsAbi3(major, minor) => {
-            let platform = target.get_platform_tag(&build_context.manylinux);
+            let platform =
+                target.get_platform_tag(&build_context.manylinux, build_context.universal2);
             vec![format!("cp{}{}-abi3-{}", major, minor, platform)]
         }
         BridgeModel::Bin | BridgeModel::Cffi => {
             build_context
                 .target
-                .get_universal_tags(&build_context.manylinux)
+                .get_universal_tags(&build_context.manylinux, build_context.universal2)
                 .1
         }
     };

diff --git a/src/main.rs b/src/main.rs
@@ -319,14 +319,19 @@ fn pep517(subcommand: PEP517Command) -> Result<()> {
             let context = build_options.into_build_context(true, strip)?;
             let tags = match context.bridge {
                 BridgeModel::Bindings(_) => {
-                    vec![context.interpreter[0].get_tag(&context.manylinux)]
+                    vec![context.interpreter[0].get_tag(&context.manylinux, context.universal2)]
                 }
                 BridgeModel::BindingsAbi3(major, minor) => {
-                    let platform = context.target.get_platform_tag(&context.manylinux);
+                    let platform = context
+                        .target
+                        .get_platform_tag(&context.manylinux, context.universal2);
                     vec![format!("cp{}{}-abi3-{}", major, minor, platform)]
                 }
                 BridgeModel::Bin | BridgeModel::Cffi => {
-                    context.target.get_universal_tags(&context.manylinux).1
+                    context
+                        .target
+                        .get_universal_tags(&context.manylinux, context.universal2)
+                        .1
                 }
             };
 

diff --git a/src/python_interpreter.rs b/src/python_interpreter.rs
@@ -362,10 +362,10 @@ impl PythonInterpreter {
     /// Don't ask me why or how, this is just what setuptools uses so I'm also going to use
     ///
     /// If abi3 is true, cpython wheels use the generic abi3 with the given version as minimum
-    pub fn get_tag(&self, manylinux: &Manylinux) -> String {
+    pub fn get_tag(&self, manylinux: &Manylinux, universal2: bool) -> String {
         match self.interpreter_kind {
             InterpreterKind::CPython => {
-                let platform = self.target.get_platform_tag(manylinux);
+                let platform = self.target.get_platform_tag(manylinux, universal2);
                 if self.target.is_unix() {
                     format!(
                         "cp{major}{minor}-cp{major}{minor}{abiflags}-{platform}",
@@ -393,7 +393,7 @@ impl PythonInterpreter {
                     );
                 }
                 // hack to never use manylinux for pypy
-                let platform = self.target.get_platform_tag(&Manylinux::Off);
+                let platform = self.target.get_platform_tag(&Manylinux::Off, universal2);
                 // pypy uses its version as part of the ABI, e.g.
                 // pypy3 v7.1 => pp371-pypy3_71-linux_x86_64.whl
                 format!(

diff --git a/src/target.rs b/src/target.rs
@@ -58,7 +58,6 @@ enum Arch {
     ARMV7L,
     POWERPC64LE,
     POWERPC64,
-    Universal2,
     X86,
     X86_64,
 }
@@ -70,7 +69,6 @@ impl fmt::Display for Arch {
             Arch::ARMV7L => write!(f, "armv7l"),
             Arch::POWERPC64LE => write!(f, "ppc64le"),
             Arch::POWERPC64 => write!(f, "ppc64"),
-            Arch::Universal2 => write!(f, "universal2"),
             Arch::X86 => write!(f, "i686"),
             Arch::X86_64 => write!(f, "x86_64"),
         }
@@ -110,13 +108,7 @@ impl Target {
             platforms::target::Arch::X86_64 => Arch::X86_64,
             platforms::target::Arch::X86 => Arch::X86,
             platforms::target::Arch::ARM => Arch::ARMV7L,
-            platforms::target::Arch::AARCH64 => {
-                if os == OS::Macos {
-                    Arch::Universal2 // macOS with Apple Silicon
-                } else {
-                    Arch::AARCH64
-                }
-            }
+            platforms::target::Arch::AARCH64 => Arch::AARCH64,
             platforms::target::Arch::POWERPC64
                 if platform.target_triple.starts_with("powerpc64-") =>
             {
@@ -134,7 +126,6 @@ impl Target {
         match (&os, &arch) {
             (OS::FreeBSD, Arch::AARCH64) => bail!("aarch64 is not supported for FreeBSD"),
             (OS::FreeBSD, Arch::ARMV7L) => bail!("armv7l is not supported for FreeBSD"),
-            (OS::FreeBSD, Arch::Universal2) => bail!("universal2 is not supported for FreeBSD"),
             (OS::FreeBSD, Arch::X86) => bail!("32-bit wheels are not supported for FreeBSD"),
             (OS::FreeBSD, Arch::X86_64) => {
                 match PlatformInfo::new() {
@@ -146,7 +137,6 @@ impl Target {
             (OS::Macos, Arch::X86) => bail!("32-bit wheels are not supported for macOS"),
             (OS::Windows, Arch::AARCH64) => bail!("aarch64 is not supported for Windows"),
             (OS::Windows, Arch::ARMV7L) => bail!("armv7l is not supported for Windows"),
-            (OS::Windows, Arch::Universal2) => bail!("universal2 is not supported for Windows"),
             (_, _) => {}
         }
         Ok(Target { os, arch })
@@ -159,7 +149,6 @@ impl Target {
             Arch::ARMV7L => 32,
             Arch::POWERPC64 => 64,
             Arch::POWERPC64LE => 64,
-            Arch::Universal2 => 64,
             Arch::X86 => 32,
             Arch::X86_64 => 64,
         }
@@ -185,18 +174,13 @@ impl Target {
         self.os == OS::Macos
     }
 
-    /// Returns true if the current platform is mac os and arch is universal2
-    pub fn is_macos_universal2(&self) -> bool {
-        self.os == OS::Macos && self.arch == Arch::Universal2
-    }
-
     /// Returns true if the current platform is windows
     pub fn is_windows(&self) -> bool {
         self.os == OS::Windows
     }
 
     /// Returns the platform part of the tag for the wheel name for cffi wheels
-    pub fn get_platform_tag(&self, manylinux: &Manylinux) -> String {
+    pub fn get_platform_tag(&self, manylinux: &Manylinux, universal2: bool) -> String {
         match (&self.os, &self.arch) {
             (OS::FreeBSD, Arch::X86_64) => {
                 let info = match PlatformInfo::new() {
@@ -207,18 +191,32 @@ impl Target {
                 format!("freebsd_{}_amd64", release)
             }
             (OS::Linux, _) => format!("{}_{}", manylinux, self.arch),
-            (OS::Macos, Arch::X86_64) => "macosx_10_7_x86_64".to_string(),
-            (OS::Macos, Arch::AARCH64) => "macosx_11_0_arm64".to_string(),
-            (OS::Macos, Arch::Universal2) => "macosx_10_9_universal2".to_string(),
+            (OS::Macos, Arch::X86_64) => {
+                if universal2 {
+                    "macosx_10_9_universal2".to_string()
+                } else {
+                    "macosx_10_7_x86_64".to_string()
+                }
+            }
+            (OS::Macos, Arch::AARCH64) => {
+                if universal2 {
+                    "macosx_10_9_universal2".to_string()
+                } else {
+                    "macosx_11_0_arm64".to_string()
+                }
+            }
             (OS::Windows, Arch::X86) => "win32".to_string(),
             (OS::Windows, Arch::X86_64) => "win_amd64".to_string(),
             (_, _) => panic!("unsupported target should not have reached get_platform_tag()"),
         }
     }
 
     /// Returns the tags for the WHEEL file for cffi wheels
-    pub fn get_py3_tags(&self, manylinux: &Manylinux) -> Vec<String> {
-        vec![format!("py3-none-{}", self.get_platform_tag(&manylinux))]
+    pub fn get_py3_tags(&self, manylinux: &Manylinux, universal2: bool) -> Vec<String> {
+        vec![format!(
+            "py3-none-{}",
+            self.get_platform_tag(&manylinux, universal2)
+        )]
     }
 
     /// Returns the platform for the tag in the shared libraries file name
@@ -233,12 +231,8 @@ impl Target {
             (OS::Linux, Arch::X86_64) => "x86_64-linux-gnu",
             (OS::Macos, Arch::X86_64) => "darwin",
             (OS::Macos, Arch::AARCH64) => "darwin",
-            (OS::Macos, Arch::Universal2) => "darwin",
             (OS::Windows, Arch::X86) => "win32",
             (OS::Windows, Arch::X86_64) => "win_amd64",
-            (OS::Linux, _) => {
-                panic!("unsupported Linux Arch should not have reached get_shared_platform_tag()")
-            }
             (OS::Macos, _) => {
                 panic!("unsupported macOS Arch should not have reached get_shared_platform_tag()")
             }
@@ -287,12 +281,16 @@ impl Target {
     }
 
     /// Returns the tags for the platform without python version
-    pub fn get_universal_tags(&self, manylinux: &Manylinux) -> (String, Vec<String>) {
+    pub fn get_universal_tags(
+        &self,
+        manylinux: &Manylinux,
+        universal2: bool,
+    ) -> (String, Vec<String>) {
         let tag = format!(
             "py3-none-{platform}",
-            platform = self.get_platform_tag(&manylinux)
+            platform = self.get_platform_tag(&manylinux, universal2)
         );
-        let tags = self.get_py3_tags(&manylinux);
+        let tags = self.get_py3_tags(&manylinux, universal2);
         (tag, tags)
     }
 }
diff --git a/tests/common/develop.rs b/tests/common/develop.rs
@@ -50,7 +50,7 @@ pub fn test_develop(package: impl AsRef<Path>, bindings: Option<String>) -> Resu
     check_installed(&package.as_ref(), &python).unwrap_err();
 
     let output = Command::new(&python)
-        .args(&["-m", "pip", "install", "cffi"])
+        .args(&["-m", "pip", "install", "-U", "pip", "cffi"])
         .output()?;
     if !output.status.success() {
         panic!(

diff --git a/tests/common/integration.rs b/tests/common/integration.rs
@@ -80,6 +80,21 @@ pub fn test_integration(package: impl AsRef<Path>, bindings: Option<String>) ->
 
         let python = target.get_venv_python(&venv_dir);
 
+        // Upgrade pip
+        let output = Command::new(&python)
+            .args(&["-m", "pip", "install", "-U", "pip"])
+            .stderr(Stdio::inherit())
+            .output()
+            .context(format!("pip install failed with {:?}", python))?;
+        if !output.status.success() {
+            bail!(
+                "pip install -U pip failed: {}\n--- Stdout:\n{}\n--- Stderr:\n{}",
+                output.status,
+                str::from_utf8(&output.stdout)?,
+                str::from_utf8(&output.stderr)?,
+            );
+        }
+
         let command = [
             "-m",
             "pip",