Skip to content

Commit 9dd1b71

Browse files
committed
fix: skip PEP 517 build during pypi satisfiability check
The lockfile-v7 refactor (#5607) introduced `read_local_package_metadata` which fell back to `get_or_build_wheel_metadata` when uv's static `requires_dist` declined. That fallback needs the host conda prefix to have Python, but cross-platform satisfiability runs on platforms that may not be in the lock at all. The resulting `PythonMissingError` panicked out through `LazyBuildDispatch::interpreter` past this path's missing `catch_unwind`, aborting `pixi run` / `pixi reinstall` on hosts whose platform was missing from the lock (e.g. macOS arm64 against a linux-64-only lock). Restrict the function to uv's static path. `Ok(Some)` feeds drift detection; `Ok(None)` (uv's "static doesn't apply" classification) trusts the lock; `Err` surfaces as `PlatformUnsat::FailedToReadLocalMetadata` to force a re-solve. Verified for uv 0.9.5 that `database.requires_dist` itself does not invoke the interpreter. Co-authored-by: Bas Zalmstra <4995967+baszalmstra@users.noreply.github.com> https://claude.ai/code/session_018Wo78WYzQZBRWQQ3zNSsE4
1 parent e3bd52a commit 9dd1b71

2 files changed

Lines changed: 125 additions & 226 deletions

File tree

crates/pixi_core/src/lock_file/satisfiability/pypi.rs

Lines changed: 125 additions & 177 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,7 @@ use url::Url;
3030
use uv_client::{BaseClientBuilder, Connectivity, FlatIndexClient, RegistryClientBuilder};
3131
use uv_configuration::RAYON_INITIALIZE;
3232
use uv_distribution::DistributionDatabase;
33-
use uv_distribution_types::{
34-
ConfigSettings, DependencyMetadata, DirectorySourceDist, Dist, HashPolicy, IndexUrl,
35-
RequirementSource, SourceDist,
36-
};
33+
use uv_distribution_types::{ConfigSettings, DependencyMetadata, IndexUrl, RequirementSource};
3734
use uv_git_types::GitReference;
3835
use uv_pypi_types::PyProjectToml;
3936
use uv_resolver::FlatIndex;
@@ -372,8 +369,9 @@ pub(super) async fn lock_pypi_packages(
372369
for record in &unresolved_pypi_environment.records {
373370
let pkg = record.as_package_data();
374371

375-
// For path-based directories, read metadata from the source tree.
376-
// The result is cached in ctx.static_metadata_cache for later use.
372+
// Drift only matters for mutable sources: local directories. Git
373+
// (commit-pinned), URLs (sha-pinned), and local archives (hashed via
374+
// `record.0.hash`) are content-pinned and trusted as-is.
377375
let metadata = if let UrlOrPath::Path(path) = &**pkg.location() {
378376
let absolute_path = if path.is_absolute() {
379377
Cow::Borrowed(Path::new(path.as_str()))
@@ -382,6 +380,15 @@ pub(super) async fn lock_pypi_packages(
382380
};
383381

384382
if absolute_path.is_dir() {
383+
// Lock says wheel but path is a directory, needs re-solve.
384+
if pkg.as_wheel().is_some() {
385+
return Err(CommandDispatcherError::Failed(Box::new(
386+
PlatformUnsat::DistributionShouldBeSource {
387+
name: pkg.name().clone(),
388+
},
389+
)));
390+
}
391+
385392
let uv_ctx = ctx
386393
.uv_context
387394
.get_or_try_init(|| {
@@ -412,35 +419,24 @@ pub(super) async fn lock_pypi_packages(
412419
static_metadata_cache: ctx.static_metadata_cache,
413420
};
414421

415-
match read_local_package_metadata(&absolute_path, pkg.name(), &build_ctx).await {
416-
Ok(m) => Some(m),
417-
Err(e) => {
418-
return Err(CommandDispatcherError::Failed(Box::new(
422+
// `Ok(None)` = static extraction declined; trust the lock.
423+
read_local_package_metadata(&absolute_path, pkg.name(), &build_ctx)
424+
.await
425+
.map_err(|e| {
426+
CommandDispatcherError::Failed(Box::new(
419427
PlatformUnsat::FailedToReadLocalMetadata(
420428
pkg.name().clone(),
421429
format!("failed to read metadata: {e}"),
422430
),
423-
)));
424-
}
425-
}
431+
))
432+
})?
426433
} else {
427434
None
428435
}
429436
} else {
430437
None
431438
};
432439

433-
// A path-based directory that was parsed as Distribution (e.g. a
434-
// directory named `foo.tar.gz`) needs a re-solve — it should be a
435-
// Source package.
436-
if pkg.as_wheel().is_some() && metadata.is_some() {
437-
return Err(CommandDispatcherError::Failed(Box::new(
438-
PlatformUnsat::DistributionShouldBeSource {
439-
name: pkg.name().clone(),
440-
},
441-
)));
442-
}
443-
444440
// Determine the version: prefer the wheel version from the lock file,
445441
// fall back to the version read from the source tree metadata.
446442
let version = pkg
@@ -471,36 +467,45 @@ struct BuildMetadataContext<'a> {
471467
static_metadata_cache: &'a DashMap<PathBuf, pypi_metadata::LocalPackageMetadata>,
472468
}
473469

474-
/// Read metadata for a local directory package using UV's DistributionDatabase.
470+
/// Read static metadata for a local directory PyPI package via uv's
471+
/// [`DistributionDatabase::requires_dist`]. Result is cached in
472+
/// [`BuildMetadataContext::static_metadata_cache`] (platform-independent).
473+
///
474+
/// Outcomes:
475+
/// - `Ok(Some(_))`: extracted; caller uses it for drift detection.
476+
/// - `Ok(None)`: static doesn't apply (dynamic field, missing pyproject,
477+
/// parse error). Caller trusts the lock.
478+
/// - `Err(FailedToReadLocalMetadata)`: uv reported a hard error
479+
/// (`MissingName`, schema mismatch, workspace I/O, ...). The source has
480+
/// diverged, propagate as drift to force a re-solve.
481+
///
482+
/// We never build here. The build fallback needs the host conda prefix to
483+
/// have Python, which fails on cross-platform satisfiability when the host
484+
/// platform isn't in the lock; the resulting `PythonMissingError` panics
485+
/// through `LazyBuildDispatch::interpreter` past this path's missing
486+
/// `catch_unwind`.
475487
///
476-
/// This first tries to extract metadata statically via `database.requires_dist()`,
477-
/// which parses the pyproject.toml without building. If static extraction fails
478-
/// (e.g., dynamic dependencies), it falls back to building the wheel metadata.
488+
/// Caveat: `Ok(None)` skips drift detection silently. `pixi run` against a
489+
/// satisfied lock won't re-introspect, so edits to genuinely-dynamic
490+
/// packages need an explicit `pixi install` / `pixi lock`.
479491
///
480-
/// Static metadata is cached across platforms since it doesn't depend on the platform.
492+
/// Assumption: `requires_dist` itself doesn't invoke `interpreter()`.
493+
/// Verified for uv 0.9.5 (`source_tree_requires_dist`). If a future uv
494+
/// breaks that, this site panics again. Wrap the call in `catch_unwind`
495+
/// and map `last_error` to `Ok(None)`.
481496
async fn read_local_package_metadata(
482497
directory: &Path,
483498
package_name: &pep508_rs::PackageName,
484499
ctx: &BuildMetadataContext<'_>,
485-
) -> Result<pypi_metadata::LocalPackageMetadata, PlatformUnsat> {
500+
) -> Result<Option<pypi_metadata::LocalPackageMetadata>, PlatformUnsat> {
486501
// Check if we already have static metadata cached for this directory
487502
if let Some(cached_metadata) = ctx.static_metadata_cache.get(directory) {
488-
tracing::debug!("Package {} - using cached static metadata", package_name);
489-
return Ok(cached_metadata.value().clone());
503+
tracing::debug!(package = %package_name, "using cached static metadata");
504+
return Ok(Some(cached_metadata.value().clone()));
490505
}
491506

492507
let pypi_options = ctx.environment.pypi_options();
493508

494-
// Look up editability from the manifest (not stored in lock file).
495-
// This affects which PEP 517 hook uv calls
496-
// (prepare_metadata_for_build_editable vs prepare_metadata_for_build_wheel).
497-
let editable = ctx
498-
.environment
499-
.pypi_dependencies(Some(ctx.platform))
500-
.get(package_name)
501-
.and_then(|specs| specs.iter().find_map(|spec| spec.editable()))
502-
.unwrap_or(false);
503-
504509
// Find the Python interpreter from locked records
505510
let python_record = ctx
506511
.locked_pixi_records
@@ -696,148 +701,91 @@ async fn read_local_package_metadata(
696701
ctx.uv_context.concurrency.downloads,
697702
);
698703

699-
// Try to read pyproject.toml and use requires_dist() first
704+
// Missing or unparsable pyproject -> trust the lock.
700705
let pyproject_path = directory.join("pyproject.toml");
701-
if let Ok(contents) = fs_err::read_to_string(&pyproject_path) {
702-
// Parse with toml_edit for version/requires_python
703-
if let Ok(toml) = contents.parse::<toml_edit::DocumentMut>() {
704-
let version = toml
705-
.get("project")
706-
.and_then(|p| p.get("version"))
707-
.and_then(|v| v.as_str())
708-
.and_then(|v| v.parse::<pep440_rs::Version>().ok());
709-
710-
let requires_python = toml
711-
.get("project")
712-
.and_then(|p| p.get("requires-python"))
713-
.and_then(|v| v.as_str())
714-
.and_then(|rp| rp.parse::<VersionSpecifiers>().ok());
715-
716-
// Parse pyproject.toml with UV's parser for requires_dist
717-
if let Ok(pyproject_toml) = PyProjectToml::from_toml(&contents) {
718-
// Try to extract requires_dist statically using UV's database
719-
// The `dynamic` flag on `RequiresDist` is true when any
720-
// field is listed in `[project.dynamic]`, not just
721-
// dependencies. Since we handle version separately (and
722-
// skip comparison when it's `None`), we accept the
723-
// statically extracted deps regardless of the flag.
724-
match database.requires_dist(directory, &pyproject_toml).await {
725-
Ok(Some(requires_dist)) => {
726-
tracing::debug!(
727-
"Package {} - extracted requires_dist using database.requires_dist(). Dynamic: {}",
728-
package_name,
729-
requires_dist.dynamic
730-
);
731-
732-
// Convert uv requirements to pep508_rs requirements
733-
let requires_dist_converted: Result<Vec<pep508_rs::Requirement>, _> =
734-
requires_dist
735-
.requires_dist
736-
.iter()
737-
.map(|req| {
738-
let req_str = req.to_string();
739-
req_str.parse::<pep508_rs::Requirement>().map_err(|e| {
740-
PlatformUnsat::FailedToReadLocalMetadata(
741-
package_name.clone(),
742-
format!("Invalid requirement: {e}"),
743-
)
744-
})
745-
})
746-
.collect();
747-
748-
if let Ok(requires_dist_vec) = requires_dist_converted {
749-
// Match the wheel METADATA shape used at lock time.
750-
let empty_optional_deps = IndexMap::new();
751-
let optional_deps = pyproject_toml
752-
.project
753-
.as_ref()
754-
.and_then(|p| p.optional_dependencies.as_ref())
755-
.unwrap_or(&empty_optional_deps);
756-
let expanded = pypi_metadata::expand_self_extras(
757-
requires_dist_vec,
758-
package_name,
759-
optional_deps,
760-
);
761-
let metadata = pypi_metadata::LocalPackageMetadata {
762-
version,
763-
requires_dist: expanded,
764-
requires_python,
765-
};
766-
ctx.static_metadata_cache
767-
.insert(directory.to_path_buf(), metadata.clone());
768-
return Ok(metadata);
769-
}
770-
}
771-
Ok(None) => {
772-
tracing::debug!(
773-
"Package {} - requires_dist() returned None, falling back to build",
774-
package_name
775-
);
776-
}
777-
Err(e) => {
778-
tracing::debug!(
779-
"Package {} - requires_dist() failed: {}, falling back to build",
780-
package_name,
781-
e
782-
);
783-
}
784-
}
785-
}
786-
}
787-
}
788-
789-
// Fall back to building the wheel metadata
790-
tracing::debug!(
791-
"Package {} - building wheel metadata with get_or_build_wheel_metadata()",
792-
package_name
793-
);
706+
let Ok(contents) = fs_err::read_to_string(&pyproject_path) else {
707+
tracing::debug!(package = %package_name, "no readable pyproject.toml");
708+
return Ok(None);
709+
};
710+
let Ok(pyproject_toml) = PyProjectToml::from_toml(&contents) else {
711+
tracing::debug!(package = %package_name, "pyproject.toml could not be parsed");
712+
return Ok(None);
713+
};
794714

795-
// Create the directory source dist
796-
let uv_package_name =
797-
uv_normalize::PackageName::from_str(package_name.as_ref()).map_err(|e| {
798-
PlatformUnsat::FailedToReadLocalMetadata(
715+
// Reuse the parsed project table for version/requires-python so a
716+
// partial uv result still gives us something to compare against the
717+
// lock. uv's `Version` is a separate type, round-trip via string.
718+
let project = pyproject_toml.project.as_ref();
719+
let version = project
720+
.and_then(|p| p.version.as_ref())
721+
.and_then(|v| v.to_string().parse::<pep440_rs::Version>().ok());
722+
let requires_python = project
723+
.and_then(|p| p.requires_python.as_ref())
724+
.and_then(|rp| rp.parse::<VersionSpecifiers>().ok());
725+
726+
// The `dynamic` flag is true when any field is in `[project.dynamic]`,
727+
// not just dependencies; since we handle version separately we accept
728+
// the deps regardless.
729+
let requires_dist = match database.requires_dist(directory, &pyproject_toml).await {
730+
Ok(Some(rd)) => {
731+
tracing::debug!(
732+
package = %package_name,
733+
dynamic = rd.dynamic,
734+
"extracted requires_dist statically",
735+
);
736+
rd
737+
}
738+
// uv: "static doesn't apply", trust lock.
739+
Ok(None) => {
740+
tracing::debug!(
741+
package = %package_name,
742+
"requires_dist returned None, trusting lock",
743+
);
744+
return Ok(None);
745+
}
746+
// uv: hard error, source diverged, force re-solve.
747+
Err(e) => {
748+
return Err(PlatformUnsat::FailedToReadLocalMetadata(
799749
package_name.clone(),
800-
format!("Invalid package name: {e}"),
801-
)
802-
})?;
750+
format!("static metadata extraction failed: {e}"),
751+
));
752+
}
753+
};
803754

804-
let install_path = directory.to_path_buf();
805-
let file_url = url::Url::from_file_path(&install_path).map_err(|_| {
806-
PlatformUnsat::FailedToReadLocalMetadata(
807-
package_name.clone(),
808-
format!("Failed to convert path to URL: {}", install_path.display()),
809-
)
810-
})?;
811-
let verbatim_url = uv_pep508::VerbatimUrl::from_url(file_url.into());
812-
let source_dist = DirectorySourceDist {
813-
name: uv_package_name,
814-
install_path: install_path.into_boxed_path(),
815-
editable: Some(editable),
816-
r#virtual: Some(false),
817-
url: verbatim_url,
755+
// Conversion failures are real bugs (uv produced an unround-trippable
756+
// requirement), not "static doesn't apply", so propagate them.
757+
let requires_dist_vec: Vec<pep508_rs::Requirement> = requires_dist
758+
.requires_dist
759+
.iter()
760+
.map(|req| {
761+
req.to_string()
762+
.parse::<pep508_rs::Requirement>()
763+
.map_err(|e| {
764+
PlatformUnsat::FailedToReadLocalMetadata(
765+
package_name.clone(),
766+
format!("Invalid requirement: {e}"),
767+
)
768+
})
769+
})
770+
.collect::<Result<_, _>>()?;
771+
772+
// Match the wheel METADATA shape used at lock time.
773+
let empty_optional_deps = IndexMap::new();
774+
let optional_deps = project
775+
.and_then(|p| p.optional_dependencies.as_ref())
776+
.unwrap_or(&empty_optional_deps);
777+
let expanded =
778+
pypi_metadata::expand_self_extras(requires_dist_vec, package_name, optional_deps);
779+
let metadata = pypi_metadata::LocalPackageMetadata {
780+
version,
781+
requires_dist: expanded,
782+
requires_python,
818783
};
819784

820-
// Build the metadata
821-
let metadata_response = database
822-
.get_or_build_wheel_metadata(
823-
&Dist::Source(SourceDist::Directory(source_dist)),
824-
HashPolicy::None,
825-
)
826-
.await
827-
.map_err(|e| {
828-
PlatformUnsat::FailedToReadLocalMetadata(
829-
package_name.clone(),
830-
format!("Failed to build metadata: {e}"),
831-
)
832-
})?;
785+
ctx.static_metadata_cache
786+
.insert(directory.to_path_buf(), metadata.clone());
833787

834-
// Convert UV metadata to our format
835-
pypi_metadata::from_uv_metadata(&metadata_response.metadata).map_err(|e| {
836-
PlatformUnsat::FailedToReadLocalMetadata(
837-
package_name.clone(),
838-
format!("Failed to convert metadata: {e}"),
839-
)
840-
})
788+
Ok(Some(metadata))
841789
}
842790

843791
#[cfg(test)]

0 commit comments

Comments
 (0)