cargo: use git gc --auto

[matthiaskrgr]

This will allow git to decide when to run gc by itself and will honor userside settings.
Implements #4495

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[matthiaskrgr]

I removed the testcase because whether git gc will run or not will now depend on git configs such as

[gc]
	auto = 1

[alexcrichton]

Thanks! I'm sort of wary removing the tests for this, though, because then there's no way of knowing whether this works over time. Can we force the auto gc to run in tests?

[Mark-Simulacrum]

I'd suggest that the auto preference in git can probably be checked and the test not run if it's set (that is, test passes). Alternatively, we might be able to configure git per-directory or something so it always runs for the test....

[matthiaskrgr]

I found out we can simply disable the gc.auto setting for that test :)

[alexcrichton]

Nice! I'm realizing now though that the gc unconditionally happens and this can sometimes slow down fetches from the network because spawning a git process has a relatively high amount of overhead associated with it. Would it be possible to leave the pre-flight checks to avoid git gc entirely sometimes?

[matthiaskrgr]

You mean, first check if cargo pack size limit is exceeded and if so, run git gc --auto?

[alexcrichton]

Indeed yeah!

[matthiaskrgr]

Looks like the test is failing on windows:

running 2 tests
test avoid_using_git ... ignored
test use_git_gc ... FAILED
failures:
---- use_git_gc stdout ----
	running `C:\projects\cargo\target\debug\cargo.exe build`
running `C:\projects\cargo\target\debug\cargo.exe update`
thread 'use_git_gc' panicked at 'packfiles before: 102
packfiles after:  104', tests\small-fd-limits.rs:82:4
note: Run with `RUST_BACKTRACE=1` for a backtrace.

Unfortunately I am not sure what to do here, I don't have a windows machine and I cannot reproduce the failure on linux. :(

[alexcrichton]

I'm not really sure what's happening on Windows, but it looks like the gc isn't running?

[sfackler]

Is this good to go?

[alexcrichton]

I would personally prefer to not land this unless we're still exercising the code paths on various platforms (especially Windows as it can be tricky for situations like this). As-is I believe it just disables the test on Windows?

[matthiaskrgr]

Yes, this is correct. I have no way of testing/debugging this on windows natively myself.
There are already tests being blacklisted for windows

#[test]
// it looks like these tests passes on some windows machines but not others,
// notably not on AppVeyor's machines. Sounds like another bug for another day.
#[cfg_attr(windows, ignore)]

and I assume this triggers here as well.

If this is a blocker on the PR feel free to reject it.

[alexcrichton]

Sorry yeah for now these implementations are tricky enough (especially across platforms) I'd like to keep the tests running across all platforms. We can always reopen though with tests that work across all platforms!