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 12, 2023
1 parent c25d524 commit 0fe1769
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 33 deletions.
74 changes: 52 additions & 22 deletions crates/puffin-build/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,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 @@ -181,6 +183,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 @@ -212,6 +232,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 @@ -226,6 +248,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 @@ -324,6 +347,7 @@ impl SourceBuild {
pep517_backend,
build_context,
source_dist,
build_kind,
)
.await?;
} else if !source_tree.join("setup.py").is_file() {
Expand All @@ -338,6 +362,7 @@ impl SourceBuild {
source_tree,
pep517_backend,
venv,
build_kind,
metadata_directory: None,
package_id: source_dist.to_string(),
})
Expand Down Expand Up @@ -423,16 +448,16 @@ 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)
.await?;
let filename = self.pep517_build(tmp_dir.path(), pep517_backend).await?;

let from = tmp_dir.path().join(&filename);
let to = wheel_dir.join(&filename);
fs_err::rename(from, to)?;

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 @@ -468,7 +493,7 @@ impl SourceBuild {
}
}

async fn pep517_build_wheel(
async fn pep517_build(
&self,
wheel_dir: &Path,
pep517_backend: &Pep517Backend,
Expand All @@ -480,26 +505,29 @@ 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).await?;
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 @@ -510,8 +538,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 @@ -533,23 +563,24 @@ 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
get_requires_for_build_wheel = getattr(backend, "get_requires_for_build_wheel", None)
if get_requires_for_build_wheel:
requires = get_requires_for_build_wheel()
get_requires_for_build = getattr(backend, "get_requires_for_build_{}", None)
if get_requires_for_build:
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 @@ -560,8 +591,7 @@ async fn create_pep517_build_environment(
drop(span);
if !output.status.success() {
return Err(Error::from_command_output(
"Build backend failed to determine extra requires with `get_requires_for_build_wheel`"
.to_string(),
format!("Build backend failed to determine extra requires with `build_{build_kind}()`",),
&output,
package_id,
));
Expand All @@ -577,7 +607,7 @@ async fn create_pep517_build_environment(
let extra_requires: Vec<Requirement> = extra_requires.map_err(|err| {
Error::from_command_output(
format!(
"Build backend failed to return extra requires with `get_requires_for_build_wheel`: {err}"
"Build backend failed to return extra requires with `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 @@ -13,7 +13,7 @@ use tracing::{debug, instrument};
use distribution_types::{CachedDist, 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_installer::{Downloader, InstallPlan, Installer, Reinstall};
Expand Down Expand Up @@ -230,6 +230,7 @@ impl BuildContext for BuildDispatch {
self,
self.source_build_context.clone(),
source_dist,
BuildKind::Wheel,
)
.await?;
Ok(builder.build(wheel_dir).await?)
Expand Down

0 comments on commit 0fe1769

Please sign in to comment.