From 003165c33ffaedebe76adfdc9fc54acec1c863e0 Mon Sep 17 00:00:00 2001 From: konstin Date: Mon, 17 Sep 2018 19:52:21 +0200 Subject: [PATCH] Go for musl and default to debug --- .travis.yml | 36 ++++++++----- Cargo.toml | 13 +++-- Changelog.md | 20 ++++++-- Readme.md | 74 ++++++++++++++------------- appveyor.yml | 9 ++-- src/build_context.rs | 46 ++++++++--------- src/build_options.rs | 105 ++++++++++++++++++++------------------ src/compile.rs | 34 ++++++------ src/develop.rs | 9 ++-- src/main.rs | 44 ++++++++++------ src/module_writer.rs | 16 +++--- src/python_interpreter.rs | 4 +- tests/test_integration.rs | 7 +-- 13 files changed, 232 insertions(+), 185 deletions(-) diff --git a/.travis.yml b/.travis.yml index b2d548756..ac936686c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,6 +11,7 @@ addons: apt: packages: - libdbus-1-dev + - musl-tools matrix: fast_finish: true @@ -18,22 +19,25 @@ matrix: # These create deployments; Apparently there is not 32 bit mac os - os: linux rust: stable - env: TARGET=x86_64-unknown-linux-gnu + env: TARGET=x86_64-unknown-linux-musl - os: osx rust: stable env: TARGET=x86_64-apple-darwin - os: linux rust: stable - env: TARGET=i686-unknown-linux-gnu + env: TARGET=i686-unknown-linux-musl addons: apt: packages: - libdbus-1-dev:i386 - - libc6-dev-i386 - - linux-libc-dev:i386 - - libssl-dev:i386 - libgmp-dev:i386 - - gcc-multilib + - binutils:i386 + # We actually only need musl-tools:i386 and gcc-multilib, but we need the others to get apt to install them + - cpp-4.8:i386 + - musl:i386 + - musl-tools:i386 + - gcc-multilib:i386 + - cpp:i386 # Those are tested - os: linux @@ -64,17 +68,24 @@ install: fi script: - - if [ "$TARGET" == "i686-unknown-linux-gnu" ]; then rustup target add $TARGET; fi - - cargo build --target $TARGET - - if [ "$TRAVIS_RUST_VERSION" == "nightly" ]; then cargo test --target $TARGET -- --nocapture; fi + - if [[ "$TARGET" == *musl ]]; then rustup target add $TARGET; fi + - if [ "$TRAVIS_RUST_VERSION" == "nightly" ]; then cargo test -- --nocapture; fi before_deploy: - - cargo build --release --target $TARGET - - cargo run --target $TARGET -- publish -b bin --cargo-extra-args="--target=$TARGET" --skip-auditwheel -u konstin + # We don't care for manylinux compliance for the downloads, so we can use the keyring. + # The stack protector story is a weird one; On 32 bit, you get errors such as the following without the option + # /usr/bin/ld: apps/openssl: hidden symbol `__stack_chk_fail_local' isn't defined + # I assume that this is a musl bug fixed in 2015 that didn't make into ubuntu 14.04, but that special + # case seems to be documented nowhere else: + # http://git.musl-libc.org/cgit/musl/commit/?id=55d061f031085f24d138664c897791aebe9a2fab + # We can't have a more recent musl on 14.04 (there's no ppa), so we have to disable that feature + - CFLAGS="-fno-stack-protector" cargo build --release --target $TARGET --features "password-storage musl" - cd target/$TARGET/release/ # You can add more file to the archive by adding them to this line - tar czf ../../../${BINARY_NAME}-$TRAVIS_TAG-$TARGET.tar.gz ${BINARY_NAME} - cd ../../.. + # We do care for manylinux compliance, so we use the musl feature to get static binaries for pypi + - CFLAGS="-fno-stack-protector" cargo run --release --target $TARGET --features musl -- publish -u konstin --release -b bin --target $TARGET --cargo-extra-args="--features=musl" deploy: - # Add zipped binary to the github release @@ -99,5 +110,4 @@ branches: - /^v\d+\.\d+\.\d+.*$/ notifications: - email: - on_success: never + email: false diff --git a/Cargo.toml b/Cargo.toml index 3199ce177..4d0190a1d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] authors = ["konstin "] name = "pyo3-pack" -version = "0.3.3" +version = "0.3.4" description = "Build and publish crates with pyo3 bindings as python packages" exclude = ["get-fourtytwo/**/*", "integration-test/**/*", "sysconfig/*"] readme = "Readme.md" @@ -50,13 +50,16 @@ openssl = { version = "0.10", features = ["vendored"], optional = true } indoc = "0.2.8" [features] -default = ["auditwheel", "upload", "password-storage"] +default = ["auditwheel", "upload"] auditwheel = ["goblin"] -# sdists can be created, but they are currently useless (#2) -sdist = ["tar", "libflate"] upload = ["reqwest"] password-storage = ["upload", "keyring"] -musl_wip = ["openssl"] + +# This will make rewquest use a statically linked version of openssl +musl = ["openssl"] + +# sdists can be created, but they are currently useless (#2) +sdist = ["tar", "libflate"] [workspace] members = [ diff --git a/Changelog.md b/Changelog.md index c1be08e6b..c15b9e2ce 100644 --- a/Changelog.md +++ b/Changelog.md @@ -5,8 +5,18 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.3.4] - 2018-09-18 -## [Unreleased] +### Added + + * A `--target` option which behaves like cargo option of the same name + +### Changed + + * Musl and auditwheel compliance: Using the new `musl` feature combined with the musl target, you can build completely static binaries. The `password-storage`, which enables keyring integration, is now disabled by default. The Pypi packages are now statically linked with musl so that they are audtiwheel compliant. + * Replaced `--debug` with `--release`. All builds are now debug by default + +## [0.3.3] - 2018-09-17 ### Added @@ -16,6 +26,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Fixed * Usage with stable + * Wrong tags in WHEEL file on non-linux platforms + * Uploading on windows ## [0.3.1] - 2017-09-14 @@ -56,8 +68,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Initial Release -[Unreleased]: https://github.com/pyo3/pyo3-pack/compare/v0.3.1...HEAD -[0.3.0]: https://github.com/pyo3/pyo3-pack/compare/v0.3.0...v0.3.1 +[Unreleased]: https://github.com/pyo3/pyo3-pack/compare/v0.3.3...HEAD +[0.3.4]: https://github.com/pyo3/pyo3-pack/compare/v0.3.3...v0.3.4 +[0.3.3]: https://github.com/pyo3/pyo3-pack/compare/v0.3.1...v0.3.3 +[0.3.1]: https://github.com/pyo3/pyo3-pack/compare/v0.3.0...v0.3.1 [0.3.0]: https://github.com/pyo3/pyo3-pack/compare/v0.2.0...v0.3.0 [0.2.0]: https://github.com/pyo3/pyo3-pack/compare/v0.1.0...v0.2.0 diff --git a/Readme.md b/Readme.md index bfccfc31e..00877420c 100644 --- a/Readme.md +++ b/Readme.md @@ -41,8 +41,40 @@ Pip allows adding so called console scripts, which are shell commands that execu get_42 = "get_fourtytwo:DummyClass.get_42" ``` +## pyo3 and rust-cpython + For pyo3 and rust-cpython, pyo3-pack can only build packages for installed python versions, so you might want to use pyenv, deadsnakes or docker for building. If you don't set your own interpreters with `-i`, a heuristic is used to search for python installations. You can get a list all found versions with the `list-python` subcommand. + +## Cffi + + Cffi wheels are compatible with all python versions, but they need to have `cffi` installed for the python used for building (`pip install cffi`). + + Until [eqrion/cbdingen#203](https://github.com/eqrion/cbindgen/issues/203) is resolved, you also need to use a build script that writes c headers to a file called `target/header.h` + +```rust +extern crate cbindgen; + +use std::env; +use std::path::Path; + +fn main() { + let crate_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); + + let mut config: cbindgen::Config = Default::default(); + config.language = cbindgen::Language::C; + cbindgen::generate_with_config(&crate_dir, config) + .expect("Unable to generate bindings") + .write_to_file(Path::new("target").join("header.h")); +} +``` + +## Manylinux and auditwheel + +For portability reasons, native python modules on linux must only dynamically link a set of very few libraries which are installed basically everywhere, hence the name manylinux. The pypa offers a special docker container and a tool called [auditwheel](https://github.com/pypa/auditwheel/) to ensure compliance with the [manylinux rules](https://www.python.org/dev/peps/pep-0513/#the-manylinux1-policy). pyo3-pack contains a reimplementation of the most important part of auditwheel that checks the generated library, so there's no need to use external tools. If you want to disable the manylinux compliance checks for some reason, use the `--skip-auditwheel` flag. + +pyo3-pack itself is manylinux compliant when compiled with the `musl` feature and a musl target, which is true for the version published on pypi. The binaries on the release pages have keyring integration (though the `password-storage` feature), which is not manylinux compliant. + ### Build ``` @@ -50,12 +82,14 @@ USAGE: pyo3-pack build [FLAGS] [OPTIONS] FLAGS: - -d, --debug Do a debug build (don't pass --release to cargo) -h, --help Prints help information + --release Pass --release to cargo --skip-auditwheel Don't check for manylinux compliance -V, --version Prints version information OPTIONS: + -m, --manifest-path The path to the Cargo.toml [default: Cargo.toml] + --target The --target option for cargo -b, --bindings-crate The crate providing the python bindings. pyo3, rust-cpython and cffi are supported @@ -65,7 +99,6 @@ OPTIONS: -i, --interpreter ... The python versions to build wheels for, given as the names of the interpreters. Uses autodiscovery if not explicitly set. - -m, --manifest-path The path to the Cargo.toml [default: Cargo.toml] -o, --out The directory to store the built wheels in. Defaults to a new "wheels" directory in the project's target directory @@ -80,12 +113,14 @@ USAGE: pyo3-pack publish [FLAGS] [OPTIONS] FLAGS: - -d, --debug Do a debug build (don't pass --release to cargo) -h, --help Prints help information + --release Pass --release to cargo --skip-auditwheel Don't check for manylinux compliance -V, --version Prints version information OPTIONS: + -m, --manifest-path The path to the Cargo.toml [default: Cargo.toml] + --target The --target option for cargo -b, --bindings-crate The crate providing the python bindings. pyo3, rust-cpython and cffi are supported @@ -95,7 +130,6 @@ OPTIONS: -i, --interpreter ... The python versions to build wheels for, given as the names of the interpreters. Uses autodiscovery if not explicitly set. - -m, --manifest-path The path to the Cargo.toml [default: Cargo.toml] -o, --out The directory to store the built wheels in. Defaults to a new "wheels" directory in the project's target directory @@ -119,7 +153,7 @@ USAGE: FLAGS: -h, --help Prints help information - --release Compile in release mode. This is useful e.g. for benchmarking + --release Pass --release to cargo -V, --version Prints version information OPTIONS: @@ -134,36 +168,6 @@ OPTIONS: Extra arguments that will be passed to rustc as `cargo rustc [...] -- [arg1] [arg2]` ``` - -## Cffi - - Cffi wheels are compatible with all python versions, but they need to have `cffi` installed to build (`pip install cffi`). Until [eqrion/cbdingen#203](https://github.com/eqrion/cbindgen/issues/203) is resolved, you also need to use a build script that writes c headers to a file called `target/header.h` - -```rust -extern crate cbindgen; - -use std::env; -use std::path::Path; - -fn main() { - let crate_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); - - let mut config: cbindgen::Config = Default::default(); - config.language = cbindgen::Language::C; - cbindgen::generate_with_config(&crate_dir, config) - .expect("Unable to generate bindings") - .write_to_file(Path::new("target").join("header.h")); -} -``` - -## Manylinux and auditwheel - -For portability reasons, native python modules on linux must only dynamically link a set of very few libraries which are installed basically everywhere, hence the name manylinux. The pypa offers a special docker container and a tool called [auditwheel](https://github.com/pypa/auditwheel/) to ensure compliance with the [manylinux rules](https://www.python.org/dev/peps/pep-0513/#the-manylinux1-policy). pyo3-pack contains a reimplementation of the most important part of auditwheel that checks the generated library, so there's no need to use external tools. If you want to disable the manylinux compliance checks for some reason, use the `--skip-auditwheel` flag. - -To ship a completely static binary with musl, you can use `pyo3-pack build -b bin --cargo-extra-args="--target=x86_64-unknown-linux-musl"`. - -Note that the pyo3-pack pip package is not manylinux compliant (A compliant package, which you get with `--no-default-features --features auditwheel`, can neither upload nor use the keyring) - ## Code The main part is the pyo3-pack library, which is completely documented and should be well integratable. The accompanying `main.rs` takes care username and password for the pypi upload and otherwise calls into the library. diff --git a/appveyor.yml b/appveyor.yml index d80053032..bd2d1f057 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -59,15 +59,16 @@ test_script: } before_deploy: - - cargo build --release - # I don't know how I can create a script deployment like in travis in appveyor, so let's hijack before_deploy - # Also we only need one version for win32 and one for win_amd64, so we only deploy to pypi for msvc - - cargo run -- publish -b bin -u konstin + - cargo build --release --features password-storage # Grab the binary and pack it into a zip archive - cd target\release\ # You can add more file to the archive by adding them to this line - 7z a ../../%APPVEYOR_PROJECT_SLUG%-%APPVEYOR_REPO_TAG_NAME%-%TARGET%.zip %BINARY_NAME% - appveyor PushArtifact ../../%APPVEYOR_PROJECT_SLUG%-%APPVEYOR_REPO_TAG_NAME%-%TARGET%.zip + - cd ../.. + # Publish pypi + - cargo run -- publish -b bin -u konstin --release + deploy: # Add zipped binary to the github release diff --git a/src/build_context.rs b/src/build_context.rs index 1d1ae172b..bd1dc4537 100644 --- a/src/build_context.rs +++ b/src/build_context.rs @@ -4,13 +4,13 @@ use auditwheel_rs; use build_source_distribution; use compile; use failure::{Context, Error, ResultExt}; -use Metadata21; -use module_writer::{write_bin, write_bindings_module, write_cffi_module}; use module_writer::WheelWriter; -use PythonInterpreter; +use module_writer::{write_bin, write_bindings_module, write_cffi_module}; use std::collections::HashMap; use std::fs; use std::path::PathBuf; +use Metadata21; +use PythonInterpreter; use Target; /// The way the rust code is bridged with python, i.e. either using extern c and cffi or @@ -54,8 +54,8 @@ pub struct BuildContext { /// The directory to store the built wheels in. Defaults to a new "wheels" /// directory in the project's target directory pub out: PathBuf, - /// Do a debug build (don't pass --release to cargo) - pub debug: bool, + /// Pass --release to cargo + pub release: bool, /// Don't check for manylinux compliance pub skip_auditwheel: bool, /// Extra arguments that will be passed to cargo as `cargo rustc [...] [arg1] [arg2] --` @@ -126,22 +126,22 @@ impl BuildContext { } #[cfg(feature = "sdist")] - { - let sdist_path = wheel_dir.join(format!( - "{}-{}.tar.gz", - &self.metadata21.get_distribution_encoded(), - &self.metadata21.get_version_encoded() - )); - - println!( - "Building the source distribution to {}", - sdist_path.display() - ); - build_source_distribution(&self, &self.metadata21, &self.scripts, &sdist_path) - .context("Failed to build the source distribution")?; - - wheels.push((sdist_path, None)); - } + { + let sdist_path = wheel_dir.join(format!( + "{}-{}.tar.gz", + &self.metadata21.get_distribution_encoded(), + &self.metadata21.get_version_encoded() + )); + + println!( + "Building the source distribution to {}", + sdist_path.display() + ); + build_source_distribution(&self, &self.metadata21, &self.scripts, &sdist_path) + .context("Failed to build the source distribution")?; + + wheels.push((sdist_path, None)); + } Ok(wheels) } @@ -168,7 +168,7 @@ impl BuildContext { if !self.skip_auditwheel && target.is_linux() { #[cfg(feature = "auditwheel")] - auditwheel_rs(&artifact).context("Failed to ensure manylinux compliance")?; + auditwheel_rs(&artifact).context("Failed to ensure manylinux compliance")?; } Ok(artifact) @@ -213,7 +213,7 @@ impl BuildContext { if !self.skip_auditwheel && self.target.is_linux() { #[cfg(feature = "auditwheel")] - auditwheel_rs(&artifact).context("Failed to ensure manylinux compliance")?; + auditwheel_rs(&artifact).context("Failed to ensure manylinux compliance")?; } let (tag, tags) = self.get_unversal_tags(); diff --git a/src/build_options.rs b/src/build_options.rs index 2bbb25908..e6ba130a7 100644 --- a/src/build_options.rs +++ b/src/build_options.rs @@ -1,19 +1,19 @@ use build_context::BridgeModel; -use BuildContext; use cargo_metadata; use cargo_toml::CargoTomlMetadata; use cargo_toml::CargoTomlMetadataPyo3Pack; -use CargoToml; +use failure::err_msg; use failure::{Error, ResultExt}; -use Metadata21; -use PythonInterpreter; use std::collections::HashMap; use std::collections::HashSet; use std::fs; use std::path::PathBuf; -use Target; -use failure::err_msg; use toml; +use BuildContext; +use CargoToml; +use Metadata21; +use PythonInterpreter; +use Target; /// High level API for building wheels from a crate which can be also used for /// the CLI @@ -28,7 +28,11 @@ pub struct BuildOptions { #[structopt(short = "b", long = "bindings-crate")] pub bindings: Option, #[structopt( - short = "m", long = "manifest-path", parse(from_os_str), default_value = "Cargo.toml" + short = "m", + long = "manifest-path", + parse(from_os_str), + default_value = "Cargo.toml", + name = "PATH" )] /// The path to the Cargo.toml pub manifest_path: PathBuf, @@ -36,12 +40,15 @@ pub struct BuildOptions { /// directory in the project's target directory #[structopt(short = "o", long = "out", parse(from_os_str))] pub out: Option, - /// Do a debug build (don't pass --release to cargo) - #[structopt(short = "d", long = "debug")] - pub debug: bool, + /// Pass --release to cargo + #[structopt(long = "release")] + pub release: bool, /// Don't check for manylinux compliance #[structopt(long = "skip-auditwheel")] pub skip_auditwheel: bool, + /// The --target option for cargo + #[structopt(long = "target", name = "TRIPLE")] + pub target: Option, /// Extra arguments that will be passed to cargo as `cargo rustc [...] [arg1] [arg2] --` #[structopt(long = "cargo-extra-args")] pub cargo_extra_args: Vec, @@ -57,8 +64,9 @@ impl Default for BuildOptions { bindings: None, manifest_path: PathBuf::from("Cargo.toml"), out: None, - debug: false, + release: false, skip_auditwheel: false, + target: None, cargo_extra_args: Vec::new(), rustc_extra_args: Vec::new(), } @@ -90,11 +98,11 @@ impl BuildOptions { .context("Failed to parse Cargo.toml into python metadata")?; let scripts = match cargo_toml.package.metadata { Some(CargoTomlMetadata { - pyo3_pack: - Some(CargoTomlMetadataPyo3Pack { - scripts: Some(ref scripts), - }), - }) => scripts.clone(), + pyo3_pack: + Some(CargoTomlMetadataPyo3Pack { + scripts: Some(ref scripts), + }), + }) => scripts.clone(), _ => HashMap::new(), }; @@ -120,18 +128,20 @@ impl BuildOptions { let bridge = find_bridge(&cargo_metadata, self.bindings.as_ref().map(|x| &**x))?; - if bridge != BridgeModel::Bin { - if module_name.contains('-') { - bail!( - "The module name must not contains a minus \ - (Make sure you have set an appropriate [lib] name in your Cargo.toml)" - ); - } + if bridge != BridgeModel::Bin && module_name.contains('-') { + bail!( + "The module name must not contains a minus \ + (Make sure you have set an appropriate [lib] name in your Cargo.toml)" + ); } - let interpreter = find_interpreter(&bridge, &self.interpreter, &target)?; + let mut cargo_extra_args = self.cargo_extra_args.clone(); + if let Some(target) = self.target { + cargo_extra_args.extend_from_slice(&["--target".to_string(), target]); + } + Ok(BuildContext { target, bridge, @@ -140,9 +150,9 @@ impl BuildOptions { module_name, manifest_path: self.manifest_path, out: wheel_dir, - debug: self.debug, + release: self.release, skip_auditwheel: self.skip_auditwheel, - cargo_extra_args: self.cargo_extra_args, + cargo_extra_args, rustc_extra_args: self.rustc_extra_args, interpreter, }) @@ -188,7 +198,11 @@ pub fn find_bridge( } } -pub fn find_interpreter(bridge: &BridgeModel, interpreter: &[String], target: &Target) -> Result, Error> { +pub fn find_interpreter( + bridge: &BridgeModel, + interpreter: &[String], + target: &Target, +) -> Result, Error> { Ok(match bridge { BridgeModel::Bindings(_) => { let interpreter = if !interpreter.is_empty() { @@ -222,22 +236,22 @@ pub fn find_interpreter(bridge: &BridgeModel, interpreter: &[String], target: &T }; let err_message = "Failed to find python interpreter for generating cffi bindings"; - let interpreter = PythonInterpreter::check_executable(executable, &target).context(err_msg(err_message))?.ok_or_else(|| err_msg(err_message))?; + let interpreter = PythonInterpreter::check_executable(executable, &target) + .context(err_msg(err_message))? + .ok_or_else(|| err_msg(err_message))?; println!("Using {} to generate the cffi bindings", interpreter); vec![interpreter] } - BridgeModel::Bin => { - vec![] - } + BridgeModel::Bin => vec![], }) } #[cfg(test)] mod test { - use std::path::Path; use super::*; + use std::path::Path; #[test] fn test_find_bridge() { @@ -250,31 +264,22 @@ mod test { cargo_metadata::metadata_deps(Some(&Path::new("points").join("Cargo.toml")), true) .unwrap(); - assert!( - match find_bridge(&get_fourtytwo, None).unwrap() { - BridgeModel::Bindings { .. } => true, - _ => false, - } - ); + assert!(match find_bridge(&get_fourtytwo, None).unwrap() { + BridgeModel::Bindings(_) => true, + _ => false, + }); - assert!( - match find_bridge(&get_fourtytwo, Some("pyo3")).unwrap() { - BridgeModel::Bindings { .. } => true, - _ => false, - } - ); + assert!(match find_bridge(&get_fourtytwo, Some("pyo3")).unwrap() { + BridgeModel::Bindings(_) => true, + _ => false, + }); assert_eq!( find_bridge(&points, Some("cffi")).unwrap(), BridgeModel::Cffi ); - assert!( - find_bridge( - &get_fourtytwo, - Some("rust-cpython"), - ).is_err() - ); + assert!(find_bridge(&get_fourtytwo, Some("rust-cpython"),).is_err()); assert!(find_bridge(&points, Some("rust-cpython")).is_err()); assert!(find_bridge(&points, Some("pyo3")).is_err()); diff --git a/src/compile.rs b/src/compile.rs index add7406e3..8b008cf26 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -1,16 +1,16 @@ use atty; use atty::Stream; use build_context::BridgeModel; -use BuildContext; use failure::{Error, ResultExt}; use indicatif::{ProgressBar, ProgressStyle}; -use PythonInterpreter; use serde_json; use std::collections::HashMap; use std::io::{BufRead, BufReader}; use std::path::PathBuf; use std::process::{Command, Stdio}; use std::str; +use BuildContext; +use PythonInterpreter; #[derive(Deserialize)] struct BuildPlanEntry { @@ -99,7 +99,6 @@ fn get_build_plan(shared_args: &[&str]) -> Result { // the rust-toolchain file is ignored .args(build_plan_args) .args(shared_args) - .stderr(Stdio::inherit()) // Forward any error to the user .output() .map_err(|e| { format_err!( @@ -117,8 +116,8 @@ fn get_build_plan(shared_args: &[&str]) -> Result { ); } - let plan: SerializedBuildPlan = - serde_json::from_slice(&build_plan.stdout).context("The build plan has an invalid format")?; + let plan: SerializedBuildPlan = serde_json::from_slice(&build_plan.stdout) + .context("The build plan has an invalid format")?; Ok(plan) } @@ -148,20 +147,16 @@ pub fn compile( shared_args.extend(&["--features", &python_feature]); } - // We need this to be allows to pass rustce extra args + // We need to pass --bins / --lib to set the rustc extra args later // TODO: What do we do when there are multiple bin targets? match bindings_crate { - BridgeModel::Bin => { - shared_args.push("--bins") - } - BridgeModel::Cffi | BridgeModel::Bindings(_) => { - shared_args.push("--lib") - } + BridgeModel::Bin => shared_args.push("--bins"), + BridgeModel::Cffi | BridgeModel::Bindings(_) => shared_args.push("--lib"), } shared_args.extend(context.cargo_extra_args.iter().map(|x| x.as_str())); - if !context.debug { + if context.release { shared_args.push("--release"); } @@ -196,9 +191,12 @@ pub fn compile( .iter() .map(|x| x.as_str()) .collect(); + if context.target.is_macos() { - let mac_args = &["-C", "link-arg=-undefined", "-C", "link-arg=dynamic_lookup"]; - rustc_args.extend(mac_args); + if let BridgeModel::Bindings(_) = bindings_crate { + let mac_args = &["-C", "link-arg=-undefined", "-C", "link-arg=dynamic_lookup"]; + rustc_args.extend(mac_args); + } } let build_args: Vec<_> = cargo_args @@ -231,9 +229,9 @@ pub fn compile( // Extract the location of the .so/.dll/etc. from cargo's json output if message.target.name == context.module_name || message.target.name == context.metadata21.name - { - artifact_messages.push(message); - } + { + artifact_messages.push(message); + } // The progress bar isn't an exact science and stuff might get out-of-sync, // but that isn't big problem since the bar is only to give the user an estimate diff --git a/src/develop.rs b/src/develop.rs index 0a9d1190e..443969b57 100644 --- a/src/develop.rs +++ b/src/develop.rs @@ -34,8 +34,9 @@ pub fn develop( bindings, manifest_path: manifest_file.to_path_buf(), out: None, - debug: !release, - skip_auditwheel: false, + skip_auditwheel: true, + target: None, + release, cargo_extra_args, rustc_extra_args, }; @@ -52,7 +53,7 @@ pub fn develop( let artifact = artifacts .get("bin") - .ok_or(format_err!("Cargo didn't build a binary"))?; + .ok_or_else(|| format_err!("Cargo didn't build a binary"))?; // Copy the artifact into the same folder as pip and python let bin_name = artifact.file_name().unwrap(); @@ -69,7 +70,7 @@ pub fn develop( &build_context.target, )?; } - BridgeModel::Bindings { .. } => { + BridgeModel::Bindings(_) => { let artifact = build_context .compile_cdylib(Some(&interpreter)) .context(context)?; diff --git a/src/main.rs b/src/main.rs index d92b2bfe2..844026716 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,6 +2,8 @@ //! //! Run with --help for usage information +#![cfg_attr(feature = "cargo-clippy", feature(tool_lints))] + extern crate core; #[macro_use] extern crate failure; @@ -37,7 +39,7 @@ use structopt::StructOpt; /// 2. keyring /// 3. stdin #[cfg(feature = "upload")] -fn get_password(username: &str) -> String { +fn get_password(_username: &str) -> String { if let Ok(password) = env::var("PYO3_PACK_PASSWORD") { return password; }; @@ -45,7 +47,7 @@ fn get_password(username: &str) -> String { #[cfg(feature = "keyring")] { let service = env!("CARGO_PKG_NAME"); - let keyring = Keyring::new(&service, &username); + let keyring = Keyring::new(&service, &_username); if let Ok(password) = keyring.get_password() { return password; }; @@ -89,7 +91,9 @@ fn complete_registry(opt: &PublishOpt) -> Result { #[derive(Debug, StructOpt)] struct PublishOpt { #[structopt( - short = "r", long = "repository-url", default_value = "https://upload.pypi.org/legacy/" + short = "r", + long = "repository-url", + default_value = "https://upload.pypi.org/legacy/" )] /// The url of registry where the wheels are uploaded to registry: String, @@ -104,7 +108,7 @@ struct PublishOpt { #[derive(Debug, StructOpt)] #[structopt(name = "pyo3-pack")] -#[cfg_attr(feature = "cargo-clippy", allow(large_enum_variant))] +#[cfg_attr(feature = "cargo-clippy", allow(clippy::large_enum_variant))] /// Build and publish crates with pyo3 bindings as python packages enum Opt { #[structopt(name = "build")] @@ -134,11 +138,14 @@ enum Opt { #[structopt(short = "b", long = "bindings-crate")] binding_crate: Option, #[structopt( - short = "m", long = "manifest-path", parse(from_os_str), default_value = "Cargo.toml" + short = "m", + long = "manifest-path", + parse(from_os_str), + default_value = "Cargo.toml" )] /// The path to the Cargo.toml manifest_path: PathBuf, - /// Compile in release mode. This is useful e.g. for benchmarking + /// Pass --release to cargo #[structopt(long = "release")] release: bool, /// Extra arguments that will be passed to cargo as `cargo rustc [...] [arg1] [arg2] --` @@ -160,6 +167,11 @@ fn run() -> Result<(), Error> { #[cfg(feature = "upload")] Opt::Publish { build, publish } => { let build_context = build.into_build_context()?; + + if !build_context.release { + eprintln!("Warning: You're publishing debug wheels"); + } + let wheels = build_context.build_wheels()?; let mut registry = complete_registry(&publish)?; @@ -203,16 +215,16 @@ fn run() -> Result<(), Error> { } let username = get_username(); - let password = rpassword::prompt_password_stdout( - "Please enter your password: ", - ).unwrap_or_else(|_| { - // So we need this fallback for pycharm on windows - let mut password = String::new(); - io::stdin() - .read_line(&mut password) - .expect("Failed to read line"); - password.trim().to_string() - }); + let password = + rpassword::prompt_password_stdout("Please enter your password: ") + .unwrap_or_else(|_| { + // So we need this fallback for pycharm on windows + let mut password = String::new(); + io::stdin() + .read_line(&mut password) + .expect("Failed to read line"); + password.trim().to_string() + }); registry = Registry::new(username, password, registry.url); println!("Retrying") diff --git a/src/module_writer.rs b/src/module_writer.rs index 212617cd5..1962bbd8d 100644 --- a/src/module_writer.rs +++ b/src/module_writer.rs @@ -63,12 +63,13 @@ pub struct DevelopModuleWriter { impl DevelopModuleWriter { /// Creates a [ModuleWriter] that adds the modul to the current virtualenv pub fn venv(target: &Target, venv_dir: &Path) -> Result { - let interpreter = PythonInterpreter::check_executable( - target.get_venv_python(&venv_dir), - &target, - )?.ok_or_else(|| { - Context::new("Expected `python` to be a python interpreter inside a virtualenv ಠ_ಠ") - })?; + let interpreter = + PythonInterpreter::check_executable(target.get_venv_python(&venv_dir), &target)? + .ok_or_else(|| { + Context::new( + "Expected `python` to be a python interpreter inside a virtualenv ಠ_ಠ", + ) + })?; let python_dir = format!("python{}.{}", interpreter.major, interpreter.minor); @@ -205,7 +206,8 @@ impl WheelWriter { let options = zip::write::FileOptions::default().compression_method(zip::CompressionMethod::Stored); let record_file = self.dist_info_dir.join("RECORD"); - self.zip.start_file(record_file.to_str().unwrap(), options)?; + self.zip + .start_file(record_file.to_str().unwrap(), options)?; for (filename, hash, len) in self.record { self.zip .write_all(format!("{},sha256={},{}\n", filename, hash, len).as_bytes())?; diff --git a/src/python_interpreter.rs b/src/python_interpreter.rs index d9f6801e5..a195bc2dc 100644 --- a/src/python_interpreter.rs +++ b/src/python_interpreter.rs @@ -37,8 +37,8 @@ print(json.dumps({ /// sure that the pointer width (32-bit or 64-bit) matches across platforms fn find_all_windows(target: &Target) -> Result, Error> { let execution = Command::new("py").arg("-0").output(); - let output = - execution.context("Couldn't run 'py' command. Do you have python installed and in PATH?")?; + let output = execution + .context("Couldn't run 'py' command. Do you have python installed and in PATH?")?; let expr = Regex::new(r" -(\d).(\d)-(\d+)(?: .*)?").unwrap(); let lines = str::from_utf8(&output.stdout).unwrap().lines(); let mut interpreter = vec![]; diff --git a/tests/test_integration.rs b/tests/test_integration.rs index 9cb358687..97eb66c20 100644 --- a/tests/test_integration.rs +++ b/tests/test_integration.rs @@ -49,7 +49,6 @@ fn test_integration(package: &Path, bindings: Option) { let mut options = BuildOptions::default(); options.manifest_path = package.join("Cargo.toml"); options.bindings = bindings; - options.debug = true; let wheels = options .into_build_context() @@ -80,8 +79,7 @@ fn test_integration(package: &Path, bindings: Option) { "-p", &venv_py_version.display().to_string(), &venv_dir.display().to_string(), - ]) - .stderr(Stdio::inherit()) + ]).stderr(Stdio::inherit()) .output() .unwrap(); if !output.status.success() { @@ -98,8 +96,7 @@ fn test_integration(package: &Path, bindings: Option) { "install", "--force-reinstall", &adjust_canonicalization(filename), - ]) - .stderr(Stdio::inherit()) + ]).stderr(Stdio::inherit()) .output() .unwrap(); if !output.status.success() {