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

Clean stale git temp files #11308

Merged
merged 1 commit into from
Nov 1, 2022
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
39 changes: 39 additions & 0 deletions src/cargo/sources/git/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,8 @@ pub fn fetch(
// request we're about to issue.
maybe_gc_repo(repo)?;

clean_repo_temp_files(repo);

// Translate the reference desired here into an actual list of refspecs
// which need to get fetched. Additionally record if we're fetching tags.
let mut refspecs = Vec::new();
Expand Down Expand Up @@ -1001,6 +1003,43 @@ fn maybe_gc_repo(repo: &mut git2::Repository) -> CargoResult<()> {
reinitialize(repo)
}

/// Removes temporary files left from previous activity.
///
/// If libgit2 is interrupted while indexing pack files, it will leave behind
/// some temporary files that it doesn't clean up. These can be quite large in
/// size, so this tries to clean things up.
///
/// This intentionally ignores errors. This is only an opportunistic cleaning,
/// and we don't really care if there are issues (there's unlikely anything
/// that can be done).
///
/// The git CLI has similar behavior (its temp files look like
/// `objects/pack/tmp_pack_9kUSA8`). Those files are normally deleted via `git
/// prune` which is run by `git gc`. However, it doesn't know about libgit2's
/// filenames, so they never get cleaned up.
fn clean_repo_temp_files(repo: &git2::Repository) {
let path = repo.path().join("objects/pack/pack_git2_*");
let pattern = match path.to_str() {
Some(p) => p,
None => {
log::warn!("cannot convert {path:?} to a string");
return;
}
};
let paths = match glob::glob(pattern) {
Ok(paths) => paths,
Err(_) => return,
};
for path in paths {
if let Ok(path) = path {
match paths::remove_file(&path) {
Ok(_) => log::debug!("removed stale temp git file {path:?}"),
Err(e) => log::warn!("failed to remove {path:?} while cleaning temp files: {e}"),
}
}
}
}

fn reinitialize(repo: &mut git2::Repository) -> CargoResult<()> {
// Here we want to drop the current repository object pointed to by `repo`,
// so we initialize temporary repository in a sub-folder, blow away the
Expand Down
33 changes: 33 additions & 0 deletions tests/testsuite/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use std::sync::Arc;
use std::thread;

use cargo_test_support::paths::{self, CargoPathExt};
use cargo_test_support::registry::Package;
use cargo_test_support::{basic_lib_manifest, basic_manifest, git, main_file, path2url, project};
use cargo_test_support::{sleep_ms, t, Project};

Expand Down Expand Up @@ -3619,3 +3620,35 @@ fn _corrupted_checkout(with_cli: bool) {
e.run();
assert!(ok.exists());
}

#[cargo_test]
fn cleans_temp_pack_files() {
// Checks that cargo removes temp files left by libgit2 when it is
// interrupted (see clean_repo_temp_files).
Package::new("bar", "1.0.0").publish();
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"

[dependencies]
bar = "1.0"
"#,
)
.file("src/lib.rs", "")
.build();
p.cargo("fetch").run();
// Simulate what happens when libgit2 is interrupted while indexing a pack file.
let tmp_path = super::git_gc::find_index().join(".git/objects/pack/pack_git2_91ab40da04fdc2e7");
fs::write(&tmp_path, "test").unwrap();
let mut perms = fs::metadata(&tmp_path).unwrap().permissions();
perms.set_readonly(true);
fs::set_permissions(&tmp_path, perms).unwrap();

// Trigger an index update.
p.cargo("generate-lockfile").run();
assert!(!tmp_path.exists());
}
2 changes: 1 addition & 1 deletion tests/testsuite/git_gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use cargo_test_support::registry::Package;

use url::Url;

fn find_index() -> PathBuf {
pub fn find_index() -> PathBuf {
let dir = paths::home().join(".cargo/registry/index");
dir.read_dir().unwrap().next().unwrap().unwrap().path()
}
Expand Down