diff --git a/Cargo.lock b/Cargo.lock index 34191580f8..36d8edaecd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5164,6 +5164,7 @@ dependencies = [ "serde", "serde_json", "thiserror 2.0.17", + "tracing", ] [[package]] diff --git a/crates/pixi_build_discovery/Cargo.toml b/crates/pixi_build_discovery/Cargo.toml index 94203e116c..3c1434332f 100644 --- a/crates/pixi_build_discovery/Cargo.toml +++ b/crates/pixi_build_discovery/Cargo.toml @@ -27,6 +27,7 @@ pixi_consts = { workspace = true } pixi_manifest = { workspace = true } pixi_spec = { workspace = true } pixi_spec_containers = { workspace = true } +tracing = { workspace = true } [dev-dependencies] insta = { workspace = true, features = [ diff --git a/crates/pixi_build_discovery/src/discovery.rs b/crates/pixi_build_discovery/src/discovery.rs index 8c86c220aa..55ae207d50 100644 --- a/crates/pixi_build_discovery/src/discovery.rs +++ b/crates/pixi_build_discovery/src/discovery.rs @@ -9,7 +9,9 @@ use ordermap::OrderMap; use pixi_build_type_conversions::{to_project_model_v1, to_target_selector_v1}; use pixi_build_types::{ProjectModelV1, TargetSelectorV1}; use pixi_config::Config; -use pixi_consts::consts::{RATTLER_BUILD_DIRS, RATTLER_BUILD_FILE_NAMES, ROS_BACKEND_FILE_NAMES}; +use pixi_consts::consts::{ + KNOWN_MANIFEST_FILES, RATTLER_BUILD_DIRS, RATTLER_BUILD_FILE_NAMES, ROS_BACKEND_FILE_NAMES, +}; use pixi_manifest::{ DiscoveryStart, ExplicitManifestError, PackageManifest, PrioritizedChannel, WithProvenance, WorkspaceDiscoverer, WorkspaceDiscoveryError, WorkspaceManifest, @@ -113,11 +115,9 @@ pub enum DiscoveryError { #[diagnostic(help("This is often caused by an internal error. Please report this issue."))] SpecConversionError(pixi_spec::SpecConversionError), - #[error("the source directory '{0}', does not contain a supported manifest")] - #[diagnostic(help( - "Ensure that the source directory contains a valid pixi.toml, pyproject.toml, recipe.yaml, package.xml or mojoproject.toml file." - ))] - FailedToDiscover(String), + #[error("the source directory '{path}', does not contain a supported manifest")] + #[diagnostic(help("{help}"))] + FailedToDiscover { path: String, help: String }, #[error("the manifest path '{0}', does not have a parent directory")] NoParentDir(PathBuf), @@ -159,7 +159,7 @@ impl DiscoveredBackend { } let source_dir = source_path .parent() - .expect("the recipe must live somewhere"); + .expect("the package.xml must live somewhere"); return Self::from_ros_package( source_dir.to_path_buf(), source_path, @@ -182,9 +182,27 @@ impl DiscoveredBackend { return Ok(pixi); } - Err(DiscoveryError::FailedToDiscover( - source_path.to_string_lossy().to_string(), - )) + // Test whether we can discover as a ROS package. + let help_msg = if enabled_protocols.enable_pixi + && ROS_BACKEND_FILE_NAMES + .iter() + .any(|&f| source_path.join(f).is_file()) + { + format!( + "ROS packages require you to specify the 'package.xml' file path directly, due to supporting more backends in the future. Or ensure that the source directory contains a valid manifest file: {}.", + KNOWN_MANIFEST_FILES.join(", ") + ) + } else { + format!( + "Ensure that the source directory contains a valid manifest file: {}.", + KNOWN_MANIFEST_FILES.join(", ") + ) + }; + + Err(DiscoveryError::FailedToDiscover { + path: source_path.to_string_lossy().to_string(), + help: help_msg, + }) } /// Construct a new instance based on a specific `recipe.yaml` file in the @@ -350,6 +368,11 @@ impl DiscoveredBackend { )?)); } } + tracing::trace!( + "No rattler-build manifests ({}) found in '{}'", + RATTLER_BUILD_FILE_NAMES.join(", "), + source_dir.display() + ); Ok(None) } diff --git a/crates/pixi_build_discovery/tests/discovery.rs b/crates/pixi_build_discovery/tests/discovery.rs index 40ed2daa4b..aa7116fb0c 100644 --- a/crates/pixi_build_discovery/tests/discovery.rs +++ b/crates/pixi_build_discovery/tests/discovery.rs @@ -122,3 +122,39 @@ fn test_direct_package_xml() { assert_discover_snapshot ! ( & path); }); } + +#[test] +fn test_ros_discovery_priority() { + // Test that ROS packages are discovered after rattler-build recipes + // Create a test case with both package.xml and recipe.yaml + let path = dunce::canonicalize(discovery_directory().join("ros-priority")).unwrap(); + let file_path = Path::new(file!()).parent().unwrap(); + let channel_config = ChannelConfig::default_with_root_dir(file_path.to_owned()); + + match DiscoveredBackend::discover(&path, &channel_config, &EnabledProtocols::default()) { + Ok(backend) => { + // Should discover rattler-build backend, not ROS + insta::with_settings!({ + filters => vec![ + (path.to_string_lossy().replace(r"\", r"\\").as_str(), "file://"), + (r"\\", r"/"), + ], + }, { + assert_yaml_snapshot!(backend, { + "[\"init-params\"][\"manifest-path\"]" => insta::dynamic_redaction(|value, _path| { + redact_path(value.as_str().unwrap()) + }), + "[\"init-params\"][\"workspace-root\"]" => insta::dynamic_redaction(|value, _path| { + redact_path(value.as_str().unwrap()) + }), + "[\"init-params\"][\"source-anchor\"]" => insta::dynamic_redaction(|value, _path| { + redact_path(value.as_str().unwrap()) + }), + }); + }); + } + Err(err) => { + assert_snapshot!(pixi_test_utils::format_diagnostic(&err)); + } + } +} diff --git a/crates/pixi_build_discovery/tests/snapshots/discovery__discovery@ros-package.snap b/crates/pixi_build_discovery/tests/snapshots/discovery__discovery@ros-package.snap index 1825210dd1..044a4d71ee 100644 --- a/crates/pixi_build_discovery/tests/snapshots/discovery__discovery@ros-package.snap +++ b/crates/pixi_build_discovery/tests/snapshots/discovery__discovery@ros-package.snap @@ -1,35 +1,8 @@ --- source: crates/pixi_build_discovery/tests/discovery.rs -expression: backend +expression: "pixi_test_utils :: format_diagnostic(& err)" input_file: tests/data/discovery/ros-package/TEST-CASE --- -backend-spec: - type: json-rpc - name: pixi-build-ros - command: - type: environment-spec - requirement: - - pixi-build-ros - - "*" - channels: - - "https://conda.anaconda.org/conda-forge" - - "https://prefix.dev/pixi-build-backends" -init-params: - workspace-root: "file:///ros-package" - source: ~ - source-anchor: "file:///ros-package" - manifest-path: "file:///ros-package/package.xml" - project-model: - name: ~ - version: ~ - description: ~ - authors: ~ - license: ~ - licenseFile: ~ - readme: ~ - homepage: ~ - repository: ~ - documentation: ~ - targets: ~ - configuration: ~ - target-configuration: ~ + × the source directory '/tests/data/discovery/ros-package', does not contain a supported manifest + help: ROS packages require you to specify the 'package.xml' file path directly, due to supporting more backends in the future. Or ensure that the source directory contains a valid manifest file: + pixi.toml, pyproject.toml, mojoproject.toml, recipe.yaml, recipe.yml. diff --git a/crates/pixi_build_discovery/tests/snapshots/discovery__ros_discovery_priority.snap b/crates/pixi_build_discovery/tests/snapshots/discovery__ros_discovery_priority.snap new file mode 100644 index 0000000000..b0eb6adbd3 --- /dev/null +++ b/crates/pixi_build_discovery/tests/snapshots/discovery__ros_discovery_priority.snap @@ -0,0 +1,22 @@ +--- +source: crates/pixi_build_discovery/tests/discovery.rs +expression: backend +--- +backend-spec: + type: json-rpc + name: pixi-build-rattler-build + command: + type: environment-spec + requirement: + - pixi-build-rattler-build + - "*" + channels: + - "https://prefix.dev/conda-forge" +init-params: + workspace-root: "file:///ros-priority" + build-source: ~ + source-anchor: "file:///ros-priority" + manifest-path: "file:///ros-priority/recipe.yaml" + project-model: ~ + configuration: ~ + target-configuration: ~ diff --git a/crates/pixi_consts/src/consts.rs b/crates/pixi_consts/src/consts.rs index aa15366341..5888af389a 100644 --- a/crates/pixi_consts/src/consts.rs +++ b/crates/pixi_consts/src/consts.rs @@ -112,10 +112,14 @@ pub const RATTLER_BUILD_FILE_NAMES: [&str; 2] = ["recipe.yaml", "recipe.yml"]; pub const RATTLER_BUILD_DIRS: [&str; 2] = ["", "recipe"]; pub const ROS_BACKEND_FILE_NAMES: [&str; 1] = ["package.xml"]; -/// Known manifest file names that indicate the path points to a file rather than a directory. -pub const KNOWN_MANIFEST_FILES: &[&str] = - &[WORKSPACE_MANIFEST, PYPROJECT_MANIFEST, MOJOPROJECT_MANIFEST]; - +pub static KNOWN_MANIFEST_FILES: LazyLock> = LazyLock::new(|| { + let mut v = Vec::new(); + v.push(WORKSPACE_MANIFEST); + v.push(PYPROJECT_MANIFEST); + v.push(MOJOPROJECT_MANIFEST); + v.extend(RATTLER_BUILD_FILE_NAMES); + v +}); pub static TASK_STYLE: LazyLock