From a768a9d11161718c8acde34eed2375ab15287f99 Mon Sep 17 00:00:00 2001 From: samypr100 <3933065+samypr100@users.noreply.github.com> Date: Mon, 15 Dec 2025 09:26:14 -0500 Subject: [PATCH] Relax error when using uv add with `UV_GIT_LFS` set (#17127) ## Summary Closes https://github.com/astral-sh/uv/issues/17083 Previously having `UV_GIT_LFS` set would cause an error when adding a non-git requirement such as ```error: `requirement` did not resolve to a Git repository, but a Git extension (`--lfs`) was provided.``` ## Test Plan Additional test has been added. --- crates/uv-cli/src/lib.rs | 6 +-- crates/uv-configuration/src/vcs.rs | 29 +++++++++++ crates/uv-settings/src/lib.rs | 2 + crates/uv-workspace/src/pyproject.rs | 16 +++--- crates/uv/src/commands/project/add.rs | 11 ++-- crates/uv/src/commands/project/mod.rs | 11 ++-- crates/uv/src/commands/tool/install.rs | 6 ++- crates/uv/src/commands/tool/run.rs | 8 ++- crates/uv/src/settings.rs | 20 ++++---- crates/uv/tests/it/edit.rs | 70 ++++++++++++++++++++++---- crates/uv/tests/it/show_settings.rs | 2 +- 11 files changed, 133 insertions(+), 48 deletions(-) diff --git a/crates/uv-cli/src/lib.rs b/crates/uv-cli/src/lib.rs index 9f9d41a13..0d5d3f198 100644 --- a/crates/uv-cli/src/lib.rs +++ b/crates/uv-cli/src/lib.rs @@ -4161,7 +4161,7 @@ pub struct AddArgs { pub branch: Option, /// Whether to use Git LFS when adding a dependency from Git. - #[arg(long, env = EnvVars::UV_GIT_LFS, value_parser = clap::builder::BoolishValueParser::new())] + #[arg(long)] pub lfs: bool, /// Extras to enable for the dependency. @@ -5139,7 +5139,7 @@ pub struct ToolRunArgs { pub refresh: RefreshArgs, /// Whether to use Git LFS when adding a dependency from Git. - #[arg(long, env = EnvVars::UV_GIT_LFS, value_parser = clap::builder::BoolishValueParser::new())] + #[arg(long)] pub lfs: bool, /// The Python interpreter to use to build the run environment. @@ -5290,7 +5290,7 @@ pub struct ToolInstallArgs { pub force: bool, /// Whether to use Git LFS when adding a dependency from Git. - #[arg(long, env = EnvVars::UV_GIT_LFS, value_parser = clap::builder::BoolishValueParser::new())] + #[arg(long)] pub lfs: bool, /// The Python interpreter to use to build the tool environment. diff --git a/crates/uv-configuration/src/vcs.rs b/crates/uv-configuration/src/vcs.rs index 13689de11..b40261a60 100644 --- a/crates/uv-configuration/src/vcs.rs +++ b/crates/uv-configuration/src/vcs.rs @@ -94,3 +94,32 @@ wheels/ # Virtual environments .venv "; + +/// Setting for Git LFS (Large File Storage) support. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum GitLfsSetting { + /// Git LFS is disabled (default). + #[default] + Disabled, + /// Git LFS is enabled. Tracks whether it came from an environment variable. + Enabled { from_env: bool }, +} + +impl GitLfsSetting { + pub fn new(from_arg: Option, from_env: Option) -> Self { + match (from_arg, from_env) { + (Some(true), _) => Self::Enabled { from_env: false }, + (_, Some(true)) => Self::Enabled { from_env: true }, + _ => Self::Disabled, + } + } +} + +impl From for Option { + fn from(setting: GitLfsSetting) -> Self { + match setting { + GitLfsSetting::Enabled { .. } => Some(true), + GitLfsSetting::Disabled => None, + } + } +} diff --git a/crates/uv-settings/src/lib.rs b/crates/uv-settings/src/lib.rs index c8383e048..4d370a6c4 100644 --- a/crates/uv-settings/src/lib.rs +++ b/crates/uv-settings/src/lib.rs @@ -590,6 +590,7 @@ pub struct EnvironmentOptions { pub python_install_registry: Option, pub install_mirrors: PythonInstallMirrors, pub log_context: Option, + pub lfs: Option, pub http_timeout: Duration, pub http_retries: u32, pub upload_http_timeout: Duration, @@ -636,6 +637,7 @@ impl EnvironmentOptions { )?, }, log_context: parse_boolish_environment_variable(EnvVars::UV_LOG_CONTEXT)?, + lfs: parse_boolish_environment_variable(EnvVars::UV_GIT_LFS)?, upload_http_timeout: parse_integer_environment_variable( EnvVars::UV_UPLOAD_HTTP_TIMEOUT, )? diff --git a/crates/uv-workspace/src/pyproject.rs b/crates/uv-workspace/src/pyproject.rs index 438e57816..e12559a9c 100644 --- a/crates/uv-workspace/src/pyproject.rs +++ b/crates/uv-workspace/src/pyproject.rs @@ -22,6 +22,7 @@ use serde::{Deserialize, Deserializer, Serialize}; use thiserror::Error; use uv_build_backend::BuildBackendSettings; +use uv_configuration::GitLfsSetting; use uv_distribution_types::{Index, IndexName, RequirementSource}; use uv_fs::{PortablePathBuf, relative_to}; use uv_git_types::GitReference; @@ -1613,13 +1614,16 @@ impl Source { rev: Option, tag: Option, branch: Option, - lfs: Option, + lfs: GitLfsSetting, root: &Path, existing_sources: Option<&BTreeMap>, ) -> Result, SourceError> { // If the user specified a Git reference for a non-Git source, try existing Git sources before erroring. if !matches!(source, RequirementSource::Git { .. }) - && (branch.is_some() || tag.is_some() || rev.is_some() || lfs.is_some()) + && (branch.is_some() + || tag.is_some() + || rev.is_some() + || matches!(lfs, GitLfsSetting::Enabled { .. })) { if let Some(sources) = existing_sources { if let Some(package_sources) = sources.get(name) { @@ -1639,7 +1643,7 @@ impl Source { rev, tag, branch, - lfs, + lfs: lfs.into(), marker: *marker, extra: extra.clone(), group: group.clone(), @@ -1657,7 +1661,7 @@ impl Source { if let Some(branch) = branch { return Err(SourceError::UnusedBranch(name.to_string(), branch)); } - if let Some(true) = lfs { + if matches!(lfs, GitLfsSetting::Enabled { from_env: false }) { return Err(SourceError::UnusedLfs(name.to_string())); } } @@ -1768,7 +1772,7 @@ impl Source { rev: rev.cloned(), tag, branch, - lfs, + lfs: lfs.into(), git: git.repository().clone(), subdirectory: subdirectory.map(PortablePathBuf::from), marker: MarkerTree::TRUE, @@ -1780,7 +1784,7 @@ impl Source { rev, tag, branch, - lfs, + lfs: lfs.into(), git: git.repository().clone(), subdirectory: subdirectory.map(PortablePathBuf::from), marker: MarkerTree::TRUE, diff --git a/crates/uv/src/commands/project/add.rs b/crates/uv/src/commands/project/add.rs index 772e37d71..28f7ee158 100644 --- a/crates/uv/src/commands/project/add.rs +++ b/crates/uv/src/commands/project/add.rs @@ -17,7 +17,8 @@ use uv_cache_key::RepositoryUrl; use uv_client::{BaseClientBuilder, FlatIndexClient, RegistryClientBuilder}; use uv_configuration::{ Concurrency, Constraints, DependencyGroups, DependencyGroupsWithDefaults, DevMode, DryRun, - ExtrasSpecification, ExtrasSpecificationWithDefaults, InstallOptions, SourceStrategy, + ExtrasSpecification, ExtrasSpecificationWithDefaults, GitLfsSetting, InstallOptions, + SourceStrategy, }; use uv_dispatch::BuildDispatch; use uv_distribution::{DistributionDatabase, LoweredExtraBuildDependencies}; @@ -85,7 +86,7 @@ pub(crate) async fn add( rev: Option, tag: Option, branch: Option, - lfs: Option, + lfs: GitLfsSetting, extras_of_dependency: Vec, package: Option, python: Option, @@ -377,7 +378,7 @@ pub(crate) async fn add( rev.as_deref(), tag.as_deref(), branch.as_deref(), - lfs, + lfs.into(), marker, ) }) @@ -797,7 +798,7 @@ fn edits( rev: Option<&str>, tag: Option<&str>, branch: Option<&str>, - lfs: Option, + lfs: GitLfsSetting, extras: &[ExtraName], index: Option<&IndexName>, toml: &mut PyProjectTomlMut, @@ -1231,7 +1232,7 @@ fn resolve_requirement( rev: Option, tag: Option, branch: Option, - lfs: Option, + lfs: GitLfsSetting, root: &Path, existing_sources: Option<&BTreeMap>, ) -> Result<(uv_pep508::Requirement, Option), anyhow::Error> { diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index bfbd4d772..e3185957c 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -12,8 +12,8 @@ use uv_cache::{Cache, CacheBucket}; use uv_cache_key::cache_digest; use uv_client::{BaseClientBuilder, FlatIndexClient, RegistryClientBuilder}; use uv_configuration::{ - Concurrency, Constraints, DependencyGroupsWithDefaults, DryRun, ExtrasSpecification, Reinstall, - TargetTriple, Upgrade, + Concurrency, Constraints, DependencyGroupsWithDefaults, DryRun, ExtrasSpecification, + GitLfsSetting, Reinstall, TargetTriple, Upgrade, }; use uv_dispatch::{BuildDispatch, SharedState}; use uv_distribution::{DistributionDatabase, LoweredExtraBuildDependencies, LoweredRequirement}; @@ -47,8 +47,7 @@ use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy}; use uv_virtualenv::remove_virtualenv; use uv_warnings::{warn_user, warn_user_once}; use uv_workspace::dependency_groups::DependencyGroupError; -use uv_workspace::pyproject::ExtraBuildDependency; -use uv_workspace::pyproject::PyProjectToml; +use uv_workspace::pyproject::{ExtraBuildDependency, PyProjectToml}; use uv_workspace::{RequiresPythonSources, Workspace, WorkspaceCache}; use crate::commands::pip::loggers::{InstallLogger, ResolveLogger}; @@ -1685,14 +1684,14 @@ pub(crate) async fn resolve_names( workspace_cache: &WorkspaceCache, printer: Printer, preview: Preview, - lfs: Option, + lfs: GitLfsSetting, ) -> Result, uv_requirements::Error> { // Partition the requirements into named and unnamed requirements. let (mut requirements, unnamed): (Vec<_>, Vec<_>) = requirements .into_iter() .map(|spec| { spec.requirement - .augment_requirement(None, None, None, lfs, None) + .augment_requirement(None, None, None, lfs.into(), None) }) .partition_map(|requirement| match requirement { UnresolvedRequirement::Named(requirement) => itertools::Either::Left(requirement), diff --git a/crates/uv/src/commands/tool/install.rs b/crates/uv/src/commands/tool/install.rs index ac8f215d3..394fe93ff 100644 --- a/crates/uv/src/commands/tool/install.rs +++ b/crates/uv/src/commands/tool/install.rs @@ -9,7 +9,9 @@ use tracing::{debug, trace}; use uv_cache::{Cache, Refresh}; use uv_cache_info::Timestamp; use uv_client::{BaseClientBuilder, RegistryClientBuilder}; -use uv_configuration::{Concurrency, Constraints, DryRun, Reinstall, TargetTriple, Upgrade}; +use uv_configuration::{ + Concurrency, Constraints, DryRun, GitLfsSetting, Reinstall, TargetTriple, Upgrade, +}; use uv_distribution::LoweredExtraBuildDependencies; use uv_distribution_types::{ ExtraBuildRequires, IndexCapabilities, NameRequirementSpecification, Requirement, @@ -58,7 +60,7 @@ pub(crate) async fn install( excludes: &[RequirementsSource], build_constraints: &[RequirementsSource], entrypoints: &[PackageName], - lfs: Option, + lfs: GitLfsSetting, python: Option, python_platform: Option, install_mirrors: PythonInstallMirrors, diff --git a/crates/uv/src/commands/tool/run.rs b/crates/uv/src/commands/tool/run.rs index f5d3ce883..6eda5de02 100644 --- a/crates/uv/src/commands/tool/run.rs +++ b/crates/uv/src/commands/tool/run.rs @@ -17,9 +17,7 @@ use uv_cache::{Cache, Refresh}; use uv_cache_info::Timestamp; use uv_cli::ExternalCommand; use uv_client::{BaseClientBuilder, RegistryClientBuilder}; -use uv_configuration::Concurrency; -use uv_configuration::Constraints; -use uv_configuration::TargetTriple; +use uv_configuration::{Concurrency, Constraints, GitLfsSetting, TargetTriple}; use uv_distribution::LoweredExtraBuildDependencies; use uv_distribution_types::InstalledDist; use uv_distribution_types::{ @@ -106,7 +104,7 @@ pub(crate) async fn run( overrides: &[RequirementsSource], build_constraints: &[RequirementsSource], show_resolution: bool, - lfs: Option, + lfs: GitLfsSetting, python: Option, python_platform: Option, install_mirrors: PythonInstallMirrors, @@ -726,7 +724,7 @@ async fn get_or_create_environment( settings: &ResolverInstallerSettings, client_builder: &BaseClientBuilder<'_>, isolated: bool, - lfs: Option, + lfs: GitLfsSetting, python_preference: PythonPreference, python_downloads: PythonDownloads, installer_metadata: bool, diff --git a/crates/uv/src/settings.rs b/crates/uv/src/settings.rs index bf879f7b4..71704b50d 100644 --- a/crates/uv/src/settings.rs +++ b/crates/uv/src/settings.rs @@ -26,10 +26,10 @@ use uv_cli::{ use uv_client::Connectivity; use uv_configuration::{ BuildIsolation, BuildOptions, Concurrency, DependencyGroups, DryRun, EditableMode, EnvFile, - ExportFormat, ExtrasSpecification, HashCheckingMode, IndexStrategy, InstallOptions, - KeyringProviderType, NoBinary, NoBuild, PipCompileFormat, ProjectBuildBackend, Reinstall, - RequiredVersion, SourceStrategy, TargetTriple, TrustedHost, TrustedPublishing, Upgrade, - VersionControlSystem, + ExportFormat, ExtrasSpecification, GitLfsSetting, HashCheckingMode, IndexStrategy, + InstallOptions, KeyringProviderType, NoBinary, NoBuild, PipCompileFormat, ProjectBuildBackend, + Reinstall, RequiredVersion, SourceStrategy, TargetTriple, TrustedHost, TrustedPublishing, + Upgrade, VersionControlSystem, }; use uv_distribution_types::{ ConfigSettings, DependencyMetadata, ExtraBuildVariables, Index, IndexLocations, IndexUrl, @@ -547,7 +547,7 @@ pub(crate) struct ToolRunSettings { pub(crate) build_constraints: Vec, pub(crate) isolated: bool, pub(crate) show_resolution: bool, - pub(crate) lfs: Option, + pub(crate) lfs: GitLfsSetting, pub(crate) python: Option, pub(crate) python_platform: Option, pub(crate) install_mirrors: PythonInstallMirrors, @@ -630,7 +630,7 @@ impl ToolRunSettings { .unwrap_or_default(); let settings = ResolverInstallerSettings::from(options.clone()); - let lfs = lfs.then_some(true); + let lfs = GitLfsSetting::new(lfs.then_some(true), environment.lfs); Self { command, @@ -689,7 +689,7 @@ pub(crate) struct ToolInstallSettings { pub(crate) overrides: Vec, pub(crate) excludes: Vec, pub(crate) build_constraints: Vec, - pub(crate) lfs: Option, + pub(crate) lfs: GitLfsSetting, pub(crate) python: Option, pub(crate) python_platform: Option, pub(crate) refresh: Refresh, @@ -744,7 +744,7 @@ impl ToolInstallSettings { .unwrap_or_default(); let settings = ResolverInstallerSettings::from(options.clone()); - let lfs = lfs.then_some(true); + let lfs = GitLfsSetting::new(lfs.then_some(true), environment.lfs); Self { package, @@ -1570,7 +1570,7 @@ pub(crate) struct AddSettings { pub(crate) rev: Option, pub(crate) tag: Option, pub(crate) branch: Option, - pub(crate) lfs: Option, + pub(crate) lfs: GitLfsSetting, pub(crate) package: Option, pub(crate) script: Option, pub(crate) python: Option, @@ -1713,7 +1713,7 @@ impl AddSettings { .unwrap_or_default(); let bounds = bounds.or(filesystem.as_ref().and_then(|fs| fs.add.add_bounds)); - let lfs = lfs.then_some(true); + let lfs = GitLfsSetting::new(lfs.then_some(true), environment.lfs); Self { lock_check: if locked { diff --git a/crates/uv/tests/it/edit.rs b/crates/uv/tests/it/edit.rs index 1cb07b383..214ae50b0 100644 --- a/crates/uv/tests/it/edit.rs +++ b/crates/uv/tests/it/edit.rs @@ -631,16 +631,6 @@ fn add_git_error() -> Result<()> { error: `flask` did not resolve to a Git repository, but a Git reference (`--branch 0.0.1`) was provided. "###); - // Request lfs without a Git source. - uv_snapshot!(context.filters(), context.add().arg("flask").arg("--lfs"), @r###" - success: false - exit_code: 2 - ----- stdout ----- - - ----- stderr ----- - error: `flask` did not resolve to a Git repository, but a Git extension (`--lfs`) was provided. - "###); - Ok(()) } @@ -14831,3 +14821,63 @@ fn add_no_install_project() -> Result<()> { Ok(()) } + +#[test] +#[cfg(feature = "git-lfs")] +fn add_git_lfs_error() -> Result<()> { + let context = TestContext::new("3.13").with_git_lfs_config(); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.13" + dependencies = [] + "#})?; + + // Request lfs (via arg) without a Git source. + uv_snapshot!(context.filters(), context.add().arg("typing-extensions").arg("--lfs"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: `typing-extensions` did not resolve to a Git repository, but a Git extension (`--lfs`) was provided. + "###); + + // Request lfs (via env var) without a Git source. + uv_snapshot!(context.filters(), context.add().arg("typing-extensions").env(EnvVars::UV_GIT_LFS, "true"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + typing-extensions==4.10.0 + "); + + // Request lfs (both arg and env var) without a Git source. + uv_snapshot!(context.filters(), context.add().arg("typing-extensions").env(EnvVars::UV_GIT_LFS, "true").arg("--lfs"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: `typing-extensions` did not resolve to a Git repository, but a Git extension (`--lfs`) was provided. + "###); + + // Request lfs from arg and disable lfs from env var (should be ignored) without a Git source. + uv_snapshot!(context.filters(), context.add().arg("typing-extensions").env(EnvVars::UV_GIT_LFS, "false").arg("--lfs"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: `typing-extensions` did not resolve to a Git repository, but a Git extension (`--lfs`) was provided. + "###); + + Ok(()) +} diff --git a/crates/uv/tests/it/show_settings.rs b/crates/uv/tests/it/show_settings.rs index 3bc7e011c..ebdd271b6 100644 --- a/crates/uv/tests/it/show_settings.rs +++ b/crates/uv/tests/it/show_settings.rs @@ -3530,7 +3530,7 @@ fn resolve_tool() -> anyhow::Result<()> { overrides: [], excludes: [], build_constraints: [], - lfs: None, + lfs: Disabled, python: None, python_platform: None, refresh: None(