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

cli: add --profile option and MATURIN_PROFILE var #774

Closed
wants to merge 4 commits into from
Closed
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
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Add support for using [`zig cc`](https://andrewkelley.me/post/zig-cc-powerful-drop-in-replacement-gcc-clang.html) as linker for easier cross compiling and manylinux compliance in [#756](https://github.com/PyO3/maturin/pull/756)
* Switch from reqwest to ureq to reduce dependencies in [#767](https://github.com/PyO3/maturin/pull/767)
* Fix missing Python submodule in wheel in [#772](https://github.com/PyO3/maturin/pull/772)
* Add `--profile` argument to match Cargo's `--profile`, and `MATURIN_PROFILE` env var in [#774](https://github.com/PyO3/maturin/pull/774)

## [0.12.6] - 2021-12-31

Expand Down
5 changes: 3 additions & 2 deletions src/build_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,9 @@ 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,
/// Pass --release to cargo
pub release: bool,
/// The `--profile` to pass to cargo. Can also be set by the `MATURIN_PROFILE` environment
/// variable.
pub profile: Option<String>,
/// Strip the library for minimum file size
pub strip: bool,
/// Skip checking the linked libraries for manylinux/musllinux compliance
Expand Down
16 changes: 8 additions & 8 deletions src/build_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ pub struct BuildOptions {
/// Only applies to macOS targets, do nothing otherwise.
#[clap(long)]
pub universal2: bool,
/// The `--profile` to pass to cargo. Can also be set by the `MATURIN_PROFILE` environment
/// variable.
#[clap(long, env = "MATURIN_PROFILE")]
pub profile: Option<String>,
}

impl Default for BuildOptions {
Expand All @@ -107,18 +111,14 @@ impl Default for BuildOptions {
cargo_extra_args: Vec::new(),
rustc_extra_args: Vec::new(),
universal2: false,
profile: None,
}
}
}

impl BuildOptions {
/// Tries to fill the missing metadata for a BuildContext by querying cargo and python
pub fn into_build_context(
self,
release: bool,
strip: bool,
editable: bool,
) -> Result<BuildContext> {
pub fn into_build_context(self, strip: bool, editable: bool) -> Result<BuildContext> {
let manifest_file = &self.manifest_path;
if !manifest_file.exists() {
let current_dir =
Expand Down Expand Up @@ -320,7 +320,6 @@ impl BuildOptions {
module_name,
manifest_path: self.manifest_path,
out: wheel_dir,
release,
strip,
skip_auditwheel,
zig: self.zig,
Expand All @@ -331,6 +330,7 @@ impl BuildOptions {
cargo_metadata,
universal2,
editable,
profile: self.profile,
})
}
}
Expand Down Expand Up @@ -829,7 +829,7 @@ mod test {
let mut options = BuildOptions::default();
options.cargo_extra_args.push("--features log".to_string());
options.bindings = Some("bin".to_string());
let context = options.into_build_context(false, false, false).unwrap();
let context = options.into_build_context(false, false).unwrap();
assert_eq!(context.cargo_extra_args, vec!["--features", "log"])
}

Expand Down
4 changes: 2 additions & 2 deletions src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ fn compile_target(

shared_args.extend(context.cargo_extra_args.iter().map(String::as_str));

if context.release {
shared_args.push("--release");
if let Some(profile) = &context.profile {
shared_args.extend(&["--profile", profile]);
}
let mut rustc_args: Vec<&str> = context
.rustc_extra_args
Expand Down
5 changes: 3 additions & 2 deletions src/develop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ pub fn develop(
cargo_extra_args: Vec<String>,
rustc_extra_args: Vec<String>,
venv_dir: &Path,
release: bool,
strip: bool,
extras: Vec<String>,
profile: Option<String>,
) -> Result<()> {
let target = Target::from_target_triple(None)?;
let python = target.get_venv_python(&venv_dir);
Expand All @@ -40,9 +40,10 @@ pub fn develop(
cargo_extra_args,
rustc_extra_args,
universal2: false,
profile,
};

let build_context = build_options.into_build_context(release, strip, true)?;
let build_context = build_options.into_build_context(strip, true)?;

let interpreter = PythonInterpreter::check_executable(&python, &target, &build_context.bridge)?
.ok_or_else(|| {
Expand Down
57 changes: 41 additions & 16 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ enum Opt {
#[clap(flatten)]
build: BuildOptions,
/// Pass --release to cargo
#[clap(long)]
#[clap(long, conflicts_with = "profile")]
release: bool,
/// Strip the library for minimum file size
#[clap(long)]
Expand All @@ -45,7 +45,7 @@ enum Opt {
#[clap(flatten)]
build: BuildOptions,
/// Do not pass --release to cargo
#[clap(long)]
#[clap(long, conflicts_with = "profile")]
debug: bool,
/// Do not strip the library for minimum file size
#[clap(long = "no-strip")]
Expand Down Expand Up @@ -76,7 +76,7 @@ enum Opt {
/// The path to the Cargo.toml
manifest_path: PathBuf,
/// Pass --release to cargo
#[clap(long)]
#[clap(long, conflicts_with = "profile")]
release: bool,
/// Strip the library for minimum file size
#[clap(long)]
Expand All @@ -102,6 +102,10 @@ enum Opt {
multiple_occurrences = false
)]
extras: Vec<String>,
/// The `--profile` to pass to cargo. Can also be set by the `MATURIN_PROFILE` environment
/// variable.
#[clap(long, env = "MATURIN_PROFILE")]
profile: Option<String>,
},
/// Build only a source distribution (sdist) without compiling.
///
Expand Down Expand Up @@ -251,15 +255,18 @@ impl FromStr for Shell {
fn pep517(subcommand: Pep517Command) -> Result<()> {
match subcommand {
Pep517Command::WriteDistInfo {
build_options,
mut build_options,
metadata_directory,
strip,
} => {
assert!(matches!(
build_options.interpreter.as_ref(),
Some(version) if version.len() == 1
));
let context = build_options.into_build_context(true, strip, false)?;
if build_options.profile.is_none() {
build_options.profile = Some("release".to_owned());
}
let context = build_options.into_build_context(strip, false)?;

// Since afaik all other PEP 517 backends also return linux tagged wheels, we do so too
let tags = match context.bridge {
Expand All @@ -285,11 +292,14 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {
println!("{}", context.metadata21.get_dist_info_dir().display());
}
Pep517Command::BuildWheel {
build_options,
mut build_options,
strip,
editable,
} => {
let build_context = build_options.into_build_context(true, strip, editable)?;
if build_options.profile.is_none() {
build_options.profile = Some("release".to_owned());
}
let build_context = build_options.into_build_context(strip, editable)?;
let wheels = build_context.build_wheels()?;
assert_eq!(wheels.len(), 1);
println!("{}", wheels[0].0.to_str().unwrap());
Expand All @@ -303,7 +313,7 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {
out: Some(sdist_directory),
..Default::default()
};
let build_context = build_options.into_build_context(false, false, false)?;
let build_context = build_options.into_build_context(false, false)?;
let (path, _) = build_context
.build_source_distribution()?
.context("Failed to build source distribution")?;
Expand All @@ -322,29 +332,39 @@ fn run() -> Result<()> {

match opt {
Opt::Build {
build,
mut build,
release,
strip,
no_sdist,
} => {
let build_context = build.into_build_context(release, strip, false)?;
if release {
assert!(build.profile.is_none()); // enforced by structopt conflicts_with
build.profile = Some("release".to_owned());
}
let build_context = build.into_build_context(strip, false)?;
if !no_sdist {
build_context.build_source_distribution()?;
}
build_context.build_wheels()?;
}
#[cfg(feature = "upload")]
Opt::Publish {
build,
mut build,
publish,
debug,
no_strip,
no_sdist,
} => {
let build_context = build.into_build_context(!debug, !no_strip, false)?;
if debug {
assert!(build.profile.is_none()); // enforced by structopt conflicts_with
build.profile = Some("dev".to_owned()); // cargo calls the debug profile dev
} else if build.profile.is_none() {
build.profile = Some("release".to_owned());
}
let build_context = build.into_build_context(!no_strip, false)?;

if !build_context.release {
eprintln!("⚠️ Warning: You're publishing debug wheels");
if build_context.profile.as_deref() != Some("release") {
eprintln!("⚠️ Warning: You're not publishing release wheels");
}

let mut wheels = build_context.build_wheels()?;
Expand Down Expand Up @@ -375,6 +395,7 @@ fn run() -> Result<()> {
release,
strip,
extras,
mut profile,
} => {
let venv_dir = match (env::var_os("VIRTUAL_ENV"), env::var_os("CONDA_PREFIX")) {
(Some(dir), None) => PathBuf::from(dir),
Expand All @@ -391,16 +412,20 @@ fn run() -> Result<()> {
)
}
};
if release {
assert!(profile.is_none()); // enforced by structopt conflicts_with
profile = Some("release".to_owned());
};

develop(
bindings,
&manifest_path,
cargo_extra_args,
rustc_extra_args,
&venv_dir,
release,
strip,
extras,
profile,
)?;
}
Opt::SDist { manifest_path, out } => {
Expand All @@ -409,7 +434,7 @@ fn run() -> Result<()> {
out,
..Default::default()
};
let build_context = build_options.into_build_context(false, false, false)?;
let build_context = build_options.into_build_context(false, false)?;
build_context
.build_source_distribution()?
.context("Failed to build source distribution")?;
Expand Down
2 changes: 1 addition & 1 deletion tests/common/develop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ pub fn test_develop(
],
vec![],
&venv_dir,
false,
cfg!(feature = "faster-tests"),
vec![],
None,
)?;

check_installed(package.as_ref(), &python)?;
Expand Down
2 changes: 1 addition & 1 deletion tests/common/editable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub fn test_editable(

let options: BuildOptions = BuildOptions::try_parse_from(cli)?;

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

for (filename, _supported_version) in wheels.iter() {
Expand Down
8 changes: 4 additions & 4 deletions tests/common/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub fn abi3_without_version() -> Result<()> {
];

let options = BuildOptions::try_parse_from(cli)?;
let result = options.into_build_context(false, cfg!(feature = "faster-tests"), false);
let result = options.into_build_context(cfg!(feature = "faster-tests"), false);
if let Err(err) = result {
assert_eq!(err.to_string(),
"You have selected the `abi3` feature but not a minimum version (e.g. the `abi3-py36` feature). \
Expand Down Expand Up @@ -42,7 +42,7 @@ pub fn pyo3_no_extension_module() -> Result<()> {

let options = BuildOptions::try_parse_from(cli)?;
let result = options
.into_build_context(false, cfg!(feature = "faster-tests"), false)?
.into_build_context(cfg!(feature = "faster-tests"), false)?
.build_wheels();
if let Err(err) = result {
if !(err
Expand Down Expand Up @@ -74,7 +74,7 @@ pub fn locked_doesnt_build_without_cargo_lock() -> Result<()> {
"--cargo-extra-args=--target-dir test-crates/targets/locked_doesnt_build_without_cargo_lock",
];
let options = BuildOptions::try_parse_from(cli)?;
let result = options.into_build_context(false, cfg!(feature = "faster-tests"), false);
let result = options.into_build_context(cfg!(feature = "faster-tests"), false);
if let Err(err) = result {
let err_string = err
.source()
Expand Down Expand Up @@ -113,7 +113,7 @@ pub fn invalid_manylinux_does_not_panic() -> Result<()> {
];
let options: BuildOptions = BuildOptions::try_parse_from(cli)?;
let result = options
.into_build_context(false, cfg!(feature = "faster-tests"), false)?
.into_build_context(false, cfg!(feature = "faster-tests"))?
.build_wheels();
if let Err(err) = result {
assert_eq!(err.to_string(), "Error ensuring manylinux_2_99 compliance");
Expand Down
3 changes: 1 addition & 2 deletions tests/common/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub fn test_integration(
}

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

let test_name = package
Expand Down Expand Up @@ -171,7 +171,6 @@ fn create_conda_env(name: &str, major: usize, minor: usize) {

#[cfg(target_os = "windows")]
pub fn test_integration_conda(package: impl AsRef<Path>, bindings: Option<String>) -> Result<()> {
use std::env;
use std::process::Stdio;

let package_string = package.as_ref().join("Cargo.toml").display().to_string();
Expand Down
6 changes: 3 additions & 3 deletions tests/common/other.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub fn test_musl() -> Result<bool> {
"test-crates/wheels/test_musl",
])?;

let build_context = options.into_build_context(false, cfg!(feature = "faster-tests"), false)?;
let build_context = options.into_build_context(cfg!(feature = "faster-tests"), false)?;
let built_lib =
PathBuf::from("test-crates/targets/test_musl/x86_64-unknown-linux-musl/debug/hello-world");
if built_lib.is_file() {
Expand Down Expand Up @@ -100,7 +100,7 @@ pub fn test_workspace_cargo_lock() -> Result<()> {
"test-crates/wheels/test_workspace_cargo_lock",
])?;

let build_context = options.into_build_context(false, false, false)?;
let build_context = options.into_build_context(false, false)?;
let source_distribution = build_context.build_source_distribution()?;
assert!(source_distribution.is_some());

Expand All @@ -126,7 +126,7 @@ pub fn test_source_distribution(
..Default::default()
};

let build_context = build_options.into_build_context(false, false, false)?;
let build_context = build_options.into_build_context(false, false)?;
let (path, _) = build_context
.build_source_distribution()?
.context("Failed to build source distribution")?;
Expand Down