Fix cross-device rename failure on LFS download
Found via real deployment testing on mbator.pl: the download path reported back to git-lfs used std::env::temp_dir() (/tmp), which git-lfs then does an atomic rename() from into .git/lfs/objects/. On systems where /tmp and the repository are on different filesystems/mounts (the VPS in question), that rename fails outright with "invalid cross-device link" rather than falling back to a copy -- git-lfs doesn't retry with a copy+delete. The actual IPFS fetch and decryption were working correctly the whole time (confirmed: download succeeded, byte size matched exactly) -- this was purely the final handoff step failing. Fix: resolve `.git/lfs/tmp` (via `git rev-parse --git-dir`, so this works whether invoked from the repo root or a subdirectory) and download there instead. This is git-lfs's own existing convention for its own internal temp downloads, so reusing it guarantees same-filesystem semantics by construction rather than hoping /tmp happens to line up. This does reintroduce a `git` binary dependency (previously removed once this crate stopped needing to read/write notes directly) -- an acceptable tradeoff, since anything using `rad`/git-lfs at all already requires git to be installed.
This commit is contained in:
parent
d67ac79f40
commit
ac4a27c098
36
src/main.rs
36
src/main.rs
|
|
@ -1,5 +1,6 @@
|
|||
mod kubo;
|
||||
|
||||
use anyhow::Context as _;
|
||||
use serde_json::{Value, json};
|
||||
use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
|
||||
use tokio::process::Command;
|
||||
|
|
@ -120,7 +121,17 @@ async fn download_inner(msg: &Value) -> Result<(String, String), (String, anyhow
|
|||
let oid = field_str(msg, "oid").map_err(|e| (String::new(), e))?;
|
||||
let size = field_i64(msg, "size").map_err(|e| (oid.clone(), e))?;
|
||||
|
||||
let tmp_path = std::env::temp_dir().join(format!("rad-lfs-{oid}"));
|
||||
// The path we report back must be on the same filesystem as
|
||||
// `.git/lfs/objects`, since git-lfs does an atomic `rename()` from
|
||||
// there into its object store rather than a copy. The system temp
|
||||
// dir (`/tmp`) can be a different filesystem/mount than the repo --
|
||||
// on such systems that rename fails outright with "invalid
|
||||
// cross-device link" rather than falling back to a copy. Using
|
||||
// `.git/lfs/tmp/` (git-lfs's own existing convention for its own
|
||||
// temp downloads) guarantees same-filesystem semantics by
|
||||
// construction.
|
||||
let tmp_dir = git_lfs_tmp_dir().await.map_err(|e| (oid.clone(), e))?;
|
||||
let tmp_path = tmp_dir.join(format!("rad-lfs-{oid}"));
|
||||
|
||||
let output = Command::new("rad")
|
||||
.args([
|
||||
|
|
@ -152,6 +163,29 @@ async fn download_inner(msg: &Value) -> Result<(String, String), (String, anyhow
|
|||
Ok((oid, tmp_path.to_string_lossy().to_string()))
|
||||
}
|
||||
|
||||
/// Resolves (and creates, if needed) `.git/lfs/tmp` for the current
|
||||
/// repository, regardless of whether the working directory is the repo
|
||||
/// root or a subdirectory of it.
|
||||
async fn git_lfs_tmp_dir() -> anyhow::Result<std::path::PathBuf> {
|
||||
let output = Command::new("git")
|
||||
.args(["rev-parse", "--git-dir"])
|
||||
.output()
|
||||
.await
|
||||
.context("failed to spawn `git rev-parse --git-dir`")?;
|
||||
if !output.status.success() {
|
||||
anyhow::bail!(
|
||||
"git rev-parse --git-dir failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
}
|
||||
let git_dir = String::from_utf8_lossy(&output.stdout).trim().to_string();
|
||||
let tmp_dir = std::path::PathBuf::from(git_dir).join("lfs").join("tmp");
|
||||
tokio::fs::create_dir_all(&tmp_dir)
|
||||
.await
|
||||
.with_context(|| format!("failed to create `{}`", tmp_dir.display()))?;
|
||||
Ok(tmp_dir)
|
||||
}
|
||||
|
||||
fn field_str(msg: &Value, key: &str) -> anyhow::Result<String> {
|
||||
msg.get(key)
|
||||
.and_then(Value::as_str)
|
||||
|
|
|
|||
Loading…
Reference in New Issue