Merge pull request #153 from PyO3/windows_encoding

Work around windows encoding errors

pyo3_pack/__init__.py

@@ -2,8 +2,11 @@
 """
 pyo3-pack's implementation of the PEP 517 interface. Calls pyo3-pack through subprocess
 
-Currently pyo3-pack doesn't have json output (nor is there a way to the user visible output off),
-so this parse the user facing messages.
+Currently, the "return value" of the rust implementation is the last line of stdout
+
+On windows, apparently pip's subprocess handling sets stdout to some windows encoding (e.g. cp1252 on my machine),
+even though the terminal supports utf8. Writing directly to the binary stdout buffer avoids ecoding errors due to
+pyo3-pack's emojis.
 
 TODO: Don't require the user to specify toml as a requirement in the pyproject.toml
 """
@@ -48,18 +51,19 @@ def get_config_options() -> List[str]:
 # noinspection PyUnusedLocal
 def build_wheel(wheel_directory, config_settings=None, metadata_directory=None):
     # The PEP 517 build environment garuantees that `python` is the correct python
-    command = ["pyo3-pack", "build", "-i", "python", "--no-sdist"]
+    command = ["pyo3-pack", "pep517", "build-wheel", "-i", "python"]
     command.extend(get_config_options())
 
     print("Running `{}`".format(" ".join(command)))
     try:
-        output = subprocess.check_output(command).decode("utf-8", "ignore")
+        output = subprocess.check_output(command)
     except subprocess.CalledProcessError as e:
         print("Error: {}".format(e))
         sys.exit(1)
-    print(output)
-    # Get the filename from `📦 Built wheel [for CPython 3.6m] to /home/user/project`
-    filename = os.path.split(output.strip().splitlines()[-1].split(" to ")[1])[1]
+    sys.stdout.buffer.write(output)
+    sys.stdout.flush()
+    output = output.decode(errors="replace")
+    filename = output.strip().splitlines()[-1]
     shutil.copy2("target/wheels/" + filename, os.path.join(wheel_directory, filename))
     return filename
 
@@ -77,11 +81,13 @@ def build_sdist(sdist_directory, config_settings=None):
 
     print("Running `{}`".format(" ".join(command)))
     try:
-        output = subprocess.check_output(command).decode("utf-8", "ignore")
+        output = subprocess.check_output(command)
     except subprocess.CalledProcessError as e:
         print(e)
         sys.exit(1)
-    print(output)
+    sys.stdout.buffer.write(output)
+    sys.stdout.flush()
+    output = output.decode(errors="replace")
     return output.strip().splitlines()[-1]
 
 
@@ -110,6 +116,12 @@ def prepare_metadata_for_build_wheel(metadata_directory, config_settings=None):
     command.extend(get_config_options())
 
     print("Running `{}`".format(" ".join(command)))
-    output = subprocess.check_output(command).decode("utf-8", "ignore")
-    print(output)
+    try:
+        output = subprocess.check_output(command)
+    except subprocess.CalledProcessError as e:
+        print("Error: {}".format(e))
+        sys.exit(1)
+    sys.stdout.buffer.write(output)
+    sys.stdout.flush()
+    output = output.decode(errors="replace")
     return output.strip().splitlines()[-1]

src/build_context.rs

@@ -216,14 +216,15 @@ impl BuildContext {
                  in the lib section of your Cargo.toml?",
             )
         })?;
-
-        let target = python_interpreter
-            .map(|x| &x.target)
-            .unwrap_or(&self.target);
-
         #[cfg(feature = "auditwheel")]
