From cf697b9e34f116198a4dc97444ea8d680430c5c0 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 31 Oct 2025 11:56:39 +0100 Subject: [PATCH 01/21] fix: use type to better capture cache data --- .../src/build/build_cache.rs | 4 +- .../pixi_command_dispatcher/src/build/mod.rs | 55 +++++++++++++++++++ .../src/install_pixi/mod.rs | 2 +- .../src/source_build/mod.rs | 12 +++- .../src/source_build_cache_status/mod.rs | 15 +++-- .../tests/integration/main.rs | 13 +++-- 6 files changed, 85 insertions(+), 16 deletions(-) diff --git a/crates/pixi_command_dispatcher/src/build/build_cache.rs b/crates/pixi_command_dispatcher/src/build/build_cache.rs index c76324df6f..97a9988249 100644 --- a/crates/pixi_command_dispatcher/src/build/build_cache.rs +++ b/crates/pixi_command_dispatcher/src/build/build_cache.rs @@ -19,7 +19,7 @@ use thiserror::Error; use tokio::io::{AsyncReadExt, AsyncSeekExt, AsyncWriteExt}; use xxhash_rust::xxh3::Xxh3; -use crate::build::source_checkout_cache_key; +use crate::build::{SourceCodeLocation, source_checkout_cache_key}; /// A cache for caching build artifacts of a source checkout. #[derive(Clone)] @@ -249,7 +249,7 @@ pub struct BuildHostPackage { pub repodata_record: RepoDataRecord, /// The source location from which the package was built. - pub source: Option, + pub source: Option, } /// A cache entry returned by [`BuildCache::entry`] which enables diff --git a/crates/pixi_command_dispatcher/src/build/mod.rs b/crates/pixi_command_dispatcher/src/build/mod.rs index 38d5b68d79..3318d74bd4 100644 --- a/crates/pixi_command_dispatcher/src/build/mod.rs +++ b/crates/pixi_command_dispatcher/src/build/mod.rs @@ -8,6 +8,7 @@ mod move_file; pub(crate) mod source_metadata_cache; mod work_dir_key; +use core::fmt; use std::hash::{Hash, Hasher}; use base64::{Engine, engine::general_purpose::URL_SAFE_NO_PAD}; @@ -22,12 +23,66 @@ pub use dependencies::{ }; pub(crate) use move_file::{MoveError, move_file}; use pixi_record::PinnedSourceSpec; +use serde::{Deserialize, Serialize}; use url::Url; pub use work_dir_key::{SourceRecordOrCheckout, WorkDirKey}; use xxhash_rust::xxh3::Xxh3; const KNOWN_SUFFIXES: [&str; 3] = [".git", ".tar.gz", ".zip"]; +/// Stores the two possible locations for the source code, +/// in the case of an out-of-tree source build. +/// +/// Something which looks like: +/// ```toml +/// [package.build] +/// source = { path = "some-path" } +/// ``` +/// +/// We want to prefer that location for our cache checks +#[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)] +pub struct SourceCodeLocation { + /// The location of the manifest and the possible source code + manifest_source: PinnedSourceSpec, + /// The location of the source code that should be queried and build + build_source: Option, +} + +impl SourceCodeLocation { + pub fn new(manifest_source: PinnedSourceSpec, build_source: Option) -> Self { + Self { + manifest_source, + build_source, + } + } + + /// Get the reference to the manifest source + pub fn manifest_source(&self) -> &PinnedSourceSpec { + &self.manifest_source + } + + /// Get the pinned source spec to the actual source code + /// This is the normally the path to the manifest_source + /// but when set is the path to the build_source + pub fn source_code(&self) -> &PinnedSourceSpec { + self.build_source.as_ref().unwrap_or(&self.manifest_source) + } +} + +impl std::fmt::Display for SourceCodeLocation { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "(manifest-src: {}, build-src: {})", + self.manifest_source(), + self.build_source + .as_ref() + .map(|build| format!("{}", build)) + .unwrap_or("undefined".to_string()) + ) + } +} + /// Try to deduce a name from a url. fn pretty_url_name(url: &Url) -> String { if let Some(last_segment) = url diff --git a/crates/pixi_command_dispatcher/src/install_pixi/mod.rs b/crates/pixi_command_dispatcher/src/install_pixi/mod.rs index 98b6b1de63..beab694cc1 100644 --- a/crates/pixi_command_dispatcher/src/install_pixi/mod.rs +++ b/crates/pixi_command_dispatcher/src/install_pixi/mod.rs @@ -230,7 +230,7 @@ impl InstallPixiEnvironmentSpec { force, // When we install a pixi environment we always build in development mode. build_profile: BuildProfile::Development, - build_source: None, + build_source: source_record.build_source.clone(), }) .await?; diff --git a/crates/pixi_command_dispatcher/src/source_build/mod.rs b/crates/pixi_command_dispatcher/src/source_build/mod.rs index 7914bf8f3f..f90e5ae2d0 100644 --- a/crates/pixi_command_dispatcher/src/source_build/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_build/mod.rs @@ -33,7 +33,7 @@ use crate::{ build::{ BuildCacheError, BuildHostEnvironment, BuildHostPackage, CachedBuild, CachedBuildSourceInfo, Dependencies, DependenciesError, MoveError, PackageBuildInputHash, - PixiRunExports, SourceRecordOrCheckout, WorkDirKey, move_file, + PixiRunExports, SourceCodeLocation, SourceRecordOrCheckout, WorkDirKey, move_file, }, package_identifier::PackageIdentifier, }; @@ -147,7 +147,10 @@ impl SourceBuildSpec { .source_build_cache_status(SourceBuildCacheStatusSpec { package: self.package.clone(), build_environment: self.build_environment.clone(), - source: self.manifest_source.clone(), + source: super::build::SourceCodeLocation::new( + self.manifest_source.clone(), + self.build_source.clone(), + ), channels: self.channels.clone(), channel_config: self.channel_config.clone(), enabled_protocols: self.enabled_protocols.clone(), @@ -463,7 +466,10 @@ impl SourceBuildSpec { .expect("the source record should be present in the result sources"); BuildHostPackage { repodata_record, - source: Some(source.manifest_source), + source: Some(SourceCodeLocation::new( + source.manifest_source, + source.build_source, + )), } } }) diff --git a/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs b/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs index 544d7f80ff..6084d0b6ae 100644 --- a/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs @@ -15,6 +15,7 @@ use crate::{ PackageIdentifier, SourceCheckoutError, build::{ BuildCacheEntry, BuildCacheError, BuildInput, CachedBuild, PackageBuildInputHashBuilder, + SourceCodeLocation, }, }; @@ -35,7 +36,7 @@ pub struct SourceBuildCacheStatusSpec { pub package: PackageIdentifier, /// Describes the source location of the package to query. - pub source: PinnedSourceSpec, + pub source: SourceCodeLocation, /// The channels to use when building source packages. pub channels: Vec, @@ -135,7 +136,7 @@ impl SourceBuildCacheStatusSpec { }; let (cached_build, build_cache_entry) = command_dispatcher .build_cache() - .entry(&self.source, &build_input) + .entry(&self.source.manifest_source(), &build_input) .await .map_err(SourceBuildCacheStatusError::BuildCache) .map_err(CommandDispatcherError::Failed)?; @@ -176,13 +177,17 @@ impl SourceBuildCacheStatusSpec { let source = &self.source; // Immutable source records are always considered valid. - if source.is_immutable() { + if source.source_code().is_immutable() { return Ok(CachedBuildStatus::UpToDate(cached_build)); } // Check if the project configuration has changed. let cached_build = match self - .check_package_configuration_changed(command_dispatcher, cached_build, source) + .check_package_configuration_changed( + command_dispatcher, + cached_build, + source.manifest_source(), + ) .await? { CachedBuildStatus::UpToDate(cached_build) | CachedBuildStatus::New(cached_build) => { @@ -196,7 +201,7 @@ impl SourceBuildCacheStatusSpec { // Determine if the package is out of date by checking the source let cached_build = match self - .check_source_out_of_date(command_dispatcher, cached_build, source) + .check_source_out_of_date(command_dispatcher, cached_build, source.source_code()) .await? { CachedBuildStatus::UpToDate(cached_build) | CachedBuildStatus::New(cached_build) => { diff --git a/crates/pixi_command_dispatcher/tests/integration/main.rs b/crates/pixi_command_dispatcher/tests/integration/main.rs index 6d016344ad..11e41c518a 100644 --- a/crates/pixi_command_dispatcher/tests/integration/main.rs +++ b/crates/pixi_command_dispatcher/tests/integration/main.rs @@ -15,7 +15,7 @@ use pixi_build_frontend::{BackendOverride, InMemoryOverriddenBackends}; use pixi_command_dispatcher::{ BuildEnvironment, CacheDirs, CommandDispatcher, Executor, InstallPixiEnvironmentSpec, InstantiateToolEnvironmentSpec, PackageIdentifier, PixiEnvironmentSpec, - SourceBuildCacheStatusSpec, + SourceBuildCacheStatusSpec, build::SourceCodeLocation, }; use pixi_config::default_channel_config; use pixi_record::PinnedPathSpec; @@ -563,10 +563,13 @@ async fn source_build_cache_status_clear_works() { let spec = SourceBuildCacheStatusSpec { package: pkg, - source: PinnedPathSpec { - path: tmp_dir.path().to_string_lossy().into_owned().into(), - } - .into(), + source: SourceCodeLocation::new( + PinnedPathSpec { + path: tmp_dir.path().to_string_lossy().into_owned().into(), + } + .into(), + None, + ), channels: Vec::::new(), build_environment: build_env, channel_config: default_channel_config(), From e1974f1243fbf732b6435d8be77c2c20f2fcd5e4 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 31 Oct 2025 16:11:28 +0100 Subject: [PATCH 02/21] wip: some logging --- crates/pixi_command_dispatcher/src/source_build/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/pixi_command_dispatcher/src/source_build/mod.rs b/crates/pixi_command_dispatcher/src/source_build/mod.rs index f90e5ae2d0..e674c102d4 100644 --- a/crates/pixi_command_dispatcher/src/source_build/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_build/mod.rs @@ -125,6 +125,7 @@ impl SourceBuildSpec { name = "source-build", fields( source= %self.manifest_source, + build_source= ?self.build_source, package = %self.package, ) )] @@ -239,6 +240,8 @@ impl SourceBuildSpec { .await .map_err_with(SourceBuildError::Discovery)?; + tracing::error!("{:?}", discovered_backend.init_params); + // Compute the package input hash for caching purposes. let package_build_input_hash = PackageBuildInputHash::from(discovered_backend.as_ref()); From 3370546e5efda587ee22e48d4ae46f0ee891c3e3 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Mon, 3 Nov 2025 14:56:48 +0100 Subject: [PATCH 03/21] fix: out-of-source caching --- crates/pixi_command_dispatcher/src/build/mod.rs | 9 ++++++++- .../src/build_backend_metadata/mod.rs | 11 +++++++++-- .../src/command_dispatcher/mod.rs | 12 +++++++++++- .../pixi_command_dispatcher/src/source_build/mod.rs | 5 +++-- .../src/source_build_cache_status/mod.rs | 13 ++++++++----- 5 files changed, 39 insertions(+), 11 deletions(-) diff --git a/crates/pixi_command_dispatcher/src/build/mod.rs b/crates/pixi_command_dispatcher/src/build/mod.rs index 3318d74bd4..08039e4ed2 100644 --- a/crates/pixi_command_dispatcher/src/build/mod.rs +++ b/crates/pixi_command_dispatcher/src/build/mod.rs @@ -8,7 +8,6 @@ mod move_file; pub(crate) mod source_metadata_cache; mod work_dir_key; -use core::fmt; use std::hash::{Hash, Hasher}; use base64::{Engine, engine::general_purpose::URL_SAFE_NO_PAD}; @@ -67,6 +66,14 @@ impl SourceCodeLocation { pub fn source_code(&self) -> &PinnedSourceSpec { self.build_source.as_ref().unwrap_or(&self.manifest_source) } + + pub fn as_source_and_alternative_root(&self) -> (&PinnedSourceSpec, Option<&PinnedSourceSpec>) { + if let Some(build_source) = &self.build_source { + (build_source, Some(&self.manifest_source)) + } else { + (&self.manifest_source, None) + } + } } impl std::fmt::Display for SourceCodeLocation { diff --git a/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs b/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs index 0272e06fa2..0c46a51db0 100644 --- a/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs +++ b/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs @@ -114,7 +114,8 @@ impl BuildBackendMetadataSpec { ) -> Result> { // Ensure that the source is checked out before proceeding. let manifest_source_checkout = command_dispatcher - .checkout_pinned_source(self.manifest_source.clone()) + // Never has an alternative root because we want to get the manifest + .checkout_pinned_source(self.manifest_source.clone(), None) .await .map_err_with(BuildBackendMetadataError::SourceCheckout)?; @@ -131,11 +132,17 @@ impl BuildBackendMetadataSpec { let build_source_checkout = if let Some(pin_override) = &self.pin_override { Some( command_dispatcher - .checkout_pinned_source(pin_override.clone()) + // We use the pinned override directly + .checkout_pinned_source(pin_override.clone(), None) .await .map_err_with(BuildBackendMetadataError::SourceCheckout)?, ) } else if let Some(build_source) = &discovered_backend.init_params.build_source { + tracing::error!( + "build source = {:?}, manifest_path = {}", + build_source, + discovered_backend.init_params.manifest_path.display() + ); Some( command_dispatcher .pin_and_checkout( diff --git a/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs b/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs index 72962739c5..518bbb8481 100644 --- a/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs +++ b/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs @@ -627,12 +627,22 @@ impl CommandDispatcher { pub async fn checkout_pinned_source( &self, pinned_spec: PinnedSourceSpec, + alternative_root: Option, ) -> Result> { match pinned_spec { PinnedSourceSpec::Path(ref path) => { + let alternative_root_path = match alternative_root { + Some(PinnedSourceSpec::Path(ref alt_path)) => Some( + self.data + .resolve_typed_path(alt_path.path.to_path(), None) + .map_err(|e| CommandDispatcherError::Failed(e.into()))?, + ), + _ => None, + }; + let source_path = self .data - .resolve_typed_path(path.path.to_path(), None) + .resolve_typed_path(path.path.to_path(), alternative_root_path.as_deref()) .map_err(SourceCheckoutError::from) .map_err(CommandDispatcherError::Failed)?; Ok(SourceCheckout { diff --git a/crates/pixi_command_dispatcher/src/source_build/mod.rs b/crates/pixi_command_dispatcher/src/source_build/mod.rs index 2992f1d4cb..3f3c82a727 100644 --- a/crates/pixi_command_dispatcher/src/source_build/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_build/mod.rs @@ -225,7 +225,8 @@ impl SourceBuildSpec { // Check out the source code. let manifest_source_checkout = command_dispatcher - .checkout_pinned_source(self.manifest_source.clone()) + // This should be relative to the workspace, so we can pass in None + .checkout_pinned_source(self.manifest_source.clone(), None) .await .map_err_with(SourceBuildError::SourceCheckout)?; @@ -265,7 +266,7 @@ impl SourceBuildSpec { // 3. Manifest source. Just assume that source is located at the same directory as the manifest. let build_source_dir = if let Some(pinned_build_source) = build_source { let build_source_checkout = command_dispatcher - .checkout_pinned_source(pinned_build_source) + .checkout_pinned_source(pinned_build_source, Some(self.manifest_source.clone())) .await .map_err_with(SourceBuildError::SourceCheckout)?; build_source_checkout.path diff --git a/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs b/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs index 6084d0b6ae..5edbc4e9db 100644 --- a/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs @@ -201,7 +201,7 @@ impl SourceBuildCacheStatusSpec { // Determine if the package is out of date by checking the source let cached_build = match self - .check_source_out_of_date(command_dispatcher, cached_build, source.source_code()) + .check_source_out_of_date(command_dispatcher, cached_build) .await? { CachedBuildStatus::UpToDate(cached_build) | CachedBuildStatus::New(cached_build) => { @@ -308,7 +308,7 @@ impl SourceBuildCacheStatusSpec { &self, command_dispatcher: &CommandDispatcher, cached_build: CachedBuild, - source: &PinnedSourceSpec, + manifest_source: &PinnedSourceSpec, ) -> Result> { let Some(source_info) = &cached_build.source else { return Ok(CachedBuildStatus::UpToDate(cached_build)); @@ -323,7 +323,8 @@ impl SourceBuildCacheStatusSpec { // Checkout the source for the package. let source_checkout = command_dispatcher - .checkout_pinned_source(source.clone()) + // Manifest source so it is relative to the workspace + .checkout_pinned_source(manifest_source.clone(), None) .await .map_err_with(SourceBuildCacheStatusError::SourceCheckout)?; @@ -361,7 +362,6 @@ impl SourceBuildCacheStatusSpec { &self, command_dispatcher: &CommandDispatcher, cached_build: CachedBuild, - source: &PinnedSourceSpec, ) -> Result> { // If there are no source globs, we always consider the cached package // up-to-date. @@ -369,9 +369,12 @@ impl SourceBuildCacheStatusSpec { return Ok(CachedBuildStatus::UpToDate(cached_build)); }; + // Convert into correct tuple + let (source, alternative_root) = self.source.as_source_and_alternative_root(); + // Checkout the source for the package. let source_checkout = command_dispatcher - .checkout_pinned_source(source.clone()) + .checkout_pinned_source(source.clone(), alternative_root.cloned()) .await .map_err_with(SourceBuildCacheStatusError::SourceCheckout)?; From 4d5dfb102710bbbe888fa0a8b8e7e4f2891dc518 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Mon, 3 Nov 2025 15:41:39 +0100 Subject: [PATCH 04/21] feat: refactor to use SourceCodeLocation in more places --- crates/pixi_cli/src/build.rs | 6 +-- .../pixi_command_dispatcher/src/build/mod.rs | 16 +++++- .../src/build_backend_metadata/mod.rs | 42 +++++++-------- .../src/command_dispatcher/mod.rs | 33 +++++++++--- .../src/install_pixi/mod.rs | 10 ++-- .../src/solve_conda/mod.rs | 2 +- .../solve_pixi/source_metadata_collector.rs | 5 +- .../src/source_build/mod.rs | 52 +++++++++---------- .../src/source_build_cache_status/mod.rs | 10 ++-- .../src/source_metadata/mod.rs | 36 ++++++------- .../tests/integration/event_tree.rs | 10 ++-- crates/pixi_global/src/project/mod.rs | 4 +- 12 files changed, 123 insertions(+), 103 deletions(-) diff --git a/crates/pixi_cli/src/build.rs b/crates/pixi_cli/src/build.rs index fd647e3ce8..9ed5e43584 100644 --- a/crates/pixi_cli/src/build.rs +++ b/crates/pixi_cli/src/build.rs @@ -5,6 +5,7 @@ use indicatif::ProgressBar; use miette::{Context, IntoDiagnostic}; use pixi_command_dispatcher::{ BuildBackendMetadataSpec, BuildEnvironment, BuildProfile, CacheDirs, SourceBuildSpec, + build::SourceCodeLocation, }; use pixi_config::ConfigCli; use pixi_core::WorkspaceLocator; @@ -145,7 +146,7 @@ pub async fn execute(args: Args) -> miette::Result<()> { // Create the build backend metadata specification. let backend_metadata_spec = BuildBackendMetadataSpec { - manifest_source: manifest_source.clone(), + source: SourceCodeLocation::new(manifest_source.clone(), None), channels: channels.clone(), channel_config: channel_config.clone(), build_environment: build_environment.clone(), @@ -183,8 +184,7 @@ pub async fn execute(args: Args) -> miette::Result<()> { package, // Build into a temporary directory first output_directory: Some(temp_output_dir.path().to_path_buf()), - manifest_source: manifest_source.clone(), - build_source: None, + source: SourceCodeLocation::new(manifest_source.clone(), None), channels: channels.clone(), channel_config: channel_config.clone(), build_environment: build_environment.clone(), diff --git a/crates/pixi_command_dispatcher/src/build/mod.rs b/crates/pixi_command_dispatcher/src/build/mod.rs index 08039e4ed2..4a81c758e5 100644 --- a/crates/pixi_command_dispatcher/src/build/mod.rs +++ b/crates/pixi_command_dispatcher/src/build/mod.rs @@ -67,6 +67,11 @@ impl SourceCodeLocation { self.build_source.as_ref().unwrap_or(&self.manifest_source) } + /// Get the optional explicit build source override. + pub fn build_source(&self) -> Option<&PinnedSourceSpec> { + self.build_source.as_ref() + } + pub fn as_source_and_alternative_root(&self) -> (&PinnedSourceSpec, Option<&PinnedSourceSpec>) { if let Some(build_source) = &self.build_source { (build_source, Some(&self.manifest_source)) @@ -84,12 +89,21 @@ impl std::fmt::Display for SourceCodeLocation { self.manifest_source(), self.build_source .as_ref() - .map(|build| format!("{}", build)) + .map(|build| format!("{build}")) .unwrap_or("undefined".to_string()) ) } } +impl From for SourceCodeLocation { + fn from(manifest_source: PinnedSourceSpec) -> Self { + Self { + manifest_source, + build_source: None, + } + } +} + /// Try to deduce a name from a url. fn pretty_url_name(url: &Url) -> String { if let Some(last_segment) = url diff --git a/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs b/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs index 0c46a51db0..2c416a5198 100644 --- a/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs +++ b/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs @@ -25,7 +25,7 @@ use crate::{ BuildEnvironment, CommandDispatcher, CommandDispatcherError, CommandDispatcherErrorResultExt, InstantiateBackendError, InstantiateBackendSpec, SourceCheckout, SourceCheckoutError, build::{ - SourceRecordOrCheckout, WorkDirKey, + SourceCodeLocation, SourceRecordOrCheckout, WorkDirKey, source_metadata_cache::{self, CachedCondaMetadata, MetadataKind, SourceMetadataKey}, }, }; @@ -49,8 +49,8 @@ fn warn_once_per_backend(backend_name: &str) { /// particular source. #[derive(Debug, Clone, Eq, PartialEq, Hash, serde::Serialize)] pub struct BuildBackendMetadataSpec { - /// The source specification where manifest is located at. - pub manifest_source: PinnedSourceSpec, + /// The manifest and optional build source location. + pub source: SourceCodeLocation, /// The channel configuration to use for the build backend. pub channel_config: ChannelConfig, @@ -81,11 +81,8 @@ pub struct BuildBackendMetadataSpec { /// The metadata of a source checkout. #[derive(Debug)] pub struct BuildBackendMetadata { - /// The source checkout that the manifest was extracted from. - pub manifest_source: PinnedSourceSpec, - - /// The source checkout from which we want to build package. - pub build_source: Option, + /// The manifest and optional build source location for this metadata. + pub source: SourceCodeLocation, /// The cache entry that contains the metadata acquired from the build /// backend. @@ -103,7 +100,8 @@ impl BuildBackendMetadataSpec { skip_all, name="backend-metadata", fields( - source = %self.manifest_source, + manifest_source = %self.source.manifest_source(), + build_source = ?self.source.build_source(), platform = %self.build_environment.host_platform, ) )] @@ -112,10 +110,12 @@ impl BuildBackendMetadataSpec { command_dispatcher: CommandDispatcher, log_sink: UnboundedSender, ) -> Result> { + let manifest_source = self.source.manifest_source().clone(); + // Ensure that the source is checked out before proceeding. let manifest_source_checkout = command_dispatcher // Never has an alternative root because we want to get the manifest - .checkout_pinned_source(self.manifest_source.clone(), None) + .checkout_pinned_source(&manifest_source) .await .map_err_with(BuildBackendMetadataError::SourceCheckout)?; @@ -133,16 +133,11 @@ impl BuildBackendMetadataSpec { Some( command_dispatcher // We use the pinned override directly - .checkout_pinned_source(pin_override.clone(), None) + .checkout_pinned_source(pin_override) .await .map_err_with(BuildBackendMetadataError::SourceCheckout)?, ) } else if let Some(build_source) = &discovered_backend.init_params.build_source { - tracing::error!( - "build source = {:?}, manifest_path = {}", - build_source, - discovered_backend.init_params.manifest_path.display() - ); Some( command_dispatcher .pin_and_checkout( @@ -174,6 +169,10 @@ impl BuildBackendMetadataSpec { } else { (manifest_source_checkout.clone(), None) }; + let source_location = SourceCodeLocation::new( + manifest_source_checkout.pinned.clone(), + build_source.clone(), + ); // Calculate the hash of the project model let additional_glob_hash = calculate_additional_glob_hash( @@ -207,10 +206,9 @@ impl BuildBackendMetadataSpec { .await? { return Ok(BuildBackendMetadata { + source: source_location.clone(), metadata, cache_entry, - manifest_source: manifest_source_checkout.pinned, - build_source, }); } } else { @@ -226,7 +224,7 @@ impl BuildBackendMetadataSpec { command_dispatcher .instantiate_backend(InstantiateBackendSpec { backend_spec: discovered_backend.backend_spec.clone().resolve( - SourceAnchor::from(SourceSpec::from(self.manifest_source.clone())), + SourceAnchor::from(SourceSpec::from(manifest_source.clone())), ), init_params: discovered_backend.init_params.clone(), build_source_dir, @@ -237,7 +235,6 @@ impl BuildBackendMetadataSpec { .map_err_with(BuildBackendMetadataError::Initialize)?; // Call the conda_outputs method to get metadata. - let manifest_source = manifest_source_checkout.pinned.clone(); if !backend.capabilities().provides_conda_outputs() { return Err(CommandDispatcherError::Failed( BuildBackendMetadataError::BackendMissingCapabilities( @@ -268,8 +265,7 @@ impl BuildBackendMetadataSpec { .map_err(CommandDispatcherError::Failed)?; Ok(BuildBackendMetadata { - manifest_source, - build_source, + source: source_location, metadata, cache_entry, }) @@ -487,7 +483,7 @@ impl BuildBackendMetadataSpec { build_environment: self.build_environment.clone(), build_variants: self.variants.clone().unwrap_or_default(), enabled_protocols: self.enabled_protocols.clone(), - pinned_source: self.manifest_source.clone(), + pinned_source: self.source.manifest_source().clone(), } } } diff --git a/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs b/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs index 518bbb8481..84c6437ac9 100644 --- a/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs +++ b/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs @@ -35,7 +35,7 @@ use crate::{ SourceBuildCacheEntry, SourceBuildCacheStatusError, SourceBuildCacheStatusSpec, SourceCheckout, SourceCheckoutError, SourceMetadata, SourceMetadataError, SourceMetadataSpec, backend_source_build::{BackendBuiltSource, BackendSourceBuildError, BackendSourceBuildSpec}, - build::{BuildCache, source_metadata_cache::SourceMetadataCache}, + build::{BuildCache, SourceCodeLocation, source_metadata_cache::SourceMetadataCache}, cache_dirs::CacheDirs, discover_backend_cache::DiscoveryCache, install_pixi::{ @@ -625,14 +625,35 @@ impl CommandDispatcher { /// - For URL sources: Extracts the archive with the exact checksum /// (unimplemented) pub async fn checkout_pinned_source( + &self, + pinned_spec: &PinnedSourceSpec, + ) -> Result> { + self.checkout_pinned_spec(pinned_spec.clone(), None).await + } + + /// Checkout a source described by a [`SourceCodeLocation`], automatically + /// falling back to the manifest path when no dedicated build source is set. + /// + /// This is useful for the case where you want to checkout a source that is at + /// different location than the manifest, most obviously in an out-of-tree build. + pub async fn checkout_source_location( + &self, + source: &SourceCodeLocation, + ) -> Result> { + let (primary, alternative_root) = source.as_source_and_alternative_root(); + self.checkout_pinned_spec(primary.clone(), alternative_root.cloned()) + .await + } + + async fn checkout_pinned_spec( &self, pinned_spec: PinnedSourceSpec, alternative_root: Option, ) -> Result> { match pinned_spec { - PinnedSourceSpec::Path(ref path) => { - let alternative_root_path = match alternative_root { - Some(PinnedSourceSpec::Path(ref alt_path)) => Some( + PinnedSourceSpec::Path(path_spec) => { + let alternative_root_path = match alternative_root.as_ref() { + Some(PinnedSourceSpec::Path(alt_path)) => Some( self.data .resolve_typed_path(alt_path.path.to_path(), None) .map_err(|e| CommandDispatcherError::Failed(e.into()))?, @@ -642,12 +663,12 @@ impl CommandDispatcher { let source_path = self .data - .resolve_typed_path(path.path.to_path(), alternative_root_path.as_deref()) + .resolve_typed_path(path_spec.path.to_path(), alternative_root_path.as_deref()) .map_err(SourceCheckoutError::from) .map_err(CommandDispatcherError::Failed)?; Ok(SourceCheckout { path: source_path, - pinned: pinned_spec, + pinned: PinnedSourceSpec::Path(path_spec), }) } PinnedSourceSpec::Git(git_spec) => self.checkout_pinned_git(git_spec).await, diff --git a/crates/pixi_command_dispatcher/src/install_pixi/mod.rs b/crates/pixi_command_dispatcher/src/install_pixi/mod.rs index beab694cc1..bd8b6bcb3a 100644 --- a/crates/pixi_command_dispatcher/src/install_pixi/mod.rs +++ b/crates/pixi_command_dispatcher/src/install_pixi/mod.rs @@ -23,8 +23,8 @@ use thiserror::Error; use crate::{ BuildEnvironment, BuildProfile, CommandDispatcher, CommandDispatcherError, - CommandDispatcherErrorResultExt, SourceBuildError, SourceBuildSpec, executor::ExecutorFutures, - install_pixi::reporter::WrappingInstallReporter, + CommandDispatcherErrorResultExt, SourceBuildError, SourceBuildSpec, build::SourceCodeLocation, + executor::ExecutorFutures, install_pixi::reporter::WrappingInstallReporter, }; #[derive(Debug, Clone, serde::Serialize)] @@ -215,7 +215,10 @@ impl InstallPixiEnvironmentSpec { .contains(&source_record.package_record.name); let built_source = command_dispatcher .source_build(SourceBuildSpec { - manifest_source: source_record.manifest_source.clone(), + source: SourceCodeLocation::new( + source_record.manifest_source.clone(), + source_record.build_source.clone(), + ), package: source_record.into(), channel_config: self.channel_config.clone(), channels: self.channels.clone(), @@ -230,7 +233,6 @@ impl InstallPixiEnvironmentSpec { force, // When we install a pixi environment we always build in development mode. build_profile: BuildProfile::Development, - build_source: source_record.build_source.clone(), }) .await?; diff --git a/crates/pixi_command_dispatcher/src/solve_conda/mod.rs b/crates/pixi_command_dispatcher/src/solve_conda/mod.rs index 3617937f42..24fa98e753 100644 --- a/crates/pixi_command_dispatcher/src/solve_conda/mod.rs +++ b/crates/pixi_command_dispatcher/src/solve_conda/mod.rs @@ -156,7 +156,7 @@ impl SolveCondaEnvironmentSpec { channel: None, }; let mut record = record.clone(); - record.build_source = source_metadata.build_source.clone(); + record.build_source = source_metadata.source.build_source().cloned(); url_to_source_package.insert(url, (record, repodata_record)); } } diff --git a/crates/pixi_command_dispatcher/src/solve_pixi/source_metadata_collector.rs b/crates/pixi_command_dispatcher/src/solve_pixi/source_metadata_collector.rs index 9438ad1b8d..c6deb8db0a 100644 --- a/crates/pixi_command_dispatcher/src/solve_pixi/source_metadata_collector.rs +++ b/crates/pixi_command_dispatcher/src/solve_pixi/source_metadata_collector.rs @@ -15,6 +15,7 @@ use thiserror::Error; use crate::{ BuildBackendMetadataSpec, BuildEnvironment, CommandDispatcher, CommandDispatcherError, SourceCheckoutError, SourceMetadataSpec, + build::SourceCodeLocation, executor::ExecutorFutures, source_metadata::{CycleEnvironment, SourceMetadata, SourceMetadataError}, }; @@ -185,7 +186,7 @@ impl SourceMetadataCollector { .source_metadata(SourceMetadataSpec { package: name.clone(), backend_metadata: BuildBackendMetadataSpec { - manifest_source: source.pinned, + source: SourceCodeLocation::new(source.pinned, None), channel_config: self.channel_config.clone(), channels: self.channels.clone(), build_environment: self.build_environment.clone(), @@ -230,7 +231,7 @@ impl SourceMetadataCollector { source_metadata.skipped_packages.clone(), ), name, - pinned_source: Box::new(source_metadata.manifest_source.clone()), + pinned_source: Box::new(source_metadata.source.manifest_source().clone()), }, )); } diff --git a/crates/pixi_command_dispatcher/src/source_build/mod.rs b/crates/pixi_command_dispatcher/src/source_build/mod.rs index 3f3c82a727..ee08dd4cb5 100644 --- a/crates/pixi_command_dispatcher/src/source_build/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_build/mod.rs @@ -51,12 +51,8 @@ pub struct SourceBuildSpec { /// The source to build pub package: PackageIdentifier, - /// The location of the source code to build. - pub manifest_source: PinnedSourceSpec, - - /// Optional source spec of sources which will be built. - #[serde(skip_serializing_if = "Option::is_none")] - pub build_source: Option, + /// The manifest and optional build source location. + pub source: SourceCodeLocation, /// The channel configuration to use when resolving metadata pub channel_config: ChannelConfig, @@ -124,8 +120,8 @@ impl SourceBuildSpec { skip_all, name = "source-build", fields( - source= %self.manifest_source, - build_source= ?self.build_source, + manifest_source = %self.source.manifest_source(), + build_source = ?self.source.build_source(), package = %self.package, ) )] @@ -135,6 +131,8 @@ impl SourceBuildSpec { reporter: Option>, log_sink: UnboundedSender, ) -> Result> { + let manifest_source = self.source.manifest_source().clone(); + // If the output directory is not set, we want to use the build cache. Read the // build cache in that case. let (output_directory, build_cache) = if let Some(output_directory) = @@ -148,10 +146,7 @@ impl SourceBuildSpec { .source_build_cache_status(SourceBuildCacheStatusSpec { package: self.package.clone(), build_environment: self.build_environment.clone(), - source: super::build::SourceCodeLocation::new( - self.manifest_source.clone(), - self.build_source.clone(), - ), + source: self.source.clone(), channels: self.channels.clone(), channel_config: self.channel_config.clone(), enabled_protocols: self.enabled_protocols.clone(), @@ -167,7 +162,7 @@ impl SourceBuildSpec { if !self.force { // If the build is up to date, we can return the cached build. tracing::debug!( - source = %self.manifest_source, + source = %self.source.manifest_source(), package = ?cached_build.record.package_record.name, build = %cached_build.record.package_record.build, output = %cached_build.record.file_name, @@ -189,7 +184,7 @@ impl SourceBuildSpec { self.package.name.as_normalized() ); tracing::debug!( - source = %self.manifest_source, + source = %self.source.manifest_source(), package = ?cached_build.record.package_record.name, build = %cached_build.record.package_record.build, output = %cached_build.record.file_name, @@ -205,7 +200,7 @@ impl SourceBuildSpec { match &*build_cache.cached_build.lock().await { CachedBuildStatus::Stale(existing) => { tracing::debug!( - source = %self.manifest_source, + source = %self.source.manifest_source(), package = ?existing.record.package_record.name, build = %existing.record.package_record.build, "rebuilding stale source build", @@ -213,7 +208,7 @@ impl SourceBuildSpec { } CachedBuildStatus::Missing => { tracing::debug!( - source = %self.manifest_source, + source = %self.source.manifest_source(), "no cached source build; starting fresh build", ); } @@ -225,8 +220,7 @@ impl SourceBuildSpec { // Check out the source code. let manifest_source_checkout = command_dispatcher - // This should be relative to the workspace, so we can pass in None - .checkout_pinned_source(self.manifest_source.clone(), None) + .checkout_pinned_source(&manifest_source) .await .map_err_with(SourceBuildError::SourceCheckout)?; @@ -250,7 +244,7 @@ impl SourceBuildSpec { // Ensure legacy lock entries that missed the git subdirectory pick it up from the // manifest so we check out the correct directory. - let mut build_source = self.build_source.clone(); + let mut build_source = self.source.build_source().cloned(); if let (Some(PinnedSourceSpec::Git(pinned_git)), Some(SourceLocationSpec::Git(git_spec))) = ( build_source.as_mut(), discovered_backend.init_params.build_source.clone(), @@ -265,8 +259,10 @@ impl SourceBuildSpec { // 2. Manifest package build. This can happen if package isn't added to the dependencies of manifest, so no pinning happens in that case. // 3. Manifest source. Just assume that source is located at the same directory as the manifest. let build_source_dir = if let Some(pinned_build_source) = build_source { + let build_location = + SourceCodeLocation::new(manifest_source.clone(), Some(pinned_build_source.clone())); let build_source_checkout = command_dispatcher - .checkout_pinned_source(pinned_build_source, Some(self.manifest_source.clone())) + .checkout_source_location(&build_location) .await .map_err_with(SourceBuildError::SourceCheckout)?; build_source_checkout.path @@ -302,7 +298,7 @@ impl SourceBuildSpec { command_dispatcher .instantiate_backend(InstantiateBackendSpec { backend_spec: discovered_backend.backend_spec.clone().resolve( - SourceAnchor::from(SourceSpec::from(self.manifest_source.clone())), + SourceAnchor::from(SourceSpec::from(manifest_source.clone())), ), init_params: discovered_backend.init_params.clone(), build_source_dir, @@ -318,7 +314,7 @@ impl SourceBuildSpec { None => command_dispatcher.cache_dirs().working_dirs().join( WorkDirKey { source: SourceRecordOrCheckout::Record { - pinned: self.manifest_source.clone(), + pinned: manifest_source.clone(), package_name: self.package.name.clone(), }, host_platform: self.build_environment.host_platform, @@ -328,7 +324,7 @@ impl SourceBuildSpec { ), }; tracing::debug!( - source = %self.manifest_source, + source = %manifest_source, work_directory = %work_directory.display(), backend = backend.identifier(), "using work directory for source build", @@ -344,7 +340,7 @@ impl SourceBuildSpec { } // Build the package using the v1 build method. - let source_for_logging = self.manifest_source.clone(); + let source_for_logging = manifest_source.clone(); let mut built_source = self .build_v1( command_dispatcher, @@ -497,7 +493,7 @@ impl SourceBuildSpec { /// Returns whether the package should be built in an editable mode. fn editable(&self) -> bool { - self.build_profile == BuildProfile::Development && self.manifest_source.is_mutable() + self.build_profile == BuildProfile::Development && self.source.source_code().is_mutable() } async fn build_v1( @@ -509,7 +505,9 @@ impl SourceBuildSpec { reporter: Option>, mut log_sink: UnboundedSender, ) -> Result> { - let source_anchor = SourceAnchor::from(SourceSpec::from(self.manifest_source.clone())); + let manifest_source = self.source.manifest_source().clone(); + + let source_anchor = SourceAnchor::from(SourceSpec::from(manifest_source.clone())); let host_platform = self.build_environment.host_platform; let build_platform = self.build_environment.build_platform; @@ -732,7 +730,7 @@ impl SourceBuildSpec { }), backend, package: self.package, - source: self.manifest_source, + source: manifest_source, work_directory, channels: self.channels, channel_config: self.channel_config, diff --git a/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs b/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs index 5edbc4e9db..1b461044dd 100644 --- a/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs @@ -136,7 +136,7 @@ impl SourceBuildCacheStatusSpec { }; let (cached_build, build_cache_entry) = command_dispatcher .build_cache() - .entry(&self.source.manifest_source(), &build_input) + .entry(self.source.manifest_source(), &build_input) .await .map_err(SourceBuildCacheStatusError::BuildCache) .map_err(CommandDispatcherError::Failed)?; @@ -323,8 +323,7 @@ impl SourceBuildCacheStatusSpec { // Checkout the source for the package. let source_checkout = command_dispatcher - // Manifest source so it is relative to the workspace - .checkout_pinned_source(manifest_source.clone(), None) + .checkout_pinned_source(manifest_source) .await .map_err_with(SourceBuildCacheStatusError::SourceCheckout)?; @@ -369,12 +368,9 @@ impl SourceBuildCacheStatusSpec { return Ok(CachedBuildStatus::UpToDate(cached_build)); }; - // Convert into correct tuple - let (source, alternative_root) = self.source.as_source_and_alternative_root(); - // Checkout the source for the package. let source_checkout = command_dispatcher - .checkout_pinned_source(source.clone(), alternative_root.cloned()) + .checkout_source_location(&self.source) .await .map_err_with(SourceBuildCacheStatusError::SourceCheckout)?; diff --git a/crates/pixi_command_dispatcher/src/source_metadata/mod.rs b/crates/pixi_command_dispatcher/src/source_metadata/mod.rs index cc6d01cfe0..9098600d2f 100644 --- a/crates/pixi_command_dispatcher/src/source_metadata/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_metadata/mod.rs @@ -10,7 +10,7 @@ use futures::TryStreamExt; use itertools::{Either, Itertools}; use miette::Diagnostic; use pixi_build_types::procedures::conda_outputs::CondaOutput; -use pixi_record::{InputHash, PinnedSourceSpec, PixiRecord, SourceRecord}; +use pixi_record::{InputHash, PixiRecord, SourceRecord}; use pixi_spec::{BinarySpec, PixiSpec, SourceAnchor, SourceSpec, SpecConversionError}; use pixi_spec_containers::DependencyMap; use rattler_conda_types::{ @@ -26,7 +26,7 @@ use crate::{ CommandDispatcherError, CommandDispatcherErrorResultExt, PixiEnvironmentSpec, SolvePixiEnvironmentError, build::{ - Dependencies, DependenciesError, PixiRunExports, conversion, + Dependencies, DependenciesError, PixiRunExports, SourceCodeLocation, conversion, source_metadata_cache::MetadataKind, }, executor::ExecutorFutures, @@ -44,13 +44,8 @@ pub struct SourceMetadataSpec { /// The result of building a particular source record. #[derive(Debug, Clone)] pub struct SourceMetadata { - /// Information about the source checkout that was used to build the - /// package. - pub manifest_source: PinnedSourceSpec, - - /// The optional location of where the actual source code is located, - /// this is used mainly for out-of-tree builds - pub build_source: Option, + /// Manifest and optional build source location for this metadata. + pub source: SourceCodeLocation, /// All the source records for this particular package. pub records: Vec, @@ -64,7 +59,8 @@ impl SourceMetadataSpec { skip_all, name = "source-metadata", fields( - source= %self.backend_metadata.manifest_source, + manifest_source= %self.backend_metadata.source.manifest_source(), + build_source=?self.backend_metadata.source.build_source(), name = %self.package.as_source(), platform = %self.backend_metadata.build_environment.host_platform, ) @@ -84,26 +80,27 @@ impl SourceMetadataSpec { match &build_backend_metadata.metadata.metadata { MetadataKind::GetMetadata { packages } => { + let source_location = build_backend_metadata.source.clone(); // Convert the metadata to source records. let records = conversion::package_metadata_to_source_records( - &build_backend_metadata.manifest_source, - build_backend_metadata.build_source.as_ref(), + source_location.manifest_source(), + source_location.build_source(), packages, &self.package, &build_backend_metadata.metadata.input_hash, ); Ok(SourceMetadata { - manifest_source: build_backend_metadata.manifest_source.clone(), + source: source_location, records, // As the GetMetadata kind returns all records at once and we don't solve them we can skip this. skipped_packages: Default::default(), - build_source: build_backend_metadata.build_source.clone(), }) } MetadataKind::Outputs { outputs } => { let mut skipped_packages = vec![]; let mut futures = ExecutorFutures::new(command_dispatcher.executor()); + let source_location = build_backend_metadata.source.clone(); for output in outputs { if output.metadata.name != self.package { skipped_packages.push(output.metadata.name.clone()); @@ -113,17 +110,15 @@ impl SourceMetadataSpec { &command_dispatcher, output, build_backend_metadata.metadata.input_hash.clone(), - build_backend_metadata.manifest_source.clone(), - build_backend_metadata.build_source.clone(), + source_location.clone(), reporter.clone(), )); } Ok(SourceMetadata { - manifest_source: build_backend_metadata.manifest_source.clone(), + source: source_location, records: futures.try_collect().await?, skipped_packages, - build_source: build_backend_metadata.build_source.clone(), }) } } @@ -134,10 +129,11 @@ impl SourceMetadataSpec { command_dispatcher: &CommandDispatcher, output: &CondaOutput, input_hash: Option, - manifest_source: PinnedSourceSpec, - build_source: Option, + source: SourceCodeLocation, reporter: Option>, ) -> Result> { + let manifest_source = source.manifest_source().clone(); + let build_source = source.build_source().cloned(); let source_anchor = SourceAnchor::from(SourceSpec::from(manifest_source.clone())); // Solve the build environment for the output. diff --git a/crates/pixi_command_dispatcher/tests/integration/event_tree.rs b/crates/pixi_command_dispatcher/tests/integration/event_tree.rs index 234df1521a..14a33ecbf3 100644 --- a/crates/pixi_command_dispatcher/tests/integration/event_tree.rs +++ b/crates/pixi_command_dispatcher/tests/integration/event_tree.rs @@ -130,7 +130,7 @@ impl EventTree { format!( "{} @ {}", &spec.package.as_source(), - spec.backend_metadata.manifest_source + spec.backend_metadata.source ), ); builder.set_event_parent((*id).into(), *context); @@ -146,7 +146,7 @@ impl EventTree { } Event::SourceMetadataFinished { .. } => {} Event::BuildBackendMetadataQueued { id, context, spec } => { - build_backend_metadata_label.insert(*id, spec.manifest_source.to_string()); + build_backend_metadata_label.insert(*id, spec.source.to_string()); builder.set_event_parent((*id).into(), *context); } Event::BuildBackendMetadataStarted { id } => { @@ -162,11 +162,7 @@ impl EventTree { Event::SourceBuildQueued { id, context, spec } => { source_build_label.insert( *id, - format!( - "{} @ {}", - spec.package.name.as_source(), - spec.manifest_source - ), + format!("{} @ {}", spec.package.name.as_source(), spec.source), ); builder.set_event_parent((*id).into(), *context); } diff --git a/crates/pixi_global/src/project/mod.rs b/crates/pixi_global/src/project/mod.rs index 7dd78704bd..6df70d0cf0 100644 --- a/crates/pixi_global/src/project/mod.rs +++ b/crates/pixi_global/src/project/mod.rs @@ -23,7 +23,7 @@ pub use parsed_manifest::{ExposedName, ParsedEnvironment, ParsedManifest}; use pixi_build_discovery::EnabledProtocols; use pixi_command_dispatcher::{ BuildBackendMetadataSpec, BuildEnvironment, CommandDispatcher, InstallPixiEnvironmentSpec, - Limits, PixiEnvironmentSpec, + Limits, PixiEnvironmentSpec, build::SourceCodeLocation, }; use pixi_config::{Config, RunPostLinkScripts, default_channel_config, pixi_home}; use pixi_consts::consts::{self}; @@ -1380,7 +1380,7 @@ impl Project { // Create the metadata spec let metadata_spec = BuildBackendMetadataSpec { - manifest_source: pinned_source_spec, + source: SourceCodeLocation::new(pinned_source_spec, None), channel_config: self.global_channel_config().clone(), channels: self .config() From 8ee563095aa6d5ece0f9fe15bfaddf947d3537ce Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Mon, 3 Nov 2025 16:36:25 +0100 Subject: [PATCH 05/21] feat: remove from and update snap --- crates/pixi_command_dispatcher/src/build/mod.rs | 9 --------- .../tests/integration/snapshots/integration__cycle.snap | 8 ++++---- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/crates/pixi_command_dispatcher/src/build/mod.rs b/crates/pixi_command_dispatcher/src/build/mod.rs index 4a81c758e5..0555467701 100644 --- a/crates/pixi_command_dispatcher/src/build/mod.rs +++ b/crates/pixi_command_dispatcher/src/build/mod.rs @@ -95,15 +95,6 @@ impl std::fmt::Display for SourceCodeLocation { } } -impl From for SourceCodeLocation { - fn from(manifest_source: PinnedSourceSpec) -> Self { - Self { - manifest_source, - build_source: None, - } - } -} - /// Try to deduce a name from a url. fn pretty_url_name(url: &Url) -> String { if let Some(last_segment) = url diff --git a/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__cycle.snap b/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__cycle.snap index b5f8bf5d7f..02e20f8a35 100644 --- a/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__cycle.snap +++ b/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__cycle.snap @@ -16,8 +16,8 @@ ERROR: TRACE: Pixi solve (package_a) -└── Source metadata (package_a @ package_a) - ├── Build backend metadata (package_a) +└── Source metadata (package_a @ (manifest-src: package_a, build-src: undefined)) + ├── Build backend metadata ((manifest-src: package_a, build-src: undefined)) └── Pixi solve (package_b) - └── Source metadata (package_b @ package_b) - └── Build backend metadata (package_b) + └── Source metadata (package_b @ (manifest-src: package_b, build-src: undefined)) + └── Build backend metadata ((manifest-src: package_b, build-src: undefined)) From e60d90d663555186cc2cdf4131341a574ec14100 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Mon, 3 Nov 2025 22:27:14 +0100 Subject: [PATCH 06/21] snapshot --- .../snapshots/integration__simple_test.snap | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__simple_test.snap b/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__simple_test.snap index 635a5822d4..e5a6313312 100644 --- a/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__simple_test.snap +++ b/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__simple_test.snap @@ -4,16 +4,16 @@ expression: event_tree.to_string() --- Pixi solve (foobar-desktop) ├── Git Checkout (https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a) -├── Source metadata (foobar-desktop @ https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a) -│ └── Build backend metadata (https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a) +├── Source metadata (foobar-desktop @ (manifest-src: https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a, build-src: undefined)) +│ └── Build backend metadata ((manifest-src: https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a, build-src: undefined)) │ └── Instantiate tool environment (pixi-build-rattler-build) │ ├── Pixi solve (pixi-build-rattler-build) │ │ └── Conda solve #0 │ └── Pixi install (pixi-build-rattler-build) -├── Source metadata (foobar @ https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a) +├── Source metadata (foobar @ (manifest-src: https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a, build-src: undefined)) └── Conda solve #1 Pixi install (test-env) -├── Source build (foobar @ https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a) +├── Source build (foobar @ (manifest-src: https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a, build-src: undefined)) │ └── Backend source build (foobar) -└── Source build (foobar-desktop @ https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a) +└── Source build (foobar-desktop @ (manifest-src: https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a, build-src: undefined)) └── Backend source build (foobar-desktop) From 6a0cff83cdc2a25e08c23533c141ee6c451ba96b Mon Sep 17 00:00:00 2001 From: Bas Zalmstra <4995967+baszalmstra@users.noreply.github.com> Date: Tue, 4 Nov 2025 14:37:54 +0100 Subject: [PATCH 07/21] fix: always make build_source relative to workspace --- .../pixi_build_discovery/src/backend_spec.rs | 14 +- crates/pixi_cli/src/build.rs | 4 +- .../src/build/dependencies.rs | 10 +- .../src/build_backend_metadata/mod.rs | 119 ++--- .../command_dispatcher/instantiate_backend.rs | 52 ++- .../src/command_dispatcher/mod.rs | 63 +-- .../src/instantiate_tool_env/mod.rs | 2 +- .../src/solve_pixi/mod.rs | 9 +- .../solve_pixi/source_metadata_collector.rs | 28 +- .../src/source_build/mod.rs | 74 ++- .../src/source_build_cache_status/mod.rs | 8 +- .../src/source_metadata/mod.rs | 10 +- .../tests/integration/event_tree.rs | 4 +- .../snapshots/integration__cycle.snap | 8 +- .../snapshots/integration__simple_test.snap | 6 +- .../src/lock_file/satisfiability/mod.rs | 5 +- crates/pixi_core/src/lock_file/update.rs | 2 +- crates/pixi_global/src/common.rs | 4 +- crates/pixi_global/src/project/mod.rs | 8 +- crates/pixi_record/src/pinned_source.rs | 429 ++++++++++++++++++ crates/pixi_spec/src/source_anchor.rs | 63 ++- 21 files changed, 656 insertions(+), 266 deletions(-) diff --git a/crates/pixi_build_discovery/src/backend_spec.rs b/crates/pixi_build_discovery/src/backend_spec.rs index e2889173a3..6bcffd4969 100644 --- a/crates/pixi_build_discovery/src/backend_spec.rs +++ b/crates/pixi_build_discovery/src/backend_spec.rs @@ -1,4 +1,4 @@ -use pixi_spec::{BinarySpec, PixiSpec, SourceAnchor}; +use pixi_spec::{BinarySpec, PixiSpec, SourceAnchor, SourceSpec}; use pixi_spec_containers::DependencyMap; use rattler_conda_types::ChannelUrl; /// Describes how a backend should be instantiated. @@ -45,8 +45,10 @@ impl JsonRpcBackendSpec { let maybe_source_spec = env_spec.requirement.1.try_into_source_spec(); let pixi_spec = match maybe_source_spec { Ok(source_spec) => { - let resolved_spec = source_anchor.resolve(source_spec); - PixiSpec::from(resolved_spec) + let resolved_spec = source_anchor.resolve(source_spec.location); + PixiSpec::from(SourceSpec { + location: resolved_spec, + }) } Err(pixi_spec) => pixi_spec, }; @@ -133,8 +135,10 @@ impl EnvironmentSpec { let maybe_source_spec = self.requirement.1.try_into_source_spec(); let pixi_spec = match maybe_source_spec { Ok(source_spec) => { - let resolved_spec = source_anchor.resolve(source_spec); - PixiSpec::from(resolved_spec) + let resolved_spec = source_anchor.resolve(source_spec.location); + PixiSpec::from(SourceSpec { + location: resolved_spec, + }) } Err(pixi_spec) => pixi_spec, }; diff --git a/crates/pixi_cli/src/build.rs b/crates/pixi_cli/src/build.rs index 9ed5e43584..6ed8f68503 100644 --- a/crates/pixi_cli/src/build.rs +++ b/crates/pixi_cli/src/build.rs @@ -146,14 +146,14 @@ pub async fn execute(args: Args) -> miette::Result<()> { // Create the build backend metadata specification. let backend_metadata_spec = BuildBackendMetadataSpec { - source: SourceCodeLocation::new(manifest_source.clone(), None), + manifest_source: manifest_source.clone(), + preferred_build_source: None, channels: channels.clone(), channel_config: channel_config.clone(), build_environment: build_environment.clone(), variants: Some(variants.clone()), variant_files: Some(variant_files.clone()), enabled_protocols: Default::default(), - pin_override: None, }; let backend_metadata = command_dispatcher .build_backend_metadata(backend_metadata_spec.clone()) diff --git a/crates/pixi_command_dispatcher/src/build/dependencies.rs b/crates/pixi_command_dispatcher/src/build/dependencies.rs index abfea9c4d4..4c137987a2 100644 --- a/crates/pixi_command_dispatcher/src/build/dependencies.rs +++ b/crates/pixi_command_dispatcher/src/build/dependencies.rs @@ -8,7 +8,7 @@ use pixi_build_types::{ }, }; use pixi_record::PixiRecord; -use pixi_spec::{BinarySpec, DetailedSpec, PixiSpec, SourceAnchor, UrlBinarySpec}; +use pixi_spec::{BinarySpec, DetailedSpec, PixiSpec, SourceAnchor, SourceSpec, UrlBinarySpec}; use pixi_spec_containers::DependencyMap; use rattler_conda_types::{ InvalidPackageNameError, MatchSpec, NamedChannelOrUrl, NamelessMatchSpec, PackageName, @@ -95,12 +95,12 @@ impl Dependencies { })?; match conversion::from_package_spec_v1(depend.spec.clone()).into_source_or_binary() { Either::Left(source) => { - let source = if let Some(anchor) = &source_anchor { - anchor.resolve(source) + let location = if let Some(anchor) = &source_anchor { + anchor.resolve(source.location) } else { - source + source.location }; - dependencies.insert(name, PixiSpec::from(source).into()); + dependencies.insert(name, PixiSpec::from(SourceSpec { location }).into()); } Either::Right(binary) => { dependencies.insert(name, PixiSpec::from(binary).into()); diff --git a/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs b/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs index 2c416a5198..cc5593b078 100644 --- a/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs +++ b/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs @@ -49,8 +49,18 @@ fn warn_once_per_backend(backend_name: &str) { /// particular source. #[derive(Debug, Clone, Eq, PartialEq, Hash, serde::Serialize)] pub struct BuildBackendMetadataSpec { - /// The manifest and optional build source location. - pub source: SourceCodeLocation, + /// The location that refers to where the manifest is stored. + pub manifest_source: PinnedSourceSpec, + + /// The optional pinned location of the source code. If not provide the + /// location in the manifest is resolved. + /// + /// This is passed as a hint. If the [`SourceSpec`] in the discovered + /// manifest does not match with the pinned source provided here, the one + /// in the manifest takes precedence and it is reresolved. + /// + /// See [`PinnedSourceSpec::matches_source_spec`] how the matching is done. + pub preferred_build_source: Option, /// The channel configuration to use for the build backend. pub channel_config: ChannelConfig, @@ -71,11 +81,6 @@ pub struct BuildBackendMetadataSpec { /// The protocols that are enabled for this source #[serde(skip_serializing_if = "crate::is_default")] pub enabled_protocols: EnabledProtocols, - - /// Optional override for the pinned build source of the current package. - /// When set, this takes precedence over any discovered build_source. - #[serde(skip)] - pub pin_override: Option, } /// The metadata of a source checkout. @@ -100,8 +105,8 @@ impl BuildBackendMetadataSpec { skip_all, name="backend-metadata", fields( - manifest_source = %self.source.manifest_source(), - build_source = ?self.source.build_source(), + manifest_source = ?self.manifest_source, + build_source = ?self.preferred_build_source, platform = %self.build_environment.host_platform, ) )] @@ -110,12 +115,10 @@ impl BuildBackendMetadataSpec { command_dispatcher: CommandDispatcher, log_sink: UnboundedSender, ) -> Result> { - let manifest_source = self.source.manifest_source().clone(); - // Ensure that the source is checked out before proceeding. let manifest_source_checkout = command_dispatcher // Never has an alternative root because we want to get the manifest - .checkout_pinned_source(&manifest_source) + .checkout_pinned_source(self.manifest_source.clone()) .await .map_err_with(BuildBackendMetadataError::SourceCheckout)?; @@ -129,38 +132,36 @@ impl BuildBackendMetadataSpec { .await .map_err_with(BuildBackendMetadataError::Discovery)?; - let build_source_checkout = if let Some(pin_override) = &self.pin_override { - Some( - command_dispatcher - // We use the pinned override directly - .checkout_pinned_source(pin_override) - .await - .map_err_with(BuildBackendMetadataError::SourceCheckout)?, - ) - } else if let Some(build_source) = &discovered_backend.init_params.build_source { - Some( - command_dispatcher - .pin_and_checkout( - build_source.clone(), + // Determine the location of the source to build from. + let manifest_source_anchor = + SourceAnchor::from(SourceSpec::from(self.manifest_source.clone())); + let build_source_checkout = match &discovered_backend.init_params.build_source { + None => None, + Some(spec) => { + // An out of tree source is provided. Resolve it against the manifest source. + let resolved_location = manifest_source_anchor.resolve(spec.clone()); + let resolved_source_build_spec = SourceSpec { + location: resolved_location.clone(), + }; + + // Check if we have a preferred build source that matches this same location + match &self.preferred_build_source { + Some(pinned) if pinned.matches_source_spec(&resolved_source_build_spec) => { Some( - discovered_backend - .init_params - .manifest_path - .parent() - .ok_or_else(|| { - SourceCheckoutError::ParentDir( - discovered_backend.init_params.manifest_path.clone(), - ) - }) - .map_err(BuildBackendMetadataError::SourceCheckout) - .map_err(CommandDispatcherError::Failed)?, - ), - ) - .await - .map_err_with(BuildBackendMetadataError::SourceCheckout)?, - ) - } else { - None + command_dispatcher + .checkout_pinned_source(pinned.clone()) + .await + .map_err_with(BuildBackendMetadataError::SourceCheckout)?, + ) + } + _ => Some( + command_dispatcher + .pin_and_checkout(resolved_location) + .await + .map_err_with(BuildBackendMetadataError::SourceCheckout)?, + ), + } + } }; let (build_source_checkout, build_source) = if let Some(checkout) = build_source_checkout { @@ -220,19 +221,23 @@ impl BuildBackendMetadataSpec { let build_source_dir = build_source_checkout.path.clone(); // Instantiate the backend with the discovered information. - let backend = - command_dispatcher - .instantiate_backend(InstantiateBackendSpec { - backend_spec: discovered_backend.backend_spec.clone().resolve( - SourceAnchor::from(SourceSpec::from(manifest_source.clone())), - ), - init_params: discovered_backend.init_params.clone(), - build_source_dir, - channel_config: self.channel_config.clone(), - enabled_protocols: self.enabled_protocols.clone(), - }) - .await - .map_err_with(BuildBackendMetadataError::Initialize)?; + let backend = command_dispatcher + .instantiate_backend(InstantiateBackendSpec { + backend_spec: discovered_backend + .backend_spec + .clone() + .resolve(manifest_source_anchor), + build_source_dir, + channel_config: self.channel_config.clone(), + enabled_protocols: self.enabled_protocols.clone(), + workspace_root: discovered_backend.init_params.workspace_root.clone(), + manifest_path: discovered_backend.init_params.manifest_path.clone(), + project_model: discovered_backend.init_params.project_model.clone(), + configuration: discovered_backend.init_params.configuration.clone(), + target_configuration: discovered_backend.init_params.target_configuration.clone(), + }) + .await + .map_err_with(BuildBackendMetadataError::Initialize)?; // Call the conda_outputs method to get metadata. if !backend.capabilities().provides_conda_outputs() { @@ -483,7 +488,7 @@ impl BuildBackendMetadataSpec { build_environment: self.build_environment.clone(), build_variants: self.variants.clone().unwrap_or_default(), enabled_protocols: self.enabled_protocols.clone(), - pinned_source: self.source.manifest_source().clone(), + pinned_source: self.manifest_source.clone(), } } } diff --git a/crates/pixi_command_dispatcher/src/command_dispatcher/instantiate_backend.rs b/crates/pixi_command_dispatcher/src/command_dispatcher/instantiate_backend.rs index 07444fb4ba..c373921484 100644 --- a/crates/pixi_command_dispatcher/src/command_dispatcher/instantiate_backend.rs +++ b/crates/pixi_command_dispatcher/src/command_dispatcher/instantiate_backend.rs @@ -1,15 +1,16 @@ use std::path::PathBuf; use miette::Diagnostic; -use pixi_build_discovery::{ - BackendInitializationParams, BackendSpec, CommandSpec, EnabledProtocols, -}; +use ordermap::OrderMap; +use pixi_build_discovery::{BackendSpec, CommandSpec, EnabledProtocols}; use pixi_build_frontend::{ Backend, BackendOverride, json_rpc, json_rpc::{CommunicationError, JsonRpcBackend}, tool::{IsolatedTool, SystemTool, Tool}, }; -use pixi_build_types::{PixiBuildApiVersion, procedures::initialize::InitializeParams}; +use pixi_build_types::{ + PixiBuildApiVersion, ProjectModelV1, TargetSelectorV1, procedures::initialize::InitializeParams, +}; use pixi_spec::SpecConversionError; use rattler_conda_types::ChannelConfig; use rattler_shell::{ @@ -33,8 +34,20 @@ pub struct InstantiateBackendSpec { /// The backend specification pub backend_spec: BackendSpec, - /// The parameters to initialize the backend with - pub init_params: BackendInitializationParams, + /// The root directory of the workspace. + pub workspace_root: PathBuf, + + /// The absolute path of the discovered manifest + pub manifest_path: PathBuf, + + /// Optionally, the manifest of the discovered package. + pub project_model: Option, + + /// Additional configuration that applies to the backend. + pub configuration: Option, + + /// Targets that apply to the backend. + pub target_configuration: Option>, /// The source directory to use for the backend pub build_source_dir: PathBuf, @@ -55,15 +68,13 @@ impl CommandDispatcher { ) -> Result> { let BackendSpec::JsonRpc(backend_spec) = spec.backend_spec; - let source_dir = spec.build_source_dir; - // Canonicalize the source_dir to ensure it's a fully resolved absolute path // without any relative components like ".." or "." - let source_dir = dunce::canonicalize(&source_dir).map_err(|e| { + let source_dir = dunce::canonicalize(&spec.build_source_dir).map_err(|e| { CommandDispatcherError::Failed(InstantiateBackendError::SpecConversionError( SpecConversionError::InvalidPath(format!( "failed to canonicalize source directory '{}': {}", - source_dir.display(), + spec.build_source_dir.display(), e )), )) @@ -79,13 +90,13 @@ impl CommandDispatcher { if let Some(in_mem) = backend { let memory = in_mem .initialize(InitializeParams { - manifest_path: spec.init_params.manifest_path, + manifest_path: spec.manifest_path, source_dir: Some(source_dir), - workspace_root: Some(spec.init_params.workspace_root), + workspace_root: Some(spec.workspace_root), cache_directory: Some(self.cache_dirs().root().clone()), - project_model: spec.init_params.project_model.map(Into::into), - configuration: spec.init_params.configuration, - target_configuration: spec.init_params.target_configuration, + project_model: spec.project_model.map(Into::into), + configuration: spec.configuration, + target_configuration: spec.target_configuration, }) .map_err(InstantiateBackendError::InMemoryError) .map_err(CommandDispatcherError::Failed)?; @@ -165,7 +176,6 @@ impl CommandDispatcher { // Make sure that the project model is compatible with the API version. if !api_version.supports_name_none() && spec - .init_params .project_model .as_ref() .is_some_and(|p| p.name.is_none()) @@ -177,11 +187,11 @@ impl CommandDispatcher { JsonRpcBackend::setup( source_dir, - spec.init_params.manifest_path, - spec.init_params.workspace_root, - spec.init_params.project_model, - spec.init_params.configuration, - spec.init_params.target_configuration, + spec.manifest_path, + spec.workspace_root, + spec.project_model, + spec.configuration, + spec.target_configuration, Some(self.cache_dirs().root().clone()), tool, ) diff --git a/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs b/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs index 84c6437ac9..c5802ed714 100644 --- a/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs +++ b/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs @@ -35,7 +35,7 @@ use crate::{ SourceBuildCacheEntry, SourceBuildCacheStatusError, SourceBuildCacheStatusSpec, SourceCheckout, SourceCheckoutError, SourceMetadata, SourceMetadataError, SourceMetadataSpec, backend_source_build::{BackendBuiltSource, BackendSourceBuildError, BackendSourceBuildSpec}, - build::{BuildCache, SourceCodeLocation, source_metadata_cache::SourceMetadataCache}, + build::{BuildCache, source_metadata_cache::SourceMetadataCache}, cache_dirs::CacheDirs, discover_backend_cache::DiscoveryCache, install_pixi::{ @@ -591,7 +591,6 @@ impl CommandDispatcher { pub async fn pin_and_checkout( &self, source_location_spec: SourceLocationSpec, - alternative_root: Option<&Path>, ) -> Result> { match source_location_spec { SourceLocationSpec::Url(url) => { @@ -600,7 +599,7 @@ impl CommandDispatcher { SourceLocationSpec::Path(path) => { let source_path = self .data - .resolve_typed_path(path.path.to_path(), alternative_root) + .resolve_typed_path(path.path.to_path()) .map_err(SourceCheckoutError::from) .map_err(CommandDispatcherError::Failed)?; Ok(SourceCheckout { @@ -625,50 +624,19 @@ impl CommandDispatcher { /// - For URL sources: Extracts the archive with the exact checksum /// (unimplemented) pub async fn checkout_pinned_source( - &self, - pinned_spec: &PinnedSourceSpec, - ) -> Result> { - self.checkout_pinned_spec(pinned_spec.clone(), None).await - } - - /// Checkout a source described by a [`SourceCodeLocation`], automatically - /// falling back to the manifest path when no dedicated build source is set. - /// - /// This is useful for the case where you want to checkout a source that is at - /// different location than the manifest, most obviously in an out-of-tree build. - pub async fn checkout_source_location( - &self, - source: &SourceCodeLocation, - ) -> Result> { - let (primary, alternative_root) = source.as_source_and_alternative_root(); - self.checkout_pinned_spec(primary.clone(), alternative_root.cloned()) - .await - } - - async fn checkout_pinned_spec( &self, pinned_spec: PinnedSourceSpec, - alternative_root: Option, ) -> Result> { match pinned_spec { - PinnedSourceSpec::Path(path_spec) => { - let alternative_root_path = match alternative_root.as_ref() { - Some(PinnedSourceSpec::Path(alt_path)) => Some( - self.data - .resolve_typed_path(alt_path.path.to_path(), None) - .map_err(|e| CommandDispatcherError::Failed(e.into()))?, - ), - _ => None, - }; - + PinnedSourceSpec::Path(ref path_spec) => { let source_path = self .data - .resolve_typed_path(path_spec.path.to_path(), alternative_root_path.as_deref()) + .resolve_typed_path(path_spec.path.to_path()) .map_err(SourceCheckoutError::from) .map_err(CommandDispatcherError::Failed)?; Ok(SourceCheckout { path: source_path, - pinned: PinnedSourceSpec::Path(path_spec), + pinned: pinned_spec, }) } PinnedSourceSpec::Git(git_spec) => self.checkout_pinned_git(git_spec).await, @@ -698,11 +666,7 @@ impl CommandDispatcherData { /// /// This function does not check if the path exists and also does not follow /// symlinks. - fn resolve_typed_path( - &self, - path_spec: Utf8TypedPath, - alternative_root: Option<&Path>, - ) -> Result { + fn resolve_typed_path(&self, path_spec: Utf8TypedPath) -> Result { if path_spec.is_absolute() { Ok(Path::new(path_spec.as_str()).to_path_buf()) } else if let Ok(user_path) = path_spec.strip_prefix("~/") { @@ -712,20 +676,7 @@ impl CommandDispatcherData { debug_assert!(home_dir.is_absolute()); normalize_absolute_path(&home_dir.join(Path::new(user_path.as_str()))) } else { - let root_dir = match alternative_root { - Some(root_path) => { - debug_assert!( - root_path.is_absolute(), - "alternative_root must be absolute, got: {root_path:?}" - ); - debug_assert!( - !root_path.is_file(), - "alternative_root should be a directory, not a file: {root_path:?}" - ); - root_path - } - None => self.root_dir.as_path(), - }; + let root_dir = self.root_dir.as_path(); let native_path = Path::new(path_spec.as_str()); debug_assert!(root_dir.is_absolute()); normalize_absolute_path(&root_dir.join(native_path)) diff --git a/crates/pixi_command_dispatcher/src/instantiate_tool_env/mod.rs b/crates/pixi_command_dispatcher/src/instantiate_tool_env/mod.rs index 3e83bd31e8..0553e60428 100644 --- a/crates/pixi_command_dispatcher/src/instantiate_tool_env/mod.rs +++ b/crates/pixi_command_dispatcher/src/instantiate_tool_env/mod.rs @@ -216,7 +216,7 @@ impl InstantiateToolEnvironmentSpec { variants: self.variants.clone(), variant_files: self.variant_files.clone(), strategy: SolveStrategy::default(), - pin_overrides: BTreeMap::new(), + preferred_build_source: BTreeMap::new(), }) .await .map_err_with(Box::new) diff --git a/crates/pixi_command_dispatcher/src/solve_pixi/mod.rs b/crates/pixi_command_dispatcher/src/solve_pixi/mod.rs index af8f645bd5..06a47339e8 100644 --- a/crates/pixi_command_dispatcher/src/solve_pixi/mod.rs +++ b/crates/pixi_command_dispatcher/src/solve_pixi/mod.rs @@ -89,9 +89,10 @@ pub struct PixiEnvironmentSpec { /// Optional override for a specific packages: use this pinned /// source for checkout and as the `package_build_source` instead - /// of pinning anew. + /// of recomputing the pinned location. #[serde(skip)] - pub pin_overrides: BTreeMap, + pub preferred_build_source: + BTreeMap, } impl Default for PixiEnvironmentSpec { @@ -110,7 +111,7 @@ impl Default for PixiEnvironmentSpec { variants: None, variant_files: None, enabled_protocols: EnabledProtocols::default(), - pin_overrides: BTreeMap::new(), + preferred_build_source: BTreeMap::new(), } } } @@ -148,7 +149,7 @@ impl PixiEnvironmentSpec { self.variants.clone(), self.variant_files.clone(), self.enabled_protocols.clone(), - self.pin_overrides.clone(), + self.preferred_build_source.clone(), ) .collect( source_specs diff --git a/crates/pixi_command_dispatcher/src/solve_pixi/source_metadata_collector.rs b/crates/pixi_command_dispatcher/src/solve_pixi/source_metadata_collector.rs index c6deb8db0a..6e40b768a1 100644 --- a/crates/pixi_command_dispatcher/src/solve_pixi/source_metadata_collector.rs +++ b/crates/pixi_command_dispatcher/src/solve_pixi/source_metadata_collector.rs @@ -15,7 +15,6 @@ use thiserror::Error; use crate::{ BuildBackendMetadataSpec, BuildEnvironment, CommandDispatcher, CommandDispatcherError, SourceCheckoutError, SourceMetadataSpec, - build::SourceCodeLocation, executor::ExecutorFutures, source_metadata::{CycleEnvironment, SourceMetadata, SourceMetadataError}, }; @@ -30,7 +29,7 @@ pub struct SourceMetadataCollector { enabled_protocols: EnabledProtocols, variants: Option>>, variant_files: Option>, - pin_overrides: BTreeMap, + preferred_build_sources: BTreeMap, } #[derive(Default)] @@ -79,7 +78,7 @@ impl SourceMetadataCollector { variants: Option>>, variant_files: Option>, enabled_protocols: EnabledProtocols, - pin_overrides: BTreeMap, + preferred_build_sources: BTreeMap, ) -> Self { Self { command_queue, @@ -89,7 +88,7 @@ impl SourceMetadataCollector { channel_config, variants, variant_files, - pin_overrides, + preferred_build_sources, } } @@ -138,7 +137,14 @@ impl SourceMetadataCollector { .map(|source_spec| (name.clone(), source_spec.clone())) }) { // We encountered a transitive source dependency. - specs.push((name, anchor.resolve(source_spec), chain.clone())); + let resolved_location = anchor.resolve(source_spec.location); + specs.push(( + name, + SourceSpec { + location: resolved_location, + }, + chain.clone(), + )); } else { // We encountered a transitive dependency that is not a source result.transitive_dependencies.push(spec); @@ -166,13 +172,13 @@ impl SourceMetadataCollector { tracing::trace!("Collecting source metadata for {name:#?}"); // Determine if we should override the build_source pin for this package. - let override_pin = self.pin_overrides.get(&name).cloned(); + let preferred_build_source = self.preferred_build_sources.get(&name).cloned(); // Always checkout the manifest-defined source location (root), discovery - // will pick build_source; we only override the build pin later. - let source = self + // will pick build_source; we only pass preferred locations. + let manifest_source_checkout = self .command_queue - .pin_and_checkout(spec.location, None) + .pin_and_checkout(spec.location) .await .map_err(|err| CollectSourceMetadataError::SourceCheckoutError { name: name.as_source().to_string(), @@ -186,14 +192,14 @@ impl SourceMetadataCollector { .source_metadata(SourceMetadataSpec { package: name.clone(), backend_metadata: BuildBackendMetadataSpec { - source: SourceCodeLocation::new(source.pinned, None), + manifest_source: manifest_source_checkout.pinned, + preferred_build_source, channel_config: self.channel_config.clone(), channels: self.channels.clone(), build_environment: self.build_environment.clone(), variants: self.variants.clone(), variant_files: self.variant_files.clone(), enabled_protocols: self.enabled_protocols.clone(), - pin_override: override_pin, }, }) .await diff --git a/crates/pixi_command_dispatcher/src/source_build/mod.rs b/crates/pixi_command_dispatcher/src/source_build/mod.rs index ee08dd4cb5..90953bb471 100644 --- a/crates/pixi_command_dispatcher/src/source_build/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_build/mod.rs @@ -220,7 +220,7 @@ impl SourceBuildSpec { // Check out the source code. let manifest_source_checkout = command_dispatcher - .checkout_pinned_source(&manifest_source) + .checkout_pinned_source(manifest_source.clone()) .await .map_err_with(SourceBuildError::SourceCheckout)?; @@ -258,55 +258,45 @@ impl SourceBuildSpec { // 1. Lock file `package_build_source`. Since we're running lock file update before building package it should pin source in there. // 2. Manifest package build. This can happen if package isn't added to the dependencies of manifest, so no pinning happens in that case. // 3. Manifest source. Just assume that source is located at the same directory as the manifest. - let build_source_dir = if let Some(pinned_build_source) = build_source { - let build_location = - SourceCodeLocation::new(manifest_source.clone(), Some(pinned_build_source.clone())); - let build_source_checkout = command_dispatcher - .checkout_source_location(&build_location) + let build_source_checkout = if let Some(pinned_build_source) = build_source { + &command_dispatcher + .checkout_pinned_source(pinned_build_source) .await - .map_err_with(SourceBuildError::SourceCheckout)?; - build_source_checkout.path + .map_err_with(SourceBuildError::SourceCheckout)? } else if let Some(manifest_build_source) = discovered_backend.init_params.build_source.clone() { - let build_source_checkout = command_dispatcher - .pin_and_checkout( - manifest_build_source, - Some( - discovered_backend - .init_params - .manifest_path - .parent() - .ok_or_else(|| { - SourceCheckoutError::ParentDir( - discovered_backend.init_params.manifest_path.clone(), - ) - }) - .map_err(SourceBuildError::SourceCheckout) - .map_err(CommandDispatcherError::Failed)?, - ), - ) + let manifest_source_anchor = + SourceAnchor::from(SourceSpec::from(manifest_source.clone())); + let resolved_build_source = manifest_source_anchor.resolve(manifest_build_source); + &command_dispatcher + .pin_and_checkout(resolved_build_source) .await - .map_err_with(SourceBuildError::SourceCheckout)?; - build_source_checkout.path + .map_err_with(SourceBuildError::SourceCheckout)? } else { - manifest_source_checkout.path + &manifest_source_checkout }; // Instantiate the backend with the discovered information. - let backend = - command_dispatcher - .instantiate_backend(InstantiateBackendSpec { - backend_spec: discovered_backend.backend_spec.clone().resolve( - SourceAnchor::from(SourceSpec::from(manifest_source.clone())), - ), - init_params: discovered_backend.init_params.clone(), - build_source_dir, - channel_config: self.channel_config.clone(), - enabled_protocols: self.enabled_protocols.clone(), - }) - .await - .map_err_with(SourceBuildError::Initialize)?; + let backend = command_dispatcher + .instantiate_backend(InstantiateBackendSpec { + backend_spec: discovered_backend + .backend_spec + .clone() + .resolve(SourceAnchor::from(SourceSpec::from( + manifest_source.clone(), + ))), + build_source_dir: build_source_checkout.path.clone(), + channel_config: self.channel_config.clone(), + enabled_protocols: self.enabled_protocols.clone(), + workspace_root: discovered_backend.init_params.workspace_root.clone(), + manifest_path: discovered_backend.init_params.manifest_path.clone(), + project_model: discovered_backend.init_params.project_model.clone(), + configuration: discovered_backend.init_params.configuration.clone(), + target_configuration: discovered_backend.init_params.target_configuration.clone(), + }) + .await + .map_err_with(SourceBuildError::Initialize)?; // Determine the working directory for the build. let work_directory = match std::mem::take(&mut self.work_directory) { @@ -786,7 +776,7 @@ impl SourceBuildSpec { variants: self.variants.clone(), variant_files: self.variant_files.clone(), enabled_protocols: self.enabled_protocols.clone(), - pin_overrides: BTreeMap::new(), + preferred_build_source: BTreeMap::new(), }) .await } diff --git a/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs b/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs index 1b461044dd..8b67630527 100644 --- a/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_build_cache_status/mod.rs @@ -323,7 +323,7 @@ impl SourceBuildCacheStatusSpec { // Checkout the source for the package. let source_checkout = command_dispatcher - .checkout_pinned_source(manifest_source) + .checkout_pinned_source(manifest_source.clone()) .await .map_err_with(SourceBuildCacheStatusError::SourceCheckout)?; @@ -369,14 +369,14 @@ impl SourceBuildCacheStatusSpec { }; // Checkout the source for the package. - let source_checkout = command_dispatcher - .checkout_source_location(&self.source) + let source_build_checkout = command_dispatcher + .checkout_pinned_source(self.source.source_code().clone()) .await .map_err_with(SourceBuildCacheStatusError::SourceCheckout)?; // Compute the modification time of the files that match the source input globs. let glob_time = match GlobModificationTime::from_patterns( - &source_checkout.path, + &source_build_checkout.path, source_info.globs.iter().map(String::as_str), ) { Ok(glob_time) => glob_time, diff --git a/crates/pixi_command_dispatcher/src/source_metadata/mod.rs b/crates/pixi_command_dispatcher/src/source_metadata/mod.rs index 9098600d2f..8600871518 100644 --- a/crates/pixi_command_dispatcher/src/source_metadata/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_metadata/mod.rs @@ -59,8 +59,8 @@ impl SourceMetadataSpec { skip_all, name = "source-metadata", fields( - manifest_source= %self.backend_metadata.source.manifest_source(), - build_source=?self.backend_metadata.source.build_source(), + manifest_source= %self.backend_metadata.manifest_source, + preferred_build_source=?self.backend_metadata.preferred_build_source, name = %self.package.as_source(), platform = %self.backend_metadata.build_environment.host_platform, ) @@ -370,9 +370,9 @@ impl SourceMetadataSpec { if dependencies.dependencies.is_empty() { return Ok(vec![]); } - let pin_overrides = self + let preferred_build_source = self .backend_metadata - .pin_override + .preferred_build_source .as_ref() .map(|pinned| BTreeMap::from([(pkg_name.clone(), pinned.clone())])) .unwrap_or_default(); @@ -399,7 +399,7 @@ impl SourceMetadataSpec { variants: self.backend_metadata.variants.clone(), variant_files: self.backend_metadata.variant_files.clone(), enabled_protocols: self.backend_metadata.enabled_protocols.clone(), - pin_overrides, + preferred_build_source, }) .await { diff --git a/crates/pixi_command_dispatcher/tests/integration/event_tree.rs b/crates/pixi_command_dispatcher/tests/integration/event_tree.rs index 14a33ecbf3..ce42d3ad55 100644 --- a/crates/pixi_command_dispatcher/tests/integration/event_tree.rs +++ b/crates/pixi_command_dispatcher/tests/integration/event_tree.rs @@ -130,7 +130,7 @@ impl EventTree { format!( "{} @ {}", &spec.package.as_source(), - spec.backend_metadata.source + spec.backend_metadata.manifest_source ), ); builder.set_event_parent((*id).into(), *context); @@ -146,7 +146,7 @@ impl EventTree { } Event::SourceMetadataFinished { .. } => {} Event::BuildBackendMetadataQueued { id, context, spec } => { - build_backend_metadata_label.insert(*id, spec.source.to_string()); + build_backend_metadata_label.insert(*id, spec.manifest_source.to_string()); builder.set_event_parent((*id).into(), *context); } Event::BuildBackendMetadataStarted { id } => { diff --git a/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__cycle.snap b/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__cycle.snap index 02e20f8a35..b5f8bf5d7f 100644 --- a/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__cycle.snap +++ b/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__cycle.snap @@ -16,8 +16,8 @@ ERROR: TRACE: Pixi solve (package_a) -└── Source metadata (package_a @ (manifest-src: package_a, build-src: undefined)) - ├── Build backend metadata ((manifest-src: package_a, build-src: undefined)) +└── Source metadata (package_a @ package_a) + ├── Build backend metadata (package_a) └── Pixi solve (package_b) - └── Source metadata (package_b @ (manifest-src: package_b, build-src: undefined)) - └── Build backend metadata ((manifest-src: package_b, build-src: undefined)) + └── Source metadata (package_b @ package_b) + └── Build backend metadata (package_b) diff --git a/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__simple_test.snap b/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__simple_test.snap index e5a6313312..05f2dd033f 100644 --- a/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__simple_test.snap +++ b/crates/pixi_command_dispatcher/tests/integration/snapshots/integration__simple_test.snap @@ -4,13 +4,13 @@ expression: event_tree.to_string() --- Pixi solve (foobar-desktop) ├── Git Checkout (https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a) -├── Source metadata (foobar-desktop @ (manifest-src: https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a, build-src: undefined)) -│ └── Build backend metadata ((manifest-src: https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a, build-src: undefined)) +├── Source metadata (foobar-desktop @ https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a) +│ └── Build backend metadata (https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a) │ └── Instantiate tool environment (pixi-build-rattler-build) │ ├── Pixi solve (pixi-build-rattler-build) │ │ └── Conda solve #0 │ └── Pixi install (pixi-build-rattler-build) -├── Source metadata (foobar @ (manifest-src: https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a, build-src: undefined)) +├── Source metadata (foobar @ https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a) └── Conda solve #1 Pixi install (test-env) ├── Source build (foobar @ (manifest-src: https://github.com/wolfv/pixi-build-examples@8d230eda9b4cdaaefd24aad87fd923d4b7c3c78a, build-src: undefined)) diff --git a/crates/pixi_core/src/lock_file/satisfiability/mod.rs b/crates/pixi_core/src/lock_file/satisfiability/mod.rs index 88c32d685c..52eaa029d3 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/mod.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/mod.rs @@ -1334,7 +1334,10 @@ pub(crate) async fn verify_package_platform_satisfiability( )) }) { - let anchored_source = anchor.resolve(source.clone()); + let anchored_location = anchor.resolve(source.location.clone()); + let anchored_source = SourceSpec { + location: anchored_location, + }; conda_queue.push(Dependency::CondaSource( package_name.clone(), spec, diff --git a/crates/pixi_core/src/lock_file/update.rs b/crates/pixi_core/src/lock_file/update.rs index 27796fc072..fcb1b5cbd2 100644 --- a/crates/pixi_core/src/lock_file/update.rs +++ b/crates/pixi_core/src/lock_file/update.rs @@ -2028,7 +2028,7 @@ async fn spawn_solve_conda_environment_task( variants: Some(variants), variant_files: Some(variant_files), enabled_protocols: Default::default(), - pin_overrides, + preferred_build_source: pin_overrides, }) .await .map_err(|source| SolveCondaEnvironmentError::SolveFailed { diff --git a/crates/pixi_global/src/common.rs b/crates/pixi_global/src/common.rs index 73e437af1b..6bbae157bc 100644 --- a/crates/pixi_global/src/common.rs +++ b/crates/pixi_global/src/common.rs @@ -1259,7 +1259,7 @@ mod tests { #[cfg(windows)] { for script_name in script_names { - let script_path = bin_dir.path().join(format!("{}.bat", script_name)); + let script_path = bin_dir.path().join(format!("{script_name}.bat")); let script = format!( r#" @"{}" %* @@ -1267,7 +1267,7 @@ mod tests { env_dir .path() .join("bin") - .join(format!("{}.exe", script_name)) + .join(format!("{script_name}.exe")) .to_string_lossy() ); tokio_fs::write(&script_path, script).await.unwrap(); diff --git a/crates/pixi_global/src/project/mod.rs b/crates/pixi_global/src/project/mod.rs index 6df70d0cf0..872dc3e6cf 100644 --- a/crates/pixi_global/src/project/mod.rs +++ b/crates/pixi_global/src/project/mod.rs @@ -23,7 +23,7 @@ pub use parsed_manifest::{ExposedName, ParsedEnvironment, ParsedManifest}; use pixi_build_discovery::EnabledProtocols; use pixi_command_dispatcher::{ BuildBackendMetadataSpec, BuildEnvironment, CommandDispatcher, InstallPixiEnvironmentSpec, - Limits, PixiEnvironmentSpec, build::SourceCodeLocation, + Limits, PixiEnvironmentSpec, }; use pixi_config::{Config, RunPostLinkScripts, default_channel_config, pixi_home}; use pixi_consts::consts::{self}; @@ -1372,7 +1372,7 @@ impl Project { ) -> Result { let command_dispatcher = self.command_dispatcher()?; let checkout = command_dispatcher - .pin_and_checkout(source_spec.location, None) + .pin_and_checkout(source_spec.location) .await .map_err(|e| InferPackageNameError::BuildBackendMetadata(Box::new(e)))?; @@ -1380,7 +1380,8 @@ impl Project { // Create the metadata spec let metadata_spec = BuildBackendMetadataSpec { - source: SourceCodeLocation::new(pinned_source_spec, None), + manifest_source: pinned_source_spec, + preferred_build_source: None, channel_config: self.global_channel_config().clone(), channels: self .config() @@ -1392,7 +1393,6 @@ impl Project { variants: None, variant_files: None, enabled_protocols: Default::default(), - pin_override: None, }; // Get the metadata using the command dispatcher diff --git a/crates/pixi_record/src/pinned_source.rs b/crates/pixi_record/src/pinned_source.rs index 0052cf01c6..05c06b81bf 100644 --- a/crates/pixi_record/src/pinned_source.rs +++ b/crates/pixi_record/src/pinned_source.rs @@ -129,6 +129,89 @@ impl PinnedSourceSpec { PinnedSourceSpec::Path(spec) => spec.identifiable_url(), } } + + /// Checks if this pinned source spec matches a given source spec. + /// + /// This method determines if the pinned source and the source spec refer to + /// the same underlying source, ignoring version-specific details like git + /// commits or archive hashes. This is useful for determining if a pinned + /// source satisfies a given source requirement. + /// + /// # Matching Rules + /// + /// - **Path sources**: Paths must be exactly equal (same normalized path) + /// - **Git sources**: Repository URLs must match (ignoring credentials and + /// case), and subdirectories must match if specified in the source spec + /// - **URL sources**: URLs must be exactly equal (including all components) + /// + /// # Examples + /// + /// ``` + /// use pixi_record::{PinnedSourceSpec, PinnedGitSpec, PinnedGitCheckout}; + /// use pixi_spec::{SourceSpec, SourceLocationSpec, GitSpec, GitReference}; + /// use pixi_git::sha::GitSha; + /// use url::Url; + /// use std::str::FromStr; + /// + /// # fn main() -> Result<(), Box> { + /// // Git source matching + /// let pinned_git = PinnedSourceSpec::Git(PinnedGitSpec { + /// git: Url::parse("https://github.com/user/repo")?, + /// source: PinnedGitCheckout { + /// commit: GitSha::from_str("abc123def456")?, + /// subdirectory: None, + /// reference: GitReference::DefaultBranch, + /// }, + /// }); + /// + /// let source_spec = SourceSpec { + /// location: SourceLocationSpec::Git(GitSpec { + /// git: Url::parse("https://github.com/user/repo.git")?, + /// rev: None, + /// subdirectory: None, + /// }), + /// }; + /// + /// assert!(pinned_git.matches_source_spec(&source_spec)); + /// # Ok(()) + /// # } + /// ``` + pub fn matches_source_spec(&self, source_spec: &SourceSpec) -> bool { + match (self, &source_spec.location) { + // Path sources: paths must be exactly equal + (PinnedSourceSpec::Path(pinned_path), SourceLocationSpec::Path(source_path)) => { + pinned_path.path == source_path.path + } + + // Git sources: repository URLs must match, subdirectories must match if specified + (PinnedSourceSpec::Git(pinned_git), SourceLocationSpec::Git(source_git)) => { + use pixi_git::url::RepositoryUrl; + + // Compare repository URLs (ignoring commit/branch details) + let pinned_repo = RepositoryUrl::new(&pinned_git.git); + let source_repo = RepositoryUrl::new(&source_git.git); + + if pinned_repo != source_repo { + return false; + } + + // If source spec specifies a subdirectory, it must match + match (&source_git.subdirectory, &pinned_git.source.subdirectory) { + (Some(source_subdir), Some(pinned_subdir)) => source_subdir == pinned_subdir, + (Some(_), None) => false, // Source expects subdirectory, but pinned doesn't have one + (None, _) => true, // Source doesn't care about subdirectory + } + } + + // URL sources: URLs must be exactly equal + (PinnedSourceSpec::Url(pinned_url), SourceLocationSpec::Url(source_url)) => { + pinned_url.url == source_url.url + } + + // Mismatched types never match + _ => false, + } + } } impl MutablePinnedSourceSpec { @@ -277,6 +360,7 @@ pub struct PinnedGitSpec { /// The URL of the repository without the revision and subdirectory /// fragment. pub git: Url, + /// The resolved git checkout. #[serde(flatten)] pub source: PinnedGitCheckout, @@ -1006,4 +1090,349 @@ mod tests { SourceMismatchError::GitSubdirectoryMismatch { .. } )); } + + mod matches_source_spec { + use super::*; + use pixi_spec::{PathSourceSpec, SourceLocationSpec, SourceSpec, UrlSourceSpec}; + use typed_path::Utf8TypedPathBuf; + + use crate::{PinnedPathSpec, PinnedSourceSpec, PinnedUrlSpec}; + + #[test] + fn test_path_exact_match() { + let pinned = PinnedSourceSpec::Path(PinnedPathSpec { + path: Utf8TypedPathBuf::from("/path/to/source"), + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Path(PathSourceSpec { + path: Utf8TypedPathBuf::from("/path/to/source"), + }), + }; + + assert!(pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_path_mismatch() { + let pinned = PinnedSourceSpec::Path(PinnedPathSpec { + path: Utf8TypedPathBuf::from("/path/to/source"), + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Path(PathSourceSpec { + path: Utf8TypedPathBuf::from("/different/path"), + }), + }; + + assert!(!pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_git_same_repo_without_git_suffix() { + let pinned = PinnedSourceSpec::Git(PinnedGitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + source: PinnedGitCheckout { + commit: GitSha::from_str("abc123def456789012345678901234567890abcd").unwrap(), + subdirectory: None, + reference: GitReference::DefaultBranch, + }, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Git(GitSpec { + git: Url::parse("https://github.com/user/repo.git").unwrap(), + rev: None, + subdirectory: None, + }), + }; + + // Should match despite .git suffix difference + assert!(pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_git_same_repo_with_git_suffix() { + let pinned = PinnedSourceSpec::Git(PinnedGitSpec { + git: Url::parse("https://github.com/user/repo.git").unwrap(), + source: PinnedGitCheckout { + commit: GitSha::from_str("abc123def456789012345678901234567890abcd").unwrap(), + subdirectory: None, + reference: GitReference::DefaultBranch, + }, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Git(GitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + rev: None, + subdirectory: None, + }), + }; + + // Should match despite .git suffix difference + assert!(pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_git_different_repos() { + let pinned = PinnedSourceSpec::Git(PinnedGitSpec { + git: Url::parse("https://github.com/user/repo1").unwrap(), + source: PinnedGitCheckout { + commit: GitSha::from_str("abc123def456789012345678901234567890abcd").unwrap(), + subdirectory: None, + reference: GitReference::DefaultBranch, + }, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Git(GitSpec { + git: Url::parse("https://github.com/user/repo2").unwrap(), + rev: None, + subdirectory: None, + }), + }; + + assert!(!pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_git_same_repo_no_subdirectory_in_spec() { + let pinned = PinnedSourceSpec::Git(PinnedGitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + source: PinnedGitCheckout { + commit: GitSha::from_str("abc123def456789012345678901234567890abcd").unwrap(), + subdirectory: Some("subdir".to_string()), + reference: GitReference::DefaultBranch, + }, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Git(GitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + rev: None, + subdirectory: None, + }), + }; + + // Should match - spec doesn't care about subdirectory + assert!(pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_git_same_repo_matching_subdirectory() { + let pinned = PinnedSourceSpec::Git(PinnedGitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + source: PinnedGitCheckout { + commit: GitSha::from_str("abc123def456789012345678901234567890abcd").unwrap(), + subdirectory: Some("subdir".to_string()), + reference: GitReference::DefaultBranch, + }, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Git(GitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + rev: None, + subdirectory: Some("subdir".to_string()), + }), + }; + + assert!(pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_git_same_repo_mismatching_subdirectory() { + let pinned = PinnedSourceSpec::Git(PinnedGitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + source: PinnedGitCheckout { + commit: GitSha::from_str("abc123def456789012345678901234567890abcd").unwrap(), + subdirectory: Some("subdir1".to_string()), + reference: GitReference::DefaultBranch, + }, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Git(GitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + rev: None, + subdirectory: Some("subdir2".to_string()), + }), + }; + + assert!(!pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_git_spec_requires_subdirectory_but_pinned_has_none() { + let pinned = PinnedSourceSpec::Git(PinnedGitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + source: PinnedGitCheckout { + commit: GitSha::from_str("abc123def456789012345678901234567890abcd").unwrap(), + subdirectory: None, + reference: GitReference::DefaultBranch, + }, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Git(GitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + rev: None, + subdirectory: Some("subdir".to_string()), + }), + }; + + // Should not match - spec requires a subdirectory that pinned doesn't have + assert!(!pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_url_exact_match() { + let pinned = PinnedSourceSpec::Url(PinnedUrlSpec { + url: Url::parse("https://example.com/archive.tar.gz").unwrap(), + sha256: rattler_digest::parse_digest_from_hex::( + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + ) + .unwrap(), + md5: None, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Url(UrlSourceSpec { + url: Url::parse("https://example.com/archive.tar.gz").unwrap(), + sha256: None, + md5: None, + }), + }; + + assert!(pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_url_mismatch() { + let pinned = PinnedSourceSpec::Url(PinnedUrlSpec { + url: Url::parse("https://example.com/archive.tar.gz").unwrap(), + sha256: rattler_digest::parse_digest_from_hex::( + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + ) + .unwrap(), + md5: None, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Url(UrlSourceSpec { + url: Url::parse("https://example.com/different.tar.gz").unwrap(), + sha256: None, + md5: None, + }), + }; + + assert!(!pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_type_mismatch_path_vs_git() { + let pinned = PinnedSourceSpec::Path(PinnedPathSpec { + path: Utf8TypedPathBuf::from("/path/to/source"), + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Git(GitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + rev: None, + subdirectory: None, + }), + }; + + assert!(!pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_type_mismatch_git_vs_url() { + let pinned = PinnedSourceSpec::Git(PinnedGitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + source: PinnedGitCheckout { + commit: GitSha::from_str("abc123def456789012345678901234567890abcd").unwrap(), + subdirectory: None, + reference: GitReference::DefaultBranch, + }, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Url(UrlSourceSpec { + url: Url::parse("https://example.com/archive.tar.gz").unwrap(), + sha256: None, + md5: None, + }), + }; + + assert!(!pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_type_mismatch_url_vs_path() { + let pinned = PinnedSourceSpec::Url(PinnedUrlSpec { + url: Url::parse("https://example.com/archive.tar.gz").unwrap(), + sha256: rattler_digest::parse_digest_from_hex::( + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + ) + .unwrap(), + md5: None, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Path(PathSourceSpec { + path: Utf8TypedPathBuf::from("/path/to/source"), + }), + }; + + assert!(!pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_git_ignores_different_commits() { + let pinned = PinnedSourceSpec::Git(PinnedGitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + source: PinnedGitCheckout { + commit: GitSha::from_str("abc123def456789012345678901234567890abcd").unwrap(), + subdirectory: None, + reference: GitReference::Rev("v1.0.0".to_string()), + }, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Git(GitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + rev: Some(GitReference::Rev("v2.0.0".to_string())), + subdirectory: None, + }), + }; + + // Should match - we only compare repository and subdirectory, not the commit/rev + assert!(pinned.matches_source_spec(&spec)); + } + + #[test] + fn test_git_case_insensitive_github() { + let pinned = PinnedSourceSpec::Git(PinnedGitSpec { + git: Url::parse("https://github.com/User/Repo").unwrap(), + source: PinnedGitCheckout { + commit: GitSha::from_str("abc123def456789012345678901234567890abcd").unwrap(), + subdirectory: None, + reference: GitReference::DefaultBranch, + }, + }); + + let spec = SourceSpec { + location: SourceLocationSpec::Git(GitSpec { + git: Url::parse("https://github.com/user/repo").unwrap(), + rev: None, + subdirectory: None, + }), + }; + + // Should match - GitHub URLs are case-insensitive + assert!(pinned.matches_source_spec(&spec)); + } + } } diff --git a/crates/pixi_spec/src/source_anchor.rs b/crates/pixi_spec/src/source_anchor.rs index e9a0f12c60..35f05867f3 100644 --- a/crates/pixi_spec/src/source_anchor.rs +++ b/crates/pixi_spec/src/source_anchor.rs @@ -13,62 +13,55 @@ pub enum SourceAnchor { Workspace, /// The source is relative to another source package. - Source(SourceSpec), + Source(SourceLocationSpec), } impl From for SourceAnchor { fn from(value: SourceSpec) -> Self { + SourceAnchor::Source(value.location) + } +} + +impl From for SourceAnchor { + fn from(value: SourceLocationSpec) -> Self { SourceAnchor::Source(value) } } impl SourceAnchor { - /// Resolve a source spec relative to this anchor. - pub fn resolve(&self, spec: SourceSpec) -> SourceSpec { + /// Resolve a source location spec relative to this anchor. + pub fn resolve(&self, spec: SourceLocationSpec) -> SourceLocationSpec { // If this instance is already anchored to the workspace we can simply return // immediately. let SourceAnchor::Source(base) = self else { - return match spec.location { - SourceLocationSpec::Url(url) => SourceSpec { - location: SourceLocationSpec::Url(url), - }, - SourceLocationSpec::Git(git) => SourceSpec { - location: SourceLocationSpec::Git(git), - }, + return match spec { + SourceLocationSpec::Url(url) => SourceLocationSpec::Url(url), + SourceLocationSpec::Git(git) => SourceLocationSpec::Git(git), SourceLocationSpec::Path(PathSourceSpec { path }) => { - SourceSpec { - location: SourceLocationSpec::Path(PathSourceSpec { - // Normalize the input path. - path: normalize_typed(path.to_path()), - }), - } + SourceLocationSpec::Path(PathSourceSpec { + // Normalize the input path. + path: normalize_typed(path.to_path()), + }) } }; }; // Only path specs can be relative. - let SourceSpec { - location: SourceLocationSpec::Path(PathSourceSpec { path }), - } = spec - else { + let SourceLocationSpec::Path(PathSourceSpec { path }) = spec else { return spec; }; // If the path is absolute we can just return it. if path.is_absolute() || path.starts_with("~") { - return SourceSpec { - location: SourceLocationSpec::Path(PathSourceSpec { path }), - }; + return SourceLocationSpec::Path(PathSourceSpec { path }); } - match &base.location { + match base { SourceLocationSpec::Path(PathSourceSpec { path: base }) => { let relative_path = normalize_typed(base.join(path).to_path()); - SourceSpec { - location: SourceLocationSpec::Path(PathSourceSpec { - path: relative_path, - }), - } + SourceLocationSpec::Path(PathSourceSpec { + path: relative_path, + }) } SourceLocationSpec::Url(UrlSourceSpec { .. }) => { unimplemented!("Cannot resolve relative paths for URL sources") @@ -83,13 +76,11 @@ impl SourceAnchor { .join(path) .to_path(), ); - SourceSpec { - location: SourceLocationSpec::Git(GitSpec { - git: git.clone(), - rev: rev.clone(), - subdirectory: Some(relative_subdir.to_string()), - }), - } + SourceLocationSpec::Git(GitSpec { + git: git.clone(), + rev: rev.clone(), + subdirectory: Some(relative_subdir.to_string()), + }) } } } From 1975d42db492e5ea63cf9c98992eed0a9ba2a9da Mon Sep 17 00:00:00 2001 From: Bas Zalmstra <4995967+baszalmstra@users.noreply.github.com> Date: Wed, 5 Nov 2025 13:38:52 +0100 Subject: [PATCH 08/21] wip: include the workspace root when serializing and deserializing the lock file --- Cargo.lock | 1 + .../src/lock_file/satisfiability/mod.rs | 4 +- crates/pixi_core/src/lock_file/update.rs | 17 +- crates/pixi_record/Cargo.toml | 1 + crates/pixi_record/src/lib.rs | 66 +-- crates/pixi_record/src/pinned_source.rs | 190 +++++++++ crates/pixi_record/src/source_record.rs | 393 +++++++++++++++--- 7 files changed, 585 insertions(+), 87 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 123a855199..6f839e8d02 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5297,6 +5297,7 @@ version = "0.1.0" dependencies = [ "file_url", "miette 7.6.0", + "pathdiff", "pixi_git", "pixi_spec", "rattler_conda_types", diff --git a/crates/pixi_core/src/lock_file/satisfiability/mod.rs b/crates/pixi_core/src/lock_file/satisfiability/mod.rs index 52eaa029d3..d5477e716f 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/mod.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/mod.rs @@ -685,9 +685,7 @@ pub async fn verify_platform_satisfiability( LockedPackageRef::Conda(conda) => { let url = conda.location().clone(); pixi_records.push( - conda - .clone() - .try_into() + PixiRecord::from_conda_package_data(conda.clone(), project_root) .map_err(|e| PlatformUnsat::CorruptedEntry(url.to_string(), e))?, ); } diff --git a/crates/pixi_core/src/lock_file/update.rs b/crates/pixi_core/src/lock_file/update.rs index fcb1b5cbd2..9d0bd41f55 100644 --- a/crates/pixi_core/src/lock_file/update.rs +++ b/crates/pixi_core/src/lock_file/update.rs @@ -502,7 +502,8 @@ impl<'p> LockFileDerivedData<'p> { LockedPackageRef::Pypi(data, _) => Either::Right(data.name.clone()), }); - let pixi_records = locked_packages_to_pixi_records(conda_packages)?; + let pixi_records = + locked_packages_to_pixi_records(conda_packages, self.workspace.root())?; let pypi_records = pypi_packages .into_iter() @@ -679,7 +680,7 @@ impl<'p> LockFileDerivedData<'p> { } else { Vec::new() }; - let records = locked_packages_to_pixi_records(packages)?; + let records = locked_packages_to_pixi_records(packages, self.workspace.root())?; // Update the conda prefix let CondaPrefixUpdated { @@ -767,12 +768,13 @@ impl PackageFilterNames { fn locked_packages_to_pixi_records( conda_packages: Vec>, + workspace_root: &std::path::Path, ) -> Result, Report> { let pixi_records = conda_packages .into_iter() .filter_map(LockedPackageRef::as_conda) .cloned() - .map(PixiRecord::try_from) + .map(|data| PixiRecord::from_conda_package_data(data, workspace_root)) .collect::, _>>() .into_diagnostic()?; Ok(pixi_records) @@ -1130,6 +1132,7 @@ impl<'p> UpdateContextBuilder<'p> { // Extract the current conda records from the lock-file // TODO: Should we parallelize this? Measure please. + let workspace_root = project.root(); let locked_repodata_records = project .environments() .into_iter() @@ -1143,7 +1146,7 @@ impl<'p> UpdateContextBuilder<'p> { .map(|(platform, records)| { records .cloned() - .map(PixiRecord::try_from) + .map(|data| PixiRecord::from_conda_package_data(data, workspace_root)) .collect::, _>>() .map(|records| { (platform, Arc::new(PixiRecordsByName::from_iter(records))) @@ -1826,7 +1829,11 @@ impl<'p> UpdateContext<'p> { for platform in environment.platforms() { if let Some(records) = self.take_latest_repodata_records(&environment, platform) { for record in records.into_inner() { - builder.add_conda_package(&environment_name, platform, record.into()); + builder.add_conda_package( + &environment_name, + platform, + record.into_conda_package_data(project.root()), + ); } } if let Some(records) = self.take_latest_pypi_records(&environment, platform) { diff --git a/crates/pixi_record/Cargo.toml b/crates/pixi_record/Cargo.toml index 31a4990a4a..92e3657806 100644 --- a/crates/pixi_record/Cargo.toml +++ b/crates/pixi_record/Cargo.toml @@ -12,6 +12,7 @@ version = "0.1.0" [dependencies] file_url = { workspace = true } miette = { workspace = true } +pathdiff = { workspace = true } pixi_git = { workspace = true } pixi_spec = { workspace = true, features = ["rattler_lock"] } rattler_conda_types = { workspace = true } diff --git a/crates/pixi_record/src/lib.rs b/crates/pixi_record/src/lib.rs index 7992cc29ea..d9e0928b1b 100644 --- a/crates/pixi_record/src/lib.rs +++ b/crates/pixi_record/src/lib.rs @@ -38,6 +38,43 @@ impl PixiRecord { } } + /// Convert to CondaPackageData with paths made relative to workspace_root. + /// This should be used when writing to the lock file. + pub fn into_conda_package_data( + self, + workspace_root: &std::path::Path, + ) -> CondaPackageData { + match self { + PixiRecord::Binary(record) => record.into(), + PixiRecord::Source(record) => { + CondaPackageData::Source(record.into_conda_source_data(workspace_root)) + } + } + } + + /// Create PixiRecord from CondaPackageData with paths resolved relative to workspace_root. + /// This should be used when reading from the lock file. + pub fn from_conda_package_data( + data: CondaPackageData, + workspace_root: &std::path::Path, + ) -> Result { + let record = match data { + CondaPackageData::Binary(value) => { + let location = value.location.clone(); + PixiRecord::Binary(value.try_into().map_err(|err| match err { + ConversionError::Missing(field) => ParseLockFileError::Missing(location, field), + ConversionError::LocationToUrlConversionError(err) => { + ParseLockFileError::InvalidRecordUrl(location, err) + } + })?) + } + CondaPackageData::Source(value) => { + PixiRecord::Source(SourceRecord::from_conda_source_data(value, workspace_root)?) + } + }; + Ok(record) + } + /// Returns a reference to the binary record if it is a binary record. pub fn as_binary(&self) -> Option<&RepoDataRecord> { match self { @@ -104,35 +141,6 @@ pub enum ParseLockFileError { PinnedSourceSpecError(#[from] pinned_source::ParseError), } -impl TryFrom for PixiRecord { - type Error = ParseLockFileError; - - fn try_from(value: CondaPackageData) -> Result { - let record = match value { - CondaPackageData::Binary(value) => { - let location = value.location.clone(); - PixiRecord::Binary(value.try_into().map_err(|err| match err { - ConversionError::Missing(field) => ParseLockFileError::Missing(location, field), - ConversionError::LocationToUrlConversionError(err) => { - ParseLockFileError::InvalidRecordUrl(location, err) - } - })?) - } - CondaPackageData::Source(value) => PixiRecord::Source(value.try_into()?), - }; - Ok(record) - } -} - -impl From for CondaPackageData { - fn from(value: PixiRecord) -> Self { - match value { - PixiRecord::Binary(record) => record.into(), - PixiRecord::Source(record) => record.into(), - } - } -} - impl Matches for NamelessMatchSpec { fn matches(&self, record: &PixiRecord) -> bool { match record { diff --git a/crates/pixi_record/src/pinned_source.rs b/crates/pixi_record/src/pinned_source.rs index 05c06b81bf..759e5c49b3 100644 --- a/crates/pixi_record/src/pinned_source.rs +++ b/crates/pixi_record/src/pinned_source.rs @@ -212,6 +212,97 @@ impl PinnedSourceSpec { _ => false, } } + + /// Makes this pinned source relative to another pinned source if both are path sources + /// or both are git sources pointing to the same repository. + /// This is useful for making `build_source` relative to `manifest_source` in lock files. + /// + /// Returns `None` if: + /// - Not a compatible combination (different types or different git repos) + /// - The sources cannot be made relative to each other + /// + /// # Arguments + /// * `base` - The base pinned source to make this path relative to (typically the manifest_source) + pub fn make_relative_to(&self, base: &PinnedSourceSpec) -> Option { + use pixi_git::url::RepositoryUrl; + use typed_path::{Utf8Component, Utf8UnixPath}; + + match (self, base) { + // Path-to-Path: Make the path relative + (PinnedSourceSpec::Path(this_path), PinnedSourceSpec::Path(base_path)) => { + // Base path is a directory (not a file path) + // Create a temporary Path for the base directory + let base_dir = Path::new(base_path.path.as_str()); + + Some(PinnedSourceSpec::Path(this_path.make_relative_to(base_dir))) + } + // Git-to-Git: If same repository, convert to a relative path based on subdirectories + (PinnedSourceSpec::Git(this_git), PinnedSourceSpec::Git(base_git)) => { + // Check if both point to the same repository + let this_repo = RepositoryUrl::new(&this_git.git); + let base_repo = RepositoryUrl::new(&base_git.git); + + if this_repo != base_repo { + // Different repositories, can't make relative + return None; + } + + // Same repository - compute relative path between subdirectories + let base_subdir = base_git.source.subdirectory.as_deref().unwrap_or(""); + let this_subdir = this_git.source.subdirectory.as_deref().unwrap_or(""); + + // If both point to the same subdirectory, don't relativize (keep as Git) + if base_subdir == this_subdir { + return None; + } + + // Always use Unix-style paths for Git subdirectories + let base_unix = Utf8UnixPath::new(base_subdir); + let this_unix = Utf8UnixPath::new(this_subdir); + + // Try to compute relative path + if let Ok(rel) = this_unix.strip_prefix(base_unix) { + // this_subdir is under base_subdir + Some(PinnedSourceSpec::Path(PinnedPathSpec { + path: rel.as_str().into(), + })) + } else { + // Need to compute relative path using .. components + let base_components: Vec<_> = base_unix.components().collect(); + let this_components: Vec<_> = this_unix.components().collect(); + + // Find common prefix + let common = base_components + .iter() + .zip(this_components.iter()) + .take_while(|(a, b)| a == b) + .count(); + + // Build relative path: go up from base, then down to target + let ups = base_components.len() - common; + let mut rel_path = String::new(); + for _ in 0..ups { + if !rel_path.is_empty() { + rel_path.push('/'); + } + rel_path.push_str(".."); + } + for comp in &this_components[common..] { + if !rel_path.is_empty() { + rel_path.push('/'); + } + rel_path.push_str(comp.as_str()); + } + + Some(PinnedSourceSpec::Path(PinnedPathSpec { + path: rel_path.into(), + })) + } + } + // Different types or incompatible sources + _ => None, + } + } } impl MutablePinnedSourceSpec { @@ -464,6 +555,105 @@ impl PinnedPathSpec { } } + /// Makes this path relative to another path if possible. + /// If the path is already relative or cannot be made relative, returns self unchanged. + /// + /// # Arguments + /// * `base` - The base path to make this path relative to + pub fn make_relative_to(&self, base: &Path) -> Self { + use typed_path::{Utf8Component, Utf8UnixPath, Utf8WindowsPath}; + + // Check if the path is already relative + let is_relative = if self.path.as_str().starts_with('/') { + // Unix absolute path + false + } else if self.path.as_str().len() >= 2 && self.path.as_str().chars().nth(1) == Some(':') { + // Windows absolute path (C:\...) + false + } else { + // Relative path + true + }; + + if is_relative { + return self.clone(); + } + + // Convert base to string for comparison + let base_str = base.to_string_lossy(); + + // Determine if we're working with Unix or Windows paths + let is_unix_style = self.path.as_str().starts_with('/'); + + if is_unix_style { + // Use Unix path logic + let this_unix = Utf8UnixPath::new(self.path.as_str()); + let base_unix = Utf8UnixPath::new(&base_str); + + if let Ok(rel) = this_unix.strip_prefix(base_unix) { + Self { + path: rel.as_str().into(), + } + } else { + // Try using pathdiff for more complex relativization + let this_components: Vec<_> = this_unix.components().collect(); + let base_components: Vec<_> = base_unix.components().collect(); + + // Find common prefix + let common = this_components + .iter() + .zip(base_components.iter()) + .take_while(|(a, b)| a == b) + .count(); + + if common == 0 { + // No common prefix, keep absolute + return self.clone(); + } + + // Build relative path: go up from base, then down to target + let ups = base_components.len() - common; + let mut rel_path = String::new(); + for _ in 0..ups { + if !rel_path.is_empty() { + rel_path.push('/'); + } + rel_path.push_str(".."); + } + for comp in &this_components[common..] { + if !rel_path.is_empty() { + rel_path.push('/'); + } + rel_path.push_str(comp.as_str()); + } + + Self { + path: rel_path.into(), + } + } + } else { + // Use Windows path logic + let this_windows = Utf8WindowsPath::new(self.path.as_str()); + let base_windows = Utf8WindowsPath::new(&base_str); + + if let Ok(rel) = this_windows.strip_prefix(base_windows) { + Self { + path: rel.as_str().into(), + } + } else { + // For Windows, fall back to pathdiff with native paths + let native_path = Path::new(self.path.as_str()); + if let Some(relative_path) = pathdiff::diff_paths(native_path, base) { + Self { + path: relative_path.to_string_lossy().to_string().into(), + } + } else { + self.clone() + } + } + } + } + /// Returns a URL that uniquely identifies this path spec. This URL is not /// portable, e.g. it might result in a different URL on different systems. pub fn identifiable_url(&self) -> Url { diff --git a/crates/pixi_record/src/source_record.rs b/crates/pixi_record/src/source_record.rs index 68bf60c0b5..99502aa7d3 100644 --- a/crates/pixi_record/src/source_record.rs +++ b/crates/pixi_record/src/source_record.rs @@ -7,7 +7,7 @@ use pixi_git::sha::GitSha; use pixi_spec::{GitReference, SourceSpec}; use rattler_conda_types::{MatchSpec, Matches, NamelessMatchSpec, PackageRecord}; use rattler_digest::{Sha256, Sha256Hash}; -use rattler_lock::{CondaPackageData, CondaSourceData, GitShallowSpec, PackageBuildSource}; +use rattler_lock::{CondaSourceData, GitShallowSpec, PackageBuildSource}; use serde::{Deserialize, Serialize}; use typed_path::Utf8TypedPathBuf; @@ -55,62 +55,86 @@ pub struct InputHash { pub globs: BTreeSet, } -impl From for CondaPackageData { - fn from(value: SourceRecord) -> Self { - let package_build_source = value.build_source.map(|s| match s { - PinnedSourceSpec::Url(pinned_url_spec) => PackageBuildSource::Url { - url: pinned_url_spec.url, - sha256: pinned_url_spec.sha256, - subdir: None, - }, - PinnedSourceSpec::Git(pinned_git_spec) => { - let subdirectory = pinned_git_spec - .source - .subdirectory - .as_deref() - .map(Utf8TypedPathBuf::from); - - let spec = match &pinned_git_spec.source.reference { - GitReference::Branch(branch) => Some(GitShallowSpec::Branch(branch.clone())), - GitReference::Tag(tag) => Some(GitShallowSpec::Tag(tag.clone())), - GitReference::Rev(_) => Some(GitShallowSpec::Rev), - GitReference::DefaultBranch => None, - }; +impl SourceRecord { + /// Convert to CondaSourceData with paths made relative to workspace_root. + /// This should be used when writing to the lock file. + /// + /// TODO: Currently this doesn't use workspace_root and just makes build_source + /// relative to manifest_source. Future implementation should make both relative + /// to workspace_root first. + pub fn into_conda_source_data( + self, + _workspace_root: &std::path::Path, + ) -> CondaSourceData { + let package_build_source = self.build_source.map(|s| { + // Try to make build_source relative to manifest_source + let s = s.make_relative_to(&self.manifest_source).unwrap_or(s); - PackageBuildSource::Git { - url: pinned_git_spec.git, - spec, - rev: pinned_git_spec.source.commit.to_string(), - subdir: subdirectory, + match s { + PinnedSourceSpec::Url(pinned_url_spec) => PackageBuildSource::Url { + url: pinned_url_spec.url, + sha256: pinned_url_spec.sha256, + subdir: None, + }, + PinnedSourceSpec::Git(pinned_git_spec) => { + let subdirectory = pinned_git_spec + .source + .subdirectory + .as_deref() + .map(Utf8TypedPathBuf::from); + + let spec = match &pinned_git_spec.source.reference { + GitReference::Branch(branch) => { + Some(GitShallowSpec::Branch(branch.clone())) + } + GitReference::Tag(tag) => Some(GitShallowSpec::Tag(tag.clone())), + GitReference::Rev(_) => Some(GitShallowSpec::Rev), + GitReference::DefaultBranch => None, + }; + + PackageBuildSource::Git { + url: pinned_git_spec.git, + spec, + rev: pinned_git_spec.source.commit.to_string(), + subdir: subdirectory, + } } + PinnedSourceSpec::Path(pinned_path) => PackageBuildSource::Path { + path: pinned_path.path, + }, } - PinnedSourceSpec::Path(pinned_path) => PackageBuildSource::Path { - path: pinned_path.path, - }, }); - CondaPackageData::Source(CondaSourceData { - package_record: value.package_record, - location: value.manifest_source.clone().into(), + + CondaSourceData { + package_record: self.package_record, + location: self.manifest_source.clone().into(), package_build_source, - input: value.input_hash.map(|i| rattler_lock::InputHash { + input: self.input_hash.map(|i| rattler_lock::InputHash { hash: i.hash, - // TODO: fix this in rattler globs: Vec::from_iter(i.globs), }), - sources: value + sources: self .sources .into_iter() .map(|(k, v)| (k, v.into())) .collect(), - }) + } } -} -impl TryFrom for SourceRecord { - type Error = ParseLockFileError; + /// Create SourceRecord from CondaSourceData with paths resolved relative to workspace_root. + /// This should be used when reading from the lock file. + /// + /// TODO: Currently this doesn't use workspace_root and just resolves build_source + /// relative to manifest_source. Future implementation should resolve both against + /// workspace_root first. + pub fn from_conda_source_data( + data: CondaSourceData, + _workspace_root: &std::path::Path, + ) -> Result { - fn try_from(value: CondaSourceData) -> Result { - let pinned_source_spec = value.package_build_source.map(|source| match source { + let manifest_source: PinnedSourceSpec = data.location.try_into()?; + + let pinned_source_spec = data.package_build_source.map(|source| match source { PackageBuildSource::Git { url, spec, @@ -143,18 +167,21 @@ impl TryFrom for SourceRecord { md5: None, }), PackageBuildSource::Path { path } => { + // Keep paths as-is from the lock file + // They will be resolved later by the code that uses them PinnedSourceSpec::Path(crate::PinnedPathSpec { path }) } }); + Ok(Self { - package_record: value.package_record, - manifest_source: value.location.try_into()?, - input_hash: value.input.map(|hash| InputHash { + package_record: data.package_record, + manifest_source, + input_hash: data.input.map(|hash| InputHash { hash: hash.hash, globs: BTreeSet::from_iter(hash.globs), }), build_source: pinned_source_spec, - sources: value + sources: data .sources .into_iter() .map(|(k, v)| (k, SourceSpec::from(v))) @@ -238,9 +265,9 @@ mod tests { sources: Default::default(), }; - let CondaPackageData::Source(conda_source) = record.clone().into() else { - panic!("expected source package data"); - }; + let conda_source = record + .clone() + .into_conda_source_data(&std::path::PathBuf::from("/workspace")); let package_build_source = conda_source .package_build_source @@ -263,7 +290,11 @@ mod tests { assert!(matches!(spec, Some(GitShallowSpec::Branch(branch)) if branch == "main")); assert_eq!(rev, "0123456789abcdef0123456789abcdef01234567"); - let roundtrip = SourceRecord::try_from(conda_source).expect("roundtrip should succeed"); + let roundtrip = SourceRecord::from_conda_source_data( + conda_source, + &std::path::PathBuf::from("/workspace"), + ) + .expect("roundtrip should succeed"); let Some(PinnedSourceSpec::Git(roundtrip_git)) = roundtrip.build_source else { panic!("expected git pinned source"); }; @@ -273,4 +304,266 @@ mod tests { ); assert_eq!(roundtrip_git.git, git_url); } + + #[test] + fn package_build_source_path_is_made_relative() { + use typed_path::Utf8TypedPathBuf; + + let package_record: PackageRecord = serde_json::from_value(json!({ + "name": "example", + "version": "1.0.0", + "build": "0", + "build_number": 0, + "subdir": "noarch", + })) + .expect("valid package record"); + + // Manifest is in /workspace/recipes directory + let manifest_source = PinnedSourceSpec::Path(crate::PinnedPathSpec { + path: Utf8TypedPathBuf::from("/workspace/recipes"), + }); + + // Build source is in /workspace/src (sibling of recipes) + let build_source = PinnedSourceSpec::Path(crate::PinnedPathSpec { + path: Utf8TypedPathBuf::from("/workspace/src"), + }); + + let record = SourceRecord { + package_record, + manifest_source: manifest_source.clone(), + build_source: Some(build_source), + input_hash: None, + sources: Default::default(), + }; + + // Convert to CondaPackageData (serialization) + let conda_source = record + .clone() + .into_conda_source_data(&std::path::PathBuf::from("/workspace")); + + let package_build_source = conda_source + .package_build_source + .as_ref() + .expect("expected package build source"); + + let PackageBuildSource::Path { path } = package_build_source else { + panic!("expected path package build source"); + }; + + // The path should now be relative to the manifest directory + // Manifest is in /workspace/recipes/, so ../src should point to /workspace/src + assert_eq!( + path.as_str(), + "../src", + "build_source should be relative to manifest_source directory" + ); + + // Convert back to SourceRecord (deserialization) + let roundtrip = SourceRecord::from_conda_source_data( + conda_source, + &std::path::PathBuf::from("/workspace"), + ) + .expect("roundtrip should succeed"); + + let Some(PinnedSourceSpec::Path(roundtrip_path)) = roundtrip.build_source else { + panic!("expected path pinned source"); + }; + + // After roundtrip, the path should remain as it was in the lock file (relative) + assert_eq!( + roundtrip_path.path.as_str(), + "../src", + "build_source should remain as relative path from lock file" + ); + } + + #[test] + fn package_build_source_path_stays_absolute_when_not_related() { + use typed_path::Utf8TypedPathBuf; + + let package_record: PackageRecord = serde_json::from_value(json!({ + "name": "example", + "version": "1.0.0", + "build": "0", + "build_number": 0, + "subdir": "noarch", + })) + .expect("valid package record"); + + // Manifest is in /workspace/recipes directory + let manifest_source = PinnedSourceSpec::Path(crate::PinnedPathSpec { + path: Utf8TypedPathBuf::from("/workspace/recipes"), + }); + + // Build source is in a completely different location + let build_source = PinnedSourceSpec::Path(crate::PinnedPathSpec { + path: Utf8TypedPathBuf::from("/completely/different/path"), + }); + + let record = SourceRecord { + package_record, + manifest_source: manifest_source.clone(), + build_source: Some(build_source), + input_hash: None, + sources: Default::default(), + }; + + // Convert to CondaPackageData (serialization) + let conda_source = record + .clone() + .into_conda_source_data(&std::path::PathBuf::from("/workspace")); + + let package_build_source = conda_source + .package_build_source + .as_ref() + .expect("expected package build source"); + + let PackageBuildSource::Path { path } = package_build_source else { + panic!("expected path package build source"); + }; + + // The path should be made relative + assert_eq!(path.as_str(), "../../completely/different/path"); + } + + #[test] + fn package_build_source_git_same_repo_is_made_relative() { + let package_record: PackageRecord = serde_json::from_value(json!({ + "name": "example", + "version": "1.0.0", + "build": "0", + "build_number": 0, + "subdir": "noarch", + })) + .expect("valid package record"); + + let git_url = Url::parse("https://github.com/user/repo.git").unwrap(); + let commit = GitSha::from_str("0123456789abcdef0123456789abcdef01234567").unwrap(); + + // Manifest is in recipes/ subdirectory + let manifest_source = PinnedSourceSpec::Git(crate::PinnedGitSpec { + git: git_url.clone(), + source: PinnedGitCheckout { + commit: commit.clone(), + subdirectory: Some("recipes".to_string()), + reference: GitReference::Branch("main".to_string()), + }, + }); + + // Build source is in src/ subdirectory (sibling of recipes) + let build_source = PinnedSourceSpec::Git(crate::PinnedGitSpec { + git: git_url.clone(), + source: PinnedGitCheckout { + commit: commit.clone(), + subdirectory: Some("src".to_string()), + reference: GitReference::Branch("main".to_string()), + }, + }); + + let record = SourceRecord { + package_record, + manifest_source: manifest_source.clone(), + build_source: Some(build_source), + input_hash: None, + sources: Default::default(), + }; + + // Convert to CondaPackageData (serialization) + let conda_source = record + .clone() + .into_conda_source_data(&std::path::PathBuf::from("/workspace")); + + let package_build_source = conda_source + .package_build_source + .as_ref() + .expect("expected package build source"); + + let PackageBuildSource::Path { path } = package_build_source else { + panic!("expected path package build source (relativized from git)"); + }; + + // The path should be relative: from recipes/ to src/ is ../src + assert_eq!( + path.as_str(), + "../src", + "build_source should be converted to relative path" + ); + + // Convert back to SourceRecord (deserialization) + let roundtrip = SourceRecord::from_conda_source_data( + conda_source, + &std::path::PathBuf::from("/workspace"), + ) + .expect("roundtrip should succeed"); + + let Some(PinnedSourceSpec::Path(roundtrip_path)) = roundtrip.build_source else { + panic!("expected path pinned source after roundtrip (deserialized from relative path in lock file)"); + }; + + // After roundtrip, it should remain as the relative path from the lock file + assert_eq!(roundtrip_path.path.as_str(), "../src"); + } + + #[test] + fn package_build_source_git_different_repos_stays_git() { + let package_record: PackageRecord = serde_json::from_value(json!({ + "name": "example", + "version": "1.0.0", + "build": "0", + "build_number": 0, + "subdir": "noarch", + })) + .expect("valid package record"); + + let manifest_git_url = Url::parse("https://github.com/user/repo1.git").unwrap(); + let build_git_url = Url::parse("https://github.com/user/repo2.git").unwrap(); + let commit1 = GitSha::from_str("0123456789abcdef0123456789abcdef01234567").unwrap(); + let commit2 = GitSha::from_str("abcdef0123456789abcdef0123456789abcdef01").unwrap(); + + // Manifest is in one repository + let manifest_source = PinnedSourceSpec::Git(crate::PinnedGitSpec { + git: manifest_git_url.clone(), + source: PinnedGitCheckout { + commit: commit1, + subdirectory: Some("recipes".to_string()), + reference: GitReference::Branch("main".to_string()), + }, + }); + + // Build source is in a different repository + let build_source = PinnedSourceSpec::Git(crate::PinnedGitSpec { + git: build_git_url.clone(), + source: PinnedGitCheckout { + commit: commit2.clone(), + subdirectory: Some("src".to_string()), + reference: GitReference::Branch("main".to_string()), + }, + }); + + let record = SourceRecord { + package_record, + manifest_source: manifest_source.clone(), + build_source: Some(build_source), + input_hash: None, + sources: Default::default(), + }; + + // Convert to CondaPackageData (serialization) + let conda_source = record + .clone() + .into_conda_source_data(&std::path::PathBuf::from("/workspace")); + + let package_build_source = conda_source + .package_build_source + .as_ref() + .expect("expected package build source"); + + let PackageBuildSource::Git { url, subdir, .. } = package_build_source else { + panic!("expected git package build source (different repos should stay git)"); + }; + + // Different repositories - should stay as Git source + assert_eq!(url, &build_git_url); + assert_eq!(subdir.as_ref().map(|s| s.as_str()), Some("src")); + } } From 889ca9d1b477c59c57ecd3b56928fbb59dc54a23 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Thu, 6 Nov 2025 16:40:57 +0100 Subject: [PATCH 09/21] feat: correct round-trip conversion for build sources --- .../src/procedures/initialize.rs | 4 +- .../src/build_backend_metadata/mod.rs | 5 +- crates/pixi_core/src/lock_file/update.rs | 4 +- crates/pixi_record/src/lib.rs | 5 +- crates/pixi_record/src/pinned_source.rs | 275 ++++++++---------- crates/pixi_record/src/source_record.rs | 96 ++++-- .../out-of-tree-source-parent/CMakeLists.txt | 5 + .../out-of-tree-source-parent/main.cpp | 6 + .../package/pixi.toml | 8 + .../out-of-tree-source-parent/pixi.lock | 81 ++++++ .../out-of-tree-source-parent/pixi.toml | 10 + .../out-of-tree-source/pixi.lock | 2 +- 12 files changed, 309 insertions(+), 192 deletions(-) create mode 100644 tests/data/satisfiability/out-of-tree-source-parent/CMakeLists.txt create mode 100644 tests/data/satisfiability/out-of-tree-source-parent/main.cpp create mode 100644 tests/data/satisfiability/out-of-tree-source-parent/package/pixi.toml create mode 100644 tests/data/satisfiability/out-of-tree-source-parent/pixi.lock create mode 100644 tests/data/satisfiability/out-of-tree-source-parent/pixi.toml diff --git a/crates/pixi_build_types/src/procedures/initialize.rs b/crates/pixi_build_types/src/procedures/initialize.rs index acaa8f244b..eb43fb4442 100644 --- a/crates/pixi_build_types/src/procedures/initialize.rs +++ b/crates/pixi_build_types/src/procedures/initialize.rs @@ -27,14 +27,14 @@ pub const METHOD_NAME: &str = "initialize"; pub struct InitializeParams { /// The manifest that the build backend should use. /// - /// This is an absolute path. + /// This is an absolute path to a manifest file. pub manifest_path: PathBuf, /// The root directory of the source code that the build backend should use. /// If this is `None`, the backend should use the directory of the /// `manifest_path` as the source directory. /// - /// This is an absolute path. + /// This is an absolute path. This is always a directory. pub source_dir: Option, /// The root directory of the workspace. diff --git a/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs b/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs index cc5593b078..02f1f448d0 100644 --- a/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs +++ b/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs @@ -135,11 +135,12 @@ impl BuildBackendMetadataSpec { // Determine the location of the source to build from. let manifest_source_anchor = SourceAnchor::from(SourceSpec::from(self.manifest_source.clone())); + // `build_source` is still relative to the `manifest_source` let build_source_checkout = match &discovered_backend.init_params.build_source { None => None, - Some(spec) => { + Some(build_source) => { // An out of tree source is provided. Resolve it against the manifest source. - let resolved_location = manifest_source_anchor.resolve(spec.clone()); + let resolved_location = manifest_source_anchor.resolve(build_source.clone()); let resolved_source_build_spec = SourceSpec { location: resolved_location.clone(), }; diff --git a/crates/pixi_core/src/lock_file/update.rs b/crates/pixi_core/src/lock_file/update.rs index 9d0bd41f55..005e45187e 100644 --- a/crates/pixi_core/src/lock_file/update.rs +++ b/crates/pixi_core/src/lock_file/update.rs @@ -1146,7 +1146,9 @@ impl<'p> UpdateContextBuilder<'p> { .map(|(platform, records)| { records .cloned() - .map(|data| PixiRecord::from_conda_package_data(data, workspace_root)) + .map(|data| { + PixiRecord::from_conda_package_data(data, workspace_root) + }) .collect::, _>>() .map(|records| { (platform, Arc::new(PixiRecordsByName::from_iter(records))) diff --git a/crates/pixi_record/src/lib.rs b/crates/pixi_record/src/lib.rs index d9e0928b1b..cf65141fd3 100644 --- a/crates/pixi_record/src/lib.rs +++ b/crates/pixi_record/src/lib.rs @@ -40,10 +40,7 @@ impl PixiRecord { /// Convert to CondaPackageData with paths made relative to workspace_root. /// This should be used when writing to the lock file. - pub fn into_conda_package_data( - self, - workspace_root: &std::path::Path, - ) -> CondaPackageData { + pub fn into_conda_package_data(self, workspace_root: &std::path::Path) -> CondaPackageData { match self { PixiRecord::Binary(record) => record.into(), PixiRecord::Source(record) => { diff --git a/crates/pixi_record/src/pinned_source.rs b/crates/pixi_record/src/pinned_source.rs index 759e5c49b3..eaf1b7e486 100644 --- a/crates/pixi_record/src/pinned_source.rs +++ b/crates/pixi_record/src/pinned_source.rs @@ -223,18 +223,18 @@ impl PinnedSourceSpec { /// /// # Arguments /// * `base` - The base pinned source to make this path relative to (typically the manifest_source) - pub fn make_relative_to(&self, base: &PinnedSourceSpec) -> Option { - use pixi_git::url::RepositoryUrl; - use typed_path::{Utf8Component, Utf8UnixPath}; - + pub fn make_relative_to(&self, base: &PinnedSourceSpec, workspace_root: &Path) -> Option { match (self, base) { // Path-to-Path: Make the path relative (PinnedSourceSpec::Path(this_path), PinnedSourceSpec::Path(base_path)) => { - // Base path is a directory (not a file path) - // Create a temporary Path for the base directory - let base_dir = Path::new(base_path.path.as_str()); + let this_path = this_path.resolve(workspace_root); + let base_path = base_path.resolve(workspace_root); + + let relative_path = pathdiff::diff_paths(this_path, base_path)?; - Some(PinnedSourceSpec::Path(this_path.make_relative_to(base_dir))) + Some(PinnedSourceSpec::Path(PinnedPathSpec { + path: Utf8TypedPathBuf::from(relative_path.to_string_lossy().as_ref()), + })) } // Git-to-Git: If same repository, convert to a relative path based on subdirectories (PinnedSourceSpec::Git(this_git), PinnedSourceSpec::Git(base_git)) => { @@ -247,58 +247,30 @@ impl PinnedSourceSpec { return None; } - // Same repository - compute relative path between subdirectories - let base_subdir = base_git.source.subdirectory.as_deref().unwrap_or(""); - let this_subdir = this_git.source.subdirectory.as_deref().unwrap_or(""); - - // If both point to the same subdirectory, don't relativize (keep as Git) - if base_subdir == this_subdir { + if this_git.source.commit != base_git.source.commit { return None; } - // Always use Unix-style paths for Git subdirectories - let base_unix = Utf8UnixPath::new(base_subdir); - let this_unix = Utf8UnixPath::new(this_subdir); - - // Try to compute relative path - if let Ok(rel) = this_unix.strip_prefix(base_unix) { - // this_subdir is under base_subdir - Some(PinnedSourceSpec::Path(PinnedPathSpec { - path: rel.as_str().into(), - })) - } else { - // Need to compute relative path using .. components - let base_components: Vec<_> = base_unix.components().collect(); - let this_components: Vec<_> = this_unix.components().collect(); - - // Find common prefix - let common = base_components - .iter() - .zip(this_components.iter()) - .take_while(|(a, b)| a == b) - .count(); - - // Build relative path: go up from base, then down to target - let ups = base_components.len() - common; - let mut rel_path = String::new(); - for _ in 0..ups { - if !rel_path.is_empty() { - rel_path.push('/'); - } - rel_path.push_str(".."); - } - for comp in &this_components[common..] { - if !rel_path.is_empty() { - rel_path.push('/'); - } - rel_path.push_str(comp.as_str()); - } + // bas = { git = "ssh@tim", subdir = "baz" } + // + // [package.build] + // source = foo/bar + // - Some(PinnedSourceSpec::Path(PinnedPathSpec { - path: rel_path.into(), - })) - } + // Same repository - compute relative path between subdirectories + // baz + let base_subdir = base_git.source.subdirectory.as_deref().unwrap_or(""); + // baz/foo/bar + let this_subdir = this_git.source.subdirectory.as_deref().unwrap_or(""); + + //foo/bar + let relative_path = pathdiff::diff_paths(this_subdir, base_subdir)?; + Some(PinnedSourceSpec::Path(PinnedPathSpec { + path: Utf8TypedPathBuf::from(relative_path.to_string_lossy().as_ref()), + })) } + (PinnedSourceSpec::Url(_), _) => unreachable!("url specs have not been implemented"), + (_, PinnedSourceSpec::Url(_)) => unreachable!("url specs have not been implemented"), // Different types or incompatible sources _ => None, } @@ -546,111 +518,12 @@ pub struct PinnedPathSpec { impl PinnedPathSpec { /// Resolves the path to an absolute path. - pub fn resolve(&self, project_root: &Path) -> PathBuf { + pub fn resolve(&self, workspace_root: &Path) -> PathBuf { let native_path = Path::new(self.path.as_str()); if native_path.is_absolute() { native_path.to_path_buf() } else { - project_root.join(native_path) - } - } - - /// Makes this path relative to another path if possible. - /// If the path is already relative or cannot be made relative, returns self unchanged. - /// - /// # Arguments - /// * `base` - The base path to make this path relative to - pub fn make_relative_to(&self, base: &Path) -> Self { - use typed_path::{Utf8Component, Utf8UnixPath, Utf8WindowsPath}; - - // Check if the path is already relative - let is_relative = if self.path.as_str().starts_with('/') { - // Unix absolute path - false - } else if self.path.as_str().len() >= 2 && self.path.as_str().chars().nth(1) == Some(':') { - // Windows absolute path (C:\...) - false - } else { - // Relative path - true - }; - - if is_relative { - return self.clone(); - } - - // Convert base to string for comparison - let base_str = base.to_string_lossy(); - - // Determine if we're working with Unix or Windows paths - let is_unix_style = self.path.as_str().starts_with('/'); - - if is_unix_style { - // Use Unix path logic - let this_unix = Utf8UnixPath::new(self.path.as_str()); - let base_unix = Utf8UnixPath::new(&base_str); - - if let Ok(rel) = this_unix.strip_prefix(base_unix) { - Self { - path: rel.as_str().into(), - } - } else { - // Try using pathdiff for more complex relativization - let this_components: Vec<_> = this_unix.components().collect(); - let base_components: Vec<_> = base_unix.components().collect(); - - // Find common prefix - let common = this_components - .iter() - .zip(base_components.iter()) - .take_while(|(a, b)| a == b) - .count(); - - if common == 0 { - // No common prefix, keep absolute - return self.clone(); - } - - // Build relative path: go up from base, then down to target - let ups = base_components.len() - common; - let mut rel_path = String::new(); - for _ in 0..ups { - if !rel_path.is_empty() { - rel_path.push('/'); - } - rel_path.push_str(".."); - } - for comp in &this_components[common..] { - if !rel_path.is_empty() { - rel_path.push('/'); - } - rel_path.push_str(comp.as_str()); - } - - Self { - path: rel_path.into(), - } - } - } else { - // Use Windows path logic - let this_windows = Utf8WindowsPath::new(self.path.as_str()); - let base_windows = Utf8WindowsPath::new(&base_str); - - if let Ok(rel) = this_windows.strip_prefix(base_windows) { - Self { - path: rel.as_str().into(), - } - } else { - // For Windows, fall back to pathdiff with native paths - let native_path = Path::new(self.path.as_str()); - if let Some(relative_path) = pathdiff::diff_paths(native_path, base) { - Self { - path: relative_path.to_string_lossy().to_string().into(), - } - } else { - self.clone() - } - } + workspace_root.join(native_path) } } @@ -1625,4 +1498,94 @@ mod tests { assert!(pinned.matches_source_spec(&spec)); } } + + mod make_relative_to { + use std::path::Path; + + use crate::{PinnedPathSpec, PinnedSourceSpec}; + + #[test] + fn test_relative_to_relative() { + // Both paths are relative - after resolution they become absolute, then relative path is computed + let workspace_root = Path::new("/workspace"); + + let this_spec = PinnedSourceSpec::Path(PinnedPathSpec { + path: "foo/bar".into(), + }); + let base_spec = PinnedSourceSpec::Path(PinnedPathSpec { path: "foo".into() }); + + let result = this_spec.make_relative_to(&base_spec, workspace_root); + + // Both resolve to /workspace/foo/bar and /workspace/foo + // Relative path should be "bar" + let PinnedSourceSpec::Path(path) = result.expect("Should return Some") else { + panic!("Expected Path variant"); + }; + assert_eq!(path.path.as_str(), "bar"); + } + + #[test] + fn test_absolute_to_absolute() { + // Both paths are absolute + let workspace_root = Path::new("/workspace"); + + let this_spec = PinnedSourceSpec::Path(PinnedPathSpec { + path: "/foo/bar/baz".into(), + }); + let base_spec = PinnedSourceSpec::Path(PinnedPathSpec { + path: "/foo/bar".into(), + }); + + let result = this_spec.make_relative_to(&base_spec, workspace_root); + + // Should compute relative path + let PinnedSourceSpec::Path(path) = result.expect("Should return Some") else { + panic!("Expected Path variant"); + }; + assert_eq!(path.path.as_str(), "baz"); + } + + #[test] + fn test_relative_to_absolute() { + // Self is relative, base is absolute - after resolution they're both absolute + let workspace_root = Path::new("/workspace"); + + let this_spec = PinnedSourceSpec::Path(PinnedPathSpec { + path: "foo/bar".into(), // Resolves to /workspace/foo/bar + }); + let base_spec = PinnedSourceSpec::Path(PinnedPathSpec { + path: "/other/path".into(), // Already absolute + }); + + let result = this_spec.make_relative_to(&base_spec, workspace_root); + + // Both are absolute after resolution, pathdiff should compute relative path + let PinnedSourceSpec::Path(path) = result.expect("Should return Some") else { + panic!("Expected Path variant"); + }; + // From /other/path to /workspace/foo/bar + assert_eq!(path.path.as_str(), "../../workspace/foo/bar"); + } + + #[test] + fn test_absolute_with_parent_navigation() { + // Test paths that require .. navigation + let workspace_root = Path::new("/workspace"); + + let this_spec = PinnedSourceSpec::Path(PinnedPathSpec { + path: "/foo/bar/qux".into(), + }); + let base_spec = PinnedSourceSpec::Path(PinnedPathSpec { + path: "/foo/baz/quux".into(), + }); + + let result = this_spec.make_relative_to(&base_spec, workspace_root); + + let PinnedSourceSpec::Path(path) = result.expect("Should return Some") else { + panic!("Expected Path variant"); + }; + // From /foo/baz/quux to /foo/bar/qux requires ../../bar/qux + assert_eq!(path.path.as_str(), "../../bar/qux"); + } + } } diff --git a/crates/pixi_record/src/source_record.rs b/crates/pixi_record/src/source_record.rs index 99502aa7d3..b28943e45e 100644 --- a/crates/pixi_record/src/source_record.rs +++ b/crates/pixi_record/src/source_record.rs @@ -1,5 +1,6 @@ use std::{ collections::{BTreeSet, HashMap}, + path::Path, str::FromStr, }; @@ -24,7 +25,8 @@ pub struct SourceRecord { pub manifest_source: PinnedSourceSpec, /// The optional pinned source where the build should be executed - /// This is used when the manifest is not in the same location ad + /// This is used when the manifest is not in the same location as the + /// source files. pub build_source: Option, /// The hash of the input that was used to build the metadata of the @@ -56,21 +58,17 @@ pub struct InputHash { } impl SourceRecord { - /// Convert to CondaSourceData with paths made relative to workspace_root. + /// Convert [`SourceRecord`] into lock-file compatible `CondaSourceData` + /// The `build_source` in the SourceRecord is always relative to the workspace. + /// However, when saving in the lock-file make these relative to the package manifest. /// This should be used when writing to the lock file. - /// - /// TODO: Currently this doesn't use workspace_root and just makes build_source - /// relative to manifest_source. Future implementation should make both relative - /// to workspace_root first. - pub fn into_conda_source_data( - self, - _workspace_root: &std::path::Path, - ) -> CondaSourceData { - let package_build_source = self.build_source.map(|s| { - // Try to make build_source relative to manifest_source - let s = s.make_relative_to(&self.manifest_source).unwrap_or(s); - - match s { + pub fn into_conda_source_data(self, workspace_root: &Path) -> CondaSourceData { + let package_build_source = self.build_source.map(|build_source| { + let pinned_source_spec = build_source + .make_relative_to(&self.manifest_source, workspace_root) + .unwrap_or(build_source); + + match pinned_source_spec { PinnedSourceSpec::Url(pinned_url_spec) => PackageBuildSource::Url { url: pinned_url_spec.url, sha256: pinned_url_spec.sha256, @@ -124,17 +122,16 @@ impl SourceRecord { /// Create SourceRecord from CondaSourceData with paths resolved relative to workspace_root. /// This should be used when reading from the lock file. /// - /// TODO: Currently this doesn't use workspace_root and just resolves build_source - /// relative to manifest_source. Future implementation should resolve both against - /// workspace_root first. + /// The inverse of `into_conda_source_data`: + /// - manifest_source: relative to workspace_root (or absolute) → resolve to absolute + /// - build_source: relative to manifest_source (or absolute) → resolve to absolute pub fn from_conda_source_data( data: CondaSourceData, - _workspace_root: &std::path::Path, + workspace_root: &std::path::Path, ) -> Result { - let manifest_source: PinnedSourceSpec = data.location.try_into()?; - let pinned_source_spec = data.package_build_source.map(|source| match source { + let build_source = data.package_build_source.map(|source| match source { PackageBuildSource::Git { url, spec, @@ -148,6 +145,7 @@ impl SourceRecord { None => GitReference::DefaultBranch, }; + // Out-of-tree git repository PinnedSourceSpec::Git(crate::PinnedGitSpec { git: url, source: PinnedGitCheckout { @@ -167,9 +165,53 @@ impl SourceRecord { md5: None, }), PackageBuildSource::Path { path } => { - // Keep paths as-is from the lock file - // They will be resolved later by the code that uses them - PinnedSourceSpec::Path(crate::PinnedPathSpec { path }) + if path.is_absolute() { + crate::PinnedSourceSpec::Path(crate::PinnedPathSpec { path }) + } else { + match &manifest_source { + PinnedSourceSpec::Url(_) => unimplemented!(), + PinnedSourceSpec::Git(pinned_git_spec) => { + let base_dir = pinned_git_spec.source.subdirectory.as_ref(); + // Make `path` relative to repository root, if `base` is not None it will + // be relative with `base` at this point, so lets join + let subdir = + base_dir.map(|base| Path::new(base).join(Path::new(path.as_str()))); + + let mut git_source = pinned_git_spec.source.clone(); + git_source.subdirectory = + subdir.map(|p| p.to_string_lossy().to_string()); + + // Reconstruct the git object + PinnedSourceSpec::Git(crate::PinnedGitSpec { + git: pinned_git_spec.git.clone(), + source: git_source, + }) + } + PinnedSourceSpec::Path(manifest_path) => { + // path is relative to manifest_source (or absolute) in the lock file + // We need to make it relative to workspace_root (or keep it absolute) + let build_source_spec = crate::PinnedPathSpec { path }; + + // If path is relative, it's relative to manifest_source + // First resolve manifest_source against workspace_root + // Then resolve path against the resolved manifest_source + // Finally make the result relative to workspace_root + let manifest_absolute = manifest_path.resolve(workspace_root); + let build_absolute = build_source_spec.resolve(&manifest_absolute); + + // Make the normalized path relative to workspace_root + let relative_to_workspace = + pathdiff::diff_paths(&build_absolute, workspace_root) + .unwrap_or_else(|| build_absolute); + + PinnedSourceSpec::Path(crate::PinnedPathSpec { + path: Utf8TypedPathBuf::from( + relative_to_workspace.to_string_lossy().as_ref(), + ), + }) + } + } + } } }); @@ -180,7 +222,7 @@ impl SourceRecord { hash: hash.hash, globs: BTreeSet::from_iter(hash.globs), }), - build_source: pinned_source_spec, + build_source, sources: data .sources .into_iter() @@ -497,7 +539,9 @@ mod tests { .expect("roundtrip should succeed"); let Some(PinnedSourceSpec::Path(roundtrip_path)) = roundtrip.build_source else { - panic!("expected path pinned source after roundtrip (deserialized from relative path in lock file)"); + panic!( + "expected path pinned source after roundtrip (deserialized from relative path in lock file)" + ); }; // After roundtrip, it should remain as the relative path from the lock file diff --git a/tests/data/satisfiability/out-of-tree-source-parent/CMakeLists.txt b/tests/data/satisfiability/out-of-tree-source-parent/CMakeLists.txt new file mode 100644 index 0000000000..5ff26babb6 --- /dev/null +++ b/tests/data/satisfiability/out-of-tree-source-parent/CMakeLists.txt @@ -0,0 +1,5 @@ +cmake_minimum_required(VERSION 3.10) +project(out-of-tree-package) + +# Simple test project for out-of-tree builds +add_executable(test_app main.cpp) diff --git a/tests/data/satisfiability/out-of-tree-source-parent/main.cpp b/tests/data/satisfiability/out-of-tree-source-parent/main.cpp new file mode 100644 index 0000000000..67610f5bfd --- /dev/null +++ b/tests/data/satisfiability/out-of-tree-source-parent/main.cpp @@ -0,0 +1,6 @@ +#include + +int main() { + std::cout << "Hello from out-of-tree build!" << std::endl; + return 0; +} diff --git a/tests/data/satisfiability/out-of-tree-source-parent/package/pixi.toml b/tests/data/satisfiability/out-of-tree-source-parent/package/pixi.toml new file mode 100644 index 0000000000..d75fbd81c9 --- /dev/null +++ b/tests/data/satisfiability/out-of-tree-source-parent/package/pixi.toml @@ -0,0 +1,8 @@ +[package] +name = "out-of-tree-package" +version = "0.1.0" + +[package.build] +backend = { name = "pixi-build-cmake", version = "0.3.*" } +# Point to source files in a different directory (out-of-tree build) +source = { path = "../" } diff --git a/tests/data/satisfiability/out-of-tree-source-parent/pixi.lock b/tests/data/satisfiability/out-of-tree-source-parent/pixi.lock new file mode 100644 index 0000000000..0e42219605 --- /dev/null +++ b/tests/data/satisfiability/out-of-tree-source-parent/pixi.lock @@ -0,0 +1,81 @@ +version: 6 +environments: + default: + channels: + - url: https://prefix.dev/pixi-build-backends/ + - url: https://prefix.dev/conda-forge/ + packages: + linux-64: + - conda: https://prefix.dev/conda-forge/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2 + - conda: https://prefix.dev/conda-forge/linux-64/_openmp_mutex-4.5-2_gnu.tar.bz2 + - conda: https://prefix.dev/conda-forge/linux-64/libgcc-15.2.0-h767d61c_7.conda + - conda: https://prefix.dev/conda-forge/linux-64/libgomp-15.2.0-h767d61c_7.conda + - conda: https://prefix.dev/conda-forge/linux-64/libstdcxx-15.2.0-h8f9b012_7.conda + - conda: ./package +packages: +- conda: https://prefix.dev/conda-forge/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2 + sha256: fe51de6107f9edc7aa4f786a70f4a883943bc9d39b3bb7307c04c41410990726 + md5: d7c89558ba9fa0495403155b64376d81 + license: None + size: 2562 + timestamp: 1578324546067 +- conda: https://prefix.dev/conda-forge/linux-64/_openmp_mutex-4.5-2_gnu.tar.bz2 + build_number: 16 + sha256: fbe2c5e56a653bebb982eda4876a9178aedfc2b545f25d0ce9c4c0b508253d22 + md5: 73aaf86a425cc6e73fcf236a5a46396d + depends: + - _libgcc_mutex 0.1 conda_forge + - libgomp >=7.5.0 + constrains: + - openmp_impl 9999 + license: BSD-3-Clause + license_family: BSD + size: 23621 + timestamp: 1650670423406 +- conda: https://prefix.dev/conda-forge/linux-64/libgcc-15.2.0-h767d61c_7.conda + sha256: 08f9b87578ab981c7713e4e6a7d935e40766e10691732bba376d4964562bcb45 + md5: c0374badb3a5d4b1372db28d19462c53 + depends: + - __glibc >=2.17,<3.0.a0 + - _openmp_mutex >=4.5 + constrains: + - libgomp 15.2.0 h767d61c_7 + - libgcc-ng ==15.2.0=*_7 + license: GPL-3.0-only WITH GCC-exception-3.1 + license_family: GPL + size: 822552 + timestamp: 1759968052178 +- conda: https://prefix.dev/conda-forge/linux-64/libgomp-15.2.0-h767d61c_7.conda + sha256: e9fb1c258c8e66ee278397b5822692527c5f5786d372fe7a869b900853f3f5ca + md5: f7b4d76975aac7e5d9e6ad13845f92fe + depends: + - __glibc >=2.17,<3.0.a0 + license: GPL-3.0-only WITH GCC-exception-3.1 + license_family: GPL + size: 447919 + timestamp: 1759967942498 +- conda: https://prefix.dev/conda-forge/linux-64/libstdcxx-15.2.0-h8f9b012_7.conda + sha256: 1b981647d9775e1cdeb2fab0a4dd9cd75a6b0de2963f6c3953dbd712f78334b3 + md5: 5b767048b1b3ee9a954b06f4084f93dc + depends: + - __glibc >=2.17,<3.0.a0 + - libgcc 15.2.0 h767d61c_7 + constrains: + - libstdcxx-ng ==15.2.0=*_7 + license: GPL-3.0-only WITH GCC-exception-3.1 + license_family: GPL + size: 3898269 + timestamp: 1759968103436 +- conda: ./package + name: out-of-tree-package + version: 0.1.0 + build: hb0f4dca_0 + subdir: linux-64 + depends: + - libstdcxx >=15 + - libgcc >=15 + input: + hash: 616e7d8611fa803e6655c354f183d6f4b405e94adcabc83c260d8b9da5e3c9d8 + globs: [] + package_build_source: + path: .. diff --git a/tests/data/satisfiability/out-of-tree-source-parent/pixi.toml b/tests/data/satisfiability/out-of-tree-source-parent/pixi.toml new file mode 100644 index 0000000000..e319b11972 --- /dev/null +++ b/tests/data/satisfiability/out-of-tree-source-parent/pixi.toml @@ -0,0 +1,10 @@ +[workspace] +channels = [ + "https://prefix.dev/pixi-build-backends", + "https://prefix.dev/conda-forge", +] +platforms = ["linux-64"] +preview = ["pixi-build"] + +[dependencies] +out-of-tree-package = { path = "./package" } diff --git a/tests/data/satisfiability/out-of-tree-source/pixi.lock b/tests/data/satisfiability/out-of-tree-source/pixi.lock index f0d728af97..473a6559f3 100644 --- a/tests/data/satisfiability/out-of-tree-source/pixi.lock +++ b/tests/data/satisfiability/out-of-tree-source/pixi.lock @@ -69,7 +69,7 @@ packages: - conda: build-config name: out-of-tree-package version: 0.1.0 - build: hbf21a9e_0 + build: hb0f4dca_0 subdir: linux-64 depends: - libstdcxx >=15 From eaa6edda755db0d68e1e0e4559fb30d958526408 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 7 Nov 2025 09:35:27 +0100 Subject: [PATCH 10/21] fix: clippy --- crates/pixi_record/src/source_record.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/pixi_record/src/source_record.rs b/crates/pixi_record/src/source_record.rs index b28943e45e..b39d0ed900 100644 --- a/crates/pixi_record/src/source_record.rs +++ b/crates/pixi_record/src/source_record.rs @@ -202,7 +202,7 @@ impl SourceRecord { // Make the normalized path relative to workspace_root let relative_to_workspace = pathdiff::diff_paths(&build_absolute, workspace_root) - .unwrap_or_else(|| build_absolute); + .unwrap_or(build_absolute); PinnedSourceSpec::Path(crate::PinnedPathSpec { path: Utf8TypedPathBuf::from( @@ -486,7 +486,7 @@ mod tests { let manifest_source = PinnedSourceSpec::Git(crate::PinnedGitSpec { git: git_url.clone(), source: PinnedGitCheckout { - commit: commit.clone(), + commit: commit, subdirectory: Some("recipes".to_string()), reference: GitReference::Branch("main".to_string()), }, @@ -496,7 +496,7 @@ mod tests { let build_source = PinnedSourceSpec::Git(crate::PinnedGitSpec { git: git_url.clone(), source: PinnedGitCheckout { - commit: commit.clone(), + commit: commit, subdirectory: Some("src".to_string()), reference: GitReference::Branch("main".to_string()), }, @@ -578,7 +578,7 @@ mod tests { let build_source = PinnedSourceSpec::Git(crate::PinnedGitSpec { git: build_git_url.clone(), source: PinnedGitCheckout { - commit: commit2.clone(), + commit: commit2, subdirectory: Some("src".to_string()), reference: GitReference::Branch("main".to_string()), }, From 903cd3b890d58a5d16c2165569118146f9dccca9 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 7 Nov 2025 12:00:01 +0100 Subject: [PATCH 11/21] added roundtrip snapshot test and fix --- Cargo.lock | 2 + .../src/lock_file/satisfiability/mod.rs | 8 +--- crates/pixi_record/Cargo.toml | 2 + ...d__tests__roundtrip_conda_source_data.snap | 39 +++++++++++++++++++ crates/pixi_record/src/source_record.rs | 30 +++++++++++++- crates/pixi_spec/src/lib.rs | 4 +- crates/pixi_spec/src/url.rs | 2 +- 7 files changed, 76 insertions(+), 11 deletions(-) create mode 100644 crates/pixi_record/src/snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap diff --git a/Cargo.lock b/Cargo.lock index 6f839e8d02..b66b6d2f10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5296,6 +5296,7 @@ name = "pixi_record" version = "0.1.0" dependencies = [ "file_url", + "insta", "miette 7.6.0", "pathdiff", "pixi_git", @@ -5306,6 +5307,7 @@ dependencies = [ "serde", "serde_json", "serde_with", + "serde_yaml", "thiserror 2.0.17", "typed-path", "url", diff --git a/crates/pixi_core/src/lock_file/satisfiability/mod.rs b/crates/pixi_core/src/lock_file/satisfiability/mod.rs index d5477e716f..19da02d82b 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/mod.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/mod.rs @@ -1508,14 +1508,8 @@ pub(crate) async fn verify_package_platform_satisfiability( continue; }; - // Get the manifest directory first - let Some(manifest_path_record) = source_record.manifest_source.as_path() else { - continue; - }; - let manifest_dir = manifest_path_record.resolve(project_root); - // Resolve build_source relative to the manifest directory - build_path_record.resolve(&manifest_dir) + build_path_record.resolve(&project_root) } else { let Some(path_record) = source_record.manifest_source.as_path() else { continue; diff --git a/crates/pixi_record/Cargo.toml b/crates/pixi_record/Cargo.toml index 92e3657806..8e3c8c2a5a 100644 --- a/crates/pixi_record/Cargo.toml +++ b/crates/pixi_record/Cargo.toml @@ -25,4 +25,6 @@ typed-path = { workspace = true } url = { workspace = true } [dev-dependencies] +insta = { workspace = true, features = ["yaml"] } serde_json = { workspace = true } +serde_yaml = { workspace = true } diff --git a/crates/pixi_record/src/snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap b/crates/pixi_record/src/snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap new file mode 100644 index 0000000000..aff2185125 --- /dev/null +++ b/crates/pixi_record/src/snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap @@ -0,0 +1,39 @@ +--- +source: crates/pixi_record/src/source_record.rs +expression: roundtrip +--- +package_record: + build: h123456_0 + build_number: 0 + depends: [] + md5: d41d8cd98f00b204e9800998ecf8427e + name: comprehensive-test + sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + subdir: noarch + version: 1.0.0 +manifest_source: + commit: abc123def456abc123def456abc123def456abc1 + git: "https://github.com/example/mono-repo.git" + reference: + branch: main + subdirectory: recipes +build_source: + commit: abc123def456abc123def456abc123def456abc1 + git: "https://github.com/example/mono-repo.git" + reference: + branch: main + subdirectory: recipes/nested/src +input_hash: + hash: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + globs: + - "**/*.py" + - "**/*.txt" +sources: + git-dep: + location: + git: "https://github.com/example/dep.git" + subdirectory: python + tag: v1.0.0 + path-dep: + location: + path: "../vendor" diff --git a/crates/pixi_record/src/source_record.rs b/crates/pixi_record/src/source_record.rs index b39d0ed900..ea732fedd7 100644 --- a/crates/pixi_record/src/source_record.rs +++ b/crates/pixi_record/src/source_record.rs @@ -15,7 +15,7 @@ use typed_path::Utf8TypedPathBuf; use crate::{ParseLockFileError, PinnedGitCheckout, PinnedSourceSpec}; /// A record of a conda package that still requires building. -#[derive(Debug, Clone, serde::Serialize)] +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct SourceRecord { /// Information about the conda package. This is metadata of the package /// after it has been build. @@ -610,4 +610,32 @@ mod tests { assert_eq!(url, &build_git_url); assert_eq!(subdir.as_ref().map(|s| s.as_str()), Some("src")); } + + #[test] + fn roundtrip_conda_source_data() { + let workspace_root = std::path::Path::new("/workspace"); + + // Load the SourceRecord from snapshot (skip YAML frontmatter) + let snapshot_content = include_str!( + "snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap" + ); + let yaml_content = snapshot_content + .split("---") + .nth(2) + .expect("snapshot should have YAML content"); + let original: SourceRecord = + serde_yaml::from_str(yaml_content).expect("failed to load snapshot"); + + // Roundtrip: SourceRecord -> CondaSourceData -> SourceRecord + let conda_data = original.clone().into_conda_source_data(workspace_root); + let roundtrip = SourceRecord::from_conda_source_data(conda_data, workspace_root) + .expect("from_conda_source_data should succeed"); + + // Snapshot the final result - should match the original + let mut settings = insta::Settings::clone_current(); + settings.set_sort_maps(true); + settings.bind(|| { + insta::assert_yaml_snapshot!(roundtrip); + }); + } } diff --git a/crates/pixi_spec/src/lib.rs b/crates/pixi_spec/src/lib.rs index 940d4922fe..7f0ca5aec0 100644 --- a/crates/pixi_spec/src/lib.rs +++ b/crates/pixi_spec/src/lib.rs @@ -362,14 +362,14 @@ impl PixiSpec { /// /// This type only represents source packages. Use [`PixiSpec`] to represent /// both binary and source packages. -#[derive(Debug, Clone, Hash, PartialEq, Eq, serde::Serialize)] +#[derive(Debug, Clone, Hash, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct SourceSpec { /// The location of the source. pub location: SourceLocationSpec, } /// A specification for a source location. -#[derive(Debug, Clone, Hash, PartialEq, Eq, serde::Serialize)] +#[derive(Debug, Clone, Hash, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[serde(untagged)] pub enum SourceLocationSpec { /// The spec is represented as an archive that can be downloaded from the diff --git a/crates/pixi_spec/src/url.rs b/crates/pixi_spec/src/url.rs index 3a47aade01..96b7732dc4 100644 --- a/crates/pixi_spec/src/url.rs +++ b/crates/pixi_spec/src/url.rs @@ -97,7 +97,7 @@ impl Display for UrlSpec { /// A specification of a source archive from a URL. #[serde_as] -#[derive(Debug, Clone, Hash, Eq, PartialEq, serde::Serialize)] +#[derive(Debug, Clone, Hash, Eq, PartialEq, serde::Serialize, serde::Deserialize)] pub struct UrlSourceSpec { /// The URL of the package pub url: Url, From 407eff280c8a7a0990877e8573729b1078c4bedc Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 7 Nov 2025 12:08:23 +0100 Subject: [PATCH 12/21] fix: clippy --- crates/pixi_record/src/source_record.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/pixi_record/src/source_record.rs b/crates/pixi_record/src/source_record.rs index ea732fedd7..b51b4e4c30 100644 --- a/crates/pixi_record/src/source_record.rs +++ b/crates/pixi_record/src/source_record.rs @@ -486,7 +486,7 @@ mod tests { let manifest_source = PinnedSourceSpec::Git(crate::PinnedGitSpec { git: git_url.clone(), source: PinnedGitCheckout { - commit: commit, + commit, subdirectory: Some("recipes".to_string()), reference: GitReference::Branch("main".to_string()), }, @@ -496,7 +496,7 @@ mod tests { let build_source = PinnedSourceSpec::Git(crate::PinnedGitSpec { git: git_url.clone(), source: PinnedGitCheckout { - commit: commit, + commit, subdirectory: Some("src".to_string()), reference: GitReference::Branch("main".to_string()), }, From 7aa0e5c83cedaf16ca7019ac65d798468b25683d Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 7 Nov 2025 12:09:30 +0100 Subject: [PATCH 13/21] fix: satisfiability --- crates/pixi_core/src/lock_file/satisfiability/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/pixi_core/src/lock_file/satisfiability/mod.rs b/crates/pixi_core/src/lock_file/satisfiability/mod.rs index 19da02d82b..8c25cf891a 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/mod.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/mod.rs @@ -1509,7 +1509,7 @@ pub(crate) async fn verify_package_platform_satisfiability( }; // Resolve build_source relative to the manifest directory - build_path_record.resolve(&project_root) + build_path_record.resolve(project_root) } else { let Some(path_record) = source_record.manifest_source.as_path() else { continue; From a061514cf9f3d98cb838a7b9775c36e6c969fc48 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 7 Nov 2025 15:15:14 +0100 Subject: [PATCH 14/21] fix: path normalization --- Cargo.lock | 1 + crates/pixi_record/Cargo.toml | 1 + crates/pixi_record/src/pinned_source.rs | 22 +- ...d__tests__roundtrip_conda_source_data.snap | 126 +++++++---- crates/pixi_record/src/source_record.rs | 195 +++++++++--------- 5 files changed, 201 insertions(+), 144 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b66b6d2f10..866442e211 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5295,6 +5295,7 @@ dependencies = [ name = "pixi_record" version = "0.1.0" dependencies = [ + "dunce", "file_url", "insta", "miette 7.6.0", diff --git a/crates/pixi_record/Cargo.toml b/crates/pixi_record/Cargo.toml index 8e3c8c2a5a..351105a7e0 100644 --- a/crates/pixi_record/Cargo.toml +++ b/crates/pixi_record/Cargo.toml @@ -10,6 +10,7 @@ repository.workspace = true version = "0.1.0" [dependencies] +dunce = { workspace = true } file_url = { workspace = true } miette = { workspace = true } pathdiff = { workspace = true } diff --git a/crates/pixi_record/src/pinned_source.rs b/crates/pixi_record/src/pinned_source.rs index eaf1b7e486..9f7f49bdf9 100644 --- a/crates/pixi_record/src/pinned_source.rs +++ b/crates/pixi_record/src/pinned_source.rs @@ -251,22 +251,20 @@ impl PinnedSourceSpec { return None; } - // bas = { git = "ssh@tim", subdir = "baz" } - // - // [package.build] - // source = foo/bar - // - - // Same repository - compute relative path between subdirectories - // baz + // Same repository and commit - compute relative path between subdirectories + // Both subdirectories are relative to the repository root let base_subdir = base_git.source.subdirectory.as_deref().unwrap_or(""); - // baz/foo/bar let this_subdir = this_git.source.subdirectory.as_deref().unwrap_or(""); - //foo/bar - let relative_path = pathdiff::diff_paths(this_subdir, base_subdir)?; + // Compute the relative path from base to this + let base_path = std::path::Path::new(base_subdir); + let this_path = std::path::Path::new(this_subdir); + + let relative = pathdiff::diff_paths(this_path, base_path)?; + let relative_str = relative.to_str()?; + Some(PinnedSourceSpec::Path(PinnedPathSpec { - path: Utf8TypedPathBuf::from(relative_path.to_string_lossy().as_ref()), + path: Utf8TypedPathBuf::from(relative_str), })) } (PinnedSourceSpec::Url(_), _) => unreachable!("url specs have not been implemented"), diff --git a/crates/pixi_record/src/snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap b/crates/pixi_record/src/snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap index aff2185125..38ff3bf88c 100644 --- a/crates/pixi_record/src/snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap +++ b/crates/pixi_record/src/snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap @@ -1,39 +1,93 @@ --- source: crates/pixi_record/src/source_record.rs -expression: roundtrip +expression: roundtrips --- -package_record: - build: h123456_0 - build_number: 0 - depends: [] - md5: d41d8cd98f00b204e9800998ecf8427e - name: comprehensive-test - sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 - subdir: noarch - version: 1.0.0 -manifest_source: - commit: abc123def456abc123def456abc123def456abc1 - git: "https://github.com/example/mono-repo.git" - reference: - branch: main - subdirectory: recipes -build_source: - commit: abc123def456abc123def456abc123def456abc1 - git: "https://github.com/example/mono-repo.git" - reference: - branch: main - subdirectory: recipes/nested/src -input_hash: - hash: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 - globs: - - "**/*.py" - - "**/*.txt" -sources: - git-dep: - location: - git: "https://github.com/example/dep.git" - subdirectory: python - tag: v1.0.0 - path-dep: - location: - path: "../vendor" +- package_record: + build: h123456_0 + build_number: 0 + depends: [] + md5: d41d8cd98f00b204e9800998ecf8427e + name: git-sibling-test + sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + subdir: noarch + version: 1.0.0 + manifest_source: + commit: abc123def456abc123def456abc123def456abc1 + git: "https://github.com/example/mono-repo.git" + reference: + branch: main + subdirectory: recipes + build_source: + commit: abc123def456abc123def456abc123def456abc1 + git: "https://github.com/example/mono-repo.git" + reference: + branch: main + subdirectory: nested/src + input_hash: + hash: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + globs: + - "**/*.py" + - "**/*.txt" + sources: {} +- package_record: + build: h234567_0 + build_number: 0 + depends: [] + md5: d41d8cd98f00b204e9800998ecf8427e + name: git-child-test + sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + subdir: noarch + version: 1.1.0 + manifest_source: + commit: abc123def456abc123def456abc123def456abc1 + git: "https://github.com/example/mono-repo.git" + reference: + branch: main + subdirectory: recipes + build_source: + commit: abc123def456abc123def456abc123def456abc1 + git: "https://github.com/example/mono-repo.git" + reference: + branch: main + subdirectory: src + input_hash: + hash: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + globs: + - "**/*.c" + sources: {} +- package_record: + build: h789012_0 + build_number: 0 + depends: [] + md5: d41d8cd98f00b204e9800998ecf8427e + name: path-sibling-test + sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + subdir: noarch + version: 2.0.0 + manifest_source: + path: recipes/my-package + build_source: + path: other-package/src + input_hash: + hash: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + globs: + - "**/*.rs" + sources: {} +- package_record: + build: h890123_0 + build_number: 0 + depends: [] + md5: d41d8cd98f00b204e9800998ecf8427e + name: path-child-test + sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + subdir: noarch + version: 2.1.0 + manifest_source: + path: recipes/my-package + build_source: + path: src/lib + input_hash: + hash: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + globs: + - "**/*.cpp" + sources: {} diff --git a/crates/pixi_record/src/source_record.rs b/crates/pixi_record/src/source_record.rs index b51b4e4c30..d45f0565e4 100644 --- a/crates/pixi_record/src/source_record.rs +++ b/crates/pixi_record/src/source_record.rs @@ -1,6 +1,7 @@ use std::{ collections::{BTreeSet, HashMap}, - path::Path, + ffi::{OsStr, OsString}, + path::{Path, PathBuf}, str::FromStr, }; @@ -171,15 +172,23 @@ impl SourceRecord { match &manifest_source { PinnedSourceSpec::Url(_) => unimplemented!(), PinnedSourceSpec::Git(pinned_git_spec) => { - let base_dir = pinned_git_spec.source.subdirectory.as_ref(); - // Make `path` relative to repository root, if `base` is not None it will - // be relative with `base` at this point, so lets join - let subdir = - base_dir.map(|base| Path::new(base).join(Path::new(path.as_str()))); + // The path is relative to the manifest subdirectory + // Need to resolve it to get the absolute subdirectory in the repo + let base_subdir = + pinned_git_spec.source.subdirectory.as_deref().unwrap_or(""); + let base_path = std::path::Path::new(base_subdir); + let relative_path = std::path::Path::new(path.as_str()); + + // Join to get the subdirectory path and normalize away `.` / `..` + let absolute_subdir = base_path.join(relative_path); + let normalized = normalize_path(&absolute_subdir); + let subdirectory = normalized + .to_str() + .expect("path should be valid UTF-8") + .to_string(); let mut git_source = pinned_git_spec.source.clone(); - git_source.subdirectory = - subdir.map(|p| p.to_string_lossy().to_string()); + git_source.subdirectory = Some(subdirectory); // Reconstruct the git object PinnedSourceSpec::Git(crate::PinnedGitSpec { @@ -198,6 +207,7 @@ impl SourceRecord { // Finally make the result relative to workspace_root let manifest_absolute = manifest_path.resolve(workspace_root); let build_absolute = build_source_spec.resolve(&manifest_absolute); + let build_absolute = normalize_path(&build_absolute); // Make the normalized path relative to workspace_root let relative_to_workspace = @@ -270,6 +280,60 @@ impl AsRef for SourceRecord { } } +fn normalize_path(path: &Path) -> PathBuf { + use std::path::Component; + + // `dunce::simplified` is purely lexical and strips UNC verbatim prefixes on Windows. + let simplified = dunce::simplified(path).to_path_buf(); + + let mut prefix: Option = None; + let mut has_root = false; + let mut parts: Vec = Vec::new(); + + for component in simplified.components() { + match component { + Component::Prefix(prefix_component) => { + prefix = Some(prefix_component.as_os_str().to_os_string()); + parts.clear(); + } + Component::RootDir => { + has_root = true; + parts.clear(); + } + Component::CurDir => {} + Component::ParentDir => { + if let Some(last) = parts.last() { + if last.as_os_str() == OsStr::new("..") { + parts.push(OsString::from("..")); + } else { + parts.pop(); + } + } else if !has_root { + parts.push(OsString::from("..")); + } + } + Component::Normal(part) => parts.push(part.to_os_string()), + } + } + + let mut normalized = PathBuf::new(); + if let Some(prefix) = prefix { + normalized.push(prefix); + } + if has_root { + normalized.push(std::path::MAIN_SEPARATOR.to_string()); + } + for part in parts { + normalized.push(part); + } + + if normalized.as_os_str().is_empty() { + simplified + } else { + normalized + } +} + #[cfg(test)] mod tests { use super::*; @@ -278,75 +342,6 @@ mod tests { use std::str::FromStr; use url::Url; - #[test] - fn package_build_source_roundtrip_preserves_git_subdirectory() { - let package_record: PackageRecord = serde_json::from_value(json!({ - "name": "example", - "version": "1.0.0", - "build": "0", - "build_number": 0, - "subdir": "noarch", - })) - .expect("valid package record"); - - let git_url = Url::parse("https://example.com/repo.git").unwrap(); - let pinned_source = PinnedSourceSpec::Git(crate::PinnedGitSpec { - git: git_url.clone(), - source: PinnedGitCheckout { - commit: GitSha::from_str("0123456789abcdef0123456789abcdef01234567").unwrap(), - subdirectory: Some("nested/project".to_string()), - reference: GitReference::Branch("main".to_string()), - }, - }); - - let record = SourceRecord { - package_record, - manifest_source: pinned_source.clone(), - build_source: Some(pinned_source.clone()), - input_hash: None, - sources: Default::default(), - }; - - let conda_source = record - .clone() - .into_conda_source_data(&std::path::PathBuf::from("/workspace")); - - let package_build_source = conda_source - .package_build_source - .as_ref() - .expect("expected package build source"); - - let PackageBuildSource::Git { - url, - spec, - rev, - subdir, - } = package_build_source - else { - panic!("expected git package build source"); - }; - - assert_eq!(url.path(), "/repo.git"); - assert_eq!(url.host_str(), Some("example.com")); - assert_eq!(subdir.as_ref().map(|s| s.as_str()), Some("nested/project")); - assert!(matches!(spec, Some(GitShallowSpec::Branch(branch)) if branch == "main")); - assert_eq!(rev, "0123456789abcdef0123456789abcdef01234567"); - - let roundtrip = SourceRecord::from_conda_source_data( - conda_source, - &std::path::PathBuf::from("/workspace"), - ) - .expect("roundtrip should succeed"); - let Some(PinnedSourceSpec::Git(roundtrip_git)) = roundtrip.build_source else { - panic!("expected git pinned source"); - }; - assert_eq!( - roundtrip_git.source.subdirectory.as_deref(), - Some("nested/project") - ); - assert_eq!(roundtrip_git.git, git_url); - } - #[test] fn package_build_source_path_is_made_relative() { use typed_path::Utf8TypedPathBuf; @@ -411,12 +406,8 @@ mod tests { panic!("expected path pinned source"); }; - // After roundtrip, the path should remain as it was in the lock file (relative) - assert_eq!( - roundtrip_path.path.as_str(), - "../src", - "build_source should remain as relative path from lock file" - ); + // After roundtrip, the path should again be from the repository root + assert_eq!(roundtrip_path.path.as_str(), "src"); } #[test] @@ -469,7 +460,7 @@ mod tests { } #[test] - fn package_build_source_git_same_repo_is_made_relative() { + fn package_build_source_roundtrip_git_with_subdir() { let package_record: PackageRecord = serde_json::from_value(json!({ "name": "example", "version": "1.0.0", @@ -538,14 +529,21 @@ mod tests { ) .expect("roundtrip should succeed"); - let Some(PinnedSourceSpec::Path(roundtrip_path)) = roundtrip.build_source else { + let Some(PinnedSourceSpec::Git(roundtrip_path)) = roundtrip.build_source else { panic!( "expected path pinned source after roundtrip (deserialized from relative path in lock file)" ); }; - // After roundtrip, it should remain as the relative path from the lock file - assert_eq!(roundtrip_path.path.as_str(), "../src"); + // After roundtrip, the path will contain .. components (not normalized) + assert_eq!( + roundtrip_path + .source + .subdirectory + .expect("subdirectory should be set") + .as_str(), + "src" + ); } #[test] @@ -615,7 +613,7 @@ mod tests { fn roundtrip_conda_source_data() { let workspace_root = std::path::Path::new("/workspace"); - // Load the SourceRecord from snapshot (skip YAML frontmatter) + // Load the SourceRecords from snapshot (skip YAML frontmatter) let snapshot_content = include_str!( "snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap" ); @@ -623,19 +621,24 @@ mod tests { .split("---") .nth(2) .expect("snapshot should have YAML content"); - let original: SourceRecord = + let originals: Vec = serde_yaml::from_str(yaml_content).expect("failed to load snapshot"); - // Roundtrip: SourceRecord -> CondaSourceData -> SourceRecord - let conda_data = original.clone().into_conda_source_data(workspace_root); - let roundtrip = SourceRecord::from_conda_source_data(conda_data, workspace_root) - .expect("from_conda_source_data should succeed"); - - // Snapshot the final result - should match the original + // Roundtrip each record: SourceRecord -> CondaSourceData -> SourceRecord + let roundtrips: Vec = originals + .into_iter() + .map(|original| { + let conda_data = original.clone().into_conda_source_data(workspace_root); + SourceRecord::from_conda_source_data(conda_data, workspace_root) + .expect("from_conda_source_data should succeed") + }) + .collect(); + + // Snapshot the final results - should match the originals let mut settings = insta::Settings::clone_current(); settings.set_sort_maps(true); settings.bind(|| { - insta::assert_yaml_snapshot!(roundtrip); + insta::assert_yaml_snapshot!(roundtrips); }); } } From 397d3506e55b6b0415a036750828f90bf6194649 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 7 Nov 2025 16:03:48 +0100 Subject: [PATCH 15/21] fix: more tests --- ...d__tests__roundtrip_conda_source_data.snap | 96 +++++++ crates/pixi_record/src/source_record.rs | 261 ++++++++++++++---- 2 files changed, 297 insertions(+), 60 deletions(-) diff --git a/crates/pixi_record/src/snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap b/crates/pixi_record/src/snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap index 38ff3bf88c..268f504cd7 100644 --- a/crates/pixi_record/src/snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap +++ b/crates/pixi_record/src/snapshots/pixi_record__source_record__tests__roundtrip_conda_source_data.snap @@ -91,3 +91,99 @@ expression: roundtrips globs: - "**/*.cpp" sources: {} +- package_record: + build: h901234_0 + build_number: 0 + depends: [] + md5: d41d8cd98f00b204e9800998ecf8427e + name: path-absolute-build-outside + sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + subdir: noarch + version: 2.2.0 + manifest_source: + path: recipes/external-package + build_source: + path: /opt/external/sources/pkg + input_hash: + hash: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + globs: + - "**/*.pyx" + sources: {} +- package_record: + build: h901235_0 + build_number: 0 + depends: [] + md5: d41d8cd98f00b204e9800998ecf8427e + name: path-root-manifest + sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + subdir: noarch + version: 2.3.0 + manifest_source: + path: "" + build_source: + path: src/root-build + input_hash: + hash: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + globs: + - pyproject.toml + sources: {} +- package_record: + build: h901236_0 + build_number: 0 + depends: [] + md5: d41d8cd98f00b204e9800998ecf8427e + name: path-absolute-manifest + sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + subdir: noarch + version: 2.4.0 + manifest_source: + path: /workspace/absolute-recipe + build_source: + path: absolute-recipe/src + input_hash: + hash: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + globs: + - "**/*.sh" + sources: {} +- package_record: + build: h901237_0 + build_number: 0 + depends: [] + md5: d41d8cd98f00b204e9800998ecf8427e + name: git-no-manifest-subdir + sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + subdir: noarch + version: 3.0.0 + manifest_source: + commit: abc123def456abc123def456abc123def456abc1 + git: "https://github.com/example/repo.git" + reference: + tag: v1.0.0 + build_source: + commit: abc123def456abc123def456abc123def456abc1 + git: "https://github.com/example/repo.git" + reference: + tag: v1.0.0 + subdirectory: build/subdir + input_hash: + hash: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + globs: + - src/**/*.rs + sources: {} +- package_record: + build: h901238_0 + build_number: 0 + depends: [] + md5: d41d8cd98f00b204e9800998ecf8427e + name: path-no-build-source + sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + subdir: noarch + version: 2.5.0 + manifest_source: + path: recipes/no-build + build_source: ~ + input_hash: + hash: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + globs: + - "**/*.md" + sources: {} diff --git a/crates/pixi_record/src/source_record.rs b/crates/pixi_record/src/source_record.rs index d45f0565e4..58bd49a9c1 100644 --- a/crates/pixi_record/src/source_record.rs +++ b/crates/pixi_record/src/source_record.rs @@ -5,13 +5,14 @@ use std::{ str::FromStr, }; -use pixi_git::sha::GitSha; +use pixi_git::{sha::GitSha, url::RepositoryUrl}; use pixi_spec::{GitReference, SourceSpec}; use rattler_conda_types::{MatchSpec, Matches, NamelessMatchSpec, PackageRecord}; use rattler_digest::{Sha256, Sha256Hash}; use rattler_lock::{CondaSourceData, GitShallowSpec, PackageBuildSource}; use serde::{Deserialize, Serialize}; use typed_path::Utf8TypedPathBuf; +use url::Url; use crate::{ParseLockFileError, PinnedGitCheckout, PinnedSourceSpec}; @@ -64,45 +65,62 @@ impl SourceRecord { /// However, when saving in the lock-file make these relative to the package manifest. /// This should be used when writing to the lock file. pub fn into_conda_source_data(self, workspace_root: &Path) -> CondaSourceData { - let package_build_source = self.build_source.map(|build_source| { - let pinned_source_spec = build_source - .make_relative_to(&self.manifest_source, workspace_root) - .unwrap_or(build_source); - - match pinned_source_spec { - PinnedSourceSpec::Url(pinned_url_spec) => PackageBuildSource::Url { - url: pinned_url_spec.url, - sha256: pinned_url_spec.sha256, - subdir: None, - }, - PinnedSourceSpec::Git(pinned_git_spec) => { - let subdirectory = pinned_git_spec - .source - .subdirectory - .as_deref() - .map(Utf8TypedPathBuf::from); - - let spec = match &pinned_git_spec.source.reference { - GitReference::Branch(branch) => { - Some(GitShallowSpec::Branch(branch.clone())) + let package_build_source = + self.build_source + .clone() + .map(|build_source| match build_source { + PinnedSourceSpec::Git(git_spec) => { + // When manifest and build refer to the same repo+commit we keep the + // build source as git, but rewrite its subdirectory relative to the + // manifest checkout as expected by the lock file format. + let mut subdirectory = git_spec.source.subdirectory.clone(); + + if let PinnedSourceSpec::Git(manifest_git) = &self.manifest_source { + if same_git_checkout(&git_spec, manifest_git) { + subdirectory = relative_repo_subdir( + manifest_git.source.subdirectory.as_deref().unwrap_or(""), + git_spec.source.subdirectory.as_deref().unwrap_or(""), + ); + } + } + + let subdirectory = subdirectory.as_deref().map(Utf8TypedPathBuf::from); + let spec = to_git_shallow(&git_spec.source.reference); + + PackageBuildSource::Git { + url: git_spec.git, + spec, + rev: git_spec.source.commit.to_string(), + subdir: subdirectory, } - GitReference::Tag(tag) => Some(GitShallowSpec::Tag(tag.clone())), - GitReference::Rev(_) => Some(GitShallowSpec::Rev), - GitReference::DefaultBranch => None, - }; - - PackageBuildSource::Git { - url: pinned_git_spec.git, - spec, - rev: pinned_git_spec.source.commit.to_string(), - subdir: subdirectory, } - } - PinnedSourceSpec::Path(pinned_path) => PackageBuildSource::Path { - path: pinned_path.path, - }, - } - }); + PinnedSourceSpec::Url(pinned_url_spec) => PackageBuildSource::Url { + url: pinned_url_spec.url, + sha256: pinned_url_spec.sha256, + subdir: None, + }, + PinnedSourceSpec::Path(pinned_path) => { + let native_path = Path::new(pinned_path.path.as_str()); + let should_relativize = !(native_path.is_absolute() + && !native_path.starts_with(workspace_root)); + + let relativized = if should_relativize { + PinnedSourceSpec::Path(pinned_path.clone()) + .make_relative_to(&self.manifest_source, workspace_root) + .unwrap_or(PinnedSourceSpec::Path(pinned_path.clone())) + } else { + PinnedSourceSpec::Path(pinned_path.clone()) + }; + + let PinnedSourceSpec::Path(result_path) = relativized else { + unreachable!("path specs must remain paths"); + }; + + PackageBuildSource::Path { + path: result_path.path, + } + } + }); CondaSourceData { package_record: self.package_record, @@ -139,19 +157,27 @@ impl SourceRecord { rev, subdir, } => { - let reference = match spec { - Some(GitShallowSpec::Branch(branch)) => GitReference::Branch(branch), - Some(GitShallowSpec::Tag(tag)) => GitReference::Tag(tag), - Some(GitShallowSpec::Rev) => GitReference::Rev(rev.clone()), - None => GitReference::DefaultBranch, - }; - - // Out-of-tree git repository + let mut subdirectory = subdir.map(|s| s.to_string()); + + if let PinnedSourceSpec::Git(manifest_git) = &manifest_source { + // For same-repo checkouts the lock stored the subdir relative to the manifest; + // restore the absolute repo subdirectory so SourceRecord stays workspace-root + // relative again. + if same_git_checkout_url_commit(manifest_git, &url, &rev) { + subdirectory = resolve_repo_subdir( + manifest_git.source.subdirectory.as_deref().unwrap_or(""), + subdirectory.as_deref(), + ); + } + } + + let reference = git_reference_from_shallow(spec, &rev); + PinnedSourceSpec::Git(crate::PinnedGitSpec { git: url, source: PinnedGitCheckout { commit: GitSha::from_str(&rev).unwrap(), - subdirectory: subdir.map(|s| s.to_string()), + subdirectory, reference, }, }) @@ -280,6 +306,7 @@ impl AsRef for SourceRecord { } } +/// Normalize a path lexically (no filesystem access) and remove redundant separators/`..`. fn normalize_path(path: &Path) -> PathBuf { use std::path::Component; @@ -327,10 +354,95 @@ fn normalize_path(path: &Path) -> PathBuf { normalized.push(part); } + normalized +} + +/// Returns true when both git specs target the same repository (ignoring URL noise) +/// and are pinned to the identical commit. +fn same_git_checkout(a: &crate::PinnedGitSpec, b: &crate::PinnedGitSpec) -> bool { + RepositoryUrl::new(&a.git) == RepositoryUrl::new(&b.git) && a.source.commit == b.source.commit +} + +/// Same check as `same_git_checkout`, but used while parsing lock data where only +/// the URL + rev string are available. +fn same_git_checkout_url_commit(manifest_git: &crate::PinnedGitSpec, url: &Url, rev: &str) -> bool { + RepositoryUrl::new(&manifest_git.git) == RepositoryUrl::new(url) + && manifest_git.source.commit.to_string() == rev +} + +/// Compute the repo-relative path from `base` (manifest subdir) to `target` +/// (build subdir). Returns `None` if the directories are identical/root. +fn relative_repo_subdir(base: &str, target: &str) -> Option { + let base_abs = repo_absolute_path(base); + let target_abs = repo_absolute_path(target); + let relative = + pathdiff::diff_paths(&target_abs, &base_abs).unwrap_or_else(|| target_abs.clone()); + let normalized = normalize_path(&relative); if normalized.as_os_str().is_empty() { - simplified + None } else { - normalized + Some(normalized.to_string_lossy().to_string()) + } +} + +/// Undo `relative_repo_subdir`: apply the manifest subdir back onto the relative +/// path that was stored in the lock, so we get the real repo subdirectory again. +fn resolve_repo_subdir(base: &str, relative: Option<&str>) -> Option { + match relative { + Some(rel) => { + let combined = repo_absolute_path(base).join(rel); + strip_repo_root(normalize_path(&combined)) + } + None => { + if base.is_empty() { + None + } else { + Some(base.to_string()) + } + } + } +} + +fn repo_absolute_path(subdir: &str) -> PathBuf { + if subdir.is_empty() { + PathBuf::from("/") + } else { + Path::new("/").join(subdir) + } +} + +fn strip_repo_root(path: PathBuf) -> Option { + let trimmed = if path.has_root() { + match path.strip_prefix(Path::new("/")) { + Ok(stripped) => stripped.to_path_buf(), + Err(_) => path, + } + } else { + path + }; + + if trimmed.as_os_str().is_empty() { + None + } else { + Some(trimmed.to_string_lossy().to_string()) + } +} + +fn to_git_shallow(reference: &GitReference) -> Option { + match reference { + GitReference::Branch(branch) => Some(GitShallowSpec::Branch(branch.clone())), + GitReference::Tag(tag) => Some(GitShallowSpec::Tag(tag.clone())), + GitReference::Rev(_) => Some(GitShallowSpec::Rev), + GitReference::DefaultBranch => None, + } +} + +fn git_reference_from_shallow(spec: Option, rev: &str) -> GitReference { + match spec { + Some(GitShallowSpec::Branch(branch)) => GitReference::Branch(branch), + Some(GitShallowSpec::Tag(tag)) => GitReference::Tag(tag), + Some(GitShallowSpec::Rev) => GitReference::Rev(rev.to_string()), + None => GitReference::DefaultBranch, } } @@ -339,7 +451,7 @@ mod tests { use super::*; use pixi_git::sha::GitSha; use serde_json::json; - use std::str::FromStr; + use std::{path::Path, str::FromStr}; use url::Url; #[test] @@ -455,8 +567,8 @@ mod tests { panic!("expected path package build source"); }; - // The path should be made relative - assert_eq!(path.as_str(), "../../completely/different/path"); + // The path should stay absolute because the build source is unrelated + assert_eq!(path.as_str(), "/completely/different/path"); } #[test] @@ -511,16 +623,11 @@ mod tests { .as_ref() .expect("expected package build source"); - let PackageBuildSource::Path { path } = package_build_source else { - panic!("expected path package build source (relativized from git)"); + let PackageBuildSource::Git { subdir, .. } = package_build_source else { + panic!("expected git package build source with relative subdir"); }; - // The path should be relative: from recipes/ to src/ is ../src - assert_eq!( - path.as_str(), - "../src", - "build_source should be converted to relative path" - ); + assert_eq!(subdir.as_ref().map(|s| s.as_str()), Some("../src")); // Convert back to SourceRecord (deserialization) let roundtrip = SourceRecord::from_conda_source_data( @@ -641,4 +748,38 @@ mod tests { insta::assert_yaml_snapshot!(roundtrips); }); } + + #[test] + fn normalize_path_collapses_parent_segments() { + let normalized = super::normalize_path(Path::new("recipes/../")); + assert!(normalized.as_os_str().is_empty()); + } + + #[test] + fn repo_subdir_helpers_round_trip() { + use super::{relative_repo_subdir, resolve_repo_subdir}; + + let manifest = "recipes"; + let build = "recipes/lib"; + + let relative = relative_repo_subdir(manifest, build).expect("should produce relative path"); + assert_eq!(relative, "lib"); + + let resolved = resolve_repo_subdir(manifest, Some(relative.as_str())); + assert_eq!(resolved.as_deref(), Some("recipes/lib")); + } + + #[test] + fn repo_subdir_helpers_handle_root() { + use super::{relative_repo_subdir, resolve_repo_subdir}; + + // Both manifest and build at repo root + assert!(relative_repo_subdir("", "").is_none()); + assert!(resolve_repo_subdir("", None).is_none()); + + // Manifest at root, build in subdir + let rel = relative_repo_subdir("", "src").expect("relative path"); + assert_eq!(rel, "src"); + assert_eq!(resolve_repo_subdir("", Some("src")).as_deref(), Some("src")); + } } From 7df20ae1f73bf5c22b129ebf87efd7c0310b722b Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 7 Nov 2025 16:06:50 +0100 Subject: [PATCH 16/21] fix: clippy --- crates/pixi_record/src/source_record.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/pixi_record/src/source_record.rs b/crates/pixi_record/src/source_record.rs index 58bd49a9c1..0fafca7b2d 100644 --- a/crates/pixi_record/src/source_record.rs +++ b/crates/pixi_record/src/source_record.rs @@ -101,8 +101,8 @@ impl SourceRecord { }, PinnedSourceSpec::Path(pinned_path) => { let native_path = Path::new(pinned_path.path.as_str()); - let should_relativize = !(native_path.is_absolute() - && !native_path.starts_with(workspace_root)); + let should_relativize = + !native_path.is_absolute() || native_path.starts_with(workspace_root); let relativized = if should_relativize { PinnedSourceSpec::Path(pinned_path.clone()) From b517bc377b8ae5f7f51ce639ea7e82fafe24f724 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 7 Nov 2025 17:43:15 +0100 Subject: [PATCH 17/21] feat: pull out path dependencies --- crates/pixi_record/src/lib.rs | 1 + crates/pixi_record/src/path_utils.rs | 196 +++++++++++++++++++++ crates/pixi_record/src/source_record.rs | 217 +++++++----------------- 3 files changed, 261 insertions(+), 153 deletions(-) create mode 100644 crates/pixi_record/src/path_utils.rs diff --git a/crates/pixi_record/src/lib.rs b/crates/pixi_record/src/lib.rs index cf65141fd3..63f6fec85b 100644 --- a/crates/pixi_record/src/lib.rs +++ b/crates/pixi_record/src/lib.rs @@ -1,3 +1,4 @@ +mod path_utils; mod pinned_source; mod source_record; diff --git a/crates/pixi_record/src/path_utils.rs b/crates/pixi_record/src/path_utils.rs new file mode 100644 index 0000000000..77033b013e --- /dev/null +++ b/crates/pixi_record/src/path_utils.rs @@ -0,0 +1,196 @@ +use std::path::{Path, PathBuf}; + +/// Normalize a path lexically (no filesystem access) and strip redundant segments. +pub(crate) fn normalize_path(path: &Path) -> PathBuf { + use std::path::Component; + + let simplified = dunce::simplified(path).to_path_buf(); + + let mut prefix: Option = None; + let mut has_root = false; + let mut parts: Vec = Vec::new(); + + for component in simplified.components() { + match component { + Component::Prefix(prefix_component) => { + prefix = Some(prefix_component.as_os_str().to_os_string()); + parts.clear(); + } + Component::RootDir => { + has_root = true; + parts.clear(); + } + Component::CurDir => {} + Component::ParentDir => { + if let Some(last) = parts.last() { + if last.as_os_str() == std::ffi::OsStr::new("..") { + parts.push(std::ffi::OsString::from("..")); + } else { + parts.pop(); + } + } else if !has_root { + parts.push(std::ffi::OsString::from("..")); + } + } + Component::Normal(part) => parts.push(part.to_os_string()), + } + } + + let mut normalized = PathBuf::new(); + if let Some(prefix) = prefix { + normalized.push(prefix); + } + if has_root { + normalized.push(std::path::MAIN_SEPARATOR.to_string()); + } + for part in parts { + normalized.push(part); + } + + normalized +} + +/// Compute the repo-relative path from `base` (manifest subdir) to `target` (build subdir). +pub(crate) fn relative_repo_subdir(base: &str, target: &str) -> Option { + let base_abs = repo_absolute_path(base); + let target_abs = repo_absolute_path(target); + let relative = + pathdiff::diff_paths(&target_abs, &base_abs).unwrap_or_else(|| target_abs.clone()); + let normalized = normalize_path(&relative); + if normalized.as_os_str().is_empty() { + None + } else { + Some(unixify_path(&normalized)) + } +} + +/// Apply a manifest subdir back onto a relative path stored in the lock. +pub(crate) fn resolve_repo_subdir(base: &str, relative: Option<&str>) -> Option { + match relative { + Some(rel) => { + let combined = repo_absolute_path(base).join(rel); + strip_repo_root(normalize_path(&combined)) + } + None => { + if base.is_empty() { + None + } else { + Some(unixify_str(base)) + } + } + } +} + +fn repo_absolute_path(subdir: &str) -> PathBuf { + if subdir.is_empty() { + PathBuf::from("/") + } else { + Path::new("/").join(subdir) + } +} + +fn strip_repo_root(path: PathBuf) -> Option { + let trimmed = if path.has_root() { + match path.strip_prefix(Path::new("/")) { + Ok(stripped) => stripped.to_path_buf(), + Err(_) => path, + } + } else { + path + }; + + if trimmed.as_os_str().is_empty() { + None + } else { + Some(unixify_path(&trimmed)) + } +} + +pub(crate) fn unixify_path(path: &Path) -> String { + path.to_string_lossy().replace('\\', "/") +} + +pub(crate) fn unixify_str(value: &str) -> String { + value.replace('\\', "/") +} + +/// Returns true if the string should be treated as absolute regardless of host OS. +pub(crate) fn is_cross_platform_absolute(path_str: &str, native_path: &Path) -> bool { + if native_path.is_absolute() { + return true; + } + + if path_str.starts_with('/') || path_str.starts_with('\\') { + return true; + } + + if path_str.len() >= 3 { + let bytes = path_str.as_bytes(); + let drive = bytes[0]; + let colon = bytes[1]; + let slash = bytes[2]; + + if drive.is_ascii_alphabetic() && colon == b':' && (slash == b'/' || slash == b'\\') { + return true; + } + } + + false +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn normalize_path_collapses_parent_segments() { + let normalized = normalize_path(Path::new("recipes/../")); + assert!(normalized.as_os_str().is_empty()); + } + + #[test] + fn repo_subdir_helpers_round_trip() { + let manifest = "recipes"; + let build = "recipes/lib"; + + let relative = relative_repo_subdir(manifest, build).expect("should produce relative path"); + assert_eq!(relative, "lib"); + + let resolved = resolve_repo_subdir(manifest, Some(relative.as_str())); + assert_eq!(resolved.as_deref(), Some("recipes/lib")); + } + + #[test] + fn repo_subdir_helpers_handle_root() { + assert!(relative_repo_subdir("", "").is_none()); + assert!(resolve_repo_subdir("", None).is_none()); + + let rel = relative_repo_subdir("", "src").expect("relative path"); + assert_eq!(rel, "src"); + assert_eq!(resolve_repo_subdir("", Some("src")).as_deref(), Some("src")); + } + + #[test] + fn cross_platform_absolute_detection() { + assert!(is_cross_platform_absolute( + "/opt/external", + Path::new("/opt/external") + )); + assert!(is_cross_platform_absolute( + r"C:\work", + Path::new(r"C:\work") + )); + assert!(is_cross_platform_absolute( + r"\\server\share", + Path::new(r"\\server\share") + )); + assert!(is_cross_platform_absolute( + "/just/slash", + Path::new("just\\slash") + )); + assert!(!is_cross_platform_absolute( + "relative/path", + Path::new("relative/path") + )); + } +} diff --git a/crates/pixi_record/src/source_record.rs b/crates/pixi_record/src/source_record.rs index 0fafca7b2d..2e3c3b7357 100644 --- a/crates/pixi_record/src/source_record.rs +++ b/crates/pixi_record/src/source_record.rs @@ -1,7 +1,6 @@ use std::{ collections::{BTreeSet, HashMap}, - ffi::{OsStr, OsString}, - path::{Path, PathBuf}, + path::Path, str::FromStr, }; @@ -14,7 +13,13 @@ use serde::{Deserialize, Serialize}; use typed_path::Utf8TypedPathBuf; use url::Url; -use crate::{ParseLockFileError, PinnedGitCheckout, PinnedSourceSpec}; +use crate::{ + ParseLockFileError, PinnedGitCheckout, PinnedSourceSpec, + path_utils::{ + is_cross_platform_absolute, normalize_path, relative_repo_subdir, resolve_repo_subdir, + unixify_path, + }, +}; /// A record of a conda package that still requires building. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] @@ -100,9 +105,14 @@ impl SourceRecord { subdir: None, }, PinnedSourceSpec::Path(pinned_path) => { - let native_path = Path::new(pinned_path.path.as_str()); - let should_relativize = - !native_path.is_absolute() || native_path.starts_with(workspace_root); + let path_str = pinned_path.path.as_str(); + let native_path = Path::new(path_str); + let is_native_absolute = native_path.is_absolute(); + let is_cross_platform_absolute = + is_cross_platform_absolute(path_str, native_path); + let is_within_workspace = + is_native_absolute && native_path.starts_with(workspace_root); + let should_relativize = !is_cross_platform_absolute || is_within_workspace; let relativized = if should_relativize { PinnedSourceSpec::Path(pinned_path.clone()) @@ -241,9 +251,7 @@ impl SourceRecord { .unwrap_or(build_absolute); PinnedSourceSpec::Path(crate::PinnedPathSpec { - path: Utf8TypedPathBuf::from( - relative_to_workspace.to_string_lossy().as_ref(), - ), + path: Utf8TypedPathBuf::from(unixify_path(&relative_to_workspace)), }) } } @@ -307,56 +315,6 @@ impl AsRef for SourceRecord { } /// Normalize a path lexically (no filesystem access) and remove redundant separators/`..`. -fn normalize_path(path: &Path) -> PathBuf { - use std::path::Component; - - // `dunce::simplified` is purely lexical and strips UNC verbatim prefixes on Windows. - let simplified = dunce::simplified(path).to_path_buf(); - - let mut prefix: Option = None; - let mut has_root = false; - let mut parts: Vec = Vec::new(); - - for component in simplified.components() { - match component { - Component::Prefix(prefix_component) => { - prefix = Some(prefix_component.as_os_str().to_os_string()); - parts.clear(); - } - Component::RootDir => { - has_root = true; - parts.clear(); - } - Component::CurDir => {} - Component::ParentDir => { - if let Some(last) = parts.last() { - if last.as_os_str() == OsStr::new("..") { - parts.push(OsString::from("..")); - } else { - parts.pop(); - } - } else if !has_root { - parts.push(OsString::from("..")); - } - } - Component::Normal(part) => parts.push(part.to_os_string()), - } - } - - let mut normalized = PathBuf::new(); - if let Some(prefix) = prefix { - normalized.push(prefix); - } - if has_root { - normalized.push(std::path::MAIN_SEPARATOR.to_string()); - } - for part in parts { - normalized.push(part); - } - - normalized -} - /// Returns true when both git specs target the same repository (ignoring URL noise) /// and are pinned to the identical commit. fn same_git_checkout(a: &crate::PinnedGitSpec, b: &crate::PinnedGitSpec) -> bool { @@ -370,64 +328,6 @@ fn same_git_checkout_url_commit(manifest_git: &crate::PinnedGitSpec, url: &Url, && manifest_git.source.commit.to_string() == rev } -/// Compute the repo-relative path from `base` (manifest subdir) to `target` -/// (build subdir). Returns `None` if the directories are identical/root. -fn relative_repo_subdir(base: &str, target: &str) -> Option { - let base_abs = repo_absolute_path(base); - let target_abs = repo_absolute_path(target); - let relative = - pathdiff::diff_paths(&target_abs, &base_abs).unwrap_or_else(|| target_abs.clone()); - let normalized = normalize_path(&relative); - if normalized.as_os_str().is_empty() { - None - } else { - Some(normalized.to_string_lossy().to_string()) - } -} - -/// Undo `relative_repo_subdir`: apply the manifest subdir back onto the relative -/// path that was stored in the lock, so we get the real repo subdirectory again. -fn resolve_repo_subdir(base: &str, relative: Option<&str>) -> Option { - match relative { - Some(rel) => { - let combined = repo_absolute_path(base).join(rel); - strip_repo_root(normalize_path(&combined)) - } - None => { - if base.is_empty() { - None - } else { - Some(base.to_string()) - } - } - } -} - -fn repo_absolute_path(subdir: &str) -> PathBuf { - if subdir.is_empty() { - PathBuf::from("/") - } else { - Path::new("/").join(subdir) - } -} - -fn strip_repo_root(path: PathBuf) -> Option { - let trimmed = if path.has_root() { - match path.strip_prefix(Path::new("/")) { - Ok(stripped) => stripped.to_path_buf(), - Err(_) => path, - } - } else { - path - }; - - if trimmed.as_os_str().is_empty() { - None - } else { - Some(trimmed.to_string_lossy().to_string()) - } -} - fn to_git_shallow(reference: &GitReference) -> Option { match reference { GitReference::Branch(branch) => Some(GitShallowSpec::Branch(branch.clone())), @@ -451,7 +351,7 @@ mod tests { use super::*; use pixi_git::sha::GitSha; use serde_json::json; - use std::{path::Path, str::FromStr}; + use std::str::FromStr; use url::Url; #[test] @@ -499,15 +399,15 @@ mod tests { panic!("expected path package build source"); }; - // The path should now be relative to the manifest directory - // Manifest is in /workspace/recipes/, so ../src should point to /workspace/src + // Because manifest + build live in the same git repo we serialize the build as a git + // source with a subdir relative to the manifest checkout. assert_eq!( path.as_str(), "../src", "build_source should be relative to manifest_source directory" ); - // Convert back to SourceRecord (deserialization) + // Convert back to SourceRecord (deserialization) and ensure we recover repo-root subdir let roundtrip = SourceRecord::from_conda_source_data( conda_source, &std::path::PathBuf::from("/workspace"), @@ -518,7 +418,7 @@ mod tests { panic!("expected path pinned source"); }; - // After roundtrip, the path should again be from the repository root + // After roundtrip the git subdirectory should be expressed from repo root again. assert_eq!(roundtrip_path.path.as_str(), "src"); } @@ -627,6 +527,7 @@ mod tests { panic!("expected git package build source with relative subdir"); }; + // Git subdir is relative to manifest checkout (recipes -> ../src) assert_eq!(subdir.as_ref().map(|s| s.as_str()), Some("../src")); // Convert back to SourceRecord (deserialization) @@ -750,36 +651,46 @@ mod tests { } #[test] - fn normalize_path_collapses_parent_segments() { - let normalized = super::normalize_path(Path::new("recipes/../")); - assert!(normalized.as_os_str().is_empty()); - } - - #[test] - fn repo_subdir_helpers_round_trip() { - use super::{relative_repo_subdir, resolve_repo_subdir}; - - let manifest = "recipes"; - let build = "recipes/lib"; - - let relative = relative_repo_subdir(manifest, build).expect("should produce relative path"); - assert_eq!(relative, "lib"); - - let resolved = resolve_repo_subdir(manifest, Some(relative.as_str())); - assert_eq!(resolved.as_deref(), Some("recipes/lib")); - } - - #[test] - fn repo_subdir_helpers_handle_root() { - use super::{relative_repo_subdir, resolve_repo_subdir}; - - // Both manifest and build at repo root - assert!(relative_repo_subdir("", "").is_none()); - assert!(resolve_repo_subdir("", None).is_none()); - - // Manifest at root, build in subdir - let rel = relative_repo_subdir("", "src").expect("relative path"); - assert_eq!(rel, "src"); - assert_eq!(resolve_repo_subdir("", Some("src")).as_deref(), Some("src")); + fn git_reference_conversion_helpers() { + use super::{git_reference_from_shallow, to_git_shallow}; + use pixi_spec::GitReference; + use rattler_lock::GitShallowSpec; + + assert!(matches!( + to_git_shallow(&GitReference::Branch("main".into())), + Some(GitShallowSpec::Branch(branch)) if branch == "main" + )); + + assert!(matches!( + to_git_shallow(&GitReference::Tag("v1".into())), + Some(GitShallowSpec::Tag(tag)) if tag == "v1" + )); + + assert!(matches!( + to_git_shallow(&GitReference::Rev("abc".into())), + Some(GitShallowSpec::Rev) + )); + + assert!(matches!(to_git_shallow(&GitReference::DefaultBranch), None)); + + assert!(matches!( + git_reference_from_shallow(Some(GitShallowSpec::Branch("dev".into())), "ignored"), + GitReference::Branch(branch) if branch == "dev" + )); + + assert!(matches!( + git_reference_from_shallow(Some(GitShallowSpec::Tag("v2".into())), "ignored"), + GitReference::Tag(tag) if tag == "v2" + )); + + assert!(matches!( + git_reference_from_shallow(Some(GitShallowSpec::Rev), "deadbeef"), + GitReference::Rev(rev) if rev == "deadbeef" + )); + + assert!(matches!( + git_reference_from_shallow(None, "deadbeef"), + GitReference::DefaultBranch + )); } } From 1d76082116c2b696ba6b11929618951e7c404091 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 7 Nov 2025 17:46:38 +0100 Subject: [PATCH 18/21] fix: clippy --- crates/pixi_record/src/source_record.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/pixi_record/src/source_record.rs b/crates/pixi_record/src/source_record.rs index 2e3c3b7357..eb11e734ce 100644 --- a/crates/pixi_record/src/source_record.rs +++ b/crates/pixi_record/src/source_record.rs @@ -671,7 +671,7 @@ mod tests { Some(GitShallowSpec::Rev) )); - assert!(matches!(to_git_shallow(&GitReference::DefaultBranch), None)); + assert!(to_git_shallow(&GitReference::DefaultBranch).is_none()); assert!(matches!( git_reference_from_shallow(Some(GitShallowSpec::Branch("dev".into())), "ignored"), From a84faf08a827c663b9b8dfe67740aef880341c4f Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 7 Nov 2025 18:04:03 +0100 Subject: [PATCH 19/21] fix: fix windows path handling again --- crates/pixi_record/src/pinned_source.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/pixi_record/src/pinned_source.rs b/crates/pixi_record/src/pinned_source.rs index 83055f9e1c..d83fb8f9b9 100644 --- a/crates/pixi_record/src/pinned_source.rs +++ b/crates/pixi_record/src/pinned_source.rs @@ -5,6 +5,7 @@ use std::{ str::FromStr, }; +use crate::path_utils::unixify_path; use miette::IntoDiagnostic; use pixi_git::{ GitUrl, @@ -233,7 +234,7 @@ impl PinnedSourceSpec { let relative_path = pathdiff::diff_paths(this_path, base_path)?; Some(PinnedSourceSpec::Path(PinnedPathSpec { - path: Utf8TypedPathBuf::from(relative_path.to_string_lossy().as_ref()), + path: Utf8TypedPathBuf::from(unixify_path(relative_path.as_path())), })) } // Git-to-Git: If same repository, convert to a relative path based on subdirectories @@ -261,7 +262,7 @@ impl PinnedSourceSpec { let this_path = std::path::Path::new(this_subdir); let relative = pathdiff::diff_paths(this_path, base_path)?; - let relative_str = relative.to_str()?; + let relative_str = unixify_path(relative.as_path()); Some(PinnedSourceSpec::Path(PinnedPathSpec { path: Utf8TypedPathBuf::from(relative_str), From f91017d3b8020c8386392f5cee036dff5aca5e8c Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Sat, 8 Nov 2025 08:26:55 +0100 Subject: [PATCH 20/21] fix: codex attempts to fix path handling once again --- crates/pixi_record/src/path_utils.rs | 49 +++++++++++++++++++++++++ crates/pixi_record/src/source_record.rs | 7 ++-- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/crates/pixi_record/src/path_utils.rs b/crates/pixi_record/src/path_utils.rs index 77033b013e..a3dd70e693 100644 --- a/crates/pixi_record/src/path_utils.rs +++ b/crates/pixi_record/src/path_utils.rs @@ -114,6 +114,34 @@ pub(crate) fn unixify_str(value: &str) -> String { value.replace('\\', "/") } +pub(crate) fn path_within_workspace( + path_str: &str, + native_path: &Path, + workspace_root: &Path, +) -> bool { + if native_path.is_absolute() { + return native_path.starts_with(workspace_root); + } + + let path_unix = unixify_str(path_str); + let mut workspace_unix = unixify_path(workspace_root); + if workspace_unix.ends_with('/') { + workspace_unix.pop(); + } + + if workspace_unix.is_empty() { + return path_unix.starts_with('/'); + } + + if path_unix == workspace_unix { + return true; + } + + path_unix.len() > workspace_unix.len() + && path_unix.starts_with(&workspace_unix) + && path_unix.as_bytes()[workspace_unix.len()] == b'/' +} + /// Returns true if the string should be treated as absolute regardless of host OS. pub(crate) fn is_cross_platform_absolute(path_str: &str, native_path: &Path) -> bool { if native_path.is_absolute() { @@ -193,4 +221,25 @@ mod tests { Path::new("relative/path") )); } + + #[test] + fn path_within_workspace_detection() { + assert!(path_within_workspace( + "/workspace/src", + Path::new("/workspace/src"), + Path::new("/workspace") + )); + + assert!(path_within_workspace( + "/workspace/src", + Path::new("workspace\\src"), + Path::new("/workspace") + )); + + assert!(!path_within_workspace( + "/other/src", + Path::new("/other/src"), + Path::new("/workspace") + )); + } } diff --git a/crates/pixi_record/src/source_record.rs b/crates/pixi_record/src/source_record.rs index eb11e734ce..687ac731be 100644 --- a/crates/pixi_record/src/source_record.rs +++ b/crates/pixi_record/src/source_record.rs @@ -16,8 +16,8 @@ use url::Url; use crate::{ ParseLockFileError, PinnedGitCheckout, PinnedSourceSpec, path_utils::{ - is_cross_platform_absolute, normalize_path, relative_repo_subdir, resolve_repo_subdir, - unixify_path, + is_cross_platform_absolute, normalize_path, path_within_workspace, relative_repo_subdir, + resolve_repo_subdir, unixify_path, }, }; @@ -107,11 +107,10 @@ impl SourceRecord { PinnedSourceSpec::Path(pinned_path) => { let path_str = pinned_path.path.as_str(); let native_path = Path::new(path_str); - let is_native_absolute = native_path.is_absolute(); let is_cross_platform_absolute = is_cross_platform_absolute(path_str, native_path); let is_within_workspace = - is_native_absolute && native_path.starts_with(workspace_root); + path_within_workspace(path_str, native_path, workspace_root); let should_relativize = !is_cross_platform_absolute || is_within_workspace; let relativized = if should_relativize { From 0910693eb7573c6ea88b636f9fc68cb848854d8f Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Sat, 8 Nov 2025 11:35:52 +0100 Subject: [PATCH 21/21] feat: added some doc comments --- crates/pixi_record/src/path_utils.rs | 5 +++-- crates/pixi_record/src/pinned_source.rs | 2 ++ crates/pixi_record/src/source_record.rs | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/pixi_record/src/path_utils.rs b/crates/pixi_record/src/path_utils.rs index a3dd70e693..a82a5d40da 100644 --- a/crates/pixi_record/src/path_utils.rs +++ b/crates/pixi_record/src/path_utils.rs @@ -50,7 +50,8 @@ pub(crate) fn normalize_path(path: &Path) -> PathBuf { normalized } -/// Compute the repo-relative path from `base` (manifest subdir) to `target` (build subdir). +/// Compute the repo-relative path from `base` (manifest subdir) to `target` (build subdir), always +/// returning `/`-separated strings so the lock format is stable across platforms. pub(crate) fn relative_repo_subdir(base: &str, target: &str) -> Option { let base_abs = repo_absolute_path(base); let target_abs = repo_absolute_path(target); @@ -64,7 +65,7 @@ pub(crate) fn relative_repo_subdir(base: &str, target: &str) -> Option { } } -/// Apply a manifest subdir back onto a relative path stored in the lock. +/// Apply a manifest subdir back onto a relative path stored in the lock, again emitting `/`. pub(crate) fn resolve_repo_subdir(base: &str, relative: Option<&str>) -> Option { match relative { Some(rel) => { diff --git a/crates/pixi_record/src/pinned_source.rs b/crates/pixi_record/src/pinned_source.rs index d83fb8f9b9..a88f0899a5 100644 --- a/crates/pixi_record/src/pinned_source.rs +++ b/crates/pixi_record/src/pinned_source.rs @@ -234,6 +234,7 @@ impl PinnedSourceSpec { let relative_path = pathdiff::diff_paths(this_path, base_path)?; Some(PinnedSourceSpec::Path(PinnedPathSpec { + // `pathdiff` yields native separators; convert to `/` for lock-file stability. path: Utf8TypedPathBuf::from(unixify_path(relative_path.as_path())), })) } @@ -262,6 +263,7 @@ impl PinnedSourceSpec { let this_path = std::path::Path::new(this_subdir); let relative = pathdiff::diff_paths(this_path, base_path)?; + // Same here: ensure lock only contains `/` even when diff runs on Windows paths. let relative_str = unixify_path(relative.as_path()); Some(PinnedSourceSpec::Path(PinnedPathSpec { diff --git a/crates/pixi_record/src/source_record.rs b/crates/pixi_record/src/source_record.rs index 687ac731be..cae64214d0 100644 --- a/crates/pixi_record/src/source_record.rs +++ b/crates/pixi_record/src/source_record.rs @@ -250,6 +250,7 @@ impl SourceRecord { .unwrap_or(build_absolute); PinnedSourceSpec::Path(crate::PinnedPathSpec { + // Lock format is always `/`-separated even if diff runs on Windows. path: Utf8TypedPathBuf::from(unixify_path(&relative_to_workspace)), }) }