From 4b3b96b6f7fc95b7bc5ed1a484372515ba85463d Mon Sep 17 00:00:00 2001 From: nichmor Date: Fri, 31 Oct 2025 16:16:30 +0200 Subject: [PATCH 1/4] fix: solve build source relative to the manifest path --- .../src/build_backend_metadata/mod.rs | 5 ++++- .../src/command_dispatcher/mod.rs | 21 ++++++++++++++----- .../solve_pixi/source_metadata_collector.rs | 2 +- .../src/source_build/mod.rs | 11 +++++++++- crates/pixi_global/src/project/mod.rs | 2 +- 5 files changed, 32 insertions(+), 9 deletions(-) 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 88e5224893..0d0735a88a 100644 --- a/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs +++ b/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs @@ -138,7 +138,10 @@ impl BuildBackendMetadataSpec { } else if let Some(build_source) = &discovered_backend.init_params.build_source { Some( command_dispatcher - .pin_and_checkout(build_source.clone()) + .pin_and_checkout( + build_source.clone(), + Some(&discovered_backend.init_params.manifest_path), + ) .await .map_err_with(BuildBackendMetadataError::SourceCheckout)?, ) diff --git a/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs b/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs index 5523a8196b..863c3709af 100644 --- a/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs +++ b/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs @@ -565,7 +565,10 @@ impl CommandDispatcher { /// /// This function resolves the source specification to a concrete checkout /// by: - /// 1. For path sources: Resolving relative paths against the root directory + /// 1. For path sources: Resolving relative paths against the root directory or against an alternative root path + /// + /// i.e. in the case of an out-of-tree build. + /// /// 2. For git sources: Cloning or fetching the repository and checking out /// the specified reference /// 3. For URL sources: Downloading and extracting the archive (currently @@ -578,6 +581,7 @@ impl CommandDispatcher { pub async fn pin_and_checkout( &self, source_location_spec: SourceLocationSpec, + alternative_root_path: Option<&Path>, ) -> Result> { match source_location_spec { SourceLocationSpec::Url(url) => { @@ -586,7 +590,7 @@ impl CommandDispatcher { SourceLocationSpec::Path(path) => { let source_path = self .data - .resolve_typed_path(path.path.to_path()) + .resolve_typed_path(path.path.to_path(), alternative_root_path) .map_err(SourceCheckoutError::from) .map_err(CommandDispatcherError::Failed)?; Ok(SourceCheckout { @@ -618,7 +622,7 @@ impl CommandDispatcher { PinnedSourceSpec::Path(ref path) => { let source_path = self .data - .resolve_typed_path(path.path.to_path()) + .resolve_typed_path(path.path.to_path(), None) .map_err(SourceCheckoutError::from) .map_err(CommandDispatcherError::Failed)?; Ok(SourceCheckout { @@ -653,7 +657,11 @@ 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) -> Result { + fn resolve_typed_path( + &self, + path_spec: Utf8TypedPath, + alternative_root_path: Option<&Path>, + ) -> 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("~/") { @@ -663,7 +671,10 @@ impl CommandDispatcherData { debug_assert!(home_dir.is_absolute()); normalize_absolute_path(&home_dir.join(Path::new(user_path.as_str()))) } else { - let root_dir = self.root_dir.as_path(); + let root_dir = match alternative_root_path { + Some(root_path) => root_path, + None => 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/solve_pixi/source_metadata_collector.rs b/crates/pixi_command_dispatcher/src/solve_pixi/source_metadata_collector.rs index d4d4faf33d..9438ad1b8d 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 @@ -171,7 +171,7 @@ impl SourceMetadataCollector { // will pick build_source; we only override the build pin later. let source = self .command_queue - .pin_and_checkout(spec.location) + .pin_and_checkout(spec.location, None) .await .map_err(|err| CollectSourceMetadataError::SourceCheckoutError { name: name.as_source().to_string(), diff --git a/crates/pixi_command_dispatcher/src/source_build/mod.rs b/crates/pixi_command_dispatcher/src/source_build/mod.rs index 7914bf8f3f..7e94d6a668 100644 --- a/crates/pixi_command_dispatcher/src/source_build/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_build/mod.rs @@ -267,7 +267,16 @@ impl SourceBuildSpec { discovered_backend.init_params.build_source.clone() { let build_source_checkout = command_dispatcher - .pin_and_checkout(manifest_build_source) + .pin_and_checkout( + manifest_build_source, + Some( + discovered_backend + .init_params + .manifest_path + .parent() + .expect("manifest path should have a parent directory"), + ), + ) .await .map_err_with(SourceBuildError::SourceCheckout)?; build_source_checkout.path diff --git a/crates/pixi_global/src/project/mod.rs b/crates/pixi_global/src/project/mod.rs index a39953ff6b..7dd78704bd 100644 --- a/crates/pixi_global/src/project/mod.rs +++ b/crates/pixi_global/src/project/mod.rs @@ -1372,7 +1372,7 @@ impl Project { ) -> Result { let command_dispatcher = self.command_dispatcher()?; let checkout = command_dispatcher - .pin_and_checkout(source_spec.location) + .pin_and_checkout(source_spec.location, None) .await .map_err(|e| InferPackageNameError::BuildBackendMetadata(Box::new(e)))?; From c8499f8370814ab215209360cf4a5322ac735ca0 Mon Sep 17 00:00:00 2001 From: nichmor Date: Fri, 31 Oct 2025 16:44:26 +0200 Subject: [PATCH 2/4] misc: change comment and error type --- .../src/command_dispatcher/mod.rs | 16 ++++++++++++---- .../src/source_build/mod.rs | 8 +++++++- .../src/source_checkout/mod.rs | 3 +++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs b/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs index 863c3709af..b8c3cdb8b5 100644 --- a/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs +++ b/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs @@ -568,6 +568,14 @@ impl CommandDispatcher { /// 1. For path sources: Resolving relative paths against the root directory or against an alternative root path /// /// i.e. in the case of an out-of-tree build. + /// Some examples for different inputs + /// /foo/bar => foo/bar + /// ./bar => /bar + /// ../bar.toml => /../bar.toml + /// + /// Usually: + /// * root_dir => workspace manifest path (set during the command dispatcher construction) + /// * alternative_root => package manifest path (set during builds of specific packages) /// /// 2. For git sources: Cloning or fetching the repository and checking out /// the specified reference @@ -581,7 +589,7 @@ impl CommandDispatcher { pub async fn pin_and_checkout( &self, source_location_spec: SourceLocationSpec, - alternative_root_path: Option<&Path>, + alternative_root: Option<&Path>, ) -> Result> { match source_location_spec { SourceLocationSpec::Url(url) => { @@ -590,7 +598,7 @@ impl CommandDispatcher { SourceLocationSpec::Path(path) => { let source_path = self .data - .resolve_typed_path(path.path.to_path(), alternative_root_path) + .resolve_typed_path(path.path.to_path(), alternative_root) .map_err(SourceCheckoutError::from) .map_err(CommandDispatcherError::Failed)?; Ok(SourceCheckout { @@ -660,7 +668,7 @@ impl CommandDispatcherData { fn resolve_typed_path( &self, path_spec: Utf8TypedPath, - alternative_root_path: Option<&Path>, + alternative_root: Option<&Path>, ) -> Result { if path_spec.is_absolute() { Ok(Path::new(path_spec.as_str()).to_path_buf()) @@ -671,7 +679,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_path { + let root_dir = match alternative_root { Some(root_path) => root_path, None => self.root_dir.as_path(), }; diff --git a/crates/pixi_command_dispatcher/src/source_build/mod.rs b/crates/pixi_command_dispatcher/src/source_build/mod.rs index 7e94d6a668..5fdd10d817 100644 --- a/crates/pixi_command_dispatcher/src/source_build/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_build/mod.rs @@ -274,7 +274,13 @@ impl SourceBuildSpec { .init_params .manifest_path .parent() - .expect("manifest path should have a parent directory"), + .ok_or_else(|| { + SourceCheckoutError::ParentDir( + discovered_backend.init_params.manifest_path.clone(), + ) + }) + .map_err(SourceBuildError::SourceCheckout) + .map_err(CommandDispatcherError::Failed)?, ), ) .await diff --git a/crates/pixi_command_dispatcher/src/source_checkout/mod.rs b/crates/pixi_command_dispatcher/src/source_checkout/mod.rs index d3b518b5ca..484b375622 100644 --- a/crates/pixi_command_dispatcher/src/source_checkout/mod.rs +++ b/crates/pixi_command_dispatcher/src/source_checkout/mod.rs @@ -32,6 +32,9 @@ pub enum SourceCheckoutError { #[error(transparent)] GitError(#[from] GitError), + + #[error("the manifest path {0} should have a parent directory")] + ParentDir(PathBuf), } #[derive(Debug, Error)] From 31cfa20616a755a5c0bddaa971ddf4c12395e68a Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 31 Oct 2025 16:46:01 +0100 Subject: [PATCH 3/4] feat: fix another build failure --- .../src/build_backend_metadata/mod.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) 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 0d0735a88a..0272e06fa2 100644 --- a/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs +++ b/crates/pixi_command_dispatcher/src/build_backend_metadata/mod.rs @@ -140,7 +140,19 @@ impl BuildBackendMetadataSpec { command_dispatcher .pin_and_checkout( build_source.clone(), - Some(&discovered_backend.init_params.manifest_path), + 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)?, From 1d390ee5baa4e05fa23d6ff1120411d3555d02e5 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Fri, 31 Oct 2025 20:01:52 +0100 Subject: [PATCH 4/4] fix: use debug_assert, update docs and added test --- .../tests/integration_rust/build_tests.rs | 91 +++++++++++++++++++ .../src/command_dispatcher/mod.rs | 26 ++++-- 2 files changed, 110 insertions(+), 7 deletions(-) diff --git a/crates/pixi/tests/integration_rust/build_tests.rs b/crates/pixi/tests/integration_rust/build_tests.rs index c028d434cf..f511c2af05 100644 --- a/crates/pixi/tests/integration_rust/build_tests.rs +++ b/crates/pixi/tests/integration_rust/build_tests.rs @@ -340,3 +340,94 @@ my-package = {{ path = "./my-package" }} "my-package", )); } + +/// Test that verifies [package.build] source.path is resolved relative to the +/// package manifest directory, not the workspace root. +/// +/// This tests the fix for out-of-tree builds where a package manifest +/// specifies `source.path = "src"` and expects it to be resolved relative +/// to the package manifest's parent directory. +#[tokio::test] +async fn test_package_build_source_relative_to_manifest() { + setup_tracing(); + + // Create a PixiControl instance with PassthroughBackend + let backend_override = BackendOverride::from_memory(PassthroughBackend::instantiator()); + let pixi = PixiControl::new() + .unwrap() + .with_backend_override(backend_override); + + // Create the package structure: + // workspace/ + // pixi.toml (workspace and package manifest) + // src/ (source directory - should be found relative to package manifest) + // pixi.toml (build source manifest) + + let package_source_dir = pixi.workspace_path().join("src"); + fs::create_dir_all(&package_source_dir).unwrap(); + + // Create a pixi.toml in the source directory that PassthroughBackend will read + let source_pixi_toml = r#" +[package] +name = "test-build-source" +version = "0.1.0" + +[package.build] +backend = { name = "in-memory", version = "0.1.0" } +"#; + fs::write(package_source_dir.join("pixi.toml"), source_pixi_toml).unwrap(); + + // Create a manifest where the package has [package.build] with source.path + // The source.path should be resolved relative to the package manifest directory + let manifest_content = format!( + r#" +[workspace] +channels = [] +platforms = ["{}"] +preview = ["pixi-build"] + +[package] +name = "test-build-source" +version = "0.1.0" +description = "Test package for build source path resolution" + +[package.build] +backend = {{ name = "in-memory", version = "0.1.0" }} +# This should resolve to /src, not /src +source.path = "src" + +[dependencies] +test-build-source = {{ path = "." }} +"#, + Platform::current(), + ); + + // Write the manifest + fs::write(pixi.manifest_path(), manifest_content).unwrap(); + + // Actually trigger the build process to test the bug + // This will call build_backend_metadata which uses alternative_root + let result = pixi.update_lock_file().await; + + // The test should succeed if the source path is resolved correctly + // If the bug exists (manifest_path instead of manifest_path.parent()), + // the build will fail because it will try to find src relative to pixi.toml (a file) + // instead of relative to the directory containing pixi.toml + assert!( + result.is_ok(), + "Lock file update should succeed when source.path is resolved correctly. Error: {:?}", + result.err() + ); + + let lock_file = result.unwrap(); + + // Verify the package was built and is in the lock file + assert!( + lock_file.contains_conda_package( + consts::DEFAULT_ENVIRONMENT_NAME, + Platform::current(), + "test-build-source", + ), + "Built package should be in the lock file" + ); +} diff --git a/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs b/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs index b8c3cdb8b5..72962739c5 100644 --- a/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs +++ b/crates/pixi_command_dispatcher/src/command_dispatcher/mod.rs @@ -568,14 +568,16 @@ impl CommandDispatcher { /// 1. For path sources: Resolving relative paths against the root directory or against an alternative root path /// /// i.e. in the case of an out-of-tree build. - /// Some examples for different inputs - /// /foo/bar => foo/bar - /// ./bar => /bar - /// ../bar.toml => /../bar.toml + /// Some examples for different inputs: + /// - `/foo/bar` => `/foo/bar` (absolute paths are unchanged) + /// - `./bar` => `/bar` + /// - `bar` => `/bar` (or `/bar` if provided) + /// - `../bar` => `/../bar` (normalized, validated for security) + /// - `~/bar` => `/bar` /// /// Usually: - /// * root_dir => workspace manifest path (set during the command dispatcher construction) - /// * alternative_root => package manifest path (set during builds of specific packages) + /// * `root_dir` => workspace root directory (parent of workspace manifest) + /// * `alternative_root` => package root directory (parent of package manifest) /// /// 2. For git sources: Cloning or fetching the repository and checking out /// the specified reference @@ -680,7 +682,17 @@ impl CommandDispatcherData { normalize_absolute_path(&home_dir.join(Path::new(user_path.as_str()))) } else { let root_dir = match alternative_root { - Some(root_path) => root_path, + 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 native_path = Path::new(path_spec.as_str());