Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

441 changes: 419 additions & 22 deletions crates/pixi_cli/src/global/global_specs.rs

Large diffs are not rendered by default.

15 changes: 12 additions & 3 deletions crates/pixi_core/src/environment/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,17 @@ pub struct EnvironmentFile {
/// be weaker than [`Self::resolved_platform`]. `None` as above.
#[serde(default)]
pub minimum_supported_platform: Option<PlatformData>,
/// Fingerprints of the manifest's source dependencies at install time,
/// keyed by package name: a hash of the source spec plus any inline
/// package definition. Only written by `pixi global`, which has no lock
/// file; `pixi global sync` compares them against the manifest to decide
/// whether a source dependency's *specification* changed (source content
/// changes are `pixi global update`'s job). Empty for workspace
/// environments and environments written by an older pixi — for
/// environments with source dependencies that means out-of-sync, which
/// degrades gracefully to a one-time rebuild.
#[serde(default, skip_serializing_if = "std::collections::BTreeMap::is_empty")]
pub source_fingerprints: std::collections::BTreeMap<String, u64>,
}

/// The path to the environment file in the `conda-meta` directory of the
Expand Down Expand Up @@ -434,9 +445,7 @@ pub fn write_environment_file(

/// Reading the environment file of the environment.
/// Removing it if it's not valid.
pub(crate) fn read_environment_file(
environment_dir: &Path,
) -> miette::Result<Option<EnvironmentFile>> {
pub fn read_environment_file(environment_dir: &Path) -> miette::Result<Option<EnvironmentFile>> {
let path = environment_file_path(environment_dir);

let contents = match fs_err::read_to_string(&path) {
Expand Down
1 change: 1 addition & 0 deletions crates/pixi_core/src/lock_file/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,7 @@ impl<'p> LockFileDerivedData<'p> {
environment_lock_file_hash: hash,
resolved_platform,
minimum_supported_platform,
source_fingerprints: Default::default(),
},
)?;

Expand Down
1 change: 1 addition & 0 deletions crates/pixi_global/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ toml-span = { workspace = true }
toml_edit = { workspace = true }
tracing = { workspace = true }
url = { workspace = true }
xxhash-rust = { workspace = true, features = ["xxh3"] }
zstd = { workspace = true }

[target.'cfg(unix)'.dependencies]
Expand Down
107 changes: 106 additions & 1 deletion crates/pixi_global/src/project/global_spec.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
//! This module makes it a bit easier to pass around a package name and the pixi specification
use std::path::Path;

use pixi_manifest::{
InlinePackageManifest, KnownPreviewFeature, Preview,
toml::{TomlPackage, WorkspacePackageProperties},
};
use pixi_spec::PixiSpec;
use rattler_conda_types::{
MatchSpec, NamelessMatchSpec, PackageName, ParseMatchSpecOptions, RepodataRevision,
Expand All @@ -10,6 +16,10 @@ use rattler_conda_types::{
pub struct GlobalSpec {
pub name: PackageName,
pub spec: PixiSpec,
/// An inline package definition (`package = { ... }`) accompanying a
/// source spec: it describes how the source is built when the source does
/// not provide its own package manifest (or to override the one it has).
pub inline: Option<InlinePackageValue>,
}

#[derive(Debug, thiserror::Error, miette::Diagnostic)]
Expand All @@ -23,7 +33,17 @@ pub enum FromMatchSpecError {
impl GlobalSpec {
/// Creates a new `GlobalSpec` with a package name and a Pixi specification.
pub fn new(name: PackageName, spec: PixiSpec) -> Self {
Self { name, spec }
Self {
name,
spec,
inline: None,
}
}

/// Attaches an inline package definition to this spec.
pub fn with_inline(mut self, inline: InlinePackageValue) -> Self {
self.inline = Some(inline);
self
}

/// Returns the package name.
Expand Down Expand Up @@ -63,3 +83,88 @@ impl GlobalSpec {
Ok(GlobalSpec::new(name.clone(), pixi_spec))
}
}

#[derive(Debug, thiserror::Error, miette::Diagnostic)]
pub enum InlinePackageValueError {
#[error("failed to parse the inline package definition: {0}")]
Parse(String),
#[error("an inline package definition cannot set `name`; it is taken from the dependency key")]
ExplicitName,
#[error(
"an inline package definition cannot set `build.source`; the source is taken from the dependency spec"
)]
ExplicitBuildSource,
#[error("failed to convert the inline package definition: {0}")]
Convert(String),
}

/// An inline package definition as the TOML value of the `package` key of a
/// dependency entry (e.g. `package.build.backend.name = "pixi-build-rust"`).
///
/// The raw TOML representation is kept so the definition can be written
/// verbatim into the global manifest document; [`Self::to_inline_manifest`]
/// converts it into the resolved [`InlinePackageManifest`] when it is needed
/// before the manifest is saved and re-parsed (e.g. for package name
/// inference).
#[derive(Debug, Clone)]
pub struct InlinePackageValue(toml_edit::InlineTable);

impl InlinePackageValue {
/// Wraps a `package` table.
pub fn new(table: toml_edit::InlineTable) -> Self {
Self(table)
}

/// Returns the definition as a TOML value, for insertion into a dependency
/// entry under the `package` key.
pub fn to_toml_value(&self) -> toml_edit::Value {
toml_edit::Value::InlineTable(self.0.clone())
}

/// Parses and converts the definition into an [`InlinePackageManifest`]
/// for the dependency `name`, mirroring the checks of the manifest parser:
/// the definition may not set `name` (it comes from the dependency key)
/// nor `build.source` (it comes from the dependency spec).
pub fn to_inline_manifest(
&self,
name: &PackageName,
root_directory: &Path,
) -> Result<InlinePackageManifest, InlinePackageValueError> {
let source = format!("package = {}", self.0);
let mut value =
toml_span::parse(&source).map_err(|e| InlinePackageValueError::Parse(e.to_string()))?;
let mut th = toml_span::de_helpers::TableHelper::new(&mut value)
.map_err(|e| InlinePackageValueError::Parse(e.to_string()))?;
let (_, mut package_value) = th
.take("package")
.expect("the `package` key was just serialized");
let package = <TomlPackage as toml_span::Deserialize>::deserialize(&mut package_value)
.map_err(|e| {
InlinePackageValueError::Parse(
e.errors
.iter()
.map(ToString::to_string)
.collect::<Vec<_>>()
.join("; "),
)
})?;

if package.name.is_some() {
return Err(InlinePackageValueError::ExplicitName);
}
if package.build.source.is_some() {
return Err(InlinePackageValueError::ExplicitBuildSource);
}

let preview = Preview::from_iter([KnownPreviewFeature::PixiBuild]);
InlinePackageManifest::from_toml_package(
name,
package,
WorkspacePackageProperties::default(),
&preview,
root_directory,
)
.map(|with_warnings| with_warnings.value)
.map_err(|e| InlinePackageValueError::Convert(e.to_string()))
}
}
135 changes: 118 additions & 17 deletions crates/pixi_global/src/project/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ impl Manifest {
/// Creates a new manifest from a string
pub fn from_str(manifest_path: &Path, contents: impl Into<String>) -> miette::Result<Self> {
let contents = contents.into();
let parsed = ParsedManifest::from_toml_str(&contents);
let root_directory = manifest_path.parent().unwrap_or(Path::new(""));
let parsed = ParsedManifest::from_toml_str(&contents, root_directory);

let (manifest, document) = match parsed.and_then(|manifest| {
contents
Expand Down Expand Up @@ -143,28 +144,60 @@ impl Manifest {
let name = named_spec.name();
let spec = named_spec.spec();

// The value to write into the document: the source spec, extended
// with the inline package definition under the `package` key when one
// is supplied.
let toml_value = match &named_spec.inline {
None => spec.to_toml_value(),
Some(inline) => {
let toml_edit::Value::InlineTable(mut table) = spec.to_toml_value() else {
miette::bail!(
"an inline package definition requires a source spec, but the spec of '{}' is `{}`",
name.as_normalized(),
spec.to_toml_value()
);
};
table.insert("package", inline.to_toml_value());
toml_edit::Value::InlineTable(table)
}
};

// Update self.parsed
self.parsed
.envs
.get_mut(env_name)
.ok_or_else(|| {
{
let env = self.parsed.envs.get_mut(env_name).ok_or_else(|| {
miette::miette!("Environment {} doesn't exist.", env_name.fancy_display())
})?
.dependencies
.specs
.insert(name.clone(), spec.clone());
})?;
env.dependencies.specs.insert(name.clone(), spec.clone());
// A previously recorded inline definition no longer applies when
// the dependency is overwritten without one.
env.inline_packages.swap_remove(name);
}

// Update self.document
self.document.insert_into_inline_table(
&["envs", env_name.as_str(), "dependencies"],
name.as_normalized(),
spec.to_toml_value(),
toml_value.clone(),
)?;

// Inline package definitions are converted with root-directory
// context by the manifest parser; re-parse the document so
// `self.parsed` picks up the converted definition.
if named_spec.inline.is_some() {
let contents = self.document.to_string();
let root_directory = self.path.parent().unwrap_or(Path::new(""));
self.parsed = match ParsedManifest::from_toml_str(&contents, root_directory) {
Ok(parsed) => parsed,
Err(e) => {
return e.to_fancy(consts::GLOBAL_MANIFEST_DEFAULT_NAME, &contents, &self.path);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do the checks before changing the self.document (e.g. in line 177)? It's just safer to do it in that order.

}
};
}

tracing::debug!(
"Added dependency {}={} to toml document for environment {}",
name.as_normalized(),
spec.to_toml_value().to_string(),
toml_value.to_string(),
env_name.fancy_display()
);
Ok(())
Expand All @@ -177,12 +210,10 @@ impl Manifest {
name: &PackageName,
) -> miette::Result<PackageName> {
// Update self.parsed
self.parsed
.envs
.get_mut(env_name)
.ok_or_else(|| {
miette::miette!("Environment {} doesn't exist.", env_name.fancy_display())
})?
let parsed_env = self.parsed.envs.get_mut(env_name).ok_or_else(|| {
miette::miette!("Environment {} doesn't exist.", env_name.fancy_display())
})?;
parsed_env
.dependencies
.specs
.swap_remove(name)
Expand All @@ -191,6 +222,7 @@ impl Manifest {
console::style(name.as_normalized()).green(),
env_name.fancy_display()
))?;
parsed_env.inline_packages.swap_remove(name);

// Update self.document
self.document
Expand Down Expand Up @@ -1060,6 +1092,75 @@ mod tests {
);
}

/// Re-adding a source dependency replaces its inline `package` definition
/// wholesale: attaching a definition records it, and re-adding without one
/// removes it rather than leaving a stale table behind.
#[test]
fn test_add_dependency_overwrites_inline_definition() {
let contents = r#"
[envs.xsv]
channels = ["conda-forge"]
[envs.xsv.dependencies]
xsv = { path = "some-source" }
"#;
let mut manifest =
Manifest::from_str(std::path::Path::new("pixi-global.toml"), contents).unwrap();

let env_name = EnvironmentName::from_str("xsv").unwrap();
let name = rattler_conda_types::PackageName::from_str("xsv").unwrap();
let source_spec = manifest
.parsed
.envs
.get(&env_name)
.unwrap()
.dependencies
.specs
.get(&name)
.unwrap()
.clone();

let dependency_toml = |manifest: &mut Manifest| {
manifest
.document
.get_or_insert_nested_table(&["envs", env_name.as_str(), "dependencies"])
.unwrap()
.get(name.as_normalized())
.unwrap()
.to_string()
};

// Attach an inline definition to the existing source spec.
let value: toml_edit::Value = r#"{ build = { backend = { name = "pixi-build-rust" } } }"#
.parse()
.unwrap();
let inline =
crate::project::InlinePackageValue::new(value.as_inline_table().unwrap().clone());
let with_inline = GlobalSpec::new(name.clone(), source_spec.clone()).with_inline(inline);
manifest.add_dependency(&env_name, &with_inline).unwrap();
assert!(
dependency_toml(&mut manifest).contains("package"),
"inline definition was not recorded"
);

// Re-add the same dependency without an inline definition.
let without_inline = GlobalSpec::new(name.clone(), source_spec);
manifest.add_dependency(&env_name, &without_inline).unwrap();
assert!(
!dependency_toml(&mut manifest).contains("package"),
"stale inline definition lingered after overwrite"
);
assert!(
manifest
.parsed
.envs
.get(&env_name)
.unwrap()
.inline_packages
.is_empty(),
"parsed inline definition lingered after overwrite"
);
}

#[test]
fn test_set_platform() {
let mut manifest = Manifest::default();
Expand Down
Loading
Loading