Skip to content

Commit

Permalink
Implement editable installs in dev command
Browse files Browse the repository at this point in the history
  • Loading branch information
@konstin
konstin committed Dec 5, 2023
1 parent 0466231 commit bb5dcfe
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 31 deletions.
75 changes: 55 additions & 20 deletions crates/puffin-build/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
//! <https://packaging.python.org/en/latest/specifications/source-distribution-format/>

use std::fmt::{Display, Formatter};
use std::io;
use std::io::BufRead;
use std::path::{Path, PathBuf};
use std::process::{Command, Output};
use std::str::FromStr;
use std::sync::Arc;
use std::{env, io};

use flate2::read::GzDecoder;
use fs_err as fs;
Expand Down Expand Up @@ -50,6 +50,8 @@ pub enum Error {
InvalidSourceDist(String),
#[error("Invalid pyproject.toml")]
InvalidPyprojectToml(#[from] toml::de::Error),
#[error("Editable installs with setup.py legacy builds are unsupported, please specify a build backend in pyproject.toml")]
EditableSetupPy,
#[error("Failed to install requirements from {0}")]
RequirementsInstall(&'static str, #[source] anyhow::Error),
#[error("Failed to create temporary virtual environment")]
Expand Down Expand Up @@ -180,6 +182,24 @@ impl Pep517Backend {
}
}

#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
pub enum BuildKind {
/// A regular PEP 517 wheel build
#[default]
Wheel,
/// A PEP 660 editable installation wheel build
Editable,
}

impl Display for BuildKind {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
BuildKind::Wheel => f.write_str("wheel"),
BuildKind::Editable => f.write_str("editable"),
}
}
}

/// Uses an [`Arc`] internally, clone freely
#[derive(Debug, Default, Clone)]
pub struct SourceBuildContext {
Expand Down Expand Up @@ -211,6 +231,8 @@ pub struct SourceBuild {
metadata_directory: Option<PathBuf>,
/// Package id such as `foo-1.2.3`, for error reporting
package_id: String,
/// Whether we do a regular PEP 517 build or an PEP 660 editable build
build_kind: BuildKind,
}

impl SourceBuild {
Expand All @@ -225,6 +247,7 @@ impl SourceBuild {
build_context: &impl BuildContext,
source_build_context: SourceBuildContext,
source_dist: &str,
build_kind: BuildKind,
) -> Result<SourceBuild, Error> {
let temp_dir = tempdir()?;

Expand Down Expand Up @@ -323,6 +346,7 @@ impl SourceBuild {
pep517_backend,
build_context,
source_dist,
build_kind,
)
.await?;
} else if !source_tree.join("setup.py").is_file() {
Expand All @@ -337,6 +361,7 @@ impl SourceBuild {
source_tree,
pep517_backend,
venv,
build_kind,
metadata_directory: None,
package_id: source_dist.to_string(),
})
Expand Down Expand Up @@ -374,7 +399,7 @@ impl SourceBuild {
name="prepare_metadata_for_build_wheel",
python_version = %self.venv.interpreter().version()
);
let output = run_python_script(&self.venv.python_executable(), &script, &self.source_tree)?;
let output = run_python_script(&self.venv, &script, &self.source_tree)?;
drop(span);
if !output.status.success() {
return Err(Error::from_command_output(
Expand Down Expand Up @@ -422,10 +447,13 @@ impl SourceBuild {
if let Some(pep517_backend) = &self.pep517_backend {
// Prevent clashes from two puffin processes building wheels in parallel
let tmp_dir = tempdir_in(&wheel_dir)?;
let filename = self.pep517_build_wheel(tmp_dir.path(), pep517_backend)?;
let filename = self.pep517_build(tmp_dir.path(), pep517_backend)?;
fs::rename(tmp_dir.path().join(&filename), wheel_dir.join(&filename))?;
Ok(filename)
} else {
if self.build_kind != BuildKind::Wheel {
return Err(Error::EditableSetupPy);
}
// We checked earlier that setup.py exists
let python_interpreter = self.venv.python_executable();
let output = Command::new(&python_interpreter)
Expand Down Expand Up @@ -457,7 +485,7 @@ impl SourceBuild {
}
}

fn pep517_build_wheel(
fn pep517_build(
&self,
wheel_dir: &Path,
pep517_backend: &Pep517Backend,
Expand All @@ -469,25 +497,28 @@ impl SourceBuild {
format!(r#""{}""#, escape_path_for_python(path))
});
debug!(
"Calling `{}.build_wheel(metadata_directory={})`",
pep517_backend.backend, metadata_directory
"Calling `{}.build_{}(metadata_directory={})`",
pep517_backend.backend, self.build_kind, metadata_directory
);
let escaped_wheel_dir = escape_path_for_python(wheel_dir);
let script = formatdoc! {
r#"{}
print(backend.build_wheel("{}", metadata_directory={}))
"#, pep517_backend.backend_import(), escaped_wheel_dir, metadata_directory
print(backend.build_{}("{}", metadata_directory={}))
"#, pep517_backend.backend_import(), self.build_kind, escaped_wheel_dir, metadata_directory
};
let span = info_span!(
"run_python_script",
name="build_wheel",
name=format!("build_{}", self.build_kind),
python_version = %self.venv.interpreter().version()
);
let output = run_python_script(&self.venv.python_executable(), &script, &self.source_tree)?;
drop(span);
if !output.status.success() {
return Err(Error::from_command_output(
"Build backend failed to build wheel through `build_wheel()`".to_string(),
format!(
"Build backend failed to build wheel through `build_{}()`",
self.build_kind
),
&output,
&self.package_id,
));
Expand All @@ -498,8 +529,10 @@ impl SourceBuild {
distribution_filename.filter(|wheel| wheel_dir.join(wheel).is_file())
else {
return Err(Error::from_command_output(
"Build backend did not return the wheel filename through `build_wheel()`"
.to_string(),
format!(
"Build backend failed to build wheel through `build_{}()`",
self.build_kind
),
&output,
&self.package_id,
));
Expand All @@ -521,22 +554,23 @@ async fn create_pep517_build_environment(
pep517_backend: &Pep517Backend,
build_context: &impl BuildContext,
package_id: &str,
build_kind: BuildKind,
) -> Result<(), Error> {
debug!(
"Calling `{}.get_requires_for_build_wheel()`",
pep517_backend.backend
"Calling `{}.get_requires_for_build_{}()`",
pep517_backend.backend, build_kind
);
let script = formatdoc! {
r#"
{}
import json
if get_requires_for_build_wheel := getattr(backend, "get_requires_for_build_wheel", None):
requires = get_requires_for_build_wheel()
if get_requires_for_build := getattr(backend, "get_requires_for_build_{}", None):
requires = get_requires_for_build()
else:
requires = []
print(json.dumps(requires))
"#, pep517_backend.backend_import()
"#, pep517_backend.backend_import(), build_kind
};
let span = info_span!(
"get_requires_for_build_wheel",
Expand All @@ -547,8 +581,9 @@ async fn create_pep517_build_environment(
drop(span);
if !output.status.success() {
return Err(Error::from_command_output(
"Build backend failed to determine extras requires with `get_requires_for_build_wheel`"
.to_string(),
format!(
"Build backend failed to determine extras requires with `build_{build_kind}()`",
),
&output,
package_id,
));
Expand All @@ -566,7 +601,7 @@ async fn create_pep517_build_environment(
Error::from_command_output(
format!(
"Build backend failed to return extras requires with \
`get_requires_for_build_wheel`: {err}"
`get_requires_for_build_{build_kind}`: {err}"
),
&output,
package_id,
Expand Down
32 changes: 22 additions & 10 deletions crates/puffin-dev/src/build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use clap::Parser;
use fs_err as fs;

use platform_host::Platform;
use puffin_build::{BuildKind, SourceBuild, SourceBuildContext};
use puffin_cache::{Cache, CacheArgs};
use puffin_client::RegistryClientBuilder;
use puffin_dispatch::BuildDispatch;
Expand All @@ -26,6 +27,10 @@ pub(crate) struct BuildArgs {
sdist: PathBuf,
/// The subdirectory to build within the source distribution.
subdirectory: Option<PathBuf>,
/// You can edit the python sources of an editable install and the changes will be used without
/// the need to reinstall it.
#[clap(short, long)]
editable: bool,
#[command(flatten)]
cache_args: CacheArgs,
}
Expand All @@ -38,6 +43,11 @@ pub(crate) async fn build(args: BuildArgs) -> Result<PathBuf> {
} else {
env::current_dir()?
};
let build_kind = if args.editable {
BuildKind::Editable
} else {
BuildKind::Wheel
};

let cache = Cache::try_from(args.cache_args)?;

Expand All @@ -52,14 +62,16 @@ pub(crate) async fn build(args: BuildArgs) -> Result<PathBuf> {
false,
IndexUrls::default(),
);
let wheel = build_dispatch
.build_source(
&args.sdist,
args.subdirectory.as_deref(),
&wheel_dir,
// Good enough for the dev command
&args.sdist.display().to_string(),
)
.await?;
Ok(wheel_dir.join(wheel))
let builder = SourceBuild::setup(
&args.sdist,
args.subdirectory.as_deref(),
&build_dispatch.interpreter(),
&build_dispatch,
SourceBuildContext::default(),
// Good enough for the dev command
&args.sdist.display().to_string(),
build_kind,
)
.await?;
Ok(wheel_dir.join(builder.build(&wheel_dir)?))
}
3 changes: 2 additions & 1 deletion crates/puffin-dispatch/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use tracing::{debug, instrument};
use distribution_types::Metadata;
use pep508_rs::Requirement;
use platform_tags::Tags;
use puffin_build::{SourceBuild, SourceBuildContext};
use puffin_build::{BuildKind, SourceBuild, SourceBuildContext};
use puffin_cache::Cache;
use puffin_client::RegistryClient;
use puffin_distribution::DistributionDatabase;
Expand Down Expand Up @@ -242,6 +242,7 @@ impl BuildContext for BuildDispatch {
self,
self.source_build_context.clone(),
source_dist,
BuildKind::Wheel,
)
.await?;
Ok(builder.build(wheel_dir)?)
Expand Down

0 comments on commit bb5dcfe

Please sign in to comment.