-        auditwheel_rs(&artifact, target, &self.manylinux)
-            .context("Failed to ensure manylinux compliance")?;
+        {
+            let target = python_interpreter
+                .map(|x| &x.target)
+                .unwrap_or(&self.target);
+
+            auditwheel_rs(&artifact, target, &self.manylinux)
+                .context("Failed to ensure manylinux compliance")?;
+        }
 
         if let Some(module_name) = module_name {
             warn_missing_py_init(&artifact, module_name)

src/main.rs

@@ -3,6 +3,7 @@
 //!
 //! Run with --help for usage information
 
+use failure::ResultExt;
 use failure::{bail, Error};
 #[cfg(feature = "human-panic")]
 use human_panic::setup_panic;
@@ -19,7 +20,6 @@ use std::path::PathBuf;
 use structopt::StructOpt;
 #[cfg(feature = "upload")]
 use {
-    failure::ResultExt,
     pyo3_pack::{upload, Registry, UploadError},
     reqwest::Url,
     rpassword,
@@ -201,6 +201,20 @@ enum PEP517Command {
         #[structopt(long)]
         strip: bool,
     },
+    #[structopt(name = "build-wheel")]
+    /// Implementation of build_wheel
+    ///
+    /// --release and --strip are currently unused by the PEP 517 implementation
+    BuildWheel {
+        #[structopt(flatten)]
+        build: BuildOptions,
+        /// Pass --release to cargo
+        #[structopt(long)]
+        release: bool,
+        /// Strip the library for minimum file size
+        #[structopt(long)]
+        strip: bool,
+    },
     /// The implementation of build_sdist
     #[structopt(name = "write-sdist")]
     WriteSDist {
@@ -388,6 +402,16 @@ fn run() -> Result<(), Error> {
                 write_dist_info(&mut writer, &context.metadata21, &context.scripts, &tags)?;
                 println!("{}", context.metadata21.get_dist_info_dir().display());
             }
+            PEP517Command::BuildWheel {
+                build,
+                release,
+                strip,
+            } => {
+                let build_context = build.into_build_context(release, strip)?;
+                let wheels = build_context.build_wheels()?;
+                assert_eq!(wheels.len(), 1);
+                println!("{}", wheels[0].0.file_name().unwrap().to_str().unwrap());
+            }
             PEP517Command::WriteSDist {
                 sdist_directory,
                 manifest_path,

src/module_writer.rs

@@ -18,7 +18,7 @@ use std::io::{Read, Write};
 #[cfg(not(target_os = "windows"))]
 use std::os::unix::fs::OpenOptionsExt;
 use std::path::{Path, PathBuf};
-use std::process::{Command, Stdio};
+use std::process::Command;
 use std::str;
 use tempfile::tempdir;
 use walkdir::WalkDir;
@@ -367,31 +367,39 @@ pub fn generate_cffi_declarations(crate_dir: &Path, python: &PathBuf) -> Result<
 
     let ffi_py = tempdir.as_ref().join("ffi.py");
 
+    // Using raw strings is important because on windows there are path like
+    // `C:\Users\JohnDoe\AppData\Local\TEmpl\pip-wheel-asdf1234` where the \U
+    // would otherwise be a broken unicode exscape sequence
     let cffi_invocation = format!(
         r#"
 import cffi
 from cffi import recompiler
 
 ffi = cffi.FFI()
-with open("{header}") as header:
+with open(r"{header}") as header:
     ffi.cdef(header.read())
-recompiler.make_py_source(ffi, "ffi", "{ffi_py}")
+recompiler.make_py_source(ffi, "ffi", r"{ffi_py}")
 "#,
         ffi_py = ffi_py.display(),
         header = header.display(),
     );
 
     let output = Command::new(python)
         .args(&["-c", &cffi_invocation])
-        .stderr(Stdio::inherit())
         .output()?;
     if !output.status.success() {
         bail!(
-            "Failed to generate cffi declarations using {}",
-            python.display()
+            "Failed to generate cffi declarations using {}: {}\n--- Stdout:\n{}\n--- Stderr:\n{}",
+            python.display(),
+            output.status,
+            str::from_utf8(&output.stdout)?,
+            str::from_utf8(&output.stderr)?,
         );
     }
 
+    // Don't swallow warnings
+    std::io::stderr().write_all(&output.stderr)?;
+
     let ffi_py_content = fs::read_to_string(ffi_py)?;
     tempdir.close()?;
     Ok(ffi_py_content)

tests/test_integration.rs

@@ -191,8 +191,12 @@ fn test_integration_conda(
     // Since the python launcher has precedence over conda, we need to deactivate it.
     // We do so by shadowing it with our own hello world binary.
     let original_path = env::var_os("PATH").expect("PATH is not defined");
-    let py_dir = env::current_dir()?.join("test-data").to_str()?.to_string();
-    let mocked_path = py_dir + ";" + original_path.to_str()?;
+    let py_dir = env::current_dir()?
+        .join("test-data")
+        .to_str()
+        .unwrap()
+        .to_string();
+    let mocked_path = py_dir + ";" + original_path.to_str().unwrap();
     env::set_var("PATH", mocked_path);
 
     // Create environments to build against, prepended with "A" to ensure that integration
@@ -222,7 +226,7 @@ fn test_integration_conda(
     for (filename, _, python_interpreter) in wheels {
         if let Some(pi) = python_interpreter {
             let executable = pi.executable;
-            if executable.to_str()?.contains("pyo3-build-env-") {
+            if executable.to_str().unwrap().contains("pyo3-build-env-") {
                 conda_wheels.push((filename, executable))
             }
         }