Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Git LFS support to uv-git crate #10335

Merged
merged 9 commits into from
Jan 13, 2025
37 changes: 37 additions & 0 deletions crates/uv-git/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ pub static GIT: LazyLock<Result<PathBuf, GitError>> = LazyLock::new(|| {
})
});

/// A global cache of the result of `which git-lfs`.
///
/// We search for the Git LFS binary instead of using `git lfs` so that we can
/// distinguish Git LFS not being installed (which we can ignore) from
/// LFS errors (which should be returned).
static GIT_LFS: LazyLock<Option<PathBuf>> = LazyLock::new(|| which::which("git-lfs").ok());
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not certain this detection method is portable to all end user systems. For example, I can have git lfs installed and working but I not have git-lfs binary on the path. I think probing git lfs directly is likely the most portable approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I've updated the logic so now instead of searching for git-lfs on the path, it just tries to run git lfs version the first time LFS is invoked and uses the success of that command as an indicator of LFS availability.

I'd definitely be interested in if anyone has a cleaner way of determining whether LFS is available. I was initially going to just check if the LFS fetch command returned 127 (command not found), but that return code isn't available in Powershell, so that approach isn't portable across shells.


/// A reference to commit or commit-ish.
#[derive(
Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, serde::Serialize, serde::Deserialize,
Expand Down Expand Up @@ -261,6 +268,8 @@ impl GitRemote {
};

if let Some(rev) = resolved_commit_hash {
fetch_lfs(&mut db.repo, self.url.as_str(), &rev)
.with_context(|| format!("failed to fetch LFS objects at {rev}"))?;
return Ok((db, rev));
}
}
Expand All @@ -280,6 +289,8 @@ impl GitRemote {
Some(rev) => rev,
None => reference.resolve(&repo)?,
};
fetch_lfs(&mut repo, self.url.as_str(), &rev)
.with_context(|| format!("failed to fetch LFS objects at {rev}"))?;

Ok((GitDatabase { repo }, rev))
}
Expand Down Expand Up @@ -635,6 +646,32 @@ fn fetch_with_cli(
// The required `on...line` callbacks currently do nothing.
// The output appears to be included in error messages by default.
cmd.exec_with_output()?;

Ok(())
}

/// Attempts to use `git-lfs` CLI to fetch required LFS objects for a given revision.
fn fetch_lfs(repo: &mut GitRepository, url: &str, revision: &GitOid) -> Result<()> {
let lfs = if let Some(lfs) = GIT_LFS.as_ref() {
debug!("Fetching Git LFS objects");
lfs
} else {
debug!("Git lfs is not installed, skipping lfs fetch");
return Ok(());
};
let mut cmd = ProcessBuilder::new(lfs);
cmd.arg("fetch")
.arg(url)
.arg(revision.as_str())
// These variables are unset for the same reason as in `fetch_with_cli`.
.env_remove(EnvVars::GIT_DIR)
.env_remove(EnvVars::GIT_WORK_TREE)
.env_remove(EnvVars::GIT_INDEX_FILE)
.env_remove(EnvVars::GIT_OBJECT_DIRECTORY)
.env_remove(EnvVars::GIT_ALTERNATE_OBJECT_DIRECTORIES)
.cwd(&repo.path);

cmd.exec_with_output()?;
Ok(())
}

Expand Down
Loading