pioduino/uv
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Support Git LFS with opt-in (#16143)

Browse Source 
## Summary

Follow up to https://github.com/astral-sh/uv/pull/15563
Closes https://github.com/astral-sh/uv/issues/13485

This is a first-pass at adding support for conditional support for Git
LFS between git sources, initial feedback welcome.

e.g.
```
[tool.uv.sources]
test-lfs-repo = { git = "https://github.com/zanieb/test-lfs-repo.git", lfs = true }
```

For context previously a user had to set `UV_GIT_LFS` to have uv fetch
lfs objects on git sources. This env var was all or nothing, meaning you
must always have it set to get consistent behavior and it applied to all
git sources. If you fetched lfs objects at a revision and then turned
off lfs (or vice versa), the git db, corresponding checkout lfs
artifacts would not be updated properly. Similarly, when git source
distributions were built, there would be no distinction between sources
with lfs and without lfs. Hence, it could corrupt the git, sdist, and
archive caches.

In order to support some sources being LFS enabled and other not, this
PR adds a stateful layer roughly similar to how `subdirectory` works but
for `lfs` since the git database, the checkouts and the corresponding
caching layers needed to be LFS aware (requested vs installed). The
caches also had to isolated and treated entirely separate when handling
LFS sources.

Summary
* Adds `lfs = true` or `lfs = false` to git sources in pyproject.toml
* Added `lfs=true` query param / fragments to most relevant url structs
(not parsed as user input)
  * In the case of uv add / uv tool, `--lfs` is supported instead
* `UV_GIT_LFS` environment variable support is still functional for
non-project entrypoints (e.g. uv pip)
* `direct-url.json` now has an custom `git_lfs` entry under VcsInfo
(note, this is not in the spec currently -- see caveats).
* git database and checkouts have an different cache key as the sources
should be treated effectively different for the same rev.
* sdists cache also differ in the cache key of a built distribution if
it was built using LFS enabled revisions to distinguish between non-LFS
same revisions. This ensures the strong assumption for archive-v0 that
an unpacked revision "doesn't change sources" stays valid.

Caveats
* `pylock.toml` import support has not been added via git_lfs=true,
going through the spec it wasn't clear to me it's something we'd support
outside of the env var (for now).
* direct-url struct was modified by adding a non-standard `git_lfs`
field under VcsInfo which may be undersirable although the PEP 610 does
say `Additional fields that would be necessary to support such VCS
SHOULD be prefixed with the VCS command name` which could be interpret
this change as ok.
* There will be a slight lockfile and cache churn for users that use
`UV_GIT_LFS` as all git lockfile entries will get a `lfs=true` fragment.
The cache version does not need an update, but LFS sources will get
their own namespace under git-v0 and sdist-v9/git hence a cache-miss
will occur once but this can be sufficient to label this as breaking for
workflows always setting `UV_GIT_LFS`.

## Test Plan

Some initial tests were added. More tests likely to follow as we reach
consensus on a final approach.

For IT test, we may want to move to use a repo under astral namespace in
order to test lfs functionality.

Manual testing was done for common pathological cases like killing LFS
fetch mid-way, uninstalling LFS after installing an sdist with it and
reinstalling, fetching LFS artifacts in different commits, etc.

PSA: Please ignore the docker build failures as its related to depot
OIDC issues.

---------

Co-authored-by: Zanie Blue <contact@zanie.dev>
Co-authored-by: konstin <konstin@mailbox.org>
This commit is contained in:
samypr100
2025-12-02 07:23:51 -05:00
committed by GitHub
parent 5947fb0c83
commit fee7f9d093
45 changed files with 1978 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/uv-git/src/git.rs
+151 -27
View File
@@ -6,15 +6,17 @@ use std::path::{Path, PathBuf};
use std::str::{self};
use std::sync::LazyLock;
use anyhow::{Context, Result};
use anyhow::{Context, Result, anyhow};
use cargo_util::{ProcessBuilder, paths};
use tracing::{debug, warn};
use owo_colors::OwoColorize;
use tracing::{debug, instrument, warn};
use url::Url;
use uv_fs::Simplified;
use uv_git_types::{GitOid, GitReference};
use uv_redacted::DisplaySafeUrl;
use uv_static::EnvVars;
use uv_warnings::warn_user_once;
/// A file indicates that if present, `git reset` has been done and a repo
/// checkout is ready to go. See [`GitCheckout::reset`] for why we need this.
@@ -24,6 +26,10 @@ const CHECKOUT_READY_LOCK: &str = ".ok";
pub enum GitError {
#[error("Git executable not found. Ensure that Git is installed and available.")]
GitNotFound,
#[error("Git LFS extension not found. Ensure that Git LFS is installed and available.")]
GitLfsNotFound,
#[error("Is Git LFS configured? Run `{}` to initialize Git LFS.", "git lfs install".green())]
GitLfsNotConfigured,
#[error(transparent)]
Other(#[from] which::Error),
#[error(
@@ -137,6 +143,8 @@ pub(crate) struct GitRemote {
pub(crate) struct GitDatabase {
/// Underlying Git repository instance for this database.
repo: GitRepository,
/// Git LFS artifacts have been initialized (if requested).
lfs_ready: Option<bool>,
}
/// A local checkout of a particular revision from a [`GitRepository`].
@@ -145,6 +153,8 @@ pub(crate) struct GitCheckout {
revision: GitOid,
/// Underlying Git repository instance for this checkout.
repo: GitRepository,
/// Git LFS artifacts have been initialized (if requested).
lfs_ready: Option<bool>,
}
/// A local Git repository.
@@ -198,6 +208,43 @@ impl GitRepository {
result.truncate(result.trim_end().len());
Ok(result.parse()?)
}
/// Verifies LFS artifacts have been initialized for a given `refname`.
#[instrument(skip_all, fields(path = %self.path.user_display(), refname = %refname))]
fn lfs_fsck_objects(&self, refname: &str) -> bool {
let mut cmd = if let Ok(lfs) = GIT_LFS.as_ref() {
lfs.clone()
} else {
warn!("Git LFS is not available, skipping LFS fetch");
return false;
};
// Requires Git LFS 3.x (2021 release)
let result = cmd
.arg("fsck")
.arg("--objects")
.arg(refname)
.cwd(&self.path)
.exec_with_output();
match result {
Ok(_) => true,
Err(err) => {
let lfs_error = err.to_string();
if lfs_error.contains("unknown flag: --objects") {
warn_user_once!(
"Skipping Git LFS validation as Git LFS extension is outdated. \
Upgrade to `git-lfs>=3.0.2` or manually verify git-lfs objects were \
properly fetched after the current operation finishes."
);
true
} else {
debug!("Git LFS validation failed: {err}");
false
}
}
}
}
}
impl GitRemote {
@@ -231,12 +278,11 @@ impl GitRemote {
locked_rev: Option<GitOid>,
disable_ssl: bool,
offline: bool,
with_lfs: bool,
) -> Result<(GitDatabase, GitOid)> {
let reference = locked_rev
.map(ReferenceOrOid::Oid)
.unwrap_or(ReferenceOrOid::Reference(reference));
let enable_lfs_fetch = std::env::var(EnvVars::UV_GIT_LFS).is_ok();
if let Some(mut db) = db {
fetch(&mut db.repo, &self.url, reference, disable_ssl, offline)
.with_context(|| format!("failed to fetch into: {}", into.user_display()))?;
@@ -247,9 +293,10 @@ impl GitRemote {
};
if let Some(rev) = resolved_commit_hash {
if enable_lfs_fetch {
fetch_lfs(&mut db.repo, &self.url, &rev, disable_ssl)
if with_lfs {
let lfs_ready = fetch_lfs(&mut db.repo, &self.url, &rev, disable_ssl)
.with_context(|| format!("failed to fetch LFS objects at {rev}"))?;
db = db.with_lfs_ready(Some(lfs_ready));
}
return Ok((db, rev));
}
@@ -272,19 +319,24 @@ impl GitRemote {
Some(rev) => rev,
None => reference.resolve(&repo)?,
};
if enable_lfs_fetch {
fetch_lfs(&mut repo, &self.url, &rev, disable_ssl)
.with_context(|| format!("failed to fetch LFS objects at {rev}"))?;
}
let lfs_ready = with_lfs
.then(|| {
fetch_lfs(&mut repo, &self.url, &rev, disable_ssl)
.with_context(|| format!("failed to fetch LFS objects at {rev}"))
})
.transpose()?;
Ok((GitDatabase { repo }, rev))
Ok((GitDatabase { repo, lfs_ready }, rev))
}
/// Creates a [`GitDatabase`] of this remote at `db_path`.
#[allow(clippy::unused_self)]
pub(crate) fn db_at(&self, db_path: &Path) -> Result<GitDatabase> {
let repo = GitRepository::open(db_path)?;
Ok(GitDatabase { repo })
Ok(GitDatabase {
repo,
lfs_ready: None,
})
}
}
@@ -300,7 +352,7 @@ impl GitDatabase {
.map(|repo| GitCheckout::new(rev, repo))
.filter(GitCheckout::is_fresh)
{
Some(co) => co,
Some(co) => co.with_lfs_ready(self.lfs_ready),
None => GitCheckout::clone_into(destination, self, rev)?,
};
Ok(checkout)
@@ -324,6 +376,18 @@ impl GitDatabase {
pub(crate) fn contains(&self, oid: GitOid) -> bool {
self.repo.rev_parse(&format!("{oid}^0")).is_ok()
}
/// Checks if `oid` contains necessary LFS artifacts in this database.
pub(crate) fn contains_lfs_artifacts(&self, oid: GitOid) -> bool {
self.repo.lfs_fsck_objects(&format!("{oid}^0"))
}
/// Set the Git LFS validation state (if any).
#[must_use]
pub(crate) fn with_lfs_ready(mut self, lfs: Option<bool>) -> Self {
self.lfs_ready = lfs;
self
}
}
impl GitCheckout {
@@ -332,7 +396,11 @@ impl GitCheckout {
///
/// * The `repo` will be the checked out Git repository.
fn new(revision: GitOid, repo: GitRepository) -> Self {
Self { revision, repo }
Self {
revision,
repo,
lfs_ready: None,
}
}
/// Clone a repo for a `revision` into a local path from a `database`.
@@ -372,8 +440,8 @@ impl GitCheckout {
let repo = GitRepository::open(into)?;
let checkout = Self::new(revision, repo);
checkout.reset()?;
Ok(checkout)
let lfs_ready = checkout.reset(database.lfs_ready)?;
Ok(checkout.with_lfs_ready(lfs_ready))
}
/// Checks if the `HEAD` of this checkout points to the expected revision.
@@ -387,22 +455,39 @@ impl GitCheckout {
}
}
/// Indicates Git LFS artifacts have been initialized (when requested).
pub(crate) fn lfs_ready(&self) -> Option<bool> {
self.lfs_ready
}
/// Set the Git LFS validation state (if any).
#[must_use]
pub(crate) fn with_lfs_ready(mut self, lfs: Option<bool>) -> Self {
self.lfs_ready = lfs;
self
}
/// This performs `git reset --hard` to the revision of this checkout, with
/// additional interrupt protection by a dummy file [`CHECKOUT_READY_LOCK`].
///
/// If we're interrupted while performing a `git reset` (e.g., we die
/// because of a signal) Cargo needs to be sure to try to check out this
/// because of a signal) uv needs to be sure to try to check out this
/// repo again on the next go-round.
///
/// To enable this we have a dummy file in our checkout, [`.cargo-ok`],
/// To enable this we have a dummy file in our checkout, [`.ok`],
/// which if present means that the repo has been successfully reset and is
/// ready to go. Hence if we start to do a reset, we make sure this file
/// ready to go. Hence, if we start to do a reset, we make sure this file
/// *doesn't* exist, and then once we're done we create the file.
///
/// [`.cargo-ok`]: CHECKOUT_READY_LOCK
fn reset(&self) -> Result<()> {
/// [`.ok`]: CHECKOUT_READY_LOCK
fn reset(&self, with_lfs: Option<bool>) -> Result<Option<bool>> {
let ok_file = self.repo.path.join(CHECKOUT_READY_LOCK);
let _ = paths::remove_file(&ok_file);
// We want to skip smudge if lfs was disabled for the repository
// as smudge filters can trigger on a reset even if lfs artifacts
// were not originally "fetched".
let lfs_skip_smudge = if with_lfs == Some(true) { "0" } else { "1" };
debug!("Reset {} to {}", self.repo.path.display(), self.revision);
// Perform the hard reset.
@@ -410,6 +495,7 @@ impl GitCheckout {
.arg("reset")
.arg("--hard")
.arg(self.revision.as_str())
.env(EnvVars::GIT_LFS_SKIP_SMUDGE, lfs_skip_smudge)
.cwd(&self.repo.path)
.exec_with_output()?;
@@ -419,12 +505,27 @@ impl GitCheckout {
.arg("update")
.arg("--recursive")
.arg("--init")
.env(EnvVars::GIT_LFS_SKIP_SMUDGE, lfs_skip_smudge)
.cwd(&self.repo.path)
.exec_with_output()
.map(drop)?;
paths::create(ok_file)?;
Ok(())
// Validate Git LFS objects (if needed) after the reset.
// See `fetch_lfs` why we do this.
let lfs_validation = match with_lfs {
None => None,
Some(false) => Some(false),
Some(true) => Some(self.repo.lfs_fsck_objects(self.revision.as_str())),
};
// The .ok file should be written when the reset is successful.
// When Git LFS is enabled, the objects must also be fetched and
// validated successfully as part of the corresponding db.
if with_lfs.is_none() || lfs_validation == Some(true) {
paths::create(ok_file)?;
}
Ok(lfs_validation)
}
}
@@ -643,7 +744,17 @@ fn fetch_with_cli(
///
/// Returns an error if Git LFS isn't available.
/// Caching the command allows us to only check if LFS is installed once.
static GIT_LFS: LazyLock<Result<ProcessBuilder>> = LazyLock::new(|| {
///
/// We also support a helper private environment variable to allow
/// controlling the LFS extension from being loaded for testing purposes.
/// Once installed, Git will always load `git-lfs` as a built-in alias
/// which takes priority over loading from `PATH` which prevents us
/// from shadowing the extension with other means.
pub static GIT_LFS: LazyLock<Result<ProcessBuilder>> = LazyLock::new(|| {
if std::env::var_os(EnvVars::UV_INTERNAL__TEST_LFS_DISABLED).is_some() {
return Err(anyhow!("Git LFS extension has been forcefully disabled."));
}
let mut cmd = ProcessBuilder::new(GIT.as_ref()?);
cmd.arg("lfs");
@@ -658,14 +769,14 @@ fn fetch_lfs(
url: &Url,
revision: &GitOid,
disable_ssl: bool,
) -> Result<()> {
) -> Result<bool> {
let mut cmd = if let Ok(lfs) = GIT_LFS.as_ref() {
debug!("Fetching Git LFS objects");
lfs.clone()
} else {
// Since this feature is opt-in, warn if not available
warn!("Git LFS is not available, skipping LFS fetch");
return Ok(());
return Ok(false);
};
if disable_ssl {
@@ -682,10 +793,23 @@ fn fetch_lfs(
.env_remove(EnvVars::GIT_INDEX_FILE)
.env_remove(EnvVars::GIT_OBJECT_DIRECTORY)
.env_remove(EnvVars::GIT_ALTERNATE_OBJECT_DIRECTORIES)
// We should not support requesting LFS artifacts with skip smudge being set.
// While this may not be necessary, it's added to avoid any potential future issues.
.env_remove(EnvVars::GIT_LFS_SKIP_SMUDGE)
.cwd(&repo.path);
cmd.exec_with_output()?;
Ok(())
// We now validate the Git LFS objects explicitly (if supported). This is
// needed to avoid issues with Git LFS not being installed or configured
// on the system and giving the wrong impression to the user that Git LFS
// objects were initialized correctly when installation finishes.
// We may want to allow the user to skip validation in the future via
// UV_GIT_LFS_NO_VALIDATION environment variable on rare cases where
// validation costs outweigh the benefit.
let validation_result = repo.lfs_fsck_objects(revision.as_str());
Ok(validation_result)
}
/// Whether `rev` is a shorter hash of `oid`.