diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index f8190a2ddae..6c6c0e05edb 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -701,6 +701,23 @@ pub fn fetch( url: &str, refspec: &str, config: &Config, +) -> CargoResult<()> { + fetch_impl(repo, url, refspec, config)?; + // We reuse repositories quite a lot, so now that we have updated the repo, + // check to see if it's a little too old and could benefit from a gc. + // `git gc` goes through lengths to avoid removing precious data. In + // principle, we could be running it in the background while we're fetching, + // but better safe(r) than sorry, we just spawn it once we're done. It will + // be spawned in the background (so effectively, cargo will likely have + // terminated before `git gc` finishes), but only if necessary. + maybe_gc_repo(repo) +} + +fn fetch_impl( + repo: &mut git2::Repository, + url: &str, + refspec: &str, + config: &Config, ) -> CargoResult<()> { if config.frozen() { anyhow::bail!( @@ -729,12 +746,6 @@ pub fn fetch( } } - // We reuse repositories quite a lot, so before we go through and update the - // repo check to see if it's a little too old and could benefit from a gc. - // In theory this shouldn't be too too expensive compared to the network - // request we're about to issue. - maybe_gc_repo(repo)?; - // Unfortunately `libgit2` is notably lacking in the realm of authentication // when compared to the `git` command line. As a result, allow an escape // hatch for users that would prefer to use `git`-the-CLI for fetching @@ -833,30 +844,15 @@ fn fetch_with_cli( /// opportunistically try a `git gc` when the pack directory looks too big, and /// failing that we just blow away the repository and start over. fn maybe_gc_repo(repo: &mut git2::Repository) -> CargoResult<()> { - // Here we arbitrarily declare that if you have more than 100 files in your - // `pack` folder that we need to do a gc. - let entries = match repo.path().join("objects/pack").read_dir() { - Ok(e) => e.count(), - Err(_) => { - debug!("skipping gc as pack dir appears gone"); - return Ok(()); - } - }; - let max = env::var("__CARGO_PACKFILE_LIMIT") - .ok() - .and_then(|s| s.parse::().ok()) - .unwrap_or(100); - if entries < max { - debug!("skipping gc as there's only {} pack files", entries); - return Ok(()); - } - - // First up, try a literal `git gc` by shelling out to git. This is pretty - // likely to fail though as we may not have `git` installed. Note that - // libgit2 doesn't currently implement the gc operation, so there's no - // equivalent there. + // First up, try a literal `git gc --auto` by shelling out to git. This + // is pretty likely to fail though as we may not have `git` installed. + // If it doesn't fail, it will check how many packs and loose objects there + // are and spawn a `git gc` in the background if there are too many. + // Note that libgit2 doesn't currently implement the gc operation, so + // there's no equivalent there. match Command::new("git") .arg("gc") + .arg("--auto") .current_dir(repo.path()) .output() { @@ -875,9 +871,7 @@ fn maybe_gc_repo(repo: &mut git2::Repository) -> CargoResult<()> { } Err(e) => debug!("git-gc failed to spawn: {}", e), } - - // Alright all else failed, let's start over. - reinitialize(repo) + Ok(()) } fn reinitialize(repo: &mut git2::Repository) -> CargoResult<()> { diff --git a/tests/testsuite/git_gc.rs b/tests/testsuite/git_gc.rs index b8401a56db3..ac8148ac63a 100644 --- a/tests/testsuite/git_gc.rs +++ b/tests/testsuite/git_gc.rs @@ -50,6 +50,9 @@ fn run_test(path_env: Option<&OsStr>) { let mut cfg = index.config().unwrap(); cfg.set_str("user.email", "foo@bar.com").unwrap(); cfg.set_str("user.name", "Foo Bar").unwrap(); + cfg.set_str("gc.autoPackLimit", "10").unwrap(); + // Ensure we check the number of packs after gc finished. + cfg.set_str("gc.autoDetach", "false").unwrap(); for _ in 0..N { git::commit(&repo); @@ -70,7 +73,6 @@ fn run_test(path_env: Option<&OsStr>) { assert!(before > N); let mut cmd = foo.cargo("update"); - cmd.env("__CARGO_PACKFILE_LIMIT", "10"); if let Some(path) = path_env { cmd.env("PATH", path); }