From a884600133a06d7c541fd2d0f15a5a8358e905d2 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Mon, 18 Nov 2024 16:12:21 +0100 Subject: [PATCH 1/6] feat: extends manifest to allow for preview features (#2489) --- Cargo.lock | 20 ++ Cargo.toml | 1 + crates/pixi_manifest/Cargo.toml | 3 +- crates/pixi_manifest/src/lib.rs | 2 + .../pixi_manifest/src/manifests/manifest.rs | 6 + crates/pixi_manifest/src/metadata.rs | 4 + crates/pixi_manifest/src/preview.rs | 203 ++++++++++++++++++ crates/pixi_manifest/src/validation.rs | 16 ++ docs/reference/project_configuration.md | 16 ++ schema/examples/valid/full.toml | 1 + schema/model.py | 7 + schema/schema.json | 24 +++ src/project/mod.rs | 20 +- 13 files changed, 320 insertions(+), 3 deletions(-) create mode 100644 crates/pixi_manifest/src/preview.rs diff --git a/Cargo.lock b/Cargo.lock index 091de2d4a..3fcc33667 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2915,6 +2915,15 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" +[[package]] +name = "ordered-float" +version = "2.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68f19d67e5a2795c94e73e0bb1cc1a7edeb2e28efd39e2e1c9b7a40c1108b11c" +dependencies = [ + "num-traits", +] + [[package]] name = "ordered-stream" version = "0.2.0" @@ -3386,6 +3395,7 @@ dependencies = [ "rstest", "serde", "serde-untagged", + "serde-value", "serde_json", "serde_with", "spdx", @@ -4849,6 +4859,16 @@ dependencies = [ "typeid", ] +[[package]] +name = "serde-value" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3a1a3341211875ef120e117ea7fd5228530ae7e7036a779fdc9117be6b3282c" +dependencies = [ + "ordered-float", + "serde", +] + [[package]] name = "serde_derive" version = "1.0.214" diff --git a/Cargo.toml b/Cargo.toml index 6bcdc51d8..f8f5d95e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,6 +67,7 @@ rstest = "0.19.0" self-replace = "1.3.7" serde = "1.0.198" serde-untagged = "0.1.5" +serde-value = "0.7.0" serde_ignored = "0.1.10" serde_json = "1.0.116" serde_with = "3.7.0" diff --git a/crates/pixi_manifest/Cargo.toml b/crates/pixi_manifest/Cargo.toml index a8cdcfa07..9abf8d73e 100644 --- a/crates/pixi_manifest/Cargo.toml +++ b/crates/pixi_manifest/Cargo.toml @@ -20,6 +20,7 @@ pixi_spec = { workspace = true } regex = { workspace = true } serde = { workspace = true } serde-untagged = { workspace = true } +serde-value = { workspace = true } serde_with = { workspace = true } spdx = { workspace = true } strsim = { workspace = true } @@ -47,5 +48,5 @@ fancy_display = { workspace = true } glob = "0.3.1" insta = { workspace = true, features = ["yaml"] } rstest = { workspace = true } -serde_json = { workspace = true } +serde_json = { workspace = true, features = ["preserve_order"] } tempfile = { workspace = true } diff --git a/crates/pixi_manifest/src/lib.rs b/crates/pixi_manifest/src/lib.rs index fbdabfe57..967558b4c 100644 --- a/crates/pixi_manifest/src/lib.rs +++ b/crates/pixi_manifest/src/lib.rs @@ -11,6 +11,7 @@ mod has_manifest_ref; mod manifests; mod metadata; mod parsed_manifest; +mod preview; pub mod pypi; pub mod pyproject; mod solve_group; @@ -48,6 +49,7 @@ use thiserror::Error; pub use features_ext::FeaturesExt; pub use has_features_iter::HasFeaturesIter; pub use has_manifest_ref::HasManifestRef; +pub use preview::{KnownPreviewFeature, Preview, PreviewFeature}; /// Errors that can occur when getting a feature. #[derive(Debug, Clone, Error, Diagnostic)] diff --git a/crates/pixi_manifest/src/manifests/manifest.rs b/crates/pixi_manifest/src/manifests/manifest.rs index c2a186668..b1dea4787 100644 --- a/crates/pixi_manifest/src/manifests/manifest.rs +++ b/crates/pixi_manifest/src/manifests/manifest.rs @@ -19,6 +19,7 @@ use crate::{ consts, error::{DependencyError, TomlError, UnknownFeature}, manifests::{ManifestSource, TomlManifest}, + preview::Preview, pypi::PyPiPackageName, pyproject::PyProjectManifest, to_options, DependencyOverwriteBehavior, Environment, EnvironmentName, Feature, FeatureName, @@ -722,6 +723,11 @@ impl Manifest { { self.parsed.environments.find(name) } + + /// Returns the preview field of the project + pub fn preview(&self) -> Option<&Preview> { + self.parsed.project.preview.as_ref() + } } #[cfg(test)] diff --git a/crates/pixi_manifest/src/metadata.rs b/crates/pixi_manifest/src/metadata.rs index 2a1dd80ce..ddf1bd7c1 100644 --- a/crates/pixi_manifest/src/metadata.rs +++ b/crates/pixi_manifest/src/metadata.rs @@ -8,6 +8,7 @@ use serde_with::{serde_as, DisplayFromStr}; use url::Url; use super::pypi::pypi_options::PypiOptions; +use crate::preview::Preview; use crate::utils::PixiSpanned; /// Describes the contents of the `[package]` section of the project manifest. @@ -64,4 +65,7 @@ pub struct ProjectMetadata { /// The pypi options supported in the project pub pypi_options: Option, + + /// Preview features + pub preview: Option, } diff --git a/crates/pixi_manifest/src/preview.rs b/crates/pixi_manifest/src/preview.rs new file mode 100644 index 000000000..c7bb190d1 --- /dev/null +++ b/crates/pixi_manifest/src/preview.rs @@ -0,0 +1,203 @@ +//! This module contains the ability to parse the preview features of the project +//! +//! e.g. +//! ```toml +//! [project] +//! # .. other project metadata +//! preview = ["new-resolve"] +//! ``` +//! +//! Features are split into Known and Unknown features. Basically you can use any string as a feature +//! but only the features defined in [`KnownFeature`] can be used. +//! We do this for backwards compatibility with the old features that may have been used in the past. +//! The [`KnownFeature`] enum contains all the known features. Extend this if you want to add support +//! for new features. +use serde::{Deserialize, Deserializer, Serialize}; + +#[derive(Debug, Serialize, Clone, PartialEq)] +#[serde(untagged)] +/// The preview features of the project +pub enum Preview { + /// All preview features are enabled + AllEnabled(bool), // For `preview = true` + /// Specific preview features are enabled + Features(Vec), // For `preview = ["feature"]` +} + +impl Preview { + /// Returns true if all preview features are enabled + pub fn all_enabled(&self) -> bool { + match self { + Preview::AllEnabled(enabled) => *enabled, + Preview::Features(_) => false, + } + } + + /// Returns true if the given preview feature is enabled + pub fn is_enabled(&self, feature: KnownPreviewFeature) -> bool { + match self { + Preview::AllEnabled(_) => true, + Preview::Features(features) => features.iter().any(|f| *f == feature), + } + } + + /// Return all unknown preview features + pub fn unknown_preview_features(&self) -> Vec<&str> { + match self { + Preview::AllEnabled(_) => vec![], + Preview::Features(features) => features + .iter() + .filter_map(|feature| match feature { + PreviewFeature::Unknown(feature) => Some(feature.as_str()), + _ => None, + }) + .collect(), + } + } +} + +impl<'de> Deserialize<'de> for Preview { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + serde_untagged::UntaggedEnumVisitor::new() + .bool(|bool| Ok(Preview::AllEnabled(bool))) + .seq(|seq| Ok(Preview::Features(seq.deserialize()?))) + .expecting("bool or list of features e.g `true` or `[\"new-resolve\"]`") + .deserialize(deserializer) + } +} + +#[derive(Debug, Serialize, Clone, PartialEq)] +#[serde(untagged)] +/// A preview feature, can be either a known feature or an unknown feature +pub enum PreviewFeature { + /// This is a known preview feature + Known(KnownPreviewFeature), + /// Unknown preview feature + Unknown(String), +} + +impl PartialEq for PreviewFeature { + fn eq(&self, other: &KnownPreviewFeature) -> bool { + match self { + PreviewFeature::Known(feature) => feature == other, + _ => false, + } + } +} + +#[derive(Debug, Serialize, Deserialize, Clone, Copy, PartialEq)] +#[serde(rename_all = "kebab-case")] +/// Currently supported preview features are listed here +pub enum KnownPreviewFeature { + // Add known features here +} + +impl<'de> Deserialize<'de> for PreviewFeature { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let value = serde_value::Value::deserialize(deserializer)?; + let known = KnownPreviewFeature::deserialize(value.clone()).map(PreviewFeature::Known); + if let Ok(feature) = known { + Ok(feature) + } else { + let unknown = String::deserialize(value) + .map(PreviewFeature::Unknown) + .map_err(serde::de::Error::custom)?; + Ok(unknown) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use insta::assert_snapshot; + use toml_edit::{de::from_str, ser::to_string}; + + /// Fake table to test the `Preview` enum + #[derive(Debug, Serialize, Deserialize)] + struct TopLevel { + preview: Preview, + } + + #[test] + fn test_preview_all_enabled() { + let input = "preview = true"; + let top: TopLevel = from_str(input).expect("should parse as `AllEnabled`"); + assert_eq!(top.preview, Preview::AllEnabled(true)); + + let output = to_string(&top).expect("should serialize back to TOML"); + assert_eq!(output.trim(), input); + } + + #[test] + fn test_preview_with_unknown_feature() { + let input = r#"preview = ["build"]"#; + let top: TopLevel = from_str(input).expect("should parse as `Features` with known feature"); + assert_eq!( + top.preview, + Preview::Features(vec![PreviewFeature::Unknown("build".to_string())]) + ); + + let output = to_string(&top).expect("should serialize back to TOML"); + assert_eq!(output.trim(), input); + } + + #[test] + fn test_insta_error_invalid_bool() { + let input = r#"preview = "not-a-bool""#; + let result: Result = from_str(input); + + assert!(result.is_err()); + assert_snapshot!( + format!("{:?}", result.unwrap_err()), + @r###"Error { inner: TomlError { message: "invalid type: map, expected bool or list of features e.g `true` or `[\"new-resolve\"]`", raw: Some("preview = \"not-a-bool\""), keys: [], span: Some(0..22) } }"### + ); + } + + #[test] + fn test_insta_error_invalid_list_item() { + let input = r#"preview = ["build", 123]"#; + let result: Result = from_str(input); + + assert!(result.is_err()); + assert_snapshot!( + format!("{:?}", result.unwrap_err()), + @r###"Error { inner: TomlError { message: "Invalid type integer `123`. Expected a string\n", raw: Some("preview = [\"build\", 123]"), keys: ["preview"], span: Some(10..24) } }"### + ); + } + + #[test] + fn test_insta_error_invalid_top_level_type() { + let input = r#"preview = 123"#; + let result: Result = from_str(input); + + assert!(result.is_err()); + assert_snapshot!( + format!("{:?}", result.unwrap_err()), + @r###"Error { inner: TomlError { message: "invalid type: integer `123`, expected bool or list of features e.g `true` or `[\"new-resolve\"]`", raw: Some("preview = 123"), keys: ["preview"], span: Some(10..13) } }"### + ); + } + + #[test] + fn test_feature_is_unknown() { + let input = r#"preview = ["new_parsing"]"#; + let top: TopLevel = from_str(input).unwrap(); + match top.preview { + Preview::AllEnabled(_) => unreachable!("this arm should not be used"), + Preview::Features(vec) => { + assert_matches::assert_matches!( + &vec[0], + PreviewFeature::Unknown(s) => { + s == &"new_parsing".to_string() + } + ); + } + } + } +} diff --git a/crates/pixi_manifest/src/validation.rs b/crates/pixi_manifest/src/validation.rs index 309f304bb..c34a38cc1 100644 --- a/crates/pixi_manifest/src/validation.rs +++ b/crates/pixi_manifest/src/validation.rs @@ -132,6 +132,22 @@ impl ParsedManifest { } } + // Warn on any unknown preview features + if let Some(preview) = self.project.preview.as_ref() { + let preview = preview.unknown_preview_features(); + if !preview.is_empty() { + let are = if preview.len() > 1 { "are" } else { "is" }; + let s = if preview.len() > 1 { "s" } else { "" }; + let preview_array = if preview.len() == 1 { + format!("{:?}", preview) + } else { + format!("[{:?}]", preview.iter().format(", ")) + }; + tracing::warn!( + "The preview feature{s}: {preview_array} {are} defined in the manifest but un-used pixi"); + } + } + Ok(()) } diff --git a/docs/reference/project_configuration.md b/docs/reference/project_configuration.md index 8f107fe5e..9897e4bec 100644 --- a/docs/reference/project_configuration.md +++ b/docs/reference/project_configuration.md @@ -769,6 +769,22 @@ When an environment comprises several features (including the default feature): - The `channels` of the environment is the union of the `channels` of all its features. Channel priorities can be specified in each feature, to ensure channels are considered in the right order in the environment. - The `platforms` of the environment is the intersection of the `platforms` of all its features. Be aware that the platforms supported by a feature (including the default feature) will be considered as the `platforms` defined at project level (unless overridden in the feature). This means that it is usually a good idea to set the project `platforms` to all platforms it can support across its environments. +## Preview features +Pixi sometimes introduces new features that are not yet stable, but that we would like for users to test out. These features are called preview features. Preview features are disabled by default and can be enabled by setting the `preview` field in the project manifest. The preview field is an array of strings that specify the preview features to enable, or the boolean value `true` to enable all preview features. + +An example of a preview feature in the project manifest: + +```toml title="Example preview features in the project manifest" +[project] +name = "foo" +channels = [] +platforms = [] +preview = ["new-resolve"] +``` + +Preview features in the documentation will be marked as such on the relevant pages. + + ## Global configuration The global configuration options are documented in the [global configuration](../reference/pixi_configuration.md) section. diff --git a/schema/examples/valid/full.toml b/schema/examples/valid/full.toml index 515b953e9..6ad5e0c4c 100644 --- a/schema/examples/valid/full.toml +++ b/schema/examples/valid/full.toml @@ -12,6 +12,7 @@ license = "MIT" license-file = "LICENSE" name = "project" platforms = ["linux-64", "win-64", "osx-64", "osx-arm64"] +preview = ["new-resolve"] readme = "README.md" repository = "https://github.com/author/project" version = "0.1.0" diff --git a/schema/model.py b/schema/model.py index 2b0062703..5d5dc9586 100644 --- a/schema/model.py +++ b/schema/model.py @@ -88,6 +88,10 @@ class ChannelPriority(str, Enum): strict = "strict" +class KnownPreviewFeature(str, Enum): + """The preview features of the project.""" + + class Project(StrictBaseModel): """The project's metadata information.""" @@ -139,6 +143,9 @@ class Project(StrictBaseModel): pypi_options: PyPIOptions | None = Field( None, alias="pypi-options", description="Options related to PyPI indexes for this project" ) + preview: list[KnownPreviewFeature | str] | bool | None = Field( + None, alias="preview", description="Defines the enabling of preview features of the project" + ) ######################## diff --git a/schema/schema.json b/schema/schema.json index ec81a102b..5016438c2 100644 --- a/schema/schema.json +++ b/schema/schema.json @@ -773,6 +773,30 @@ "$ref": "#/$defs/Platform" } }, + "preview": { + "title": "Preview", + "description": "Defines the enabling of preview features of the project", + "anyOf": [ + { + "type": "array", + "items": { + "anyOf": [ + { + "title": "KnownPreviewFeature", + "description": "The preview features of the project.", + "enum": [] + }, + { + "type": "string" + } + ] + } + }, + { + "type": "boolean" + } + ] + }, "pypi-options": { "$ref": "#/$defs/PyPIOptions", "description": "Options related to PyPI indexes for this project" diff --git a/src/project/mod.rs b/src/project/mod.rs index 30d8e24da..d79167e77 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -32,8 +32,8 @@ use pixi_config::{Config, PinningStrategy}; use pixi_consts::consts; use pixi_manifest::{ pypi::PyPiPackageName, DependencyOverwriteBehavior, EnvironmentName, Environments, FeatureName, - FeaturesExt, HasFeaturesIter, HasManifestRef, Manifest, ParsedManifest, PypiDependencyLocation, - SpecType, + FeaturesExt, HasFeaturesIter, HasManifestRef, KnownPreviewFeature, Manifest, ParsedManifest, + PypiDependencyLocation, SpecType, }; use pixi_utils::reqwest::build_reqwest_clients; use pypi_mapping::{ChannelName, CustomMapping, MappingLocation, MappingSource}; @@ -962,6 +962,22 @@ impl Project { Ok(implicit_constraints) } + + /// Returns true if all preview features are enabled + pub fn all_preview_features_enabled(&self) -> bool { + self.manifest + .preview() + .map(|preview| preview.all_enabled()) + .unwrap_or(false) + } + + /// Returns true if the given preview feature is enabled + pub fn is_preview_feature_enabled(&self, feature: KnownPreviewFeature) -> bool { + self.manifest + .preview() + .map(|preview| preview.is_enabled(feature)) + .unwrap_or(false) + } } pub struct UpdateDeps { From 4c1cb86de7b812f9b0f45c81125a837fc76d9f16 Mon Sep 17 00:00:00 2001 From: Hofer-Julian <30049909+Hofer-Julian@users.noreply.github.com> Date: Mon, 18 Nov 2024 17:54:57 +0100 Subject: [PATCH 2/6] ci: only update one channel when updating test channel (#2502) --- pixi.toml | 4 +-- tests/data/channels/update-channels.py | 45 ++++++++++++++------------ 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/pixi.toml b/pixi.toml index 36dc88961..5af7d42bb 100644 --- a/pixi.toml +++ b/pixi.toml @@ -59,8 +59,8 @@ test-integration-slow = { cmd = "pytest --numprocesses=auto --durations=10 tests # you can also pass a specific test function, like this: # /path/to/test.py::test_function test-specific-test = { cmd = "pytest", depends-on = ["build-release"] } -# Update one test channel by passing on key of `mappings.toml` -# e.g. "trampoline/trampoline_2.yaml" +# Update one test channel by passing on value of `mappings.toml` +# e.g. "multiple_versions_channel_1" update-test-channel = { cmd = "python update-channels.py", cwd = "tests/data/channels" } [feature.dev.dependencies] diff --git a/tests/data/channels/update-channels.py b/tests/data/channels/update-channels.py index 9e9d0e626..6c84b888c 100644 --- a/tests/data/channels/update-channels.py +++ b/tests/data/channels/update-channels.py @@ -2,35 +2,40 @@ from pathlib import Path import shutil import tomllib +import argparse def main() -> None: + parser = argparse.ArgumentParser(description="Update a single channel.") + parser.add_argument("channel", help="The channel to update") + args = parser.parse_args() + platforms = ["win-64", "linux-64", "osx-arm64", "osx-64"] mappings = tomllib.loads(Path("mappings.toml").read_text()) - channels_dir = Path("channels") + channels_dir = Path("channels", args.channel) shutil.rmtree(channels_dir, ignore_errors=True) - channels_dir.mkdir() for recipe, channel in mappings.items(): - print(recipe, channel) - for platform in platforms: - subprocess.run( - [ - "rattler-build", - "build", - "--target-platform", - platform, - "--no-include-recipe", - "--output-dir", - f"channels/{channel}", - "--recipe", - f"recipes/{recipe}", - ], - check=True, - ) + if channel == args.channel: + print(recipe, channel) + for platform in platforms: + subprocess.run( + [ + "rattler-build", + "build", + "--target-platform", + platform, + "--no-include-recipe", + "--output-dir", + f"channels/{channel}", + "--recipe", + f"recipes/{recipe}", + ], + check=True, + ) - # Remove the build directory using shutil - shutil.rmtree(Path(channel, "bld"), ignore_errors=True) + # Remove the build directory using shutil + shutil.rmtree(channels_dir.joinpath("bld"), ignore_errors=True) if __name__ == "__main__": From e56f2e6300ba2defbd322fc13a45f224886a89c4 Mon Sep 17 00:00:00 2001 From: Liam Connors Date: Tue, 19 Nov 2024 00:16:50 -0500 Subject: [PATCH 3/6] docs: update project structure in Python tutorial (#2506) Follow up on https://github.com/prefix-dev/pixi/pull/2452 I'd missed that the structure was referenced later here --- docs/tutorials/python.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/tutorials/python.md b/docs/tutorials/python.md index 69537b9a3..7a50d3fe3 100644 --- a/docs/tutorials/python.md +++ b/docs/tutorials/python.md @@ -249,7 +249,7 @@ You can execute commands in this environment with e.g. `pixi run -e test python` ## Getting code to run Let's add some code to the `pixi-py` package. -We will add a new function to the `pixi_py/__init__.py` file: +We will add a new function to the `src/pixi_py/__init__.py` file: ```python from rich import print @@ -288,8 +288,9 @@ Giving us the following project structure: ```shell . ├── pixi.lock -├── pixi_py -│   └── __init__.py +├── src +│ └── pixi_py +│ └── __init__.py ├── pyproject.toml └── tests/test_me.py ``` From 68e3262f2d4383160861f1ab3fa973d900eaca61 Mon Sep 17 00:00:00 2001 From: Hofer-Julian <30049909+Hofer-Julian@users.noreply.github.com> Date: Tue, 19 Nov 2024 11:35:33 +0100 Subject: [PATCH 4/6] fix: always print a warning when config is invalid (#2508) ![grafik](https://github.com/user-attachments/assets/468e31d0-4d80-437b-ade2-73c43c50b18b) If one of the configs is broken, that's the warning I get --- crates/pixi_config/src/lib.rs | 7 +++++-- tests/integration_python/test_main_cli.py | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/crates/pixi_config/src/lib.rs b/crates/pixi_config/src/lib.rs index be80ec53a..5157d3de1 100644 --- a/crates/pixi_config/src/lib.rs +++ b/crates/pixi_config/src/lib.rs @@ -666,10 +666,13 @@ impl Config { let mut config = Self::load_system(); for p in config_path_global() { + if !p.is_file() { + continue; + } match Self::from_path(&p) { Ok(c) => config = config.merge_config(c), - Err(e) => tracing::debug!( - "Failed to load global config: {} (error: {})", + Err(e) => tracing::warn!( + "Failed to load global config '{}' with error: {}", p.display(), e ), diff --git a/tests/integration_python/test_main_cli.py b/tests/integration_python/test_main_cli.py index 5b44e2a08..5cf2be3f9 100644 --- a/tests/integration_python/test_main_cli.py +++ b/tests/integration_python/test_main_cli.py @@ -168,6 +168,27 @@ def test_project_commands(pixi: Path, tmp_path: Path) -> None: ) +def test_broken_config(pixi: Path, tmp_path: Path) -> None: + env = {"PIXI_HOME": str(tmp_path)} + config = tmp_path.joinpath("config.toml") + toml = """ + [repodata-config."https://prefix.dev/"] + disable-sharded = false + """ + config.write_text(toml) + + # Test basic commands + verify_cli_command([pixi, "info"], env=env) + + toml_invalid = toml + "invalid" + config.write_text(toml_invalid) + verify_cli_command([pixi, "info"], env=env, stderr_contains="parse error") + + toml_unknown = "invalid-key = true" + toml + config.write_text(toml_unknown) + verify_cli_command([pixi, "info"], env=env, stderr_contains="Ignoring 'invalid-key'") + + @pytest.mark.slow def test_search(pixi: Path) -> None: verify_cli_command( From f50f89ba0a58ea97d257a6179845f39ab36f7c1f Mon Sep 17 00:00:00 2001 From: Hofer-Julian <30049909+Hofer-Julian@users.noreply.github.com> Date: Tue, 19 Nov 2024 14:15:48 +0100 Subject: [PATCH 5/6] test: make pixi tests work with local config (#2509) When I changed the defaults-channel in my config some tests stopped working. I've adapted `test_add_dependency` accordingly. I didn't manage to adapt `default_channel` so that it would stop parsing the local config --- src/global/project/manifest.rs | 13 ++++++++++++- tests/integration_rust/init_tests.rs | 20 -------------------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/global/project/manifest.rs b/src/global/project/manifest.rs index afef0dd6b..5ad76c865 100644 --- a/src/global/project/manifest.rs +++ b/src/global/project/manifest.rs @@ -501,6 +501,7 @@ mod tests { use indexmap::IndexSet; use insta::assert_snapshot; use itertools::Itertools; + use pixi_consts::consts::DEFAULT_CHANNELS; use rattler_conda_types::ParseStrictness; use super::*; @@ -769,7 +770,17 @@ mod tests { MatchSpec::from_str("pythonic ==3.15.0", ParseStrictness::Strict).unwrap(); // Add environment - manifest.add_environment(&env_name, None).unwrap(); + manifest + .add_environment( + &env_name, + Some( + DEFAULT_CHANNELS + .iter() + .map(|&name| NamedChannelOrUrl::Name(name.to_string())) + .collect(), + ), + ) + .unwrap(); // Add dependency manifest diff --git a/tests/integration_rust/init_tests.rs b/tests/integration_rust/init_tests.rs index b0300618b..235e5330f 100644 --- a/tests/integration_rust/init_tests.rs +++ b/tests/integration_rust/init_tests.rs @@ -55,26 +55,6 @@ async fn specific_channel() { ) } -/// Tests that when initializing an empty project the default channel -/// `conda-forge` is used. -#[tokio::test] -async fn default_channel() { - let pixi = PixiControl::new().unwrap(); - - // Init a new project - pixi.init().no_fast_prefix_overwrite(true).await.unwrap(); - - // Load the project - let project = pixi.project().unwrap(); - - // The only channel should be the "conda-forge" channel - let channels = Vec::from_iter(project.default_environment().channels()); - assert_eq!( - channels, - [&NamedChannelOrUrl::Name(String::from("conda-forge"))] - ) -} - // Test the initialization from an existing pyproject.toml file without the pixi information #[tokio::test] async fn init_from_existing_pyproject_toml() { From a642b35da4c04e7f455f95c7392c969416531865 Mon Sep 17 00:00:00 2001 From: Daniel Elsner Date: Tue, 19 Nov 2024 14:46:06 +0100 Subject: [PATCH 6/6] feat: Add versions/build list to `pixi search` output (#2440) Co-authored-by: Pavel Zwerschke --- src/cli/search.rs | 180 ++++++++++++++---- tests/integration_rust/common/builders.rs | 6 +- tests/integration_rust/search_tests.rs | 72 +++++++ ...earch_tests__search_multiple_versions.snap | 23 +++ 4 files changed, 246 insertions(+), 35 deletions(-) create mode 100644 tests/integration_rust/snapshots/integration_rust__search_tests__search_multiple_versions.snap diff --git a/src/cli/search.rs b/src/cli/search.rs index 6cd33952c..33ae21060 100644 --- a/src/cli/search.rs +++ b/src/cli/search.rs @@ -5,6 +5,7 @@ use std::io::{self, Write}; use std::str::FromStr; use clap::Parser; +use indexmap::IndexMap; use itertools::Itertools; use miette::IntoDiagnostic; use pixi_config::default_channel_config; @@ -54,6 +55,7 @@ async fn search_package_by_filter( all_package_names: Vec, repodata_query_func: QF, filter_func: F, + only_latest: bool, ) -> miette::Result> where F: Fn(&PackageName, &PackageName) -> bool, @@ -76,32 +78,38 @@ where let repos: Vec = repodata_query_func(specs).await.into_diagnostic()?; - let mut latest_packages: Vec = Vec::new(); - - for repo in repos { - // sort records by version, get the latest one of each package - let records_of_repo: HashMap = repo - .into_iter() - .sorted_by(|a, b| a.package_record.version.cmp(&b.package_record.version)) - .map(|record| { - ( - record.package_record.name.as_normalized().to_string(), - record.clone(), - ) - }) - .collect(); - - latest_packages.extend(records_of_repo.into_values().collect_vec()); + let mut packages: Vec = Vec::new(); + if only_latest { + for repo in repos { + // sort records by version, get the latest one of each package + let records_of_repo: HashMap = repo + .into_iter() + .sorted_by(|a, b| a.package_record.version.cmp(&b.package_record.version)) + .map(|record| { + ( + record.package_record.name.as_normalized().to_string(), + record.clone(), + ) + }) + .collect(); + + packages.extend(records_of_repo.into_values().collect_vec()); + } + // sort all versions across all channels and platforms + packages.sort_by(|a, b| a.package_record.version.cmp(&b.package_record.version)); + } else { + for repo in repos { + packages.extend(repo.into_iter().cloned().collect_vec()); + } } - // sort all versions across all channels and platforms - latest_packages.sort_by(|a, b| a.package_record.version.cmp(&b.package_record.version)); - - Ok(latest_packages) + Ok(packages) } -pub async fn execute_impl(args: Args) -> miette::Result>> { - let stdout = io::stdout(); +pub async fn execute_impl( + args: Args, + out: &mut W, +) -> miette::Result>> { let project = Project::load_or_else_discover(args.project_config.manifest_path.as_deref()).ok(); // Resolve channels from project / CLI args @@ -156,7 +164,7 @@ pub async fn execute_impl(args: Args) -> miette::Result miette::Result miette::Result miette::Result<()> { - execute_impl(args).await?; + let mut out = io::stdout(); + execute_impl(args, &mut out).await?; Ok(()) } @@ -181,7 +190,7 @@ async fn search_exact_package( package_name: PackageName, all_repodata_names: Vec, repodata_query_func: QF, - out: W, + out: &mut W, ) -> miette::Result>> where QF: Fn(Vec) -> FR, @@ -193,33 +202,85 @@ where all_repodata_names, repodata_query_func, |pn, n| pn == n, + false, ) .await?; + // Sort packages by version, build number and build string + let packages = packages + .iter() + .sorted_by(|a, b| { + Ord::cmp( + &( + &a.package_record.version, + a.package_record.build_number, + &a.package_record.build, + ), + &( + &b.package_record.version, + b.package_record.build_number, + &b.package_record.build, + ), + ) + }) + .cloned() + .collect::>(); if packages.is_empty() { let normalized_package_name = package_name.as_normalized(); return Err(miette::miette!("Package {normalized_package_name} not found, please use a wildcard '*' in the search name for a broader result.")); } - let package = packages.last(); - if let Some(package) = package { - if let Err(e) = print_package_info(package, out) { + let newest_package = packages.last(); + if let Some(newest_package) = newest_package { + let other_versions = packages + .iter() + .filter(|p| p.package_record != newest_package.package_record) + .collect::>(); + if let Err(e) = print_package_info(newest_package, &other_versions, out) { if e.kind() != std::io::ErrorKind::BrokenPipe { return Err(e).into_diagnostic(); } } } - Ok(package.map(|package| vec![package.clone()])) + Ok(newest_package.map(|package| vec![package.clone()])) +} + +fn format_additional_builds_string(builds: Option>) -> String { + let builds = builds.unwrap_or_default(); + match builds.len() { + 0 => String::new(), + 1 => " (+ 1 build)".to_string(), + _ => format!(" (+ {} builds)", builds.len()), + } } -fn print_package_info(package: &RepoDataRecord, mut out: W) -> io::Result<()> { +fn print_package_info( + package: &RepoDataRecord, + other_versions: &Vec<&RepoDataRecord>, + out: &mut W, +) -> io::Result<()> { writeln!(out)?; let package = package.clone(); let package_name = package.package_record.name.as_source(); let build = &package.package_record.build; - let package_info = format!("{} {}", console::style(package_name), console::style(build)); + let mut grouped_by_version = IndexMap::new(); + for version in other_versions { + grouped_by_version + .entry(&version.package_record.version) + .or_insert_with(Vec::new) + .insert(0, *version); + } + let other_builds = grouped_by_version.shift_remove(&package.package_record.version); + let package_info = format!( + "{}-{}-{}{}", + console::style(package.package_record.name.as_source()), + console::style(package.package_record.version.to_string()), + console::style(&package.package_record.build), + console::style(format_additional_builds_string(other_builds)) + ); + writeln!(out, "{}", package_info)?; writeln!(out, "{}\n", "-".repeat(package_info.chars().count()))?; @@ -314,6 +375,55 @@ fn print_package_info(package: &RepoDataRecord, mut out: W) -> io::Res writeln!(out, " - {}", dependency)?; } + // Print summary of older versions for package + if !grouped_by_version.is_empty() { + writeln!(out, "\nOther Versions ({}):", grouped_by_version.len())?; + let version_width = grouped_by_version + .keys() + .map(|v| v.to_string().len()) + .chain(["Version".len()].iter().cloned()) + .max() + .unwrap() + + 1; + let build_width = other_versions + .iter() + .map(|v| v.package_record.build.len()) + .chain(["Build".len()].iter().cloned()) + .max() + .unwrap() + + 1; + writeln!( + out, + "{:version_width$} {:build_width$}", + console::style("Version").bold(), + console::style("Build").bold(), + version_width = version_width, + build_width = build_width + )?; + let max_displayed_versions = 4; + let mut counter = 0; + for (version, builds) in grouped_by_version.iter().rev() { + writeln!( + out, + "{:version_width$} {:build_width$}{}", + console::style(version.to_string()), + console::style(builds[0].package_record.build.clone()), + console::style(format_additional_builds_string(Some(builds[1..].to_vec()))).dim(), + version_width = version_width, + build_width = build_width + )?; + counter += 1; + if counter == max_displayed_versions { + writeln!( + out, + "... and {} more", + grouped_by_version.len() - max_displayed_versions + )?; + break; + } + } + } + Ok(()) } @@ -323,7 +433,7 @@ async fn search_package_by_wildcard( all_package_names: Vec, repodata_query_func: QF, limit: Option, - out: W, + out: &mut W, ) -> miette::Result>> where QF: Fn(Vec) -> FR + Clone, @@ -340,6 +450,7 @@ where all_package_names.clone(), repodata_query_func.clone(), |pn, _| wildcard_pattern.is_match(pn.as_normalized()), + true, ) .await?; @@ -354,6 +465,7 @@ where all_package_names, repodata_query_func, |pn, n| jaro(pn.as_normalized(), n.as_normalized()) > similarity, + true, ) .await }) @@ -391,7 +503,7 @@ where fn print_matching_packages( packages: &[RepoDataRecord], - mut out: W, + out: &mut W, limit: Option, ) -> io::Result<()> { writeln!( diff --git a/tests/integration_rust/common/builders.rs b/tests/integration_rust/common/builders.rs index 05a8ed003..d2cff748a 100644 --- a/tests/integration_rust/common/builders.rs +++ b/tests/integration_rust/common/builders.rs @@ -26,6 +26,7 @@ use pixi::cli::cli_config::{PrefixUpdateConfig, ProjectConfig}; use std::{ future::{Future, IntoFuture}, + io, path::{Path, PathBuf}, pin::Pin, str::FromStr, @@ -232,7 +233,10 @@ impl IntoFuture for SearchBuilder { type IntoFuture = Pin + 'static>>; fn into_future(self) -> Self::IntoFuture { - search::execute_impl(self.args).boxed_local() + Box::pin(async move { + let mut out = io::stdout(); + search::execute_impl(self.args, &mut out).await + }) } } diff --git a/tests/integration_rust/search_tests.rs b/tests/integration_rust/search_tests.rs index 57796ac82..dac19c9ad 100644 --- a/tests/integration_rust/search_tests.rs +++ b/tests/integration_rust/search_tests.rs @@ -1,3 +1,5 @@ +use insta::assert_snapshot; +use pixi::cli::search; use rattler_conda_types::Platform; use tempfile::TempDir; use url::Url; @@ -65,3 +67,73 @@ async fn search_return_latest_across_everything() { assert_eq!(found_package.package_record.version.as_str(), "4"); } + +#[tokio::test] +async fn test_search_multiple_versions() { + let mut package_database = PackageDatabase::default(); + + // Add package with multiple versions and build strings + package_database.add_package( + Package::build("foo", "0.1.0") + .with_build("h60d57d3_0") + .with_subdir(Platform::NoArch) + .finish(), + ); + package_database.add_package( + Package::build("foo", "0.1.0") + .with_build("h60d57d3_1") + .with_subdir(Platform::NoArch) + .finish(), + ); + package_database.add_package( + Package::build("foo", "0.2.0") + .with_build("h60d57d3_0") + .with_subdir(Platform::NoArch) + .finish(), + ); + package_database.add_package( + Package::build("foo", "0.2.0") + .with_build("h60d57d3_1") + .with_subdir(Platform::NoArch) + .finish(), + ); + let temp_dir = TempDir::new().unwrap(); + let channel_dir = temp_dir.path().join("channel"); + package_database.write_repodata(&channel_dir).await.unwrap(); + let channel = Url::from_file_path(channel_dir).unwrap(); + let platform = Platform::current(); + let pixi = PixiControl::from_manifest(&format!( + r#" + [project] + name = "test-multiple-versions" + channels = ["{channel}"] + platforms = ["{platform}"] + + "# + )) + .unwrap(); + + let mut out = Vec::new(); + let builder = pixi.search("foo".to_string()); + let result = search::execute_impl(builder.args, &mut out) + .await + .unwrap() + .unwrap(); + let output = String::from_utf8(out).unwrap(); + let output = output + // Remove ANSI escape codes from output + .replace("\x1b[0m", "") + .replace("\x1b[1m", "") + .replace("\x1b[2m", ""); + + assert_eq!(result.len(), 1); + assert_eq!(result[0].package_record.version.as_str(), "0.2.0"); + assert_eq!(result[0].package_record.build, "h60d57d3_1"); + let output = output + .lines() + // Filter out URL line since temporary directory name is random. + .filter(|line| !line.starts_with("URL")) + .collect::>() + .join("\n"); + assert_snapshot!(output); +} diff --git a/tests/integration_rust/snapshots/integration_rust__search_tests__search_multiple_versions.snap b/tests/integration_rust/snapshots/integration_rust__search_tests__search_multiple_versions.snap new file mode 100644 index 000000000..f2a268b4d --- /dev/null +++ b/tests/integration_rust/snapshots/integration_rust__search_tests__search_multiple_versions.snap @@ -0,0 +1,23 @@ +--- +source: tests/integration_rust/search_tests.rs +assertion_line: 137 +expression: output +--- +foo-0.2.0-h60d57d3_1 (+ 1 build) +-------------------------------- + +Name foo +Version 0.2.0 +Build h60d57d3_1 +Size Not found. +License Not found. +Subdir noarch +File Name foo-0.2.0-h60d57d3_1.conda +MD5 046ee97456e4c968d514dcb0b4632a8c +SHA256 a19e6a97fb5def2f6bd7385348ba603c54859500a30d168455cb19d3b347c7e7 + +Dependencies: + +Other Versions (1): +Version Build +0.1.0 h60d57d3_1 (+ 1 build)