From adc92cd1969df776bbe9d16427e717888a7d549a Mon Sep 17 00:00:00 2001 From: nichmor Date: Mon, 29 Dec 2025 17:41:28 +0200 Subject: [PATCH 01/11] fix: preserver branch reference --- .../pixi_core/src/lock_file/resolve/pypi.rs | 186 +++++++++++++++--- 1 file changed, 162 insertions(+), 24 deletions(-) diff --git a/crates/pixi_core/src/lock_file/resolve/pypi.rs b/crates/pixi_core/src/lock_file/resolve/pypi.rs index fddd4ce4c7..68b225a594 100644 --- a/crates/pixi_core/src/lock_file/resolve/pypi.rs +++ b/crates/pixi_core/src/lock_file/resolve/pypi.rs @@ -121,6 +121,64 @@ fn parse_hashes_from_hash_vec(hashes: &HashDigests) -> Result Result { + use pixi_uv_conversions::into_uv_git_reference; + + let git_locked_url = LockedGitUrl::from(location.clone()); + let pinned_git_spec = git_locked_url + .to_pinned_git_spec() + .map_err(|e| LockedGitSourceError::Parse(e.to_string()))?; + + // Create VerbatimUrl from the original location + let verbatim_url = VerbatimUrl::from(location.clone()); + + // Get the display safe URL from the PinnedGitSpec (without git+ prefix) + let display_safe = DisplaySafeUrl::from(pinned_git_spec.git.clone()); + + // Parse the commit OID + let git_oid = uv_git_types::GitOid::from_str(&pinned_git_spec.source.commit.to_string())?; + + // Convert the pixi git reference to uv git reference + // This preserves the branch/tag/rev from the locked spec + let uv_reference = into_uv_git_reference(pinned_git_spec.source.reference.clone().into()); + + // Create the GitUrl with the correct reference and precise commit + let git_url = GitUrl::from_commit(display_safe, uv_reference, git_oid)?; + + Ok(RequirementSource::Git { + git: git_url, + subdirectory: pinned_git_spec + .source + .subdirectory + .map(|s| PathBuf::from(s).into_boxed_path()), + url: verbatim_url, + }) +} + #[derive(Debug, thiserror::Error)] enum ProcessPathUrlError { #[error("expected given path for {0} but none found")] @@ -612,30 +670,10 @@ pub async fn resolve_pypi( if let Some(location) = package_data.location.as_url() { // now check if it's a git url if LockedGitUrl::is_locked_git_url(location) { - // we need to parse back `LockedGitUrl` in order to get the `PinnedGitSpec` - // then we will precise commit to set for the `GitUrl` - // that will be used in the `RequirementSource::Git` below - let git_locked_url = LockedGitUrl::from(location.clone()); - let pinned_git_spec = git_locked_url - .to_pinned_git_spec() - .map_err(PixiPreferencesError::Other)?; - // we need to create VerbatimUrl from the original location - let verbatim_url = VerbatimUrl::from(location.clone()); - - // but the display safe url should come from the `PinnedGitSpec` url - // which don't have anymore git+ prefix - let display_safe = DisplaySafeUrl::from(pinned_git_spec.git.clone()); - - let git_oid = - uv_git_types::GitOid::from_str(&pinned_git_spec.source.commit.to_string())?; - - let git_url = GitUrl::try_from(display_safe)?.with_precise(git_oid); - - let constraint_source = RequirementSource::Git { - git: git_url, - subdirectory: None, - url: verbatim_url, - }; + // Create a constraint source from the locked git URL, + // preserving the git reference (branch/tag/rev) + let constraint_source = locked_git_url_to_requirement_source(location) + .map_err(|e| PixiPreferencesError::Other(miette::miette!("{e}")))?; // find this requirements in dependencies and skip adding it as preference let req_from_dep = requirements.iter_mut().find(|r| r.name == requirement.name); @@ -1297,4 +1335,104 @@ mod tests { .unwrap(); assert_eq!(path.as_str(), "C:/a/b/c"); } + + /// Test for issue #5185: git reference must be preserved when enriching + /// requirements from locked packages. + /// + /// When a locked git package has a specific reference (e.g., `branch=master`), + /// the constraint_source created from it should preserve that reference. + #[test] + fn test_git_reference_preserved_in_constraint_source() { + use super::locked_git_url_to_requirement_source; + + // Create a locked git URL with a specific branch reference + // This simulates what's stored in the lock file for a git dependency with `rev = "master"` + let locked_url = url::Url::parse( + "git+https://github.com/example/repo.git?branch=master#abc123def456abc123def456abc123def456abc1", + ) + .unwrap(); + + // Use the extracted function to create the constraint source + let constraint_source = locked_git_url_to_requirement_source(&locked_url).unwrap(); + + // Verify that the constraint source has the correct git reference + match constraint_source { + uv_distribution_types::RequirementSource::Git { git, .. } => { + assert_eq!( + git.reference(), + &uv_git_types::GitReference::Branch("master".into()), + "The git reference should be preserved from the locked spec. \ + This is the fix for issue #5185." + ); + } + _ => panic!("Expected RequirementSource::Git"), + } + } + + /// Test that various git reference types are preserved correctly. + #[test] + fn test_git_reference_types_preserved() { + use super::locked_git_url_to_requirement_source; + + // Test branch reference + let url_with_branch = url::Url::parse( + "git+https://github.com/example/repo.git?branch=develop#abc123def456abc123def456abc123def456abc1", + ).unwrap(); + let source = locked_git_url_to_requirement_source(&url_with_branch).unwrap(); + if let uv_distribution_types::RequirementSource::Git { git, .. } = source { + assert_eq!( + git.reference(), + &uv_git_types::GitReference::Branch("develop".into()) + ); + } + + // Test tag reference + let url_with_tag = url::Url::parse( + "git+https://github.com/example/repo.git?tag=v1.0.0#abc123def456abc123def456abc123def456abc1", + ).unwrap(); + let source = locked_git_url_to_requirement_source(&url_with_tag).unwrap(); + if let uv_distribution_types::RequirementSource::Git { git, .. } = source { + assert_eq!( + git.reference(), + &uv_git_types::GitReference::Tag("v1.0.0".into()) + ); + } + + // Test rev reference (short string that doesn't look like a commit hash) + // "abc123" is only 6 chars, so it becomes BranchOrTag (needs 7+ hex chars for commit hash) + let url_with_rev = url::Url::parse( + "git+https://github.com/example/repo.git?rev=abc123#abc123def456abc123def456abc123def456abc1", + ).unwrap(); + let source = locked_git_url_to_requirement_source(&url_with_rev).unwrap(); + if let uv_distribution_types::RequirementSource::Git { git, .. } = source { + // Rev with <7 chars becomes BranchOrTag in uv + assert!(matches!( + git.reference(), + uv_git_types::GitReference::BranchOrTag(r) if r == "abc123" + )); + } + + // Test rev reference with commit-like hash (7+ hex chars) + let url_with_commit_rev = url::Url::parse( + "git+https://github.com/example/repo.git?rev=abc1234#abc123def456abc123def456abc123def456abc1", + ).unwrap(); + let source = locked_git_url_to_requirement_source(&url_with_commit_rev).unwrap(); + if let uv_distribution_types::RequirementSource::Git { git, .. } = source { + // Rev with 7+ hex chars becomes BranchOrTagOrCommit in uv + assert!(matches!( + git.reference(), + uv_git_types::GitReference::BranchOrTagOrCommit(r) if r == "abc1234" + )); + } + + // Test default branch (no explicit reference) + let url_default = url::Url::parse( + "git+https://github.com/example/repo.git#abc123def456abc123def456abc123def456abc1", + ) + .unwrap(); + let source = locked_git_url_to_requirement_source(&url_default).unwrap(); + if let uv_distribution_types::RequirementSource::Git { git, .. } = source { + assert_eq!(git.reference(), &uv_git_types::GitReference::DefaultBranch); + } + } } From 13adf6392e26e307a68aed2370ca45721ba5475b Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 30 Dec 2025 10:27:01 +0200 Subject: [PATCH 02/11] misc: remove some comments --- crates/pixi_core/src/lock_file/resolve/pypi.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/pixi_core/src/lock_file/resolve/pypi.rs b/crates/pixi_core/src/lock_file/resolve/pypi.rs index 68b225a594..422d677aeb 100644 --- a/crates/pixi_core/src/lock_file/resolve/pypi.rs +++ b/crates/pixi_core/src/lock_file/resolve/pypi.rs @@ -25,6 +25,7 @@ use pixi_manifest::{ use pixi_pypi_spec::PixiPypiSpec; use pixi_record::{LockedGitUrl, PixiRecord}; use pixi_reporters::{UvReporter, UvReporterOptions}; +use pixi_uv_conversions::into_uv_git_reference; use pixi_uv_conversions::{ ConversionError, as_uv_req, configure_insecure_hosts_for_tls_bypass, convert_uv_requirements_to_pep508, into_pinned_git_spec, pypi_options_to_build_options, @@ -51,7 +52,6 @@ use uv_distribution_types::{ IndexCapabilities, IndexUrl, Name, RequirementSource, RequiresPython, Resolution, ResolvedDist, SourceDist, ToUrlError, }; -use uv_git_types::GitUrl; use uv_pep508::VerbatimUrl; use uv_pypi_types::{Conflicts, HashAlgorithm, HashDigests}; use uv_redacted::DisplaySafeUrl; @@ -146,20 +146,15 @@ pub enum LockedGitSourceError { pub(crate) fn locked_git_url_to_requirement_source( location: &Url, ) -> Result { - use pixi_uv_conversions::into_uv_git_reference; - let git_locked_url = LockedGitUrl::from(location.clone()); let pinned_git_spec = git_locked_url .to_pinned_git_spec() .map_err(|e| LockedGitSourceError::Parse(e.to_string()))?; - // Create VerbatimUrl from the original location let verbatim_url = VerbatimUrl::from(location.clone()); - // Get the display safe URL from the PinnedGitSpec (without git+ prefix) let display_safe = DisplaySafeUrl::from(pinned_git_spec.git.clone()); - // Parse the commit OID let git_oid = uv_git_types::GitOid::from_str(&pinned_git_spec.source.commit.to_string())?; // Convert the pixi git reference to uv git reference @@ -167,7 +162,7 @@ pub(crate) fn locked_git_url_to_requirement_source( let uv_reference = into_uv_git_reference(pinned_git_spec.source.reference.clone().into()); // Create the GitUrl with the correct reference and precise commit - let git_url = GitUrl::from_commit(display_safe, uv_reference, git_oid)?; + let git_url = uv_git_types::GitUrl::from_commit(display_safe, uv_reference, git_oid)?; Ok(RequirementSource::Git { git: git_url, From 231ee78bc7a1227885bb1a99540bb3c20f9b0a9b Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 30 Dec 2025 10:28:34 +0200 Subject: [PATCH 03/11] misc: simplify doc comment --- crates/pixi_core/src/lock_file/resolve/pypi.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/crates/pixi_core/src/lock_file/resolve/pypi.rs b/crates/pixi_core/src/lock_file/resolve/pypi.rs index 46f8e56f9f..72ae23392d 100644 --- a/crates/pixi_core/src/lock_file/resolve/pypi.rs +++ b/crates/pixi_core/src/lock_file/resolve/pypi.rs @@ -132,17 +132,9 @@ pub enum LockedGitSourceError { GitUrlParse(#[from] uv_git_types::GitUrlParseError), } -/// Creates a [`RequirementSource::Git`] from a locked git URL. -/// -/// This function is used to create a constraint source from a previously locked -/// git dependency, preserving the git reference (branch, tag, or rev) so that +/// Creates a [`RequirementSource::Git`] from a previously locked git URL +/// preserving the git reference (branch, tag, or rev) so that /// the resolver can use the same reference when re-resolving. -/// -/// # Arguments -/// * `location` - The locked git URL from the lock file -/// -/// # Returns -/// A `RequirementSource::Git` that can be used as a constraint for the resolver. pub(crate) fn locked_git_url_to_requirement_source( location: &Url, ) -> Result { From 18ea574130e6767b34f1648728fb50ca59c457e5 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 30 Dec 2025 15:14:37 +0200 Subject: [PATCH 04/11] misc: remove enriching requirements --- .../pixi_core/src/lock_file/resolve/pypi.rs | 185 +----------------- .../src/lock_file/satisfiability/mod.rs | 26 ++- 2 files changed, 31 insertions(+), 180 deletions(-) diff --git a/crates/pixi_core/src/lock_file/resolve/pypi.rs b/crates/pixi_core/src/lock_file/resolve/pypi.rs index 72ae23392d..bbbe9be049 100644 --- a/crates/pixi_core/src/lock_file/resolve/pypi.rs +++ b/crates/pixi_core/src/lock_file/resolve/pypi.rs @@ -18,14 +18,12 @@ use indicatif::ProgressBar; use itertools::{Either, Itertools}; use miette::{Context, IntoDiagnostic}; use pixi_consts::consts; -use pixi_git::git::GitReference; use pixi_manifest::{ EnvironmentName, SolveStrategy, SystemRequirements, pypi::pypi_options::PypiOptions, }; use pixi_pypi_spec::PixiPypiSpec; use pixi_record::{LockedGitUrl, PixiRecord}; use pixi_reporters::{UvReporter, UvReporterOptions}; -use pixi_uv_conversions::into_uv_git_reference; use pixi_uv_conversions::{ ConversionError, as_uv_req, configure_insecure_hosts_for_tls_bypass, convert_uv_requirements_to_pep508, into_pinned_git_spec, pypi_options_to_build_options, @@ -52,9 +50,7 @@ use uv_distribution_types::{ IndexCapabilities, IndexUrl, Name, RequirementSource, RequiresPython, Resolution, ResolvedDist, SourceDist, ToUrlError, }; -use uv_pep508::VerbatimUrl; use uv_pypi_types::{Conflicts, HashAlgorithm, HashDigests}; -use uv_redacted::DisplaySafeUrl; use uv_requirements::LookaheadResolver; use uv_resolver::{ AllowedYanks, DefaultResolverProvider, FlatIndex, InMemoryIndex, Manifest, Options, Preference, @@ -121,51 +117,6 @@ fn parse_hashes_from_hash_vec(hashes: &HashDigests) -> Result Result { - let git_locked_url = LockedGitUrl::from(location.clone()); - let pinned_git_spec = git_locked_url - .to_pinned_git_spec() - .map_err(|e| LockedGitSourceError::Parse(e.to_string()))?; - - let verbatim_url = VerbatimUrl::from(location.clone()); - - let display_safe = DisplaySafeUrl::from(pinned_git_spec.git.clone()); - - let git_oid = uv_git_types::GitOid::from_str(&pinned_git_spec.source.commit.to_string())?; - - // Convert the pixi git reference to uv git reference - // This preserves the branch/tag/rev from the locked spec - let uv_reference = into_uv_git_reference(pinned_git_spec.source.reference.clone().into()); - - // Create the GitUrl with the correct reference and precise commit - let git_url = uv_git_types::GitUrl::from_commit(display_safe, uv_reference, git_oid)?; - - Ok(RequirementSource::Git { - git: git_url, - subdirectory: pinned_git_spec - .source - .subdirectory - .map(|s| PathBuf::from(s).into_boxed_path()), - url: verbatim_url, - }) -} - #[derive(Debug, thiserror::Error)] enum ProcessPathUrlError { #[error("expected given path for {0} but none found")] @@ -392,7 +343,7 @@ pub async fn resolve_pypi( tracing::info!("there are no python packages installed by conda"); } - let mut requirements = dependencies + let requirements = dependencies .into_iter() .flat_map(|(name, req)| { req.into_iter() @@ -624,9 +575,8 @@ pub async fn resolve_pypi( #[error(transparent)] GitUrlParse(#[from] uv_git_types::GitUrlParseError), - - #[error("{0}")] - Other(miette::ErrReport), + // #[error("{0}")] + // Other(miette::ErrReport), } // Create preferences from the locked pypi packages @@ -656,31 +606,12 @@ pub async fn resolve_pypi( // This will help the resolver to pick the commit that we already have locked // instead of updating to a newer commit that also matches the requirement. if let Some(location) = package_data.location.as_url() { - // now check if it's a git url + // For git URLs, we don't use locked constraints to enrich requirements. + // This is because uv's resolver cache is keyed by (url, reference), and + // setting a precise commit from the lock file without going through uv's + // normal resolution flow causes cache lookup failures and panics. if LockedGitUrl::is_locked_git_url(location) { - // Create a constraint source from the locked git URL, - // preserving the git reference (branch/tag/rev) - let constraint_source = locked_git_url_to_requirement_source(location) - .map_err(|e| PixiPreferencesError::Other(miette::miette!("{e}")))?; - - // find this requirements in dependencies and skip adding it as preference - let req_from_dep = requirements.iter_mut().find(|r| r.name == requirement.name); - if let Some(req) = req_from_dep { - // we need to update the requirement source in the requirements list - // to use the precise git commit - // only if the requirements do not already have a source set with something specific - if let RequirementSource::Git { git, .. } = &req.source { - // only update if the git url does not already have a precise commit - if git.precise().is_none() && !GitReference::looks_like_commit_hash(git.reference().as_rev()) { - tracing::debug!( - "updating requirement source to precise git commit for requirement: {:?}", - &req - ); - req.source = constraint_source.clone(); - } - - } - } + return Ok(None); } Ok(None) } else { @@ -1323,104 +1254,4 @@ mod tests { .unwrap(); assert_eq!(path.as_str(), "C:/a/b/c"); } - - /// Test for issue #5185: git reference must be preserved when enriching - /// requirements from locked packages. - /// - /// When a locked git package has a specific reference (e.g., `branch=master`), - /// the constraint_source created from it should preserve that reference. - #[test] - fn test_git_reference_preserved_in_constraint_source() { - use super::locked_git_url_to_requirement_source; - - // Create a locked git URL with a specific branch reference - // This simulates what's stored in the lock file for a git dependency with `rev = "master"` - let locked_url = url::Url::parse( - "git+https://github.com/example/repo.git?branch=master#abc123def456abc123def456abc123def456abc1", - ) - .unwrap(); - - // Use the extracted function to create the constraint source - let constraint_source = locked_git_url_to_requirement_source(&locked_url).unwrap(); - - // Verify that the constraint source has the correct git reference - match constraint_source { - uv_distribution_types::RequirementSource::Git { git, .. } => { - assert_eq!( - git.reference(), - &uv_git_types::GitReference::Branch("master".into()), - "The git reference should be preserved from the locked spec. \ - This is the fix for issue #5185." - ); - } - _ => panic!("Expected RequirementSource::Git"), - } - } - - /// Test that various git reference types are preserved correctly. - #[test] - fn test_git_reference_types_preserved() { - use super::locked_git_url_to_requirement_source; - - // Test branch reference - let url_with_branch = url::Url::parse( - "git+https://github.com/example/repo.git?branch=develop#abc123def456abc123def456abc123def456abc1", - ).unwrap(); - let source = locked_git_url_to_requirement_source(&url_with_branch).unwrap(); - if let uv_distribution_types::RequirementSource::Git { git, .. } = source { - assert_eq!( - git.reference(), - &uv_git_types::GitReference::Branch("develop".into()) - ); - } - - // Test tag reference - let url_with_tag = url::Url::parse( - "git+https://github.com/example/repo.git?tag=v1.0.0#abc123def456abc123def456abc123def456abc1", - ).unwrap(); - let source = locked_git_url_to_requirement_source(&url_with_tag).unwrap(); - if let uv_distribution_types::RequirementSource::Git { git, .. } = source { - assert_eq!( - git.reference(), - &uv_git_types::GitReference::Tag("v1.0.0".into()) - ); - } - - // Test rev reference (short string that doesn't look like a commit hash) - // "abc123" is only 6 chars, so it becomes BranchOrTag (needs 7+ hex chars for commit hash) - let url_with_rev = url::Url::parse( - "git+https://github.com/example/repo.git?rev=abc123#abc123def456abc123def456abc123def456abc1", - ).unwrap(); - let source = locked_git_url_to_requirement_source(&url_with_rev).unwrap(); - if let uv_distribution_types::RequirementSource::Git { git, .. } = source { - // Rev with <7 chars becomes BranchOrTag in uv - assert!(matches!( - git.reference(), - uv_git_types::GitReference::BranchOrTag(r) if r == "abc123" - )); - } - - // Test rev reference with commit-like hash (7+ hex chars) - let url_with_commit_rev = url::Url::parse( - "git+https://github.com/example/repo.git?rev=abc1234#abc123def456abc123def456abc123def456abc1", - ).unwrap(); - let source = locked_git_url_to_requirement_source(&url_with_commit_rev).unwrap(); - if let uv_distribution_types::RequirementSource::Git { git, .. } = source { - // Rev with 7+ hex chars becomes BranchOrTagOrCommit in uv - assert!(matches!( - git.reference(), - uv_git_types::GitReference::BranchOrTagOrCommit(r) if r == "abc1234" - )); - } - - // Test default branch (no explicit reference) - let url_default = url::Url::parse( - "git+https://github.com/example/repo.git#abc123def456abc123def456abc123def456abc1", - ) - .unwrap(); - let source = locked_git_url_to_requirement_source(&url_default).unwrap(); - if let uv_distribution_types::RequirementSource::Git { git, .. } = source { - assert_eq!(git.reference(), &uv_git_types::GitReference::DefaultBranch); - } - } } diff --git a/crates/pixi_core/src/lock_file/satisfiability/mod.rs b/crates/pixi_core/src/lock_file/satisfiability/mod.rs index 6dcf2d4da5..050c0f0443 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/mod.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/mod.rs @@ -970,10 +970,30 @@ pub(crate) fn pypi_satifisfies_requirement( } .into()); } - // If the spec does not specify a revision than any will do - // E.g `git.com/user/repo` is the same as `git.com/user/repo@adbdd` + // If the spec uses DefaultBranch, we need to check what the lock has: + // - If lock has Branch("main") or Tag("v1.0") → NOT satisfiable + // (user removed explicit branch/tag, should re-resolve) + // - If lock has DefaultBranch or Rev (specific commit) → satisfiable + // (specific commit is still valid even without explicit ref) if *reference == GitReference::DefaultBranch { - return Ok(()); + match &pinned_git_spec.source.reference { + // These are explicit named references - not satisfiable + // when manifest has DefaultBranch (user removed the explicit ref) + pixi_spec::GitReference::Branch(_) + | pixi_spec::GitReference::Tag(_) => { + return Err(PlatformUnsat::LockedPyPIGitRefMismatch { + name: spec.name.clone().to_string(), + expected_ref: reference.to_string(), + found_ref: pinned_git_spec.source.reference.to_string(), + } + .into()); + } + // These are satisfiable - either DefaultBranch or specific commits + pixi_spec::GitReference::DefaultBranch + | pixi_spec::GitReference::Rev(_) => { + return Ok(()); + } + } } if pinned_git_spec.source.subdirectory From 0b1577b3c9c0b20f427b688eeebfadf1a4c76420 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 30 Dec 2025 15:54:44 +0200 Subject: [PATCH 05/11] misc: return back enrichment logic --- .../pixi_core/src/lock_file/resolve/pypi.rs | 102 +++++++++++++++++- 1 file changed, 97 insertions(+), 5 deletions(-) diff --git a/crates/pixi_core/src/lock_file/resolve/pypi.rs b/crates/pixi_core/src/lock_file/resolve/pypi.rs index bbbe9be049..c282cfb2d1 100644 --- a/crates/pixi_core/src/lock_file/resolve/pypi.rs +++ b/crates/pixi_core/src/lock_file/resolve/pypi.rs @@ -50,7 +50,9 @@ use uv_distribution_types::{ IndexCapabilities, IndexUrl, Name, RequirementSource, RequiresPython, Resolution, ResolvedDist, SourceDist, ToUrlError, }; +use uv_pep508::VerbatimUrl; use uv_pypi_types::{Conflicts, HashAlgorithm, HashDigests}; +use uv_redacted::DisplaySafeUrl; use uv_requirements::LookaheadResolver; use uv_resolver::{ AllowedYanks, DefaultResolverProvider, FlatIndex, InMemoryIndex, Manifest, Options, Preference, @@ -117,6 +119,51 @@ fn parse_hashes_from_hash_vec(hashes: &HashDigests) -> Result commit mapping before using this, otherwise uv's redirect +/// mechanism will panic in debug mode. +pub(crate) fn locked_git_url_to_requirement_source( + location: &Url, +) -> Result { + let git_locked_url = LockedGitUrl::from(location.clone()); + let pinned_git_spec = git_locked_url + .to_pinned_git_spec() + .map_err(|e| LockedGitSourceError::Parse(e.to_string()))?; + + let verbatim_url = VerbatimUrl::from(location.clone()); + + let display_safe = DisplaySafeUrl::from(pinned_git_spec.git.clone()); + + let git_oid = uv_git_types::GitOid::from_str(&pinned_git_spec.source.commit.to_string())?; + + // Use try_from + with_precise. The cache should already have the + // (url, DefaultBranch) -> commit mapping from pre-population. + let git_url = uv_git_types::GitUrl::try_from(display_safe)?.with_precise(git_oid); + + Ok(RequirementSource::Git { + git: git_url, + subdirectory: pinned_git_spec + .source + .subdirectory + .map(|s| PathBuf::from(s).into_boxed_path()), + url: verbatim_url, + }) +} + #[derive(Debug, thiserror::Error)] enum ProcessPathUrlError { #[error("expected given path for {0} but none found")] @@ -343,7 +390,7 @@ pub async fn resolve_pypi( tracing::info!("there are no python packages installed by conda"); } - let requirements = dependencies + let mut requirements = dependencies .into_iter() .flat_map(|(name, req)| { req.into_iter() @@ -579,6 +626,34 @@ pub async fn resolve_pypi( // Other(miette::ErrReport), } + // Pre-populate the GitResolver cache with locked git commits. + // This is necessary because when we enrich requirements with locked precise commits, + // uv's redirect mechanism looks up the commit in the GitResolver cache. + // Without pre-population, the lookup fails and causes a panic in debug mode. + for (package_data, _) in locked_pypi_packages.iter() { + if let Some(location) = package_data.location.as_url() { + if LockedGitUrl::is_locked_git_url(location) { + if let Ok(pinned_git_spec) = + LockedGitUrl::from(location.clone()).to_pinned_git_spec() + { + // Create the repository reference that matches what the manifest will use + let display_safe = + uv_redacted::DisplaySafeUrl::from(pinned_git_spec.git.clone()); + if let Ok(git_url) = uv_git_types::GitUrl::try_from(display_safe) { + // Parse the commit SHA + if let Ok(git_oid) = uv_git_types::GitOid::from_str( + &pinned_git_spec.source.commit.to_string(), + ) { + // Insert the (url, reference) -> commit mapping into the cache + let repo_ref = uv_git::RepositoryReference::from(&git_url); + context.shared_state.git().insert(repo_ref, git_oid); + } + } + } + } + } + } + // Create preferences from the locked pypi packages // This will ensure minimal lock file updates let preferences = locked_pypi_packages @@ -606,11 +681,28 @@ pub async fn resolve_pypi( // This will help the resolver to pick the commit that we already have locked // instead of updating to a newer commit that also matches the requirement. if let Some(location) = package_data.location.as_url() { - // For git URLs, we don't use locked constraints to enrich requirements. - // This is because uv's resolver cache is keyed by (url, reference), and - // setting a precise commit from the lock file without going through uv's - // normal resolution flow causes cache lookup failures and panics. + // Check if it's a git URL that we can enrich with locked commit if LockedGitUrl::is_locked_git_url(location) { + // Find this requirement in the requirements list + let req_from_dep = requirements.iter_mut().find(|r| r.name == requirement.name); + if let Some(req) = req_from_dep { + // Only enrich git requirements that don't already have a precise commit + if let RequirementSource::Git { git, .. } = &req.source { + if git.precise().is_none() { + // Create the constraint source with precise commit + // The GitResolver cache was pre-populated above, so this won't panic + if let Ok(constraint_source) = + locked_git_url_to_requirement_source(location) + { + tracing::debug!( + "updating requirement source to precise git commit for requirement: {:?}", + &req + ); + req.source = constraint_source; + } + } + } + } return Ok(None); } Ok(None) From 299a03f3ed3546fcc013510db99ebfb7b17bf8b3 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 30 Dec 2025 17:40:55 +0200 Subject: [PATCH 06/11] misc: preserve the reference --- .../pixi_core/src/lock_file/resolve/pypi.rs | 25 ++++++++++++++++++- crates/pixi_uv_conversions/src/conversions.rs | 16 ++++++++++-- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/crates/pixi_core/src/lock_file/resolve/pypi.rs b/crates/pixi_core/src/lock_file/resolve/pypi.rs index c282cfb2d1..d3ac2d701d 100644 --- a/crates/pixi_core/src/lock_file/resolve/pypi.rs +++ b/crates/pixi_core/src/lock_file/resolve/pypi.rs @@ -390,6 +390,20 @@ pub async fn resolve_pypi( tracing::info!("there are no python packages installed by conda"); } + // Build a lookup map of original git references before consuming dependencies. + // This is used later to preserve branch/tag info in the lock file that uv normalizes away. + let original_git_references = dependencies + .iter() + .filter_map(|(name, specs)| { + specs.iter().find_map(|spec| { + spec.source + .as_git() + .and_then(|git_spec| git_spec.rev.clone()) + .map(|rev| (name.clone(), rev)) + }) + }) + .collect(); + let mut requirements = dependencies .into_iter() .flat_map(|(name, req)| { @@ -870,6 +884,7 @@ pub async fn resolve_pypi( &context.capabilities, context.concurrency.downloads, project_root, + &original_git_references, ) .await?; @@ -1000,6 +1015,7 @@ async fn lock_pypi_packages( index_capabilities: &IndexCapabilities, concurrent_downloads: usize, abs_project_root: &Path, + original_git_references: &HashMap, ) -> miette::Result> { let mut locked_packages = LockedPypiPackages::with_capacity(resolution.len()); let database = @@ -1119,8 +1135,15 @@ async fn lock_pypi_packages( (direct_url.into(), hash, false) } SourceDist::Git(git) => { + // Look up the original git reference from the manifest dependencies + // to preserve branch/tag info that uv normalizes away + let package_name = uv_normalize::PackageName::from(git.name.clone()); + let original_reference = + original_git_references.get(&package_name).cloned(); + // convert resolved source dist into a pinned git spec - let pinned_git_spec = into_pinned_git_spec(git.clone()); + let pinned_git_spec = + into_pinned_git_spec(git.clone(), original_reference); ( pinned_git_spec.into_locked_git_url().to_url().into(), hash, diff --git a/crates/pixi_uv_conversions/src/conversions.rs b/crates/pixi_uv_conversions/src/conversions.rs index 17ef4432f1..3de3825ef3 100644 --- a/crates/pixi_uv_conversions/src/conversions.rs +++ b/crates/pixi_uv_conversions/src/conversions.rs @@ -292,8 +292,20 @@ pub fn into_pixi_reference(git_reference: uv_git_types::GitReference) -> PixiRef } /// Convert a solved [`GitSourceDist`] into [`PinnedGitSpec`] -pub fn into_pinned_git_spec(dist: GitSourceDist) -> PinnedGitSpec { - let reference = into_pixi_reference(dist.git.reference().clone()); +/// +/// The `original_reference` parameter allows preserving the original git reference +/// from the manifest (e.g., `Branch("main")`). When uv resolves a git dependency, +/// it may normalize branch references to `DefaultBranch`, losing the original +/// branch information. If provided, this original reference will be used instead +/// of the one from uv's resolution. +pub fn into_pinned_git_spec( + dist: GitSourceDist, + original_reference: Option, +) -> PinnedGitSpec { + // Use the original reference from the manifest if provided, + // otherwise fall back to what uv returned + let reference = + original_reference.unwrap_or_else(|| into_pixi_reference(dist.git.reference().clone())); // Necessary to convert between our gitsha and uv gitsha. let git_sha = PixiGitSha::from_str( From 9f820baa9de4dca4ae952fe229d5c969c66137d1 Mon Sep 17 00:00:00 2001 From: nichmor Date: Thu, 1 Jan 2026 22:26:53 +0200 Subject: [PATCH 07/11] misc: prepopulate git solver --- Cargo.lock | 1 + Cargo.toml | 1 + .../pixi/tests/integration_rust/add_tests.rs | 126 +++++++++++++ crates/pixi_core/Cargo.toml | 1 + .../pixi_core/src/lock_file/resolve/pypi.rs | 178 ++++++------------ crates/pixi_uv_conversions/src/conversions.rs | 16 +- 6 files changed, 198 insertions(+), 125 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 26607db938..5fd79b7bcc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5395,6 +5395,7 @@ dependencies = [ "url", "uv-build-frontend", "uv-cache", + "uv-cache-key", "uv-client", "uv-configuration", "uv-dispatch", diff --git a/Cargo.toml b/Cargo.toml index 5a06f8c1f1..e3fd98d14c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -189,6 +189,7 @@ uv-auth = { git = "https://github.com/astral-sh/uv", tag = "0.9.5" } uv-build-frontend = { git = "https://github.com/astral-sh/uv", tag = "0.9.5" } uv-cache = { git = "https://github.com/astral-sh/uv", tag = "0.9.5" } uv-cache-info = { git = "https://github.com/astral-sh/uv", tag = "0.9.5" } +uv-cache-key = { git = "https://github.com/astral-sh/uv", tag = "0.9.5" } uv-client = { git = "https://github.com/astral-sh/uv", tag = "0.9.5" } uv-configuration = { git = "https://github.com/astral-sh/uv", tag = "0.9.5" } uv-dispatch = { git = "https://github.com/astral-sh/uv", tag = "0.9.5" } diff --git a/crates/pixi/tests/integration_rust/add_tests.rs b/crates/pixi/tests/integration_rust/add_tests.rs index 64599a9b8f..dbd06befef 100644 --- a/crates/pixi/tests/integration_rust/add_tests.rs +++ b/crates/pixi/tests/integration_rust/add_tests.rs @@ -1095,6 +1095,132 @@ platforms = ["{platform}"] }); } +/// Test that git branch references are correctly preserved in lock files. +/// This tests issue #5185: git references should be preserved across lock file updates. +#[tokio::test] +#[cfg_attr(not(feature = "online_tests"), ignore)] +async fn test_pypi_git_branch_preservation() { + setup_tracing(); + + let pixi = PixiControl::from_manifest( + format!( + r#" +[workspace] +name = "test-git-branch-preservation" +channels = ["https://prefix.dev/conda-forge"] +platforms = ["{platform}"] + +[dependencies] +python = ">=3.13" +"#, + platform = Platform::current() + ) + .as_str(), + ) + .unwrap(); + + // Step 1: Add git dependency WITHOUT branch + pixi.add("boltons") + .set_pypi(true) + .with_git_url(Url::parse("https://github.com/mahmoud/boltons.git").unwrap()) + .await + .unwrap(); + + // Verify: lock file should NOT have ?branch= when no branch specified + let lock = pixi.lock_file().await.unwrap(); + let url1 = lock + .default_environment() + .unwrap() + .pypi_packages(Platform::current()) + .unwrap() + .find(|(p, _)| p.name.to_string() == "boltons") + .map(|(p, _)| p.location.to_string()) + .unwrap(); + assert!( + !url1.contains("branch="), + "Should not have branch= without manifest branch, got: {url1}" + ); + + // Step 2: Update manifest to ADD branch = "master" (boltons uses master, not main) + pixi.update_manifest( + format!( + r#" +[workspace] +name = "test-git-branch-preservation" +channels = ["https://prefix.dev/conda-forge"] +platforms = ["{platform}"] + +[dependencies] +python = ">=3.13" + +[pypi-dependencies] +boltons = {{ git = "https://github.com/mahmoud/boltons.git", branch = "master" }} +"#, + platform = Platform::current() + ) + .as_str(), + ) + .unwrap(); + + // Re-lock to update + pixi.lock().await.unwrap(); + + // Verify: lock file should have ?branch=master + let lock = pixi.lock_file().await.unwrap(); + let url2 = lock + .default_environment() + .unwrap() + .pypi_packages(Platform::current()) + .unwrap() + .find(|(p, _)| p.name.to_string() == "boltons") + .map(|(p, _)| p.location.to_string()) + .unwrap(); + assert!( + url2.contains("branch=master"), + "Should have branch=master after adding to manifest, got: {url2}" + ); + + // Step 3: Update manifest to REMOVE branch + pixi.update_manifest( + format!( + r#" +[workspace] +name = "test-git-branch-preservation" +channels = ["https://prefix.dev/conda-forge"] +platforms = ["{platform}"] + +[dependencies] +python = ">=3.13" + +[pypi-dependencies] +boltons = {{ git = "https://github.com/mahmoud/boltons.git" }} +"#, + platform = Platform::current() + ) + .as_str(), + ) + .unwrap(); + + // Re-lock to update + pixi.lock().await.unwrap(); + + // Verify: lock file should NOT have branch=main anymore + let lock = pixi.lock_file().await.unwrap(); + let url3 = lock + .default_environment() + .unwrap() + .pypi_packages(Platform::current()) + .unwrap() + .find(|(p, _)| p.name.to_string() == "boltons") + .map(|(p, _)| p.location.to_string()) + .unwrap(); + assert!( + !url3.contains("branch="), + "Should not have branch= after removing from manifest, got: {url3}" + ); + // The URL may have rev= or just #commit, but should NOT have branch= +} + #[tokio::test] async fn add_git_dependency_without_preview_feature_fails() { setup_tracing(); diff --git a/crates/pixi_core/Cargo.toml b/crates/pixi_core/Cargo.toml index f55d26176b..e67a764571 100644 --- a/crates/pixi_core/Cargo.toml +++ b/crates/pixi_core/Cargo.toml @@ -79,6 +79,7 @@ typed-path = { workspace = true } url = { workspace = true } uv-build-frontend = { workspace = true } uv-cache = { workspace = true } +uv-cache-key = { workspace = true } uv-client = { workspace = true } uv-configuration = { workspace = true } uv-dispatch = { workspace = true } diff --git a/crates/pixi_core/src/lock_file/resolve/pypi.rs b/crates/pixi_core/src/lock_file/resolve/pypi.rs index d3ac2d701d..88730ac241 100644 --- a/crates/pixi_core/src/lock_file/resolve/pypi.rs +++ b/crates/pixi_core/src/lock_file/resolve/pypi.rs @@ -26,9 +26,10 @@ use pixi_record::{LockedGitUrl, PixiRecord}; use pixi_reporters::{UvReporter, UvReporterOptions}; use pixi_uv_conversions::{ ConversionError, as_uv_req, configure_insecure_hosts_for_tls_bypass, - convert_uv_requirements_to_pep508, into_pinned_git_spec, pypi_options_to_build_options, - pypi_options_to_index_locations, to_exclude_newer, to_index_strategy, to_normalize, - to_prerelease_mode, to_requirements, to_uv_normalize, to_uv_version, to_version_specifiers, + convert_uv_requirements_to_pep508, into_pinned_git_spec, into_uv_git_reference, + into_uv_git_sha, pypi_options_to_build_options, pypi_options_to_index_locations, + to_exclude_newer, to_index_strategy, to_normalize, to_prerelease_mode, to_requirements, + to_uv_normalize, to_uv_version, to_version_specifiers, }; use pypi_modifiers::{ pypi_marker_env::determine_marker_environment, @@ -40,6 +41,7 @@ use rattler_lock::{ }; use typed_path::Utf8TypedPathBuf; use url::Url; +use uv_cache_key::RepositoryUrl; use uv_client::{ BaseClientBuilder, Connectivity, FlatIndexClient, RegistryClient, RegistryClientBuilder, }; @@ -50,9 +52,8 @@ use uv_distribution_types::{ IndexCapabilities, IndexUrl, Name, RequirementSource, RequiresPython, Resolution, ResolvedDist, SourceDist, ToUrlError, }; -use uv_pep508::VerbatimUrl; +use uv_git::RepositoryReference; use uv_pypi_types::{Conflicts, HashAlgorithm, HashDigests}; -use uv_redacted::DisplaySafeUrl; use uv_requirements::LookaheadResolver; use uv_resolver::{ AllowedYanks, DefaultResolverProvider, FlatIndex, InMemoryIndex, Manifest, Options, Preference, @@ -119,51 +120,6 @@ fn parse_hashes_from_hash_vec(hashes: &HashDigests) -> Result commit mapping before using this, otherwise uv's redirect -/// mechanism will panic in debug mode. -pub(crate) fn locked_git_url_to_requirement_source( - location: &Url, -) -> Result { - let git_locked_url = LockedGitUrl::from(location.clone()); - let pinned_git_spec = git_locked_url - .to_pinned_git_spec() - .map_err(|e| LockedGitSourceError::Parse(e.to_string()))?; - - let verbatim_url = VerbatimUrl::from(location.clone()); - - let display_safe = DisplaySafeUrl::from(pinned_git_spec.git.clone()); - - let git_oid = uv_git_types::GitOid::from_str(&pinned_git_spec.source.commit.to_string())?; - - // Use try_from + with_precise. The cache should already have the - // (url, DefaultBranch) -> commit mapping from pre-population. - let git_url = uv_git_types::GitUrl::try_from(display_safe)?.with_precise(git_oid); - - Ok(RequirementSource::Git { - git: git_url, - subdirectory: pinned_git_spec - .source - .subdirectory - .map(|s| PathBuf::from(s).into_boxed_path()), - url: verbatim_url, - }) -} - #[derive(Debug, thiserror::Error)] enum ProcessPathUrlError { #[error("expected given path for {0} but none found")] @@ -404,7 +360,37 @@ pub async fn resolve_pypi( }) .collect(); - let mut requirements = dependencies + // Pre-populate the git resolver with locked git references. + // This ensures that when uv resolves git dependencies, it will find the cached commit + // and not panic in `url_to_precise` function. + for (package_data, _) in locked_pypi_packages { + if let Some(location) = package_data.location.as_url() + && LockedGitUrl::is_locked_git_url(location) + { + let locked_url = LockedGitUrl::new(location.clone()); + if let Ok(pinned_git_spec) = locked_url.to_pinned_git_spec() { + // Convert pixi types to uv types and insert into the git resolver + // pixi_spec::GitReference -> pixi_git::git::GitReference -> uv_git_types::GitReference + let pixi_git_ref = pinned_git_spec.source.reference.clone().into(); + + let uv_reference = into_uv_git_reference(pixi_git_ref); + let uv_sha = into_uv_git_sha(pinned_git_spec.source.commit); + + let display_safe_url = pinned_git_spec.git.clone().into(); + + let repository_url = RepositoryUrl::new(&display_safe_url); + let reference = RepositoryReference { + url: repository_url, + reference: uv_reference, + }; + + tracing::debug!("Pre-populating git resolver: {:?} -> {}", reference, uv_sha); + context.shared_state.git().insert(reference, uv_sha); + } + } + } + + let requirements = dependencies .into_iter() .flat_map(|(name, req)| { req.into_iter() @@ -640,34 +626,6 @@ pub async fn resolve_pypi( // Other(miette::ErrReport), } - // Pre-populate the GitResolver cache with locked git commits. - // This is necessary because when we enrich requirements with locked precise commits, - // uv's redirect mechanism looks up the commit in the GitResolver cache. - // Without pre-population, the lookup fails and causes a panic in debug mode. - for (package_data, _) in locked_pypi_packages.iter() { - if let Some(location) = package_data.location.as_url() { - if LockedGitUrl::is_locked_git_url(location) { - if let Ok(pinned_git_spec) = - LockedGitUrl::from(location.clone()).to_pinned_git_spec() - { - // Create the repository reference that matches what the manifest will use - let display_safe = - uv_redacted::DisplaySafeUrl::from(pinned_git_spec.git.clone()); - if let Ok(git_url) = uv_git_types::GitUrl::try_from(display_safe) { - // Parse the commit SHA - if let Ok(git_oid) = uv_git_types::GitOid::from_str( - &pinned_git_spec.source.commit.to_string(), - ) { - // Insert the (url, reference) -> commit mapping into the cache - let repo_ref = uv_git::RepositoryReference::from(&git_url); - context.shared_state.git().insert(repo_ref, git_oid); - } - } - } - } - } - } - // Create preferences from the locked pypi packages // This will ensure minimal lock file updates let preferences = locked_pypi_packages @@ -688,47 +646,26 @@ pub async fn resolve_pypi( origin: None, }; - // When iterating over locked packages, - // instead of adding git requirements as preferences - // we enrich previous defined requirements in the `requirements` list - // as they have been pinned to a precise commit - // This will help the resolver to pick the commit that we already have locked - // instead of updating to a newer commit that also matches the requirement. - if let Some(location) = package_data.location.as_url() { - // Check if it's a git URL that we can enrich with locked commit - if LockedGitUrl::is_locked_git_url(location) { - // Find this requirement in the requirements list - let req_from_dep = requirements.iter_mut().find(|r| r.name == requirement.name); - if let Some(req) = req_from_dep { - // Only enrich git requirements that don't already have a precise commit - if let RequirementSource::Git { git, .. } = &req.source { - if git.precise().is_none() { - // Create the constraint source with precise commit - // The GitResolver cache was pre-populated above, so this won't panic - if let Ok(constraint_source) = - locked_git_url_to_requirement_source(location) - { - tracing::debug!( - "updating requirement source to precise git commit for requirement: {:?}", - &req - ); - req.source = constraint_source; - } - } - } - } - return Ok(None); - } - Ok(None) - } else { - let named = uv_requirements_txt::RequirementsTxtRequirement::Named(requirement); - let entry = uv_requirements_txt::RequirementEntry { - requirement: named, - hashes: Default::default(), - }; - - Ok(Preference::from_entry(entry)?) + // For git packages, we don't add them as preferences. + // because they are resolved based on the reference (branch/tag/rev) in the manifest. + // This matches how uv handles git dependencies - it doesn't try to pin them via preferences. + // The git resolver cache (pre-populated above) ensures the locked commit is preferred. + if let Some(location) = package_data.location.as_url() + && LockedGitUrl::is_locked_git_url(location) + { + // Skip git packages - they'll be resolved based on manifest reference + // with the cached commit from the git resolver + return Ok(None); } + + // Create preference for registry and URL packages + let named = uv_requirements_txt::RequirementsTxtRequirement::Named(requirement); + let entry = uv_requirements_txt::RequirementEntry { + requirement: named, + hashes: Default::default(), + }; + + Ok(Preference::from_entry(entry)?) }) .filter_map(|pref| pref.transpose()) .collect::, PixiPreferencesError>>() @@ -1007,6 +944,7 @@ fn get_url_or_path( } /// Create a vector of locked packages from a resolution +#[allow(clippy::too_many_arguments)] async fn lock_pypi_packages( conda_python_packages: CondaPythonPackages, pixi_build_dispatch: &LazyBuildDispatch<'_>, @@ -1137,7 +1075,7 @@ async fn lock_pypi_packages( SourceDist::Git(git) => { // Look up the original git reference from the manifest dependencies // to preserve branch/tag info that uv normalizes away - let package_name = uv_normalize::PackageName::from(git.name.clone()); + let package_name = git.name.clone(); let original_reference = original_git_references.get(&package_name).cloned(); diff --git a/crates/pixi_uv_conversions/src/conversions.rs b/crates/pixi_uv_conversions/src/conversions.rs index 3de3825ef3..86487730c5 100644 --- a/crates/pixi_uv_conversions/src/conversions.rs +++ b/crates/pixi_uv_conversions/src/conversions.rs @@ -298,15 +298,15 @@ pub fn into_pixi_reference(git_reference: uv_git_types::GitReference) -> PixiRef /// it may normalize branch references to `DefaultBranch`, losing the original /// branch information. If provided, this original reference will be used instead /// of the one from uv's resolution. +/// +/// If no original reference is provided (user didn't specify branch/tag/rev), +/// we store the resolved commit as `Rev(commit)` rather than `DefaultBranch`. +/// This ensures the lock file has a precise reference that doesn't require +/// cache lookups when re-resolving (similar to how uv's lockfile works). pub fn into_pinned_git_spec( dist: GitSourceDist, original_reference: Option, ) -> PinnedGitSpec { - // Use the original reference from the manifest if provided, - // otherwise fall back to what uv returned - let reference = - original_reference.unwrap_or_else(|| into_pixi_reference(dist.git.reference().clone())); - // Necessary to convert between our gitsha and uv gitsha. let git_sha = PixiGitSha::from_str( &dist @@ -317,6 +317,12 @@ pub fn into_pinned_git_spec( ) .expect("we expect it to be a valid sha"); + // Use the original reference from the manifest if provided. + // If no explicit reference was specified (DefaultBranch), store the resolved + // commit as Rev(commit). This avoids DefaultBranch which requires cache lookups + // and can cause panics when re-resolving. + let reference = original_reference.unwrap_or_else(|| PixiReference::Rev(git_sha.to_string())); + let pinned_checkout = PinnedGitCheckout::new( git_sha, dist.subdirectory.map(|sd| sd.to_string_lossy().to_string()), From e672d5a08980e12e1da8e5518b787e443010c048 Mon Sep 17 00:00:00 2001 From: nichmor Date: Thu, 1 Jan 2026 22:39:31 +0200 Subject: [PATCH 08/11] misc: remove comment --- crates/pixi/tests/integration_rust/add_tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/pixi/tests/integration_rust/add_tests.rs b/crates/pixi/tests/integration_rust/add_tests.rs index dbd06befef..d8337c8ba6 100644 --- a/crates/pixi/tests/integration_rust/add_tests.rs +++ b/crates/pixi/tests/integration_rust/add_tests.rs @@ -1218,7 +1218,6 @@ boltons = {{ git = "https://github.com/mahmoud/boltons.git" }} !url3.contains("branch="), "Should not have branch= after removing from manifest, got: {url3}" ); - // The URL may have rev= or just #commit, but should NOT have branch= } #[tokio::test] From 7cbf5a05251199f0f1a5c41ff1b6eb42ad6c47fb Mon Sep 17 00:00:00 2001 From: nichmor Date: Sun, 11 Jan 2026 17:22:50 +0200 Subject: [PATCH 09/11] misc: fix satisfability in the lockfile --- .../src/lock_file/satisfiability/mod.rs | 47 ++++++++++++++----- crates/pixi_uv_conversions/src/conversions.rs | 8 ++-- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/crates/pixi_core/src/lock_file/satisfiability/mod.rs b/crates/pixi_core/src/lock_file/satisfiability/mod.rs index 39e513dec8..1a6e507f3a 100644 --- a/crates/pixi_core/src/lock_file/satisfiability/mod.rs +++ b/crates/pixi_core/src/lock_file/satisfiability/mod.rs @@ -970,17 +970,16 @@ pub(crate) fn pypi_satifisfies_requirement( } .into()); } - // If the spec uses DefaultBranch, we need to check what the lock has: - // - If lock has Branch("main") or Tag("v1.0") → NOT satisfiable - // (user removed explicit branch/tag, should re-resolve) - // - If lock has DefaultBranch or Rev (specific commit) → satisfiable - // (specific commit is still valid even without explicit ref) + // If the spec uses DefaultBranch, we need to check what the lock has + // DefaultBranch in it + // otherwise any explicit ref in lock is not satisfiable if *reference == GitReference::DefaultBranch { match &pinned_git_spec.source.reference { - // These are explicit named references - not satisfiable + // Any explicit reference in lock is not satisfiable // when manifest has DefaultBranch (user removed the explicit ref) pixi_spec::GitReference::Branch(_) - | pixi_spec::GitReference::Tag(_) => { + | pixi_spec::GitReference::Tag(_) + | pixi_spec::GitReference::Rev(_) => { return Err(PlatformUnsat::LockedPyPIGitRefMismatch { name: spec.name.clone().to_string(), expected_ref: reference.to_string(), @@ -988,9 +987,8 @@ pub(crate) fn pypi_satifisfies_requirement( } .into()); } - // These are satisfiable - either DefaultBranch or specific commits - pixi_spec::GitReference::DefaultBranch - | pixi_spec::GitReference::Rev(_) => { + // Only DefaultBranch in lock is satisfiable + pixi_spec::GitReference::DefaultBranch => { return Ok(()); } } @@ -2619,12 +2617,35 @@ mod tests { .unwrap(); // This should not pypi_satifisfies_requirement(&non_matching_spec, &locked_data, &project_root).unwrap_err(); - // Removing the rev from the Requirement should satisfy any revision - let spec = pep508_requirement_to_uv_requirement( + + // Removing the rev from the Requirement should NOT satisfy when lock has explicit Rev. + // This ensures that when a user removes an explicit ref from the manifest, + // the lock file gets re-resolved. + let spec_without_rev = pep508_requirement_to_uv_requirement( pep508_rs::Requirement::from_str("mypkg @ git+https://github.com/mypkg").unwrap(), ) .unwrap(); - pypi_satifisfies_requirement(&spec, &locked_data, &project_root).unwrap(); + pypi_satifisfies_requirement(&spec_without_rev, &locked_data, &project_root).unwrap_err(); + + // When lock has DefaultBranch (no explicit ref), removing rev from manifest should satisfy + let locked_data_default_branch = PypiPackageData { + name: "mypkg".parse().unwrap(), + version: Version::from_str("0.1.0").unwrap(), + // No ?rev= query param, only the fragment with commit hash + location: "git+https://github.com/mypkg.git#29932f3915935d773dc8d52c292cadd81c81071d" + .parse() + .expect("failed to parse url"), + hash: None, + requires_dist: vec![], + requires_python: None, + editable: false, + }; + pypi_satifisfies_requirement( + &spec_without_rev, + &locked_data_default_branch, + &project_root, + ) + .unwrap(); } // Currently this test is missing from `good_satisfiability`, so we test the diff --git a/crates/pixi_uv_conversions/src/conversions.rs b/crates/pixi_uv_conversions/src/conversions.rs index 86487730c5..2c2a07f5af 100644 --- a/crates/pixi_uv_conversions/src/conversions.rs +++ b/crates/pixi_uv_conversions/src/conversions.rs @@ -318,10 +318,10 @@ pub fn into_pinned_git_spec( .expect("we expect it to be a valid sha"); // Use the original reference from the manifest if provided. - // If no explicit reference was specified (DefaultBranch), store the resolved - // commit as Rev(commit). This avoids DefaultBranch which requires cache lookups - // and can cause panics when re-resolving. - let reference = original_reference.unwrap_or_else(|| PixiReference::Rev(git_sha.to_string())); + // If no explicit reference was specified, use DefaultBranch. + // The precise commit is already captured in the fragment (`#commit`), + // so we don't need to duplicate it in the query string as `?rev=commit`. + let reference = original_reference.unwrap_or(PixiReference::DefaultBranch); let pinned_checkout = PinnedGitCheckout::new( git_sha, From 36aa0a08e9af24cffd97d76df7d6689d32b2eb3c Mon Sep 17 00:00:00 2001 From: nichmor Date: Sun, 11 Jan 2026 18:30:34 +0200 Subject: [PATCH 10/11] misc: remove commented error code --- crates/pixi_core/src/lock_file/resolve/pypi.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/pixi_core/src/lock_file/resolve/pypi.rs b/crates/pixi_core/src/lock_file/resolve/pypi.rs index ccbe8ca5e3..0674f3adf3 100644 --- a/crates/pixi_core/src/lock_file/resolve/pypi.rs +++ b/crates/pixi_core/src/lock_file/resolve/pypi.rs @@ -615,8 +615,6 @@ pub async fn resolve_pypi( #[error(transparent)] GitUrlParse(#[from] uv_git_types::GitUrlParseError), - // #[error("{0}")] - // Other(miette::ErrReport), } // Create preferences from the locked pypi packages From 25a3337071794bc8b0f1476ddab99e57a9d3fd4a Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Tue, 13 Jan 2026 10:30:30 +0100 Subject: [PATCH 11/11] Update crates/pixi_core/src/lock_file/resolve/pypi.rs --- crates/pixi_core/src/lock_file/resolve/pypi.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/pixi_core/src/lock_file/resolve/pypi.rs b/crates/pixi_core/src/lock_file/resolve/pypi.rs index 0674f3adf3..26fd90f5a1 100644 --- a/crates/pixi_core/src/lock_file/resolve/pypi.rs +++ b/crates/pixi_core/src/lock_file/resolve/pypi.rs @@ -384,7 +384,7 @@ pub async fn resolve_pypi( reference: uv_reference, }; - tracing::debug!("Pre-populating git resolver: {:?} -> {}", reference, uv_sha); + tracing::debug!("pre-populating git resolver: {:?} -> {}", reference, uv_sha); context.shared_state.git().insert(reference, uv_sha); } }