Use git gc --auto in maybe_gc_repo

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::<usize>().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<()> {

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);
     }