Skip to content

fix rename temp file to cache file #4

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

Merged
merged 4 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
steps:
- uses: actions/checkout@master
- name: Compile and release
uses: rust-build/rust-build.action@v1.4.0
uses: rust-build/rust-build.action@v1.75.0
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "lfspull"
version = "0.3.0"
version = "0.3.1"
edition = "2021"
license = "MIT"
authors = ["Volume Graphics GmbH"]
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ Please see our docs.rs for example code and the gherkin tests for how to check t

## Changelog

### 0.3.1

- fix bug when trying to rename temp file to cache file, but cache file is already created and locked by other parallel job

### 0.3.0

- use stream_bytes to download object directly into a temporary files and avoid 'memory allocation of x bytes failed'
31 changes: 22 additions & 9 deletions src/repo_tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use glob::glob;
use primitives::get_repo_root;
use std::path::{Path, PathBuf};
use tokio::fs;
use tracing::{debug, info};
use tracing::{debug, error, info};
use url::Url;
use vg_errortools::{fat_io_wrap_tokio, FatIOError};

Expand Down Expand Up @@ -120,14 +120,27 @@ async fn get_file_cached<P: AsRef<Path>>(

let temp_file =
primitives::download_file(metadata, &repo_url, access_token, randomizer_bytes).await?;
fs::rename(&temp_file.path(), cache_file.as_path())
.map_err(|e| {
LFSError::FatFileIOError(FatIOError::from_std_io_err(
e,
temp_file.path().to_path_buf(),
))
})
.await?;
if cache_file.exists() {
info!(
"cache file {:?} is already written from other process",
&cache_file
);
} else {
fs::rename(&temp_file.path(), cache_file.as_path())
.map_err(|e| {
error!(
"Could not rename {:?} to {:?}: {:?}",
temp_file.path(),
cache_file.as_path(),
&e
);
LFSError::FatFileIOError(FatIOError::from_std_io_err(
e,
temp_file.path().to_path_buf(),
))
})
.await?;
}

Ok((cache_file, FilePullMode::DownloadedFromRemote))
}
Expand Down
28 changes: 10 additions & 18 deletions src/repo_tools/primitives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::path::PathBuf;
use tempfile::NamedTempFile;
use tokio::fs;
use tokio::io::AsyncReadExt;
use tracing::{debug, info};
use tracing::{debug, error, info};
use url::Url;
use vg_errortools::{fat_io_wrap_tokio, FatIOError};

Expand Down Expand Up @@ -119,18 +119,6 @@ fn url_with_auth(url: &str, access_token: Option<&str>) -> Result<Url, LFSError>
Ok(url)
}

// //no need for tempfile crate
// pub(crate) struct TempFile {
// pub(crate) path: PathBuf,
// pub(crate) file: fs::File,
// }
//
// impl Drop for TempFile {
// fn drop(&mut self) {
// let _ = std::fs::remove_file(&self.path);
// }
// }

pub async fn download_file(
meta_data: &MetaData,
repo_remote_url: &str,
Expand Down Expand Up @@ -215,11 +203,14 @@ pub async fn download_file(
.tempfile_in(TEMP_FOLDER)
.map_err(|e| LFSError::TempFile(e.to_string()))?;

debug!("created tempfile: {:?}", &temp_file);

let mut hasher = Sha256::new();
let mut stream = response.bytes_stream();
while let Some(chunk_result) = stream.next().await {
let chunk = chunk_result?;
temp_file.as_file().write_all(&chunk).map_err(|e| {
error!("Could not write tempfile");
LFSError::FatFileIOError(FatIOError::from_std_io_err(
e,
temp_file.path().to_path_buf(),
Expand All @@ -228,6 +219,7 @@ pub async fn download_file(
hasher.update(chunk);
}
temp_file.as_file().flush().map_err(|e| {
error!("Could not flush tempfile");
LFSError::FatFileIOError(FatIOError::from_std_io_err(
e,
temp_file.path().to_path_buf(),
Expand Down Expand Up @@ -302,19 +294,19 @@ impl Object {

#[cfg(test)]
mod tests {
const URL: &str = "https://dev.azure.com/rohdealx/devops/_git/git-lfs-test";
const URL: &str = "https://dev.azure.com/buildvgmpsmi/buildvg/_git/git-lfs-test";
use super::*;
const LFS_TEST_DATA: &str = r#"version https://git-lfs.github.com/spec/v1
oid sha256:4329aab31bc9c72a897f57e038fe60655d31df6e5ddf2cf897669a845d64edbc
size 665694"#;
oid sha256:0fae26606afd128d4d2f730462c8451b90931d25813e06e55239a2ca00e74c74
size 226848"#;
#[test]
fn test_parsing_of_string() {
let parsed = parse_lfs_string(LFS_TEST_DATA).expect("Could not parse demo-string!");
assert_eq!(parsed.size, 665694);
assert_eq!(parsed.size, 226848);
assert_eq!(parsed.version, "https://git-lfs.github.com/spec/v1");
assert_eq!(
parsed.oid,
"4329aab31bc9c72a897f57e038fe60655d31df6e5ddf2cf897669a845d64edbc"
"0fae26606afd128d4d2f730462c8451b90931d25813e06e55239a2ca00e74c74"
);
assert_eq!(parsed.hash, Some(Hash::SHA256));
}
Expand Down
8 changes: 4 additions & 4 deletions tests/features/lfspull.feature
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,24 @@ Feature: A simple git-lfs implementation for pulling single files
Given the test-repository is correctly setup
When pulling the file
Then the file was pulled from origin
And the file size is 665694
And the file size is 226848

Scenario: Pulling a single file when already cached
Given the test-repository is correctly setup
And the file was pulled already
When resetting the file
And pulling the file
Then the file was pulled from local cache
And the file size is 665694
And the file size is 226848

Scenario: Pulling a file that already exists
Given the test-repository is correctly setup
And the file was pulled already
When pulling the file
Then the file was already there
And the file size is 665694
And the file size is 226848

Scenario: Pulling a complete directory
Given the test-repository is correctly setup
When pulling the complete directory
Then the file size is 665694
Then the file size is 226848
6 changes: 3 additions & 3 deletions tests/lfspull.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const TEST_GIT_CONFIG: &str = r#"[core]
bare = false
logallrefupdates = true
[remote "origin"]
url = https://dev.azure.com/rohdealx/devops/_git/git-lfs-test
url = https://dev.azure.com/buildvgmpsmi/buildvg/_git/git-lfs-test
fetch = +refs/heads/*:refs/remotes/origin/*
[branch "main"]
remote = origin
Expand All @@ -27,8 +27,8 @@ const TEST_GIT_CONFIG: &str = r#"[core]
"#;

const TEST_LFS_FILE: &str = r#"version https://git-lfs.github.com/spec/v1
oid sha256:4329aab31bc9c72a897f57e038fe60655d31df6e5ddf2cf897669a845d64edbc
size 665694
oid sha256:0fae26606afd128d4d2f730462c8451b90931d25813e06e55239a2ca00e74c74
size 226848
"#;

const TEST_LFS_FILE_NAME: &str = "beer_tornado.mp4";
Expand Down
Loading