From c04a95e03741de41bf84e8e52ebaa3fda47e368b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 1 Jun 2024 08:40:11 -0400 Subject: [PATCH] Respect resolved Git SHAs in `uv lock` (#3956) ## Summary This PR ensures that if a lockfile already contains a resolved reference (e.g., you locked with `main` previously, and it locked to a specific commit), and you run `uv lock`, we use the same SHA, even if it's not the latest SHA for that tag. This avoids upgrading Git dependencies without `--upgrade`. Closes #3920. --- crates/uv-git/src/lib.rs | 4 +- crates/uv-git/src/resolver.rs | 20 ++- crates/uv-requirements/src/upgrade.rs | 57 ++++--- crates/uv-resolver/src/lock.rs | 22 ++- crates/uv/src/commands/project/lock.rs | 8 +- crates/uv/tests/lock.rs | 206 ++++++++++++++++++++----- 6 files changed, 251 insertions(+), 66 deletions(-) diff --git a/crates/uv-git/src/lib.rs b/crates/uv-git/src/lib.rs index dd344250169f..5270fcdc4ea6 100644 --- a/crates/uv-git/src/lib.rs +++ b/crates/uv-git/src/lib.rs @@ -2,7 +2,9 @@ use std::str::FromStr; use url::Url; pub use crate::git::GitReference; -pub use crate::resolver::{GitResolver, GitResolverError, RepositoryReference}; +pub use crate::resolver::{ + GitResolver, GitResolverError, RepositoryReference, ResolvedRepositoryReference, +}; pub use crate::sha::{GitOid, GitSha, OidParseError}; pub use crate::source::{Fetch, GitSource, Reporter}; diff --git a/crates/uv-git/src/resolver.rs b/crates/uv-git/src/resolver.rs index 5a3689be8be0..6904bae76a5d 100644 --- a/crates/uv-git/src/resolver.rs +++ b/crates/uv-git/src/resolver.rs @@ -27,6 +27,15 @@ pub enum GitResolverError { pub struct GitResolver(Arc>); impl GitResolver { + /// Initialize a [`GitResolver`] with a set of resolved references. + pub fn from_refs(refs: Vec) -> Self { + Self(Arc::new( + refs.into_iter() + .map(|ResolvedRepositoryReference { reference, sha }| (reference, sha)) + .collect(), + )) + } + /// Returns the [`GitSha`] for the given [`RepositoryReference`], if it exists. pub fn get(&self, reference: &RepositoryReference) -> Option> { self.0.get(reference) @@ -136,11 +145,20 @@ impl GitResolver { } } +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct ResolvedRepositoryReference { + /// An abstract reference to a Git repository, including the URL and the commit (e.g., a branch, + /// tag, or revision). + pub reference: RepositoryReference, + /// The precise commit SHA of the reference. + pub sha: GitSha, +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct RepositoryReference { /// The URL of the Git repository, with any query parameters and fragments removed. pub url: RepositoryUrl, - /// The reference to the commit to use, which could be a branch, tag or revision. + /// The reference to the commit to use, which could be a branch, tag, or revision. pub reference: GitReference, } diff --git a/crates/uv-requirements/src/upgrade.rs b/crates/uv-requirements/src/upgrade.rs index 4cff3b0be341..09308da37ec3 100644 --- a/crates/uv-requirements/src/upgrade.rs +++ b/crates/uv-requirements/src/upgrade.rs @@ -7,8 +7,17 @@ use requirements_txt::RequirementsTxt; use uv_client::{BaseClientBuilder, Connectivity}; use uv_configuration::Upgrade; use uv_distribution::ProjectWorkspace; +use uv_git::ResolvedRepositoryReference; use uv_resolver::{Lock, Preference, PreferenceError}; +#[derive(Debug, Default)] +pub struct LockedRequirements { + /// The pinned versions from the lockfile. + pub preferences: Vec, + /// The pinned Git SHAs from the lockfile. + pub git: Vec, +} + /// Load the preferred requirements from an existing `requirements.txt`, applying the upgrade strategy. pub async fn read_requirements_txt( output_file: Option<&Path>, @@ -58,10 +67,10 @@ pub async fn read_requirements_txt( pub async fn read_lockfile( project: &ProjectWorkspace, upgrade: &Upgrade, -) -> Result> { +) -> Result { // As an optimization, skip reading the lockfile is we're upgrading all packages anyway. if upgrade.is_all() { - return Ok(Vec::new()); + return Ok(LockedRequirements::default()); } // If an existing lockfile exists, build up a set of preferences. @@ -71,32 +80,36 @@ pub async fn read_lockfile( Ok(lock) => lock, Err(err) => { eprint!("Failed to parse lockfile; ignoring locked requirements: {err}"); - return Ok(Vec::new()); + return Ok(LockedRequirements::default()); } }, Err(err) if err.kind() == std::io::ErrorKind::NotFound => { - return Ok(Vec::new()); + return Ok(LockedRequirements::default()); } Err(err) => return Err(err.into()), }; - // Map each entry in the lockfile to a preference. - let preferences: Vec = lock - .distributions() - .iter() - .map(Preference::from_lock) - .collect(); + let mut preferences = Vec::new(); + let mut git = Vec::new(); - // Apply the upgrade strategy to the requirements. - Ok(match upgrade { - // Respect all pinned versions from the existing lockfile. - Upgrade::None => preferences, - // Ignore all pinned versions from the existing lockfile. - Upgrade::All => vec![], - // Ignore pinned versions for the specified packages. - Upgrade::Packages(packages) => preferences - .into_iter() - .filter(|preference| !packages.contains(preference.name())) - .collect(), - }) + for dist in lock.distributions() { + // Skip the distribution if it's not included in the upgrade strategy. + if match upgrade { + Upgrade::None => false, + Upgrade::All => true, + Upgrade::Packages(packages) => packages.contains(dist.name()), + } { + continue; + } + + // Map each entry in the lockfile to a preference. + preferences.push(Preference::from_lock(dist)); + + // Map each entry in the lockfile to a Git SHA. + if let Some(git_ref) = dist.as_git_ref() { + git.push(git_ref); + } + } + + Ok(LockedRequirements { preferences, git }) } diff --git a/crates/uv-resolver/src/lock.rs b/crates/uv-resolver/src/lock.rs index e1685e3166ea..fb0595e33852 100644 --- a/crates/uv-resolver/src/lock.rs +++ b/crates/uv-resolver/src/lock.rs @@ -7,6 +7,7 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use anyhow::Result; +use cache_key::RepositoryUrl; use rustc_hash::FxHashMap; use toml_edit::{value, Array, ArrayOfTables, InlineTable, Item, Table, Value}; use url::Url; @@ -21,7 +22,7 @@ use pep440_rs::Version; use pep508_rs::{MarkerEnvironment, VerbatimUrl}; use platform_tags::{TagCompatibility, TagPriority, Tags}; use pypi_types::{HashDigest, ParsedArchiveUrl, ParsedGitUrl}; -use uv_git::{GitReference, GitSha}; +use uv_git::{GitReference, GitSha, RepositoryReference, ResolvedRepositoryReference}; use uv_normalize::{ExtraName, PackageName}; use crate::resolution::AnnotatedDist; @@ -478,6 +479,25 @@ impl Distribution { } best.map(|(_, i)| i) } + + /// Returns the [`PackageName`] of the distribution. + pub fn name(&self) -> &PackageName { + &self.id.name + } + + /// Returns the [`ResolvedRepositoryReference`] for the distribution, if it is a Git source. + pub fn as_git_ref(&self) -> Option { + match &self.id.source.kind { + SourceKind::Git(git) => Some(ResolvedRepositoryReference { + reference: RepositoryReference { + url: RepositoryUrl::new(&self.id.source.url), + reference: GitReference::from(git.kind.clone()), + }, + sha: git.precise, + }), + _ => None, + } + } } #[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord, serde::Deserialize)] diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index 2b88f26062c5..9c7eb9b846a0 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -13,7 +13,7 @@ use uv_dispatch::BuildDispatch; use uv_distribution::ProjectWorkspace; use uv_git::GitResolver; use uv_interpreter::PythonEnvironment; -use uv_requirements::upgrade::read_lockfile; +use uv_requirements::upgrade::{read_lockfile, LockedRequirements}; use uv_resolver::{ExcludeNewer, FlatIndex, InMemoryIndex, Lock, OptionsBuilder}; use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy, InFlight}; use uv_warnings::warn_user; @@ -105,7 +105,6 @@ pub(super) async fn do_lock( let config_settings = ConfigSettings::default(); let extras = ExtrasSpecification::default(); let flat_index = FlatIndex::default(); - let git = GitResolver::default(); let in_flight = InFlight::default(); let index = InMemoryIndex::default(); let index_locations = IndexLocations::default(); @@ -119,7 +118,10 @@ pub(super) async fn do_lock( let options = OptionsBuilder::new().exclude_newer(exclude_newer).build(); // If an existing lockfile exists, build up a set of preferences. - let preferences = read_lockfile(project, &upgrade).await?; + let LockedRequirements { preferences, git } = read_lockfile(project, &upgrade).await?; + + // Create the Git resolver. + let git = GitResolver::from_refs(git); // Create a build dispatch. let build_dispatch = BuildDispatch::new( diff --git a/crates/uv/tests/lock.rs b/crates/uv/tests/lock.rs index b3295dafa9d7..fb7833d78fe5 100644 --- a/crates/uv/tests/lock.rs +++ b/crates/uv/tests/lock.rs @@ -348,7 +348,7 @@ fn lock_sdist_git() -> Result<()> { [project] name = "project" version = "0.1.0" - dependencies = ["anyio @ git+https://github.com/agronholm/anyio@3.7.0"] + dependencies = ["uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0.0.1"] "#, )?; @@ -359,7 +359,7 @@ fn lock_sdist_git() -> Result<()> { ----- stderr ----- warning: `uv lock` is experimental and may change without warning. - Resolved 4 packages in [TIME] + Resolved 2 packages in [TIME] "###); let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock"))?; @@ -371,29 +371,6 @@ fn lock_sdist_git() -> Result<()> { lock, @r###" version = 1 - [[distribution]] - name = "anyio" - version = "3.7.0" - source = "git+https://github.com/agronholm/anyio?rev=3.7.0#f7a880ffac4766efb39e6fb60fc28d944f5d2f65" - sdist = { url = "https://github.com/agronholm/anyio?rev=3.7.0#f7a880ffac4766efb39e6fb60fc28d944f5d2f65" } - - [[distribution.dependencies]] - name = "idna" - version = "3.6" - source = "registry+https://pypi.org/simple" - - [[distribution.dependencies]] - name = "sniffio" - version = "1.3.1" - source = "registry+https://pypi.org/simple" - - [[distribution]] - name = "idna" - version = "3.6" - source = "registry+https://pypi.org/simple" - sdist = { url = "https://files.pythonhosted.org/packages/bf/3f/ea4b9117521a1e9c50344b909be7886dd00a519552724809bb1f486986c2/idna-3.6.tar.gz", hash = "sha256:9ecdbbd083b06798ae1e86adcbfe8ab1479cf864e4ee30fe4e46a003d12491ca", size = 175426 } - wheels = [{ url = "https://files.pythonhosted.org/packages/c2/e7/a82b05cf63a603df6e68d59ae6a68bf5064484a0718ea5033660af4b54a9/idna-3.6-py3-none-any.whl", hash = "sha256:c05567e9c24a6b9faaa835c4821bad0590fbb9d5779e7caa6e1cc4978e7eb24f", size = 61567 }] - [[distribution]] name = "project" version = "0.1.0" @@ -401,16 +378,15 @@ fn lock_sdist_git() -> Result<()> { sdist = { url = "file://[TEMP_DIR]/" } [[distribution.dependencies]] - name = "anyio" - version = "3.7.0" - source = "git+https://github.com/agronholm/anyio?rev=3.7.0#f7a880ffac4766efb39e6fb60fc28d944f5d2f65" + name = "uv-public-pypackage" + version = "0.1.0" + source = "git+https://github.com/astral-test/uv-public-pypackage?rev=0.0.1#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" [[distribution]] - name = "sniffio" - version = "1.3.1" - source = "registry+https://pypi.org/simple" - sdist = { url = "https://files.pythonhosted.org/packages/a2/87/a6771e1546d97e7e041b6ae58d80074f81b7d5121207425c964ddf5cfdbd/sniffio-1.3.1.tar.gz", hash = "sha256:f4324edc670a0f49750a81b895f35c3adb843cca46f0530f79fc1babb23789dc", size = 20372 } - wheels = [{ url = "https://files.pythonhosted.org/packages/e9/44/75a9c9421471a6c4805dbf2356f7c181a29c1879239abab1ea2cc8f38b40/sniffio-1.3.1-py3-none-any.whl", hash = "sha256:2f6da418d1f1e0fddd844478f41680e794e6051915791a034ff65e5f100525a2", size = 10235 }] + name = "uv-public-pypackage" + version = "0.1.0" + source = "git+https://github.com/astral-test/uv-public-pypackage?rev=0.0.1#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" + sdist = { url = "https://github.com/astral-test/uv-public-pypackage?rev=0.0.1#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" } "### ); }); @@ -423,12 +399,10 @@ fn lock_sdist_git() -> Result<()> { ----- stderr ----- warning: `uv sync` is experimental and may change without warning. - Downloaded 4 packages in [TIME] - Installed 4 packages in [TIME] - + anyio==3.7.0 (from git+https://github.com/agronholm/anyio@f7a880ffac4766efb39e6fb60fc28d944f5d2f65) - + idna==3.6 + Downloaded 2 packages in [TIME] + Installed 2 packages in [TIME] + project==0.1.0 (from file://[TEMP_DIR]/) - + sniffio==1.3.1 + + uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979) "###); Ok(()) @@ -915,3 +889,159 @@ fn lock_preference() -> Result<()> { Ok(()) } + +/// Respect locked versions with `uv lock`, unless `--upgrade` is passed. +#[test] +fn lock_git_sha() -> Result<()> { + let context = TestContext::new("3.12"); + + // Lock to a specific commit on `main`. + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "project" + version = "0.1.0" + dependencies = ["uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979"] + "#, + )?; + + uv_snapshot!(context.filters(), context.lock(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv lock` is experimental and may change without warning. + Resolved 2 packages in [TIME] + "###); + + let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock"))?; + + insta::with_settings!({ + filters => context.filters(), + }, { + assert_snapshot!( + lock, @r###" + version = 1 + + [[distribution]] + name = "project" + version = "0.1.0" + source = "editable+file://[TEMP_DIR]/" + sdist = { url = "file://[TEMP_DIR]/" } + + [[distribution.dependencies]] + name = "uv-public-pypackage" + version = "0.1.0" + source = "git+https://github.com/astral-test/uv-public-pypackage?rev=0dacfd662c64cb4ceb16e6cf65a157a8b715b979#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" + + [[distribution]] + name = "uv-public-pypackage" + version = "0.1.0" + source = "git+https://github.com/astral-test/uv-public-pypackage?rev=0dacfd662c64cb4ceb16e6cf65a157a8b715b979#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" + sdist = { url = "https://github.com/astral-test/uv-public-pypackage?rev=0dacfd662c64cb4ceb16e6cf65a157a8b715b979#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" } + "### + ); + }); + + // Rewrite the lockfile, as if it were locked against `main`. + let lock = lock.replace("rev=0dacfd662c64cb4ceb16e6cf65a157a8b715b979", "rev=main"); + fs_err::write(context.temp_dir.join("uv.lock"), lock)?; + + // Lock `anyio` against `main`. + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "project" + version = "0.1.0" + dependencies = ["uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@main"] + "#, + )?; + + uv_snapshot!(context.filters(), context.lock(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv lock` is experimental and may change without warning. + Resolved 2 packages in [TIME] + "###); + + let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock"))?; + + // The lockfile should resolve to `0dacfd662c64cb4ceb16e6cf65a157a8b715b979`, even though it's + // not the latest commit on `main`. + insta::with_settings!({ + filters => context.filters(), + }, { + assert_snapshot!( + lock, @r###" + version = 1 + + [[distribution]] + name = "project" + version = "0.1.0" + source = "editable+file://[TEMP_DIR]/" + sdist = { url = "file://[TEMP_DIR]/" } + + [[distribution.dependencies]] + name = "uv-public-pypackage" + version = "0.1.0" + source = "git+https://github.com/astral-test/uv-public-pypackage?rev=main#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" + + [[distribution]] + name = "uv-public-pypackage" + version = "0.1.0" + source = "git+https://github.com/astral-test/uv-public-pypackage?rev=main#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" + sdist = { url = "https://github.com/astral-test/uv-public-pypackage?rev=main#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" } + "### + ); + }); + + // Relock with `--upgrade`. + uv_snapshot!(context.filters(), context.lock().arg("--upgrade-package").arg("uv-public-pypackage"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv lock` is experimental and may change without warning. + Resolved 2 packages in [TIME] + "###); + + let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock"))?; + + // The lockfile should resolve to `b270df1a2fb5d012294e9aaf05e7e0bab1e6a389`, the latest commit + // on `main`. + insta::with_settings!({ + filters => context.filters(), + }, { + assert_snapshot!( + lock, @r###" + version = 1 + + [[distribution]] + name = "project" + version = "0.1.0" + source = "editable+file://[TEMP_DIR]/" + sdist = { url = "file://[TEMP_DIR]/" } + + [[distribution.dependencies]] + name = "uv-public-pypackage" + version = "0.1.0" + source = "git+https://github.com/astral-test/uv-public-pypackage?rev=main#b270df1a2fb5d012294e9aaf05e7e0bab1e6a389" + + [[distribution]] + name = "uv-public-pypackage" + version = "0.1.0" + source = "git+https://github.com/astral-test/uv-public-pypackage?rev=main#b270df1a2fb5d012294e9aaf05e7e0bab1e6a389" + sdist = { url = "https://github.com/astral-test/uv-public-pypackage?rev=main#b270df1a2fb5d012294e9aaf05e7e0bab1e6a389" } + "### + ); + }); + + Ok(()) +